-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #700 #701
fix #700 #701
Conversation
Use CMAKE_INSTALL_PREFIX for install paths
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMAKE_INSTALL_PREFIX not needed.
Hi, Vasilii. Did you remove cmake cache before the test?
пт, 30 мар. 2018 г., 9:41 Vasiliy Glazov <[email protected]>:
… ***@***.**** commented on this pull request.
CMAKE_INSTALL_PREFIX not needed.
------------------------------
In CMakeLists.txt
<#701 (comment)>:
> @@ -5,17 +5,7 @@ set(PROJECT_DESCRIPTION "Open source version of the STMicroelectronics Stlink To
set(STLINK_UDEV_RULES_DIR "/etc/udev/rules.d" CACHE PATH "Udev rules directory")
set(STLINK_MODPROBED_DIR "/etc/modprobe.d" CACHE PATH "modprobe.d directory")
set(LIB_INSTALL_DIR "lib" CACHE PATH "Main library directory")
-
-if(IS_DIRECTORY CMAKE_LIBRARY_PATH)
-else()
- set(CMAKE_LIBRARY_PATH "lib")
-endif()
-
-if (WIN32)
- set(STLINK_LIBRARY_PATH ${CMAKE_LIBRARY_PATH} CACHE PATH "Target lib directory")
-else()
- set(STLINK_LIBRARY_PATH "${LIB_INSTALL_DIR}/${CMAKE_LIBRARY_PATH}" CACHE PATH "Target lib directory")
-endif(WIN32)
+set(STLINK_LIBRARY_PATH "${CMAKE_INSTALL_PREFIX}/${LIB_INSTALL_DIR}" CACHE PATH "Target lib directory")
Seems that LIB_INSTALL_DIR already /usr/lib64 and CMAKE_INSTALL_PREFIX =
/usr.
So lib installed to /usr/usr/lib64/
Need to remove CMAKE_INSTALL_PREFIX from this line.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#701 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABgKcUfQ5oAJC0kqqvYzvtixsKAhwd12ks5tjdO1gaJpZM4TBU4N>
.
|
I don't know how to remove it. But on clean installed build environment it has same error:
Full build log https://kojipkgs.fedoraproject.org//work/tasks/5046/26055046/build.log P.S. Don't see to version 1.5.0, it use zip from your branch fix-linux-install. |
Not work.
|
Vasiliy. You pass to cmake those variables from a command line: Can you change that to: And I, respectively, will set in cmake only: without any checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong parameters expansion.
I can't change it. You need just correct variables in "if(" |
Now all OK. |
Will wait for approving from Debian guy. |
I'm not a CMake expert (in fact I have an intense dislike for it...), but something is not right:
I got what I needed:
Building from this PR this doesn't work anymore, CMAKE_LIBRARY_PATH is ignored, this is where libraries are installed now:
Setting LIB_INSTALL_DIR:
Makes it even worse, not sure where that path is coming from:
Not sure what to change though, sorry |
@bluca You set |
That works, thanks (as I said, not CMake expert). Note that headers have changed path too - that is fine if they are static and arch-independent |
@bluca Headers always arch independent and placed to /usr/include/stlink. |
@bluca I understand the frustration of cmake compared to autoconf build system. But it is unmaintainable and hard to write and read for developers in a cross platform way, it is easier to understand for packaging people (almost standard build parameters). Are we good now with this PR? What about the defaults without using any build parameters does it work for |
@slyshykO had asked me to test his branch. It doesn't work for me here with Debian 9! IMO the
|
The install files are correct, as mentioned above the cmake options are changing in an incompatible way so debian/rules needs to be updated accordingly |
@bluca the removal of the wildcard in But there are at least two glitches in the
|
@slyshykO could you please also apply this patch together with your changes:
The CMake options are being changed so the CMake parameter used in d/rules doesn't work anymore. The fix is above.
Multiarch, see section 9.1.1.4 of the Policy: https://www.debian.org/doc/debian-policy/
dh_install works fine with either, but feel free to change that if you wish
The unversioned symlink goes into the -dev package, see section 8.4 of the Policy: |
@slyshykO you should use |
1ce17f2
to
ae4e6a0
Compare
I've uploaded it to Debian, it will be available after it's been reviewed (no ETA), afterwards I'll upload to stretch-backports as well |
@bluca |
That's what I meant with "afterwards" - once it's approved and it's in unstable/testing |
@xor-gate , I think, it is ready for merge. |
Use CMAKE_INSTALL_PREFIX for install paths
Need to approve from @cedricboudinet and @Vascom