From dac31ef3c405709aadd467eab21686f5a51930db Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Tue, 5 Jul 2022 14:48:32 +0200 Subject: Remove fetch from depfile (#31433) --- lib/spack/docs/environments.rst | 20 ++++++++------------ lib/spack/spack/cmd/env.py | 40 +++++++++------------------------------- lib/spack/spack/test/cmd/env.py | 10 ++-------- 3 files changed, 19 insertions(+), 51 deletions(-) diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 62ec2f74f7..ee95d4f779 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -1013,7 +1013,7 @@ The following advanced example shows how generated targets can be used in a SPACK ?= spack - .PHONY: all clean fetch env + .PHONY: all clean env all: env @@ -1022,9 +1022,6 @@ The following advanced example shows how generated targets can be used in a env.mk: spack.lock $(SPACK) -e . env depfile -o $@ --make-target-prefix spack - - fetch: spack/fetch - $(info Environment fetched!) env: spack/env $(info Environment installed!) @@ -1037,10 +1034,10 @@ The following advanced example shows how generated targets can be used in a endif When ``make`` is invoked, it first "remakes" the missing include ``env.mk`` -from its rule, which triggers concretization. When done, the generated targets -``spack/fetch`` and ``spack/env`` are available. In the above -example, the ``env`` target uses the latter as a prerequisite, meaning -that it can make use of the installed packages in its commands. +from its rule, which triggers concretization. When done, the generated target +``spack/env`` is available. In the above example, the ``env`` target uses this generated +target as a prerequisite, meaning that it can make use of the installed packages in +its commands. As it is typically undesirable to remake ``env.mk`` as part of ``make clean``, the include is conditional. @@ -1048,7 +1045,6 @@ the include is conditional. .. note:: When including generated ``Makefile``\s, it is important to use - the ``--make-target-prefix`` flag and use the non-phony targets - ``/env`` and ``/fetch`` as - prerequisites, instead of the phony targets ``/all`` - and ``/fetch-all`` respectively. + the ``--make-target-prefix`` flag and use the non-phony target + ``/env`` as prerequisite, instead of the phony target + ``/all``. diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index f33d98bd2b..daf57a0e0b 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -559,11 +559,11 @@ def env_depfile(args): target_prefix = args.make_target_prefix def get_target(name): - # The `all`, `fetch` and `clean` targets are phony. It doesn't make sense to + # The `all` and `clean` targets are phony. It doesn't make sense to # have /abs/path/to/env/metadir/{all,clean} targets. But it *does* make - # sense to have a prefix like `env/all`, `env/fetch`, `env/clean` when they are + # sense to have a prefix like `env/all`, `env/clean` when they are # supposed to be included - if name in ('all', 'fetch-all', 'clean') and os.path.isabs(target_prefix): + if name in ('all', 'clean') and os.path.isabs(target_prefix): return name else: return os.path.join(target_prefix, name) @@ -571,9 +571,6 @@ def env_depfile(args): def get_install_target(name): return os.path.join(target_prefix, '.install', name) - def get_fetch_target(name): - return os.path.join(target_prefix, '.fetch', name) - for _, spec in env.concretized_specs(): for s in spec.traverse(root=True): hash_to_spec[s.dag_hash()] = s @@ -588,46 +585,29 @@ def env_depfile(args): # All package install targets, not just roots. all_install_targets = [get_install_target(h) for h in hash_to_spec.keys()] - # Fetch targets for all packages in the environment, not just roots. - all_fetch_targets = [get_fetch_target(h) for h in hash_to_spec.keys()] - buf = six.StringIO() buf.write("""SPACK ?= spack -.PHONY: {} {} {} - -{}: {} - -{}: {} +.PHONY: {} {} {}: {} -\t@touch $@ {}: {} -\t@touch $@ {}: -\t@mkdir -p {} {} +\t@mkdir -p {} {}: | {} -\t$(info Fetching $(SPEC)) -\t$(SPACK) -e '{}' fetch $(SPACK_FETCH_FLAGS) /$(notdir $@) && touch $@ - -{}: {} \t$(info Installing $(SPEC)) \t{}$(SPACK) -e '{}' install $(SPACK_INSTALL_FLAGS) --only-concrete --only=package \ --no-add /$(notdir $@) && touch $@ -""".format(get_target('all'), get_target('fetch-all'), get_target('clean'), +""".format(get_target('all'), get_target('clean'), get_target('all'), get_target('env'), - get_target('fetch-all'), get_target('fetch'), get_target('env'), ' '.join(root_install_targets), - get_target('fetch'), ' '.join(all_fetch_targets), - get_target('dirs'), get_target('.fetch'), get_target('.install'), - get_target('.fetch/%'), get_target('dirs'), - env.path, - get_target('.install/%'), get_target('.fetch/%'), + get_target('dirs'), get_target('.install'), + get_target('.install/%'), get_target('dirs'), '+' if args.jobserver else '', env.path)) # Targets are of the form /: [/]..., @@ -657,11 +637,9 @@ def env_depfile(args): # --make-target-prefix can be any existing directory we do not control, # including empty string (which means deleting the containing folder # would delete the folder with the Makefile) - buf.write("{}:\n\trm -f -- {} {} {} {}\n".format( + buf.write("{}:\n\trm -f -- {} {}\n".format( get_target('clean'), get_target('env'), - get_target('fetch'), - ' '.join(all_fetch_targets), ' '.join(all_install_targets))) makefile = buf.getvalue() diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 8edfaec027..ba67e98a3c 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2898,14 +2898,8 @@ def test_environment_depfile_makefile(tmpdir, mock_packages): with ev.read('test') as e: for _, root in e.concretized_specs(): for spec in root.traverse(root=True): - for task in ('.fetch', '.install'): - tgt = os.path.join('prefix', task, spec.dag_hash()) - assert 'touch {}'.format(tgt) in all_out - - # Check whether make prefix/fetch-all only fetches - fetch_out = make('prefix/fetch-all', '-n', '-f', makefile, output=str) - assert '.install/' not in fetch_out - assert '.fetch/' in fetch_out + tgt = os.path.join('prefix', '.install', spec.dag_hash()) + assert 'touch {}'.format(tgt) in all_out def test_environment_depfile_out(tmpdir, mock_packages): -- cgit v1.2.3-70-g09d2