fix check in Score-P's configure scripts that may fail if the path to certain dependencies include yes or no

[bedroge]

This prevents issues with dependencies located in paths containing yes or no, see:
https://gitlab.com/score-p/scorep/-/issues/1008

I'm restricting this to versions 8.0-8.4, as @Thyre mentioned that it will probably be fixed soon, and version 7 doesn't seem to have this check.

[Thyre]

Thanks a lot for the PR. The fix certainly goes in the correct direction, but doesn't fully fix the issue. With the proposed change, passing yes or no to both --with-libbfd-include and --with-libbfd-lib will not trigger the error anymore, as both values are concatenated. Errors will then likely occur later on during the actual build process.

This is the broken check in Score-P 8.x (https://gitlab.com/score-p/scorep/-/blob/b84a3703563b61d99312169b4f6a6cef1c050005/build-config/common/m4/afs_external_lib.m4#L209):

AS_CASE([${_afs_lib_withval_lib}${_afs_lib_withval_include}],
    [*yes*|*no*], [AC_MSG_ERROR([Both, --with-_afs_lib_name-lib and --with-_afs_lib_name-include require a <path>.])],
[...]

our current idea is to replace this with the following block (not yet tested):

[AS_CASE([${_afs_lib_withval_lib},${_afs_lib_withval_include}],
    [yes,*|no,*|*,yes|*,no], [AC_MSG_ERROR([Both, --with-_afs_lib_name-lib and --with-_afs_lib_name-include require a <path>.])],
[...]

This should fix the issue reported in https://gitlab.com/score-p/scorep/-/issues/1008.

---

Fixing this in the EasyBlock will require a few more replacements aside from the one for *yes*|*no* unfortunately, as three libraries (libbfd, libunwind, liblustre) are affected by this.

However, given that the EasyBlock always tries to pass full paths for the dependencies

easybuild-easyblocks/easybuild/easyblocks/s/score_p.py

Line 146 in 80c3995

'binutils': ['--with-libbfd-include=%s/include',

this fix might be sufficient enough for now

[bedroge]

Oh, right, I overlooked that the values are concatenated. Thanks! I'll change the regex and replace it by the one you provided (yes,*|no,*|*,yes|*,no).

Fixing this in the EasyBlock will require a few more replacements aside from the one for yes|no unfortunately, as three libraries (libbfd, libunwind, liblustre) are affected by this.

The current regex should replace all *yes*|*no* occurrences in all build*/configure scripts, do you mean that doesn't cover everything? From what I can tell, these scripts (e.g. build-mpi/configure) does indeed seem to have the same check for all three libraries, and hence the regex should fix them all in the same way?

[Thyre]

The current regex should replace all yes|no occurrences in all build*/configure scripts, do you mean that doesn't cover everything?

There's a very tiny additional difference, which is needed for the check to succeed:

- AS_CASE([${_afs_lib_withval_lib}${_afs_lib_withval_include}],
+ AS_CASE([${_afs_lib_withval_lib},${_afs_lib_withval_include}],

In the resulting configure, this means that the check for libbfd, libunwind and liblustre changes slightly as well (this is from master, therefore NVTX shows up as well):

--- build-backend/configure.old 2024-10-29 12:44:11.495387487 +0100
+++ build-backend/configure     2024-10-29 12:44:40.043430447 +0100
@@ -37729,8 +37729,8 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_libbfd_lib}${with_libbfd_include} in #(
-  *yes*|*no*) :
+    case ${with_libbfd_lib},${with_libbfd_include} in #(
+  yes,*|no,*|*,yes|*,no) :
     as_fn_error $? "Both, --with-libbfd-lib and --with-libbfd-include require a <path>." "$LINENO" 5 ;; #(
   *) :
 
@@ -46862,8 +46862,8 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_libunwind_lib}${with_libunwind_include} in #(
-  *yes*|*no*) :
+    case ${with_libunwind_lib},${with_libunwind_include} in #(
+  yes,*|no,*|*,yes|*,no) :
     as_fn_error $? "Both, --with-libunwind-lib and --with-libunwind-include require a <path>." "$LINENO" 5 ;; #(
   *) :
 
@@ -53492,8 +53492,8 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_libnvToolsExt_lib}${with_libnvToolsExt_include} in #(
-  *yes*|*no*) :
+    case ${with_libnvToolsExt_lib},${with_libnvToolsExt_include} in #(
+  yes,*|no,*|*,yes|*,no) :
     as_fn_error $? "Both, --with-libnvToolsExt-lib and --with-libnvToolsExt-include require a <path>." "$LINENO" 5 ;; #(
   *) :
 
@@ -57476,8 +57476,8 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_liblustreapi_lib}${with_liblustreapi_include} in #(
-  *yes*|*no*) :
+    case ${with_liblustreapi_lib},${with_liblustreapi_include} in #(
+  yes,*|no,*|*,yes|*,no) :
     as_fn_error $? "Both, --with-liblustreapi-lib and --with-liblustreapi-include require a <path>." "$LINENO" 5 ;; #(
   *) :

From what I can tell, these scripts (e.g. build-mpi/configure) does indeed seem to have the same check for all three libraries, and hence the regex should fix them all in the same way?

That's true. We call the same check for build-mpi and build-shmem. If we add the , as well, everything should work.

[Thyre]

This additional change should be sufficient to add the ,:

$ diff -u build-backend/configure.old build-backend/configure
$ sed -i 's/_lib}${with_/_lib},${with_/g' build-*/configure
$ diff -u build-backend/configure.old build-backend/configure    
--- build-backend/configure.old	2024-10-29 12:44:11.495387487 +0100
+++ build-backend/configure	2024-10-29 12:57:00.940559462 +0100
@@ -37729,7 +37729,7 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_libbfd_lib}${with_libbfd_include} in #(
+    case ${with_libbfd_lib},${with_libbfd_include} in #(
   *yes*|*no*) :
     as_fn_error $? "Both, --with-libbfd-lib and --with-libbfd-include require a <path>." "$LINENO" 5 ;; #(
   *) :
@@ -46862,7 +46862,7 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_libunwind_lib}${with_libunwind_include} in #(
+    case ${with_libunwind_lib},${with_libunwind_include} in #(
   *yes*|*no*) :
     as_fn_error $? "Both, --with-libunwind-lib and --with-libunwind-include require a <path>." "$LINENO" 5 ;; #(
   *) :
@@ -53492,7 +53492,7 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_libnvToolsExt_lib}${with_libnvToolsExt_include} in #(
+    case ${with_libnvToolsExt_lib},${with_libnvToolsExt_include} in #(
   *yes*|*no*) :
     as_fn_error $? "Both, --with-libnvToolsExt-lib and --with-libnvToolsExt-include require a <path>." "$LINENO" 5 ;; #(
   *) :
@@ -57476,7 +57476,7 @@
 fi ;;
 esac ;; #(
       set2set3) :
-    case ${with_liblustreapi_lib}${with_liblustreapi_include} in #(
+    case ${with_liblustreapi_lib},${with_liblustreapi_include} in #(
   *yes*|*no*) :
     as_fn_error $? "Both, --with-liblustreapi-lib and --with-liblustreapi-include require a <path>." "$LINENO" 5 ;; #(
   *) :

[bedroge]

Thanks @Thyre , I was clearly too hasty here... 😅 I've added the additional regex, and also added a fix for the other one, as it didn't completely do the right thing.

I've just done a build with the latest changes, and got the following diff for these two configure scripts:

$ diff /data/eb/build/ScoreP/8.4/gompi-2023b/scorep-8.4/build-mpi/configure.orig.eb /data/eb/build/ScoreP/8.4/gompi-2023b/scorep-8.4/build-mpi/configure
35715,35716c35715,35716
<     case ${with_libbfd_lib}${with_libbfd_include} in #(
<   *yes*|*no*) :
---
>     case ${with_libbfd_lib},${with_libbfd_include} in #(
>   yes,*|no,*|*,yes|*,no) :
45108,45109c45108,45109
<     case ${with_libunwind_lib}${with_libunwind_include} in #(
<   *yes*|*no*) :
---
>     case ${with_libunwind_lib},${with_libunwind_include} in #(
>   yes,*|no,*|*,yes|*,no) :
54091,54092c54091,54092
<     case ${with_liblustreapi_lib}${with_liblustreapi_include} in #(
<   *yes*|*no*) :
---
>     case ${with_liblustreapi_lib},${with_liblustreapi_include} in #(
>   yes,*|no,*|*,yes|*,no) :


$ diff /data/eb/build/ScoreP/8.4/gompi-2023b/scorep-8.4/build-shmem/configure.orig.eb /data/eb/build/ScoreP/8.4/gompi-2023b/scorep-8.4/build-shmem/configure
35526,35527c35526,35527
<     case ${with_libbfd_lib}${with_libbfd_include} in #(
<   *yes*|*no*) :
---
>     case ${with_libbfd_lib},${with_libbfd_include} in #(
>   yes,*|no,*|*,yes|*,no) :
44732,44733c44732,44733
<     case ${with_libunwind_lib}${with_libunwind_include} in #(
<   *yes*|*no*) :
---
>     case ${with_libunwind_lib},${with_libunwind_include} in #(
>   yes,*|no,*|*,yes|*,no) :
53701,53702c53701,53702
<     case ${with_liblustreapi_lib}${with_liblustreapi_include} in #(
<   *yes*|*no*) :
---
>     case ${with_liblustreapi_lib},${with_liblustreapi_include} in #(
>   yes,*|no,*|*,yes|*,no) :

I think this is the right result?

Edit: let me also change the build-*, maybe that's a bit too aggressive if we only need to change the ones in build-mpi and build-shmem.

[Thyre]

This seems to look good now, yeah. Any objections @cfeld, @geimer?

Edit: let me also change the build-*, maybe that's a bit too aggressive if we only need to change the ones in build-mpi and build-shmem?

We need to touch build-backend, build-mpi and build-shmem, since all three do the checks. But I agree, it might be a bit too aggressive to touch all other build directories, even though it shouldn't cause any issues.

I'll do some test installations later on our systems if I find the time, hopefully today.

[bedroge]

Also applied the same regex to build-backend/configure, and a test build now shows the following diff for that one:

$ diff /data/eb/build/ScoreP/8.4/gompi-2023b/scorep-8.4/build-backend/configure.orig.eb /data/eb/build/ScoreP/8.4/gompi-2023b/scorep-8.4/build-backend/configure
37758,37759c37758,37759
<     case ${with_libbfd_lib}${with_libbfd_include} in #(
<   *yes*|*no*) :
---
>     case ${with_libbfd_lib},${with_libbfd_include} in #(
>   yes,*|no,*|*,yes|*,no) :
46770,46771c46770,46771
<     case ${with_libunwind_lib}${with_libunwind_include} in #(
<   *yes*|*no*) :
---
>     case ${with_libunwind_lib},${with_libunwind_include} in #(
>   yes,*|no,*|*,yes|*,no) :
56921,56922c56921,56922
<     case ${with_liblustreapi_lib}${with_liblustreapi_include} in #(
<   *yes*|*no*) :
---
>     case ${with_liblustreapi_lib},${with_liblustreapi_include} in #(
>   yes,*|no,*|*,yes|*,no) :

[cfeld]

This seems to look good now, yeah. Any objections @cfeld, @geimer?

Looks reasonable to me.

[bedroge]

Test report by @bedroge

Overview of tested easyconfigs (in order)

• SUCCESS Score-P-8.4-gompi-2023b.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
bob-Latitude-5300 - Linux Ubuntu 24.04.1 LTS (Noble Numbat), x86_64, Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz, Python 3.12.3
See https://gist.github.com/bedroge/a0f3ff6820a494c896326ba3d8147db7 for a full test report.

[bedroge]

@boegelbot please test @ generoso
EB_ARGS="--installpath /tmp/pr3496 Score-P-8.0-gompi-2022a.eb Score-P-8.1-gompi-2023a.eb Score-P-8.3-gompi-2022b.eb Score-P-8.4-gompi-2024a.eb"

[boegelbot]

@bedroge: Request for testing this PR well received on login1

PR test command 'EB_PR=3496 EB_ARGS="--installpath /tmp/pr3496 Score-P-8.0-gompi-2022a.eb Score-P-8.1-gompi-2023a.eb Score-P-8.3-gompi-2022b.eb Score-P-8.4-gompi-2024a.eb" EB_CONTAINER= EB_REPO=easybuild-easyblocks /opt/software/slurm/bin/sbatch --job-name test_PR_3496 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

• exit code: 0
• output:

Submitted batch job 14591

Test results coming soon (I hope)...

Details

- notification for comment with ID 2444177078 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

[bedroge]

Tested on a whole bunch of CPUs (including three Arm CPUs, all of them have no in the libbfd paths and were affected by the issue) in EESSI/software-layer#797, and all builds succeeded 🎉

[boegelbot]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS Score-P-8.0-gompi-2022a.eb
• SUCCESS Score-P-8.1-gompi-2023a.eb
• SUCCESS Score-P-8.3-gompi-2022b.eb
• FAIL (build issue) PDT-3.25.2-GCCcore-13.3.0.eb (partial log available at https://gist.github.com/boegelbot/3046ebf96bf25f5c0c044dc6fba7d89c)
• FAIL (build issue) Score-P-8.4-gompi-2024a.eb (partial log available at https://gist.github.com/boegelbot/5fe22bbb21ca20b7eee4a762b35482d5)

Build succeeded for 3 out of 5 (4 easyconfigs in total)
cns1 - Linux Rocky Linux 8.9, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/e6ab62ff7edad32300be8b3096217791 for a full test report.

[ocaisa]

LGTM

[ocaisa]

The failure in #3496 (comment) is due to a failed download of PDT and not relevant to this PR