Skip to content

config: make path replacements lazy#34758

Merged
tgamblin merged 2 commits intodevelopfrom
indirect-replacements
Jan 13, 2023
Merged

config: make path replacements lazy#34758
tgamblin merged 2 commits intodevelopfrom
indirect-replacements

Conversation

@tgamblin
Copy link
Copy Markdown
Member

@tgamblin tgamblin commented Jan 1, 2023

Currently, all of the replacements in spack.util.path.replacements() get evaluated for each replacement. This makes it easy to get bootstrap issues, because config is used very early on in Spack.

Right now, if I run test_autotools_gnuconfig_replacement_no_gnuconfig on my M1 mac, I get the circular reference error below. This fixes the issue by making all of the path replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for $env -- it's consistent with other substitutions now.

  • make all path replacements() lazy
  • clean up handling of $env
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError

@tgamblin tgamblin requested review from alalazo and becker33 January 1, 2023 00:51
@spackbot-app spackbot-app bot added core PR affects Spack core functionality utilities labels Jan 1, 2023
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Jan 2, 2023

@spackbot fix style

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Jan 2, 2023

Let me see if I can fix that for you!

@alalazo alalazo self-assigned this Jan 2, 2023
@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Jan 2, 2023

I was able to run spack style --fix for you!

spack style --fix
==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  lib/spack/spack/util/path.py
==> Running isort checks
Fixing /tmp/tmpokzax4_7/spack/lib/spack/spack/util/path.py
  isort checks were clean
==> Running mypy checks
Success: no issues found in 572 source files
  mypy checks were clean
==> Running black checks
All done! ✨ 🍰 ✨
1 file left unchanged.
  black checks were clean
==> Running flake8 checks
lib/spack/spack/util/path.py:58: [E302] expected 2 blank lines, found 1
  flake8 found errors
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

alalazo
alalazo previously requested changes Jan 2, 2023
Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

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

LGTM, but there's still a style issue (blank spaces) that spackbot can't fix

becker33
becker33 previously approved these changes Jan 10, 2023
Currently, all of the replacements in `spack.util.path.replacements()` get evaluated for
each replacement. This makes it easy to get bootstrap issues, because config is used
very early on in Spack.

Right now, if I run `test_autotools_gnuconfig_replacement_no_gnuconfig` on my M1 mac, I
get the circular reference error below. This fixes the issue by making all of the path
replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for `$env` -- it's consistent with
other substitutions now.

- [x] make all path `replacements()` lazy
- [x] clean up handling of `$env`

```console
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @Property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError
```
@tgamblin tgamblin force-pushed the indirect-replacements branch from 5958227 to 9b19b71 Compare January 10, 2023 23:11
becker33
becker33 previously approved these changes Jan 10, 2023
@tgamblin tgamblin enabled auto-merge (squash) January 10, 2023 23:41
@tgamblin tgamblin merged commit 88a604e into develop Jan 13, 2023
@tgamblin tgamblin deleted the indirect-replacements branch January 13, 2023 18:27
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 24, 2023
Currently, all of the replacements in `spack.util.path.replacements()` get evaluated for
each replacement. This makes it easy to get bootstrap issues, because config is used
very early on in Spack.

Right now, if I run `test_autotools_gnuconfig_replacement_no_gnuconfig` on my M1 mac, I
get the circular reference error below. This fixes the issue by making all of the path
replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for `$env` -- it's consistent with
other substitutions now.

- [x] make all path `replacements()` lazy
- [x] clean up handling of `$env`

```console
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @Property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError
```
nhanford pushed a commit to nhanford/spack that referenced this pull request Feb 15, 2023
Currently, all of the replacements in `spack.util.path.replacements()` get evaluated for
each replacement. This makes it easy to get bootstrap issues, because config is used
very early on in Spack.

Right now, if I run `test_autotools_gnuconfig_replacement_no_gnuconfig` on my M1 mac, I
get the circular reference error below. This fixes the issue by making all of the path
replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for `$env` -- it's consistent with
other substitutions now.

- [x] make all path `replacements()` lazy
- [x] clean up handling of `$env`

```console
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @Property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError
```
amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023
Currently, all of the replacements in `spack.util.path.replacements()` get evaluated for
each replacement. This makes it easy to get bootstrap issues, because config is used
very early on in Spack.

Right now, if I run `test_autotools_gnuconfig_replacement_no_gnuconfig` on my M1 mac, I
get the circular reference error below. This fixes the issue by making all of the path
replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for `$env` -- it's consistent with
other substitutions now.

- [x] make all path `replacements()` lazy
- [x] clean up handling of `$env`

```console
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @Property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError
```
techxdave pushed a commit to Tech-XCorp/spack that referenced this pull request Feb 17, 2023
Currently, all of the replacements in `spack.util.path.replacements()` get evaluated for
each replacement. This makes it easy to get bootstrap issues, because config is used
very early on in Spack.

Right now, if I run `test_autotools_gnuconfig_replacement_no_gnuconfig` on my M1 mac, I
get the circular reference error below. This fixes the issue by making all of the path
replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for `$env` -- it's consistent with
other substitutions now.

- [x] make all path `replacements()` lazy
- [x] clean up handling of `$env`

```console
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @Property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError
```
jmcarcell pushed a commit to key4hep/spack that referenced this pull request Apr 13, 2023
Currently, all of the replacements in `spack.util.path.replacements()` get evaluated for
each replacement. This makes it easy to get bootstrap issues, because config is used
very early on in Spack.

Right now, if I run `test_autotools_gnuconfig_replacement_no_gnuconfig` on my M1 mac, I
get the circular reference error below. This fixes the issue by making all of the path
replacements lazy lambdas.

As a bonus, this cleans up the way we do substitution for `$env` -- it's consistent with
other substitutions now.

- [x] make all path `replacements()` lazy
- [x] clean up handling of `$env`

```console
> spack unit-test -k test_autotools_gnuconfig_replacement_no_gnuconfig

...

==> [2022-12-31-15:44:21.771459] Error: AttributeError:

The 'autotools-config-replacement' package cannot find an attribute while trying to build from sources. This might be due to a change in Spack's package format to support multiple build-systems for a single package. You can fix this by updating the build recipe, and you can also report the issue as a bug. More information at https://spack.readthedocs.io/en/latest/packaging_guide.html#installation-procedure

/Users/gamblin2/src/spack/lib/spack/spack/package_base.py:1332, in prefix:
       1330    @Property
       1331    def prefix(self):
  >>   1332        """Get the prefix into which this package should be installed."""
       1333        return self.spec.prefix

Traceback (most recent call last):
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 1030, in _setup_pkg_and_run
    kwargs["env_modifications"] = setup_package(
                                  ^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 757, in setup_package
    set_module_variables_for_package(pkg)
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_environment.py", line 596, in set_module_variables_for_package
    m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/build_systems/cmake.py", line 241, in std_args
    define("CMAKE_INSTALL_PREFIX", pkg.prefix),
                                   ^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/package_base.py", line 1333, in prefix
    return self.spec.prefix
           ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1710, in prefix
    self.prefix = spack.store.layout.path_for_spec(self)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 336, in path_for_spec
    path = self.relative_path_for_spec(spec)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/directory_layout.py", line 106, in relative_path_for_spec
    projection = spack.projections.get_projection(self.projections, spec)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/projections.py", line 13, in get_projection
    if spec.satisfies(spec_like, strict=True):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 3642, in satisfies
    if not self.virtual and other.virtual:
           ^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/spec.py", line 1622, in virtual
    return spack.repo.path.is_virtual(self.name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 890, in is_virtual
    return have_name and pkg_name in self.provider_index
                                     ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 770, in provider_index
    self._provider_index.merge(repo.provider_index)
                               ^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 1096, in provider_index
    return self.index["providers"]
           ~~~~~~~~~~^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 592, in __getitem__
    self._build_all_indexes()
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 607, in _build_all_indexes
    self.indexes[name] = self._build_index(name, indexer)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/spack/repo.py", line 616, in _build_index
    index_mtime = self.cache.mtime(cache_filename)
                  ^^^^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 826, in __getattr__
    return getattr(self.instance, name)
                   ^^^^^^^^^^^^^
  File "/Users/gamblin2/src/spack/lib/spack/llnl/util/lang.py", line 825, in __getattr__
    raise AttributeError()
AttributeError
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core PR affects Spack core functionality utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants