Skip to content

setup_platform_environment before package env mods#41205

Merged
haampie merged 2 commits intospack:developfrom
haampie:fix/platform-before-pkg-env-changes
Nov 22, 2023
Merged

setup_platform_environment before package env mods#41205
haampie merged 2 commits intospack:developfrom
haampie:fix/platform-before-pkg-env-changes

Conversation

@haampie
Copy link
Copy Markdown
Member

@haampie haampie commented Nov 21, 2023

This restores the order from before where AutotoolsPackage would
override the env variable set in setup_platform_environment.

This restores the order from before where `AutotoolsPackage` would
override the env variable set in `setup_platform_environment`.
@spackbot-app spackbot-app bot added build-environment core PR affects Spack core functionality labels Nov 21, 2023
Copy link
Copy Markdown
Member

@tgymnich tgymnich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fix the issue for me.

sethrj
sethrj previously approved these changes Nov 22, 2023
Copy link
Copy Markdown
Contributor

@sethrj sethrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be improved by a comment describing why (and that!) the order matters

@haampie haampie added this to the v0.21.1 milestone Nov 22, 2023
@haampie
Copy link
Copy Markdown
Member Author

haampie commented Nov 22, 2023

I noticed that the original order was:

  • dependencies
  • platform specific stuff
  • root

I don't think that makes much sense given that setup_dependent_build_environment is called from dependencies and would be below the platform stuff, while setup_build_environment is part of root.

It may matter for cray, but I think it's unlikely, cause compiler modules and external package modules are loaded after these 3 steps anyways, so dependencies / root are Spack installed packages.

What a mess!

@haampie haampie merged commit 24a38e6 into spack:develop Nov 22, 2023
@haampie haampie deleted the fix/platform-before-pkg-env-changes branch November 22, 2023 16:32
@climbfuji climbfuji mentioned this pull request Nov 22, 2023
4 tasks
climbfuji pushed a commit to AlexanderRichert-NOAA/spack that referenced this pull request Nov 22, 2023
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
@climbfuji climbfuji mentioned this pull request Nov 22, 2023
3 tasks
haampie added a commit that referenced this pull request Nov 23, 2023
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
@haampie haampie mentioned this pull request Nov 23, 2023
36 tasks
gabrielctn pushed a commit to gabrielctn/spack that referenced this pull request Nov 24, 2023
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
mtaillefumier pushed a commit to mtaillefumier/spack that referenced this pull request Dec 14, 2023
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
alalazo pushed a commit that referenced this pull request Jan 10, 2024
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 31, 2024
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
haampie added a commit to haampie/spack that referenced this pull request Feb 28, 2024
The deps were added in spack#40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people the
autoreconf script caused issues, later fixed in spack#41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

I did not check what that error is about exactly, but it looks like
Spack's `def autoreconf` step does way less than the `autgen.sh` script
provided in the source tarball.

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since spack#41205, therefore, get rid of all this madness.
alalazo pushed a commit that referenced this pull request Feb 29, 2024
The deps were added in #40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people reported the
autoreconf script caused issues, later fixed in #41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since #41205.
mathomp4 pushed a commit to mathomp4/spack that referenced this pull request Mar 27, 2024
The deps were added in spack#40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people reported the
autoreconf script caused issues, later fixed in spack#41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since spack#41205.
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
The deps were added in spack#40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people reported the
autoreconf script caused issues, later fixed in spack#41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since spack#41205.
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
This roughly restores the order of operation from Spack 0.20,
where where `AutotoolsPackage.setup_build_environment` would
override the env variable set in `setup_platform_environment` on
macOS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build-environment core PR affects Spack core functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants