Simple rewiring of spliced specs by nhanford · Pull Request #26873 · spack/spack

nhanford · 2021-10-21T15:25:29Z

Rewiring is an operation on concrete, installed, spliced specs which carefully relocates any relevant prefixes throughout all the specs in a spliced DAG and then performs necessary operations for the spliced spec to be seen as "installed." This operation remains somewhat "dangerous" and can lead to ABI breaks and other unexpected behavior. It should be used with caution.
Rewiring extends splicing (#20262) and honors all the semantics therein. For example, when a transitive splice is rewired, it will bring in all the dependencies of the newly-spliced dependency. Like splicing, rewiring is not destructive--it creates a new spec that conforms to the new spec from the splice.

…to features/splice-rewire

vsoch · 2021-11-11T20:07:03Z

lib/spack/spack/rewiring.py

+    shutil.copytree(os.path.join(tempdir, spec.dag_hash()), spec.prefix,
+                    ignore=shutil.ignore_patterns('.spack/spec.json'))


This is going to get angry if the prefix already exists - if it does and has the same hash I think we already have the spec and don't need to do again?

Suggested change

shutil.copytree(os.path.join(tempdir, spec.dag_hash()), spec.prefix,

ignore=shutil.ignore_patterns('.spack/spec.json'))

if not os.path.exists(spec.prefix):

shutil.copytree(os.path.join(tempdir, spec.dag_hash()), spec.prefix,

ignore=shutil.ignore_patterns('.spack/spec.json'))

but if it's the case that the relocations are different (and the hash is the same) then you'd instead want to do:

Suggested change

shutil.copytree(os.path.join(tempdir, spec.dag_hash()), spec.prefix,

ignore=shutil.ignore_patterns('.spack/spec.json'))

if os.path.exists(spec.prefix):

shutil.rmtree(spec.prefix)

shutil.copytree(os.path.join(tempdir, spec.dag_hash()), spec.prefix,

ignore=shutil.ignore_patterns('.spack/spec.json'))

If the relocations are different the hash has to be different too -- the hash contains all of the information on all of the splices.

Yup sorry for not resolving this earlier. We wound up fixing the issue with the check to install missing specs.

lib/spack/spack/rewiring.py

vsoch · 2021-11-24T22:33:59Z

Are we going to be able to splice different specs in? E.g., openmpi or mpich or vice versa - is that going to be another PR?

nhanford · 2021-12-02T20:06:51Z

Sorry for missing this question: It will be in an upcoming PR.

becker33 · 2022-02-16T23:42:22Z

lib/spack/spack/test/rewiring.py

+    spec.package.do_install(force=True)
+    dep.package.do_install(force=True)
+    spliced_spec = spec.splice(dep, transitive=transitive)
+    assert spec is not spliced_spec


Is this checking for something in particular?

Removed per our discussion; this is already tested elsewhere.

lib/spack/spack/test/rewiring.py

becker33 · 2022-02-16T23:44:25Z

lib/spack/spack/test/rewiring.py

+        manifest_file_path = os.path.join(node.prefix,
+                                          spack.store.layout.metadata_dir,
+                                          spack.store.layout.manifest_file_name)
+        assert os.path.exists(manifest_file_path)


Is this just checking that the manifest file exists? Should we check that it reflects the new information?

A new test, test_rewire_writes_new_metadata has been added with the described check.

becker33 · 2022-02-16T23:44:46Z

lib/spack/spack/test/rewiring.py

+        # test install manifest
+        spack.store.layout.ensure_installed(node)
+        manifest_file_path = os.path.join(node.prefix,
+                                          spack.store.layout.metadata_dir,
+                                          spack.store.layout.manifest_file_name)
+        assert os.path.exists(manifest_file_path)


This seems to be covered by the previous test?

This duplicated code has been eliminated.

becker33 · 2022-02-24T23:36:25Z

lib/spack/spack/rewiring.py

+
+    manifest = bindist.get_buildfile_manifest(spec.build_spec)
+    platform = spack.platforms.by_name(spec.platform)
+    if manifest.get('text_to_relocate'):


Suggested change

if manifest.get('text_to_relocate'):

text_to_relocate = manifest.get('text_to_relocate', None):

if text_to_relocate:

Then you can use it inside the block wiithout needing a separate lookup

This has been changed accordingly in both the text and binary cases.

* tests for rewiring pure specs to spliced specs * relocate text, binaries, and links * using llnl.util.symlink for windows compat. Note: This does not include CLI hooks for relocation. Co-authored-by: Nathan Hanford <[email protected]>

This PR allows users to configure explicit splicing replacement of an abstract spec in the concretizer. concretizer: splice: explicit: - target: mpi replacement: mpich/abcdef transitive: true This config block would mean "for any spec that concretizes to use mpi, splice in mpich/abcdef in place of the mpi it would naturally concretize to use. See #20262, #26873, #27919, and #46382 for PRs enabling splicing in the Spec object. This PR will be the first place the splice method is used in a user-facing manner. See https://spack.readthedocs.io/en/latest/spack.html#spack.spec.Spec.splice for more information on splicing. This will allow users to reuse generic public binaries while splicing in the performant local mpi implementation on their system. In the config file, the target may be any abstract spec. The replacement must be a spec that includes an abstract hash `/abcdef`. The transitive key is optional, defaulting to true if left out. Two important items to note: 1. When writing explicit splice config, the user is in charge of ensuring that the replacement specs they use are binary compatible with whatever targets they replace. In practice, this will likely require either specific knowledge of what packages will be installed by the user's workflow, or somewhat more specific abstract "target" specs for splicing, to ensure binary compatibility. 2. Explicit splices can cause the output of the concretizer not to satisfy the input. For example, using the config above and consider a package in a binary cache `hdf5/xyzabc` that depends on mvapich2. Then the command `spack install hdf5/xyzabc` will instead install the result of splicing `mpich/abcdef` into `hdf5/xyzabc` in place of whatever mvapich2 spec it previously depended on. When this occurs, a warning message is printed `Warning: explicit splice configuration has caused the the concretized spec {concrete_spec} not to satisfy the input spec {input_spec}". Highlighted technical details of implementation: 1. This PR required modifying the installer to have two separate types of Tasks, `RewireTask` and `BuildTask`. Spliced specs are queued as `RewireTask` and standard specs are queued as `BuildTask`. Each spliced spec retains a pointer to its build_spec for provenance. If a RewireTask is dequeued and the associated `build_spec` is neither available in the install_tree nor from a binary cache, the RewireTask is requeued with a new dependency on a BuildTask for the build_spec, and BuildTasks are queued for the build spec and its dependencies. 2. Relocation is modified so that a spack binary can be simultaneously installed and rewired. This ensures that installing the build_spec is not necessary when splicing from a binary cache. 3. The splicing model is modified to more accurately represent build dependencies -- that is, spliced specs do not have build dependencies, as spliced specs are never built. Their build_specs retain the build dependencies, as they may be built as part of installing the spliced spec. 4. There were vestiges of the compiler bootstrapping logic that were not removed in #46237 because I asked alalazo to leave them in to avoid making the rebase for this PR harder than it needed to be. Those last remains are removed in this PR. Co-authored-by: Nathan Hanford <[email protected]> Co-authored-by: Gregory Becker <[email protected]> Co-authored-by: Tamara Dahlgren <[email protected]>

This PR allows users to configure explicit splicing replacement of an abstract spec in the concretizer. concretizer: splice: explicit: - target: mpi replacement: mpich/abcdef transitive: true This config block would mean "for any spec that concretizes to use mpi, splice in mpich/abcdef in place of the mpi it would naturally concretize to use. See spack#20262, spack#26873, spack#27919, and spack#46382 for PRs enabling splicing in the Spec object. This PR will be the first place the splice method is used in a user-facing manner. See https://spack.readthedocs.io/en/latest/spack.html#spack.spec.Spec.splice for more information on splicing. This will allow users to reuse generic public binaries while splicing in the performant local mpi implementation on their system. In the config file, the target may be any abstract spec. The replacement must be a spec that includes an abstract hash `/abcdef`. The transitive key is optional, defaulting to true if left out. Two important items to note: 1. When writing explicit splice config, the user is in charge of ensuring that the replacement specs they use are binary compatible with whatever targets they replace. In practice, this will likely require either specific knowledge of what packages will be installed by the user's workflow, or somewhat more specific abstract "target" specs for splicing, to ensure binary compatibility. 2. Explicit splices can cause the output of the concretizer not to satisfy the input. For example, using the config above and consider a package in a binary cache `hdf5/xyzabc` that depends on mvapich2. Then the command `spack install hdf5/xyzabc` will instead install the result of splicing `mpich/abcdef` into `hdf5/xyzabc` in place of whatever mvapich2 spec it previously depended on. When this occurs, a warning message is printed `Warning: explicit splice configuration has caused the the concretized spec {concrete_spec} not to satisfy the input spec {input_spec}". Highlighted technical details of implementation: 1. This PR required modifying the installer to have two separate types of Tasks, `RewireTask` and `BuildTask`. Spliced specs are queued as `RewireTask` and standard specs are queued as `BuildTask`. Each spliced spec retains a pointer to its build_spec for provenance. If a RewireTask is dequeued and the associated `build_spec` is neither available in the install_tree nor from a binary cache, the RewireTask is requeued with a new dependency on a BuildTask for the build_spec, and BuildTasks are queued for the build spec and its dependencies. 2. Relocation is modified so that a spack binary can be simultaneously installed and rewired. This ensures that installing the build_spec is not necessary when splicing from a binary cache. 3. The splicing model is modified to more accurately represent build dependencies -- that is, spliced specs do not have build dependencies, as spliced specs are never built. Their build_specs retain the build dependencies, as they may be built as part of installing the spliced spec. 4. There were vestiges of the compiler bootstrapping logic that were not removed in spack#46237 because I asked alalazo to leave them in to avoid making the rebase for this PR harder than it needed to be. Those last remains are removed in this PR. Co-authored-by: Nathan Hanford <[email protected]> Co-authored-by: Gregory Becker <[email protected]> Co-authored-by: Tamara Dahlgren <[email protected]>

nhanford added 7 commits October 13, 2021 09:00

initial modifications to splice tests

2698bd4

initial modifications to splice tests

10b3824

simple text relocation

7e69bbe

Merge branch 'features/splice-rewire' of github.com:nhanford/spack in…

7994c45

…to features/splice-rewire

initial modifications to splice tests

24867e8

simple text relocation

4ade6a8

Merge branch 'features/splice-rewire' of github.com:nhanford/spack in…

8abc6c2

…to features/splice-rewire

spackbot-app bot added the tests General test capability(ies) label Oct 21, 2021

nhanford added 6 commits October 26, 2021 20:11

binaries with test

0291e3b

cleanup and resolve issue

eef74da

cleanup

f88521a

cleanup

4c8297e

cleanup

59f6a12

refactoring link relocation

109ad32

nhanford added the binary-packages label Oct 28, 2021

nhanford marked this pull request as ready for review October 28, 2021 15:19

nhanford requested review from becker33 and tgamblin October 28, 2021 15:19

nhanford added 4 commits October 28, 2021 08:42

parameterized tests

5193d96

Merge branch 'develop' into features/splice-rewire

ee72e94

removed duplicate db add

8513f1b

Merge branch 'develop' into features/splice-rewire

83b1622

vsoch reviewed Nov 11, 2021

View reviewed changes

nhanford added 5 commits November 11, 2021 12:12

attempting to fix duplicate install issue

b84c809

fixed a copytree issue

c57601e

link relocation delimiter issue

ddf3d42

removed vestigial debug print statement

64ded0e

modify rewire to install missing pure specs

28579ad

vsoch reviewed Nov 16, 2021

View reviewed changes

lib/spack/spack/rewiring.py Outdated Show resolved Hide resolved

link relocation fixed

7675395

nhanford added 2 commits November 24, 2021 19:22

Merge branch 'develop' into features/splice-rewire

3313b60

removing 2.6 backport

4ad3684

nhanford dismissed becker33’s stale review via 4ad3684 November 25, 2021 03:23

nhanford added 4 commits December 10, 2021 10:43

Merge branch 'develop' into features/splice-rewire

25a9518

Merge branch 'develop' into features/splice-rewire

2fb21f0

Merge branch 'develop' into features/splice-rewire

4c04b2b

update from to in layout calls.

18e07ec

becker33 requested changes Feb 24, 2022

View reviewed changes

becker33 reviewed Feb 24, 2022

View reviewed changes

nhanford added 12 commits March 9, 2022 11:54

added test and cleaned up changes

ee66e87

Merge branch 'develop' into features/splice-rewire

66cb813

added test for uninstalling rewired packages

6f45cfb

reintroduced error for rewiring not installed

ef99a2d

Merge branch 'develop' into features/splice-rewire

bd9ba1f

modified and tested not installed error

5664748

using new symlink for windows compat.

439cd22

remove force install from tests

391f2c5

Merge branch 'develop' into features/splice-rewire

5cee96f

attempting to exempt tests from Windows testing

40d698f

text relocation also doesn't work on windows

09d7e5e

style

38e8a71

becker33 approved these changes Apr 4, 2022

View reviewed changes

becker33 merged commit 88d8ca9 into spack:develop Apr 4, 2022

becker33 mentioned this pull request Oct 10, 2024

Installer: rewire spliced specs via RewireTask #39136

Merged

		shutil.copytree(os.path.join(tempdir, spec.dag_hash()), spec.prefix,
		ignore=shutil.ignore_patterns('.spack/spec.json'))

	if manifest.get('text_to_relocate'):
	text_to_relocate = manifest.get('text_to_relocate', None):
	if text_to_relocate:

Conversation

nhanford commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsoch Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vsoch commented Nov 24, 2021

Uh oh!

nhanford commented Dec 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

becker33 Feb 24, 2022 • edited by vsoch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nhanford commented Oct 21, 2021 •

edited

Loading

vsoch Nov 11, 2021 •

edited

Loading

becker33 Feb 24, 2022 •

edited by vsoch

Loading