Skip to content
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

packaging: Fix debian packaging #485

Merged
merged 1 commit into from
Oct 4, 2016

Conversation

nekromant
Copy link
Contributor

@nekromant nekromant commented Sep 23, 2016

The sole reason debian packaging got broken - the addition of the toplevel makefile broke debhelper's autodetection. This patchset does the following:

  • Forces the buildsystem to be always cmake with all relevant debhelper magic involved.
  • Installs modprobe.d/*.conf and udev rules to the relevant locations
  • Adjust debian file lists to avoid any conflicts
  • Removes all the useless code from debian/rules to keep it at bare minimum

This pretty much passes most of the lintian checks, the remaining are:

W: libstlink source: missing-license-paragraph-in-dep5-copyright bsd-3-clause (paragraph at line 10)
W: libstlink source: out-of-date-standards-version 3.9.5 (current is 3.9.6)
E: stlink-tools: extended-description-is-empty
E: stlink-gui: extended-description-is-empty
W: stlink-gui: binary-without-manpage usr/bin/stlink-gui
W: libstlink: package-name-doesnt-match-sonames libstlink1
W: libstlink: description-starts-with-leading-spaces
W: libstlink: non-dev-pkg-with-shlib-symlink usr/lib/x86_64-linux-gnu/libstlink.so.1.2.0 usr/lib/x86_64-linux-gnu/libstlink.so
W: libstlink-dev: description-starts-with-leading-spaces

But these are easy to fix and are only relevant if we're going to integrate it to debian (I'm not a debian maintainer myself, don't know any debian maintainers, so I can't help out with this task). @xor-gate, @OsterlaD, comments are welcome.

@nekromant nekromant mentioned this pull request Sep 23, 2016
@OsterlaD
Copy link
Contributor

Acked.

Is DH_VERBOSE intended or just a misstake?

@nekromant
Copy link
Contributor Author

Looks like a left-over from troubleshooting. I'm doing this in a terrible rush switching from day-job so these are expected ;(

And it looks like installing configs to /etc/ broke cpack somehow. I wonder if @xor-gate came across this one in the past. Meanwhile I'll see if I can fix it.

@nekromant
Copy link
Contributor Author

That should hopefully work-around the remaining problems

@nekromant
Copy link
Contributor Author

@xor-gate Could you look at CMakeLists.txt changes that break cpack and hence the travis build run? The only way I found to workaround failing installation of configs to etc is to add an optional parameter when running cmake, e,g. cmake -DINSTALL_UDEV_RULES=yes. Perhaps you know a cleaner solution.

@xor-gate
Copy link
Member

You should probably use something like: -DCMAKE_INSTALL_PREFIX=$PWD/build/Release/_install or -DCMAKE_INSTALL_PREFIX=$PWD/build/Debug/_install When generating the makefiles.

@xor-gate
Copy link
Member

@nekromant
Copy link
Contributor Author

The debian packaging works nicely, dh calls cmake with -DCMAKE_PREFIX=/usr and later uses DESTDIR= when installing and cmake handles all of that nicely.
It's cpack that fails for some reason trying to install the files into the absolute paths. See the travis log above

@xor-gate
Copy link
Member

I will investigate why it fails in the upcoming week!

@OsterlaD
Copy link
Contributor

Default build wants to install to CMAKE_PREFIX, but this change does not respect this variable.
There are three use-cases:

  • sandbox install ./_install/...
  • direct install /usr/local/... and /etc/...
  • debian package install /usr/... and /lib/...

Maybe it is better to keep documented behavior for sandbox and direct install, to install udev rules and modconfig by hand. For debian package we can do in debian/rules.

@xor-gate
Copy link
Member

I had a busy week, will take a look this weekend.

@xor-gate xor-gate added this to the v1.3.0 milestone Sep 30, 2016
@xor-gate xor-gate self-assigned this Sep 30, 2016
@xor-gate
Copy link
Member

xor-gate commented Oct 3, 2016

I have a patch which also installs modprobe, udev only for linux:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index bc00d18..b9fe67d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -127,12 +127,12 @@ install(TARGETS st-flash st-info
        RUNTIME DESTINATION bin
 )

-if (UNIX AND (NOT CPACK_GENERATOR))
+if (LINUX)
        file(GLOB RULES_FILES etc/udev/rules.d/*.rules)
        install(FILES etc/modprobe.d/stlink_v1.conf
-       DESTINATION /etc)
+       DESTINATION etc)
        install(FILES ${RULES_FILES}
-       DESTINATION /lib/udev/rules.d/)
+       DESTINATION lib/udev/rules.d)
 endif()

 add_subdirectory(src/gdbserver)

The cmake documentation states:

install
DESTINATION
Specify the directory on disk to which a file will be installed. If a full path (with a leading slash or drive letter) is given it is used directly. If a relative path is given it is interpreted relative to the value of the CMAKE_INSTALL_PREFIX variable. The prefix can be relocated at install time using the DESTDIR mechanism explained in the CMAKE_INSTALL_PREFIX variable documentation.
https://cmake.org/cmake/help/v3.0/command/install.html

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

Sounds like it would install the files under /usr/etc and /usr/lib/udev.
This would be wrong.

Anyway I would prefer to keep normal cmake install as it is documented, so only debian packager knows about module config and udev rules installation to /lib.

Hopefully I can spends some time today.

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

It is likely for distro package maintainers to have a different guidelines for installing modprobe and udev helper files. I know it is not the best way to go how my patch works, it would be better to have seperate paths in the cmake file then.

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

You mean something like a switch for install directory.
Sounds good.
Can you provide a proposal?

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

Probably we should not apply my patch. As on the cmake mailing list they say:

No you can perfectly install in absolute dir. like /etc without trouble AND
obtain a relocatable RPM.

For this CPackRPM generates %config directive in the RPM spec file
precisely because usually when installing file using absolute path this is
for config files.
In fact this is the "RPM" way to do it.

So you CAN obtain relocatable RPM with absolutely installed path
BUT
You cannot get this kind of package if you do set CPACK_SET_DESTDIR to ON
you have to let CPackRPM do his homework alone.
http://public.kitware.com/pipermail/cmake/2011-July/045627.html

So that mean when setting CPACK_SET_DESTDIR we should be good to go for travis/cpack builds (make package target). And the debian building remains the same.

And the pack doc says exactly what we need:

Manually setting CPACK_SET_DESTDIR may help (or simply be necessary) if some install rules uses absolute DESTINATION (see CMake INSTALL command). However, starting with CPack/CMake 2.8.3 RPM and DEB installers tries to handle DESTDIR automatically so that it is seldom necessary for the user to set it.

We need an absolute destination for this.

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

Seems this way is a dead road.

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

Now I have a real patch which works (on top of this pull request):
https://github.com/xor-gate/stlink/commit/51a270334da0c91f61b83a96d39447017e516a52

@nekromant could you cherry-pick my commit into your PR and check if it works? When good you can update the pull request, squash-merge and then we should be able to close this.

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

Files didn´t get installed.

Install the project...
/usr/bin/cmake -P cmake_install.cmake
-- Install configuration: "None"
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/lib/x86_64-linux-gnu/libstlink.so.1.3.0
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/lib/x86_64-linux-gnu/libstlink.so.1
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/lib/x86_64-linux-gnu/libstlink.so
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/lib/x86_64-linux-gnu/libstlink.a
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/st-flash
-- Removed runtime path from "/home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/st-flash"
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/st-info
-- Removed runtime path from "/home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/st-info"
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/st-util
-- Removed runtime path from "/home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/st-util"
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/stlink-gui
-- Removed runtime path from "/home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/bin/stlink-gui"
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/share/stlink/stlink-gui.ui
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/lib/x86_64-linux-gnu/pkgconfig/stlink.pc
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/commands.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/logging.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/usb.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/chipid.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/backend.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/reg.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/sg.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/flash_loader.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/mmap.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/include/x86_64-linux-gnu/stlink/version.h
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/share/man/man1/st-util.1
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/share/man/man1/st-flash.1
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/share/man/man1/st-info.1
-- Installing: /home/osterlad/buildbox/stlink/stlink-nekromant/debian/tmp/usr/share/man/man1/st-term.1
make[1]: Leaving directory `/home/osterlad/buildbox/stlink/stlink-nekromant/obj-x86_64-linux-gnu'
   dh_install -O--buildsystem=cmake
dh_install: libstlink missing files (lib/udev/rules.d/*.rules), aborting

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

I'm not sure but I have not change the normal make install behaviour. Could you try to remove the conditional check: https://github.com/xor-gate/stlink/blob/deb-pkg-fixup/CMakeLists.txt#L130

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

It´s because of your change to CMakeLists.txt:
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 01f7147..bc00d18 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -127,7 +127,7 @@ install(TARGETS st-flash st-info
        RUNTIME DESTINATION bin
 )

-if (LINUX)
+if (UNIX AND (NOT CPACK_GENERATOR))
        file(GLOB RULES_FILES etc/udev/rules.d/*.rules)
        install(FILES etc/modprobe.d/stlink_v1.conf
        DESTINATION /etc)

And it works ;-)

Why do you want to change this line?

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

I'm running mostly on a mac (main machine) and I have forgotten LINUX is not defined at al.... I prefer not to have it in the package target for !Linux systems (mac, windows, bsd). As I was mixing up as APPLE is defined on my machine -_-".

We should probably replace it by:

if (CMAKE_SYSTEM_NAME STREQUAL "Linux")
...
endif ()

@nekromant
Copy link
Contributor Author

nekromant commented Oct 4, 2016

Why do you want to change this line?

@OsterlaD DESTINATION /etc breaks cpack packaging ;(

I think the only less dirty solution would be to skip the installation to /etc when running cpack, perhaps only install the configs when -DSTLINK_INSTALL_ETC=yes is passed to cmake

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

I prefer cpack packaging as you can create zipfile,debfile,rpm just with one tool. I would like not to make any difference in the CMakeLists.txt between cpack and non-cpack builds. Every non-standard option and variable introduced needs documentation and confuse packagers, power-users.

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

if (CMAKE_SYSTEM_NAME STREQUAL "Linux")
works

@nekromant I guess you mean @xor-gate

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

@nekromant do you agree to the changes suggested by @xor-gate?

…system

The top-level makefile that was used to drive cmake broke the autodetection
of the used buildsystem by debhelper. This commit fixes it and moves most
of the installation stuff to CMakeLists.txt to keep debian/ folder contents
to the bare minimum.

Signed-off-by: Andrew Andrianov <andrew@ncrmnt.org>
@nekromant
Copy link
Contributor Author

That should be it, packaging works once again. Everything's merged into one commit
@OsterlaD, please check if everything works for you as well.

@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

Looks good to me. And the Mac/Linux travis build seems to be satisfied 👍

@OsterlaD
Copy link
Contributor

OsterlaD commented Oct 4, 2016

Looks good.

@xor-gate xor-gate merged commit 2e50937 into stlink-org:master Oct 4, 2016
@xor-gate
Copy link
Member

xor-gate commented Oct 4, 2016

Finally :+), thanks guys.

@Nightwalker-87 Nightwalker-87 linked an issue Mar 17, 2020 that may be closed by this pull request
2 tasks
@Nightwalker-87 Nightwalker-87 changed the title Fix debian packaging packaging: Fix debian packaging Mar 17, 2020
@stlink-org stlink-org locked as resolved and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants