Use self.start_dir in LLVM easyblock

[Flamefire]

self.cfg['start_dir'] gets initialized in guess_start_dir and hence is always set to an absolute path, so non-empty.
Note that path.join(anything, abs-path) == abs-path

So llvm_src_dir is always equal to self.start_dir and we can remove this property.

@Crivella am I missing anything?

I'm also wondering about the src extension in llvm-project-%s.src. Haven't seen that in my tests anywhere and it likely didn't cause issues due to the above which means that code path wasn't used.

[Crivella]

Umh if start_dir is always set i guess the else part would not be used

I'm also wondering about the src extension in llvm-project-%s.src

All the sources are of the type llvm-project-XXX.src.tar.xz and gets uncompressed to the llvm-project-XXX.src in the build directory (Not sure if you are seeing a different behavior)

Note that path.join(anything, abs-path) == abs-path

We could add a check to be more transparent about this behavior and not do the join in case of an abspath.
I think we still need the join as the starting directory could be different from the base one in case of other builds like the ROCM-llvm
@bedroge can probably comment on this

[Flamefire]

Umh if start_dir is always set i guess the else part would not be used

It is and even checked for existence: https://github.com/easybuilders/easybuild-framework/blob/87b3d9d959b5f3ec739f3db4a350982b531c58fb/easybuild/framework/easyblock.py#L2299-L2338

All the sources are of the type llvm-project-XXX.src.tar.xz and gets uncompressed to the llvm-project-XXX.src in the build directory (Not sure if you are seeing a different behavior)

Ah, the release sources yes. I was using a specific commit (required for Triton) where that extension is missing.

Note that path.join(anything, abs-path) == abs-path

We could add a check to be more transparent about this behavior and not do the join in case of an abspath. I think we still need the join as the starting directory could be different from the base one in case of other builds like the ROCM-llvm @bedroge can probably comment on this

start_dir is always absolute, at least now. Do you know a situation where either llvm*.tar.gz is NOT the first source of the easyconfig/component or where you want a different folder?
I found a bug which might be related, or the actual cause, of #3680 for which you designed the fix: For Bundle-components start_dir wasn't set correctly and always set to builddir. So I guess that might have been why this piece of code was required.

I'm fixing that in #3769

[Flamefire]

After some more work on the Bundle easyblock I don't see a way to fix it without breaking existing easyconfigs/easyblocks.

But it still stands: self.cfg['start_dir'] is always an absolute path at this point, so this PR doesn't introduce an effective change, and just cleans up code that might lead to misunderstandings

BTW: I haven't found any easyconfigs using LLVM in components. Do you know any?

[Flamefire]

Test report by @Flamefire
'Failed to process easyconfig /home/s3248973/.local/EasyBuildDev/easybuild-easyconfigs/easybuild/easyconfigs/l/LLVM/LLVM-14.0.3-GCCcore-11.3.0.eb: You may have some typos in your easyconfig file: use_polly -> usepolly'
Failed during parsing of the easyconfigs, so no ecs were built (5 easyconfigs in total)
i7007 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor (zen2), Python 3.9.18
See https://gist.github.com/Flamefire/107c2e420c8bf07b3acb4d023188d6a7 for a full test report.

[Thyre]

BTW: I haven't found any easyconfigs using LLVM in components. Do you know any?

ROCm-LLVM will, see:

https://github.com/Thyre/easybuild-custom/blob/support-passing-amdgcn/easybuild/easyconfigs/r/ROCm-LLVM/ROCm-LLVM-6.4.1-GCCcore-14.2.0.eb

This order is required, due to the weird handling of dependencies by AMD.

CC @bedroge

[Flamefire]

I'm convinced that would still work: You use the (usual) workaround of manually setting start_dir (because the Bundle easyblock doesn't):
https://github.com/Thyre/easybuild-custom/blob/8d141968038f4fa48026f09f284e80969b1b2ca7/easybuild/easyconfigs/r/ROCm-LLVM/ROCm-LLVM-6.4.1-GCCcore-14.2.0.eb#L66

So it will be set by the point it gets to the if here and the else is also not used nor required

[Flamefire]

Test report by @Flamefire

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-14.0.3-GCCcore-11.3.0.eb
• SUCCESS LLVM-16.0.6-GCCcore-12.3.0.eb
• SUCCESS LLVM-16.0.6-GCCcore-13.2.0.eb
• FAIL (build issue) LLVM-18.1.8-GCCcore-13.3.0.eb (partial log available at https://gist.github.com/Flamefire/08f9bbda90fd581e868f9668287dd752)
• FAIL (build issue) LLVM-20.1.4-GCCcore-13.3.0.eb (partial log available at https://gist.github.com/Flamefire/a8d0efee2c24b1a14f1eb73fcdd7f2ac)

Build succeeded for 3 out of 5 (5 easyconfigs in total)
n1148 - Linux RHEL 8.9 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (sapphirerapids), Python 3.9.18
See https://gist.github.com/Flamefire/18eade8918e08d191f429477f2087851 for a full test report.

Failure in LLVM 20 caused by #3758 fixed in #3755

Failure in 18.1 is present before: #3741 (comment)

[Flamefire]

Test report by @Flamefire

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-14.0.3-GCCcore-11.3.0.eb
• SUCCESS LLVM-16.0.6-GCCcore-12.3.0.eb
• SUCCESS LLVM-16.0.6-GCCcore-13.2.0.eb
• SUCCESS LLVM-18.1.8-GCCcore-13.3.0.eb
• FAIL (build issue) LLVM-20.1.4-GCCcore-13.3.0.eb (partial log available at https://gist.github.com/Flamefire/7e9bfec627a06e30bd66c7c9ace5dcd7)

Build succeeded for 4 out of 5 (5 easyconfigs in total)
i7016 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor (zen2), Python 3.9.18
See https://gist.github.com/Flamefire/38d79f8eaee632170a77e4f8042db58f for a full test report.

Failure caused by #3758 fixed in #3755

[Flamefire]

Test report by @Flamefire

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-20.1.4-GCCcore-13.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
n1071 - Linux RHEL 8.9 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (sapphirerapids), Python 3.9.18
See https://gist.github.com/Flamefire/df5d842c70fb7d1058d69e4f58fc4d18 for a full test report.

[Flamefire]

Test report by @Flamefire

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-20.1.4-GCCcore-13.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
i7008 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor (zen2), Python 3.9.18
See https://gist.github.com/Flamefire/48d270a6b47f4881b9d0b7d9607e5485 for a full test report.

[Flamefire]

Can this be merged or do you want further tests or information? @Thyre @bedroge

[Thyre]

I'm not sure if the first commit (f710d05) can have any side-effects. I remember @Crivella adding them explicitly in 94b960b, but I don't remember why.

Looks good to me otherwise.

[Crivella]

For me I am fine with the if/else being removed but i would leave the llvm_src_dir attribute as is, as it is already being used in other derived easyblocks (even if not upstream), and in general i think it is good to keep it separate from start_dir as it makes clearer that attribute points to the source directory of the llvm project (especially if in the future we might want to add some third party project/sources to the build).

[Crivella]

I'm not sure if the first commit (f710d05) can have any side-effects. I remember @Crivella adding them explicitly in 94b960b, but I don't remember why.

That can be removed, at some point i also added it to being called inside _create_compiler_config_file for safety but did not remove the calls outside

easybuild-easyblocks/easybuild/easyblocks/l/llvm.py

Lines 838 to 840 in f710d05

	def _create_compiler_config_file(self, installdir):
	"""Create a config file for the compiler to point to the correct GCC installation."""
	self._set_gcc_prefix()

[Flamefire]

I would leave the llvm_src_dir attribute as is, as it is already being used in other derived easyblocks (even if not upstream), and in general i think it is good to keep it separate from start_dir as it makes clearer that attribute points to the source directory of the llvm project (especially if in the future we might want to add some third party project/sources to the build).

Fully agreed on the first point to not break anyone.
Not sure I understand the concern of the second part. Even the official sources contain "clang", "flang", "llvm", "openmp", etc. folders. So I would expect llvm_src_dir to be start_dir/llvm, so not using it makes it clear it is the the folder of the first source (which it currently is, probably except for some uses of Bundles where LLVM isn't the first component where it would be build_dir)
Having more names for the same thing could also be confusing itself and I can't see what we could change it too, especially without breaking existing easyconfigs.

especially if in the future we might want to add some third party project/sources to the build

That wouldn't affect start_dir unless we put them before the the source of the llvm-project for which I can't see any reason.
So I'd say this is a safe assumption.

I added a property (which is read-only by default) for existing, derived easyblocks. Does that work for you?

That can be removed, at some point i also added it to being called inside _create_compiler_config_file for safety but did not remove the calls outside

That was what I thought. I'd even go further and use a property which initializes itself on access so we never end up reading None when we forget a call to _set_gcc_prefix

[Crivella]

That was what I thought. I'd even go further and use a property which initializes itself on access so we never end up reading None when we forget a call to _set_gcc_prefix

I think that would be a good idea, but i would leave that for a separate PR

The second part is in case we download extra packages outside of the llvm project, but I agree it is fine like this and in case we fixit in the future.

Gonna do one final bot run and than merge this

[Crivella]

@boegelbot please test @ jsc-zen3
EB_ARGS="--installpath /tmp/$USER/ebpr-3770v2 LVM-16.0.6-GCCcore-12.3.0.eb"

[boegelbot]

@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3770 EB_ARGS="--installpath /tmp/$USER/ebpr-3770v2 LVM-16.0.6-GCCcore-12.3.0.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3770 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6962

Test results coming soon (I hope)...

Details

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

[Crivella]

@boegelbot please test @ jsc-zen3
EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LLVM-20.1.5-GCCcore-13.3.0.eb"
CORE_CNT=16

[Flamefire]

I think that would be a good idea, but i would leave that for a separate PR

I opened a PR for that: #3793

[boegelbot]

@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3770 EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LLVM-20.1.5-GCCcore-13.3.0.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3770 --ntasks="16" ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6963

Test results coming soon (I hope)...

Details

- notification for comment with ID 2996187054 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]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-20.1.5-GCCcore-13.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/c0b08473815b207a78511a73dc67d0c6 for a full test report.

[Crivella]

@boegelbot please test @ jsc-zen3
EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LVM-16.0.6-GCCcore-12.3.0.eb"

[boegelbot]

@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3770 EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LVM-16.0.6-GCCcore-12.3.0.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3770 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6974

Test results coming soon (I hope)...

Details

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

[Flamefire]

@boegelbot please test @ jsc-zen3 EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LVM-16.0.6-GCCcore-12.3.0.eb"

You have a missing L in the easyconfig name.

[boegelbot]

@Flamefire: I noticed your comment, but I only dance when @akesandgren or @bartoldeman or @bedroge or @boegel or @branfosj or @casparvl or @Crivella or @jfgrimm or @lexming or @Micket or @migueldiascosta or @ocaisa or @SebastianAchilles or @smoors or @verdurin or @WilleBell or @robert-mijakovic or @deniskristak or @ItIsI-Orient or @PetrKralCZ or @sassy-crick or @laraPPr or @pavelToman or @Louwrensth or @Thyre tells me (for now), I'm sorry...

Details

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

[Thyre]

@boegelbot please test @ jsc-zen3
EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LLVM-16.0.6-GCCcore-12.3.0.eb"

[boegelbot]

@Thyre: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3770 EB_ARGS="--installpath /tmp/$USER/ebpr-3770 LLVM-16.0.6-GCCcore-12.3.0.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3770 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6975

Test results coming soon (I hope)...

Details

- notification for comment with ID 2999133091 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]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-16.0.6-GCCcore-12.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/241906c51da5ba217d29da02955078e2 for a full test report.

[Crivella]

LGTM

[Crivella]

Going in, thanks @Flamefire!

[Flamefire]

Thanks!

#3793 updated after merge

[Flamefire]

Test report by @Flamefire

Overview of tested easyconfigs (in order)

• SUCCESS LLVM-19.1.7-GCCcore-13.3.0.eb
• SUCCESS LLVM-20.1.5-GCCcore-13.3.0.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
c14 - Linux AlmaLinux 9.4, x86_64, AMD EPYC 9334 32-Core Processor (zen4), 4 x NVIDIA NVIDIA H100, 560.35.03, Python 3.9.18
See https://gist.github.com/Flamefire/bd65526d487f917e5a38e44fc7d8dbd0 for a full test report.