-
Notifications
You must be signed in to change notification settings - Fork 17
modular sqfs #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
modular sqfs #224
Conversation
8a44d89
to
b99664b
Compare
remove bootstrap from schema remove compilers.yaml add squashfs-mount for base-uenvs cleanup cleanup change order typo typo typo skip compiler target convert to string fix path skip base uenvs in post-install loop.last tab look for squashfs in global tree always install squashfs add dependency do not generate {{env}}/compilers.yaml skip {{env}}/compilers.yaml in envvars.py skip -C /user-environment/config do not find compilers cleanup deps remove compilers.yaml install squashfs to bind mounted /tmp do not use filesystem locks remove dependency remove -d install squashfs in an env fix fix cleanup install -D the mount point doesn't need to exist base-uenv.json is optional wip fix squashfs-mount-wrapper allow to use a non-existent directory for STORE comment add padded_length for cache add missing file undo use padded_lenght in env do not use self.mount for mirror path use same sandbox in stack-debug.sh Revert "do not use self.mount for mirror path" This reverts commit f187450. ignore store/mountpath for build cache fix stack-debug.sh fix bwrap-store Revert "ignore store/mountpath for build cache" This reverts commit a13fc2b. remove padded_length remove stdout add generation of packages.yaml for externals fix chmod wip avoid spack --color fix structure set buildable false fix gen packages.yaml update do not use -all-root remove unused compiler target v2.0 repo format path must contain spack_repo repo -> spack_repo repo path must include namespace repo -> spack_repo update base-uenv.json schema compilers.yaml -> packages.yaml fix sandbox fix path, version doesn't belong here add missing version to packages.yaml set extra_attributes for compilers fix fix split debug Revert "debug" This reverts commit 4172a0a. fix find command cleanup update the schema remove compilers.yaml and the bits related to it from recipe.py remove more occurences of compilers.yaml avoid empty packages.all in environments.spack.yaml update for new base_uenv.json schema typo in schema base_uenv schema, default null for optional entries fix schema wip schema fix schema typo fix environments.spack.yaml typo fix environment.spack.yaml include exports from the gpu sub uenv fix schema change title remove compiler from env schema remove compiler from _internal_utils
8490715
to
edbd6d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few cleanup comments.
I did not look at the modular squashfs parts, only briefly at the changes to remove compilers.yaml
and compiler bootstrapping, and that looks fine to me.
stackinator/recipe.py
Outdated
# for name, config in environments.items(): | ||
# compilers = config["compiler"] | ||
# if len(compilers) == 1: | ||
# config["toolchain_constraints"] = [] | ||
# continue | ||
# requires = [f"%{compilers[0]['spec']}"] | ||
# for spec in config["specs"]: | ||
# if "%" in spec: | ||
# requires.append(spec) | ||
|
||
# config["toolchain_constraints"] = requires |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This I forgot to mention yesterday, the constraints for using nvhpc
and gcc
are missing at the moment. They used to be generated from the compiler
section in environments.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that the packages:all:compiler:gcc
constraints (modulo me misremembering which exact config section it is; i.e. making sure environments are concretized only using one compiler)? If yes, I think it's anyway currently ignored on spack develop and we can maybe add that back separately from this PR? I think it's #237.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was referring to this:
https://github.com/eth-cscs/stackinator/blob/master/stackinator/templates/environments.spack.yaml#L24
In packages.all.require
we used one_of: [%gcc, pkgA%nvhpc, pkgB%nvhpc]
to ensure that packages were built using gcc, unless explicitly requiring nvhpc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. I was referring to
compiler: [{% for c in config.compiler %}{{ separator() }}'{{ c.spec }}'{% endfor %}] |
To keep things simple for this PR, can we live without the constraint for now? Or how complicated do you think it would be to add back the constraints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've made a mess there. I can't recall why I commented the lines. environments.compiler
perhaps should be kept as it was. Implicitly generating the compiler preference gcc/nvhpc based on which compilers are present in the modular uenv isn't a good idea, better to make this explicit.
Summarizing the quick chat we had with @simonpintarelli, @albestro, and myself today: Assuming that this PR keeps non-modular uenvs/recipes working exactly as before we try to merge this PR first for support for spack 1.0. The modular uenv stuff can be considered "experimental" and tested further on main. After merging this:
|
mount_point=${elem#*:} | ||
sqfs=${elem%%:*} | ||
tmp_mount_point="${BUILD_ROOT}/tmp/mounts/${mount_point}" | ||
mkdir -p ${tmp_mount_point} | ||
build_root_mounts="${build_root_mounts} ${sqfs}:${tmp_mount_point}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: formatting to be fixed?
@@ -3,7 +3,7 @@ | |||
"title": "Schema for Spack Stack compilers.yaml recipe file", | |||
"type": "object", | |||
"additionalProperties": false, | |||
"required": ["bootstrap", "gcc"], | |||
"required": ["gcc"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it mean that we can remove defs:properties:bootstrap
as well?
@@ -3,35 +3,22 @@ include ../Make.user | |||
CONFIG_DIR = $(STORE)/config | |||
MODULE_DIR = $(BUILD_ROOT)/modules | |||
|
|||
# These will be the prefixes of the GCCs, LLVMs and NVHPCs in the respective environments. | |||
ALL_COMPILER_PREFIXES ={% for compiler in all_compilers %} $$($(SPACK_HELPER) -e ../compilers/{{ compiler }} find --format='{prefix}' gcc llvm nvhpc){% endfor %} | |||
# COMPILER_PREFIXES ={% for compiler in release_compilers %} $$($(SPACK_HELPER) -e ../compilers/{{ compiler }} find --format='{prefix}' gcc llvm nvhpc){% endfor %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# COMPILER_PREFIXES ={% for compiler in release_compilers %} $$($(SPACK_HELPER) -e ../compilers/{{ compiler }} find --format='{prefix}' gcc llvm nvhpc){% endfor %} |
?
SANDBOX := $(SQFS_MOUNT) -- $(BUILD_ROOT)/bwrap-mutable-root.sh $\ | ||
--tmpfs ~ $\ | ||
--bind $(BUILD_ROOT)/tmp /tmp $\ | ||
--bind $(BUILD_ROOT)/store $(STORE) $\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--bind $(BUILD_ROOT)/store $(STORE) $\ | |
--bind $(BUILD_ROOT)/store $(STORE) $\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm missing why this differs from SANDBOX_NO_BASE
for what concerns store. Can we use bwrap-store
here too?
--tmpfs ~ $\ | ||
--bind $(BUILD_ROOT)/tmp /tmp $\ | ||
--bind $(BUILD_ROOT)/store $(STORE) | ||
-- $(BUILD_ROOT)/bwrap-store.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-- $(BUILD_ROOT)/bwrap-store.sh | |
-- $(BUILD_ROOT)/bwrap-store.sh |
}, | ||
"image": { | ||
"$ref": "#/defs/image", | ||
"description": "TOOD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mispelled TODO that might escape the search for TODOs
/user-environment/gcc/12.3.0
) are regular stackinator sqfs images located in a subdirectory of the root image.squashfs-mount-wrapper.sh
$BUILDROOT/tmp/mounts/
viasquashfs-mount
bwrap-mutable-root.sh
squashfs-run
does), this is required for building base images where mount-points don't exist (for example/user-environment/gcc
)$BUILDROOT/tmp/mounts/
to their final destination.stack-debug.sh
to mount the base uenvscompilers.toolchain
has been removed fromenvironments.yaml
. The json description can be used to generate the same constraints inspack.yaml
env/default/packages.yaml
. The dependent uenvs then include thesepackages.yaml
inspack.include
. The python script used to generatepackages.yaml
fromspack.lock
is in gen_packages_yaml.pypackages
fromenvironments.yaml
is ignored (it was inMakefile.compilers
). Why not usepackages
(inclvariants
like in regularspack.yaml
? And then use it as override for auto generated package section.The corresponding cluster-config is in https://github.com/eth-cscs/alps-cluster-config/tree/feat/modular-uenv. It contains the repo changes for spack 1.0 and gcc12 configured in packages.yaml for daint/eiger (overlaps/conflicts with eth-cscs/alps-cluster-config#35)
Prototype for
uenv start/run
which can mount modular sqfs from the json description: eth-cscs/uenv2#86Example recipe for
cuda
/gcc
andosu-micro-benchmarks
: https://github.com/simonpintarelli/daint-modular-uenv