build nodejs with OpenSSL and ICU provided as proper dependencies #16529

lexming · 2022-11-02T23:34:21Z

(created using eb --new-pr)

Changelog:

build nodejs with external OpenSSL to easily get security fixes
enable full ICU support, fixes issue “jupyter lab build” fails with Invalid regular expression jupyterlab/jupyterlab#10487
reduce duplication in postinstallcmds
enhance sanity checks by also checking the added symlinks

lexming · 2022-11-03T00:33:06Z

@boegelbot: please test @ generoso

boegelbot · 2022-11-03T00:35:08Z

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

PR test command 'EB_PR=16529 EB_ARGS= EB_CONTAINER= /opt/software/slurm/bin/sbatch --job-name test_PR_16529 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

exit code: 0
output:

Submitted batch job 9420

Test results coming soon (I hope)...

- notification for comment with ID 1301521887 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).

boegelbot · 2022-11-03T03:35:37Z

Test report by @boegelbot
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
cns3 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/2a28a1cf5a4afa3fee8fcdbf1206a9cc for a full test report.

verdurin · 2022-11-11T16:45:42Z

easybuild/easyconfigs/n/nodejs/nodejs-14.17.0-GCCcore-10.3.0.eb

@@ -2,7 +2,7 @@ easyblock = 'ConfigureMake'

 name = 'nodejs'
 version = '14.17.0'  # LTS on 2021-06-03
-local_libversion = '83'
+_libversion = '83'


@lexming why this change?

I systematically remove local from custom variables in easyconfigs since the change allowing _variable as name. It's a better format.

I think we're pretty consistent in using local_* for local variables in easyconfigs, so we should stick to that in the central easyconfigs repo, I think... "It's a better format" is highly subjective, no?

That said, it's not like we should be looking for reasons to keep PRs open longer either.

So as long as we don't enforce either _* or local_* in the tests, this shouldn't block a PR from being merged.

Ok, wasted some more time prefixing those variables. Keep in mind that I cannot justify spending tens of hours on minor PRs for easyconfigs. Time is not infinite. Moreover we actively allowed _* for custom variables some time ago, so that should be accepted without questions.

It's not wasted effort, because I think we should actually enforce the use of local_* in the easyconfig test suite.

EasyBuild allows using either local_* or _*, but in the central easyconfigs repo it makes sense to be a bit more strict to keep some consistency across easyconfigs we include. Similar, in some sense, to how we require use_pip, and restrict dependency versions in a particular easyconfig generation.

Some "stats": there's currently 205 easyconfigs that use _* for local variables, and 3910 that use local_*.

So it's not consistent currently, but making it consistent should definitely go towards using local_* everywhere instead of _*.

@boegel any ideas on the ratio for recent (new) contributions? From my experience, a lot of the local_ use is in bumped older ECs
edit for reference: amongst recent-ish PRs #(13000-current) that used the "new" label, use of local_* was only ~3x as common as _*

verdurin · 2022-11-11T16:46:25Z

easybuild/easyconfigs/n/nodejs/nodejs-16.15.1-GCCcore-11.3.0.eb

@@ -2,7 +2,7 @@ easyblock = 'ConfigureMake'

 name = 'nodejs'
 version = '16.15.1'  # LTS on 2022-06-12
-local_libversion = '93'
+_libversion = '93'


@lexming also here - why the change?

I systematically remove local from custom variables in easyconfigs since the change allowing _variable as name. It's a better format.

…asyconfigs into 20221103003408_new_pr_nodejs14170

lexming · 2022-11-23T11:55:07Z

@boegelbot: please test @ generoso

boegelbot · 2022-11-23T12:00:11Z

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

PR test command 'EB_PR=16529 EB_ARGS= EB_CONTAINER= /opt/software/slurm/bin/sbatch --job-name test_PR_16529 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

exit code: 0
output:

Submitted batch job 9652

Test results coming soon (I hope)...

- notification for comment with ID 1324944685 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).

boegel · 2022-11-23T14:52:29Z

Test report by @boegel
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
node3106.skitty.os - Linux RHEL 8.4, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/079da90b6f974cc1c454586a83ae5dfc for a full test report.

boegelbot · 2022-11-23T14:58:34Z

Test report by @boegelbot
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
cns3 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/a67a9393604fc31df2cb7c9a4fa2d4bf for a full test report.

boegel

lgtm

boegel · 2022-11-23T15:42:12Z

Going in, thanks @lexming!

jfgrimm · 2022-11-23T16:02:21Z

Test report by @jfgrimm
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
node148.pri.viking.alces.network - Linux CentOS Linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/f2286b97a4c05684869e84c6c94636eb for a full test report.

build nodejs with external OpenSSL and ICU

d73fe2e

lexming added enhancement bug fix labels Nov 2, 2022

lexming added this to the next release (4.6.3?) milestone Nov 2, 2022

easybuilders deleted a comment from boegelbot Nov 2, 2022

add patch to fix ICU v69 support in nodejs v14

9a40329

easybuilders deleted a comment from boegelbot Nov 3, 2022

fix dependency on OpenSSL to use SYSTEM template

d2457c0

easybuilders deleted a comment from boegelbot Nov 3, 2022

This was referenced Nov 7, 2022

{tools}[GCCcore/11.3.0] jupyterlmod v3.0.0, jupyterlab-lmod v1.0.2 w/ Python 3.10.4 #16563

Merged

{tools}[GCCcore/10.3.0] jupyterlab-lmod v1.0.2, jupyterlmod v3.0.0, jupyter-server v1.13.4 + add jupyter-server dependency to JupyterLab #16562

Closed

verdurin reviewed Nov 11, 2022

View reviewed changes

lexming requested a review from verdurin November 21, 2022 12:50

boegel changed the title ~~build nodejs with external OpenSSL and ICU~~ build nodejs with OpenSSL and ICU provided as proper dependencies Nov 23, 2022

Merge branch 'develop' of https://github.com/easybuilders/easybuild-e…

8b425be

…asyconfigs into 20221103003408_new_pr_nodejs14170

lexming force-pushed the 20221103003408_new_pr_nodejs14170 branch from 9b1a7aa to a8a950f Compare November 23, 2022 11:47

prefix custom parameters with local in nodejs

811ef35

lexming force-pushed the 20221103003408_new_pr_nodejs14170 branch from a8a950f to 811ef35 Compare November 23, 2022 11:48

boegel mentioned this pull request Nov 23, 2022

{lang}[GCCcore/12.2.0] nodejs v18.12.1 #16713

Merged

boegel approved these changes Nov 23, 2022

View reviewed changes

boegel merged commit 636837b into easybuilders:develop Nov 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build nodejs with OpenSSL and ICU provided as proper dependencies #16529

build nodejs with OpenSSL and ICU provided as proper dependencies #16529

lexming commented Nov 2, 2022 •

edited

Loading

lexming commented Nov 3, 2022

boegelbot commented Nov 3, 2022

boegelbot commented Nov 3, 2022

verdurin Nov 11, 2022

lexming Nov 11, 2022

boegel Nov 23, 2022

lexming Nov 23, 2022 •

edited

Loading

boegel Nov 23, 2022

boegel Nov 23, 2022

jfgrimm Nov 23, 2022 •

edited

Loading

verdurin Nov 11, 2022

lexming Nov 11, 2022

lexming commented Nov 23, 2022

boegelbot commented Nov 23, 2022

boegel commented Nov 23, 2022

boegelbot commented Nov 23, 2022

boegel left a comment

boegel commented Nov 23, 2022

jfgrimm commented Nov 23, 2022

build nodejs with OpenSSL and ICU provided as proper dependencies #16529

build nodejs with OpenSSL and ICU provided as proper dependencies #16529

Conversation

lexming commented Nov 2, 2022 • edited Loading

lexming commented Nov 3, 2022

boegelbot commented Nov 3, 2022

boegelbot commented Nov 3, 2022

verdurin Nov 11, 2022

Choose a reason for hiding this comment

lexming Nov 11, 2022

Choose a reason for hiding this comment

boegel Nov 23, 2022

Choose a reason for hiding this comment

lexming Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

boegel Nov 23, 2022

Choose a reason for hiding this comment

boegel Nov 23, 2022

Choose a reason for hiding this comment

jfgrimm Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

verdurin Nov 11, 2022

Choose a reason for hiding this comment

lexming Nov 11, 2022

Choose a reason for hiding this comment

lexming commented Nov 23, 2022

boegelbot commented Nov 23, 2022

boegel commented Nov 23, 2022

boegelbot commented Nov 23, 2022

boegel left a comment

Choose a reason for hiding this comment

boegel commented Nov 23, 2022

jfgrimm commented Nov 23, 2022

lexming commented Nov 2, 2022 •

edited

Loading

lexming Nov 23, 2022 •

edited

Loading

jfgrimm Nov 23, 2022 •

edited

Loading