Remove fetch from depfile (#31433)

This commit is contained in:
Harmen Stoppels 2022-07-05 14:48:32 +02:00 committed by GitHub
parent bb3a663392
commit dac31ef3c4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 19 additions and 51 deletions

View file

@ -1013,7 +1013,7 @@ The following advanced example shows how generated targets can be used in a
SPACK ?= spack SPACK ?= spack
.PHONY: all clean fetch env .PHONY: all clean env
all: env all: env
@ -1022,9 +1022,6 @@ The following advanced example shows how generated targets can be used in a
env.mk: spack.lock env.mk: spack.lock
$(SPACK) -e . env depfile -o $@ --make-target-prefix spack $(SPACK) -e . env depfile -o $@ --make-target-prefix spack
fetch: spack/fetch
$(info Environment fetched!)
env: spack/env env: spack/env
$(info Environment installed!) $(info Environment installed!)
@ -1037,10 +1034,10 @@ The following advanced example shows how generated targets can be used in a
endif endif
When ``make`` is invoked, it first "remakes" the missing include ``env.mk`` When ``make`` is invoked, it first "remakes" the missing include ``env.mk``
from its rule, which triggers concretization. When done, the generated targets from its rule, which triggers concretization. When done, the generated target
``spack/fetch`` and ``spack/env`` are available. In the above ``spack/env`` is available. In the above example, the ``env`` target uses this generated
example, the ``env`` target uses the latter as a prerequisite, meaning target as a prerequisite, meaning that it can make use of the installed packages in
that it can make use of the installed packages in its commands. its commands.
As it is typically undesirable to remake ``env.mk`` as part of ``make clean``, As it is typically undesirable to remake ``env.mk`` as part of ``make clean``,
the include is conditional. the include is conditional.
@ -1048,7 +1045,6 @@ the include is conditional.
.. note:: .. note::
When including generated ``Makefile``\s, it is important to use When including generated ``Makefile``\s, it is important to use
the ``--make-target-prefix`` flag and use the non-phony targets the ``--make-target-prefix`` flag and use the non-phony target
``<target-prefix>/env`` and ``<target-prefix>/fetch`` as ``<target-prefix>/env`` as prerequisite, instead of the phony target
prerequisites, instead of the phony targets ``<target-prefix>/all`` ``<target-prefix>/all``.
and ``<target-prefix>/fetch-all`` respectively.

View file

@ -559,11 +559,11 @@ def env_depfile(args):
target_prefix = args.make_target_prefix target_prefix = args.make_target_prefix
def get_target(name): 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 # 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 # 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 return name
else: else:
return os.path.join(target_prefix, name) return os.path.join(target_prefix, name)
@ -571,9 +571,6 @@ def get_target(name):
def get_install_target(name): def get_install_target(name):
return os.path.join(target_prefix, '.install', 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 _, spec in env.concretized_specs():
for s in spec.traverse(root=True): for s in spec.traverse(root=True):
hash_to_spec[s.dag_hash()] = s hash_to_spec[s.dag_hash()] = s
@ -588,46 +585,29 @@ def get_fetch_target(name):
# All package install targets, not just roots. # All package install targets, not just roots.
all_install_targets = [get_install_target(h) for h in hash_to_spec.keys()] 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 = six.StringIO()
buf.write("""SPACK ?= spack 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$(info Installing $(SPEC))
\t{}$(SPACK) -e '{}' install $(SPACK_INSTALL_FLAGS) --only-concrete --only=package \ \t{}$(SPACK) -e '{}' install $(SPACK_INSTALL_FLAGS) --only-concrete --only=package \
--no-add /$(notdir $@) && touch $@ --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('all'), get_target('env'),
get_target('fetch-all'), get_target('fetch'),
get_target('env'), ' '.join(root_install_targets), get_target('env'), ' '.join(root_install_targets),
get_target('fetch'), ' '.join(all_fetch_targets), get_target('dirs'), get_target('.install'),
get_target('dirs'), get_target('.fetch'), get_target('.install'), get_target('.install/%'), get_target('dirs'),
get_target('.fetch/%'), get_target('dirs'),
env.path,
get_target('.install/%'), get_target('.fetch/%'),
'+' if args.jobserver else '', env.path)) '+' if args.jobserver else '', env.path))
# Targets are of the form <prefix>/<name>: [<prefix>/<depname>]..., # Targets are of the form <prefix>/<name>: [<prefix>/<depname>]...,
@ -657,11 +637,9 @@ def get_fetch_target(name):
# --make-target-prefix can be any existing directory we do not control, # --make-target-prefix can be any existing directory we do not control,
# including empty string (which means deleting the containing folder # including empty string (which means deleting the containing folder
# would delete the folder with the Makefile) # 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('clean'),
get_target('env'), get_target('env'),
get_target('fetch'),
' '.join(all_fetch_targets),
' '.join(all_install_targets))) ' '.join(all_install_targets)))
makefile = buf.getvalue() makefile = buf.getvalue()

View file

@ -2898,14 +2898,8 @@ def test_environment_depfile_makefile(tmpdir, mock_packages):
with ev.read('test') as e: with ev.read('test') as e:
for _, root in e.concretized_specs(): for _, root in e.concretized_specs():
for spec in root.traverse(root=True): for spec in root.traverse(root=True):
for task in ('.fetch', '.install'): tgt = os.path.join('prefix', '.install', spec.dag_hash())
tgt = os.path.join('prefix', task, spec.dag_hash()) assert 'touch {}'.format(tgt) in all_out
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
def test_environment_depfile_out(tmpdir, mock_packages): def test_environment_depfile_out(tmpdir, mock_packages):