build: resurrect `make deb` #1082

grondo · 2023-09-28T14:37:37Z

This PR brings back the make deb target, which we've been using to build and install packages on test systems.

A make dist target, required by scripts/debbuild.sh, is added (and is now used by make distcheck), and make dist is fixed to work with slightly older versions of git-archive that do not support the --add-virtual-file option. Finally, the build system is forced to cmake in debian/rules since the fake configure script doesn't support all the requisite options.

grondo · 2023-09-28T17:55:51Z

Hold off on merging this until #1083 is solved

trws

Oh, I missed that you had already put in a fix for #1083 here @grondo, I'm good with this as a fix as well if it takes care of the issue. Thanks for the fix!

grondo · 2023-09-29T14:25:27Z

Thanks @trws! The one issue I'm still fighting with is #1087.

So far, from lots of online searching I've come up with this as the best way to install a link:

diff --git a/src/python/CMakeLists.txt b/src/python/CMakeLists.txt
index b4228f69..3efd2dd4 100644
--- a/src/python/CMakeLists.txt
+++ b/src/python/CMakeLists.txt
@@ -12,3 +12,22 @@ install(DIRECTORY fluxion
   DESTINATION "${PYTHON_INSTALL_SITELIB}"
   FILES_MATCHING PATTERN "*.py")
 
+set(PYNAME "python${Python_VERSION_MAJOR}.${Python_VERSION_MINOR}")
+set(PYDIR "${CMAKE_INSTALL_FULL_LIBDIR}/flux/${PYNAME}")
+set(LINKTARGET "${PYTHON_INSTALL_SITELIB}/fluxion")
+
+INSTALL(CODE "\
+  execute_process( \
+  COMMAND \
+    mkdir -p \$ENV{DESTDIR}/${PYDIR} \
+  )
+  execute_process( \
+  COMMAND \
+    rm -f \$ENV{DESTDIR}/${PYDIR}/fluxion \
+  )
+  MESSAGE (STATUS \
+    \"linking ${PYDIR}/fluxion -> ${PYTHON_INSTALL_SITELIB}/fluxion\")
+  execute_process( \
+  COMMAND \
+    ln -s ${LINKTARGET} \$ENV{DESTDIR}/${PYDIR}/fluxion \
+  )")

Actually I was going to report that this works for make install DESTDIR= but not for make deb, but this morning it started working in both cases. However, this seems to be magically working in both cases now, so I'll probably incorporate these changes here. However, does the above look correct to you?

Also, is there some way to prevent cmake from rebuilding the whole project when you touch one file? That makes it really painful to work on the install target when you end up building from scratch with each test (especially with Fluxion)...

Problem: dh detects an autoconf build system when building a debian package, but the fake 'configure' script doesn't support all the necessary options to build a package. Since dh appears to support cmake, force a cmake build system so the package build works.

Problem: The 'make deb' target was dropped in the conversion to cmake, but having an easy way to build test .deb packages has become an essential component of testing. Add a custom 'deb' target to CmakeLists.txt. Since this target requires 'make dist', add a 'dist' custom target as well. Some versions of git do not support the `--add-virtual-file` option, so instead of using that to add the flux-sched.ver file to the archive, just create the file, add it, and subsequently remove it. Fixes flux-framework#1072

Problem: The cmake-based build installs flux-ion-R.py to <libdir>/libexec/cmd/ instead of <libexecdir>/flux/cmd. Fix FLUX_CMD_DIR so that this program is installed to the correct path. Fixes flux-framework#1083

Problem: In the previous autotools based build system, a soft link from libdir/python<ver>/fluxion was created to point to the real fluxion directory in python site packages. This was done because the flux(1) command driver actually prepends this non-standard PATH to PYTHONPATH in order to prepend just the path to the Flux modules instead of the path to all site-installed modules. In the conversion to cmake this link was dropped from `make install`. Re-establish this python link in `make install`. Fixes flux-framework#1087

Problem: The build of flux-sched now requires cmake, but this is not reflected in debian/control. Add cmake to build requirements in debian/control.

Problem: dh_auto_configure(1) does not seem to set libexecdir automatically for package builds. Add -DCMAKE_INSTALL_LIBEXECDIR to get the correct libexecdir for debian systems in the cmake based build.

codecov · 2023-09-29T14:52:07Z

Codecov Report

Merging #1082 (01e1ad5) into master (c51fc86) will decrease coverage by 0.1%.
The diff coverage is n/a.

❗ Current head 01e1ad5 differs from pull request most recent head 74c2219. Consider uploading reports for the commit 74c2219 to get more accurate results

@@           Coverage Diff            @@
##           master   #1082     +/-   ##
========================================
- Coverage    71.7%   71.7%   -0.1%     
========================================
  Files          89      89             
  Lines       11665   11665             
========================================
- Hits         8374    8371      -3     
- Misses       3291    3294      +3

see 1 file with indirect coverage changes

grondo · 2023-09-29T14:59:25Z

Ok, I've pushed the latest hopefully working set of changes. @garlick, want to give this branch a try?

trws · 2023-09-29T15:00:35Z

That looks reasonable. Usually it's best practice to use the cmake -E ... commands for that kind of thing for portability, but we require a unix-like environment in so many places it really doesn't matter.

I'm looking into the rebuild issue. To be honest I'm not sure, I have a feeling it's being defensive due to the use of some of the custom commands, but I'm not completely sure why yet.

trws · 2023-09-29T15:05:21Z

Blarg, yes, there are some dirty edges because I missed specifying explicit inputs for a couple of targets. Hopefully that will take care of it once I clean those up.

garlick · 2023-09-29T15:28:18Z

Looking good! I did notice a couple formerly "noinst" programs have leaked into the install

-rwxr-xr-x root/root     35392 2023-09-29 16:04 ./usr/bin/grug2dot
-rwxr-xr-x root/root    495480 2023-09-29 16:04 ./usr/bin/resource-query

but that's not an issue with the deb per se so I'll approve and set MWP.

Thank you! It will be good to be able to install test releases of fluxion on my test cluster again.

garlick

👍

grondo · 2023-09-29T19:31:30Z

Awesome. Thanks @trws and @garlick!

grondo force-pushed the make-deb-fix branch from 2a488bc to 32ebb75 Compare September 28, 2023 14:38

grondo requested a review from trws September 28, 2023 15:25

grondo force-pushed the make-deb-fix branch from 32ebb75 to 0b76610 Compare September 28, 2023 15:45

garlick mentioned this pull request Sep 28, 2023

not ok 6 - no jobs received alloc-check exception #1084

Closed

trws approved these changes Sep 29, 2023

View reviewed changes

grondo added 6 commits September 29, 2023 07:32

build: fix install path for flux-ion-R(1)

dfaf59e

Problem: The cmake-based build installs flux-ion-R.py to <libdir>/libexec/cmd/ instead of <libexecdir>/flux/cmd. Fix FLUX_CMD_DIR so that this program is installed to the correct path. Fixes flux-framework#1083

debian: add cmake to build requirements

627f076

Problem: The build of flux-sched now requires cmake, but this is not reflected in debian/control. Add cmake to build requirements in debian/control.

debian: set libexecdir for deb build

74c2219

Problem: dh_auto_configure(1) does not seem to set libexecdir automatically for package builds. Add -DCMAKE_INSTALL_LIBEXECDIR to get the correct libexecdir for debian systems in the cmake based build.

grondo force-pushed the make-deb-fix branch from 4719df2 to 74c2219 Compare September 29, 2023 14:39

garlick approved these changes Sep 29, 2023

View reviewed changes

garlick added the merge-when-passing mergify.io - merge PR automatically once CI passes label Sep 29, 2023

mergify bot merged commit d956e50 into flux-framework:master Sep 29, 2023
19 checks passed

garlick mentioned this pull request Sep 29, 2023

build: don't install grug2dot and resource-query #1089

Closed

trws mentioned this pull request Sep 29, 2023

build: reduce rebuilds, don't install dev tools #1090

Merged

grondo deleted the make-deb-fix branch September 29, 2023 19:30

grondo mentioned this pull request Oct 3, 2023

cmake: use GNUInstallDirs to normalize libexecdir #1088

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: resurrect `make deb` #1082

build: resurrect `make deb` #1082

grondo commented Sep 28, 2023

grondo commented Sep 28, 2023

trws left a comment

grondo commented Sep 29, 2023 •

edited

Loading

codecov bot commented Sep 29, 2023

grondo commented Sep 29, 2023

trws commented Sep 29, 2023

trws commented Sep 29, 2023

garlick commented Sep 29, 2023

garlick left a comment

grondo commented Sep 29, 2023

build: resurrect make deb #1082

build: resurrect make deb #1082

Conversation

grondo commented Sep 28, 2023

grondo commented Sep 28, 2023

trws left a comment

Choose a reason for hiding this comment

grondo commented Sep 29, 2023 • edited Loading

codecov bot commented Sep 29, 2023

Codecov Report

grondo commented Sep 29, 2023

trws commented Sep 29, 2023

trws commented Sep 29, 2023

garlick commented Sep 29, 2023

garlick left a comment

Choose a reason for hiding this comment

grondo commented Sep 29, 2023

build: resurrect `make deb` #1082

build: resurrect `make deb` #1082

grondo commented Sep 29, 2023 •

edited

Loading