Allow the user to use wrap as part of the bootstrap process#479
Allow the user to use wrap as part of the bootstrap process#479MaxHearnden wants to merge 8 commits intofosslinux:masterfrom
Conversation
The lib directory for python modules conflicted with the symlinks for a merged usr so this moves the directory out of the way
|
I suggest adding a GitHub CI workflow using the -w option. Also, mind the Python linter. |
|
Done, just waiting for the CI to finish |
|
If you're comfortable with it, could you use a matrix for the Github workflow, instead of copy-pasting the workflow file? https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow If you're not comfortable I can do that. |
|
I've added a test matrix, would you like me to use separate step 1 and step 2 artifacts for bwrap and wrap. |
|
Yes, that would be preferable. |
|
It now uses separate artifacts |
fosslinux
left a comment
There was a problem hiding this comment.
Thanks for the contribution! There's a few structural things that I'd like to see changed a bit, they go against convention that we've been using. Particularly, we try to make the bootstrap flow as mode-agnostic as possible, and simply wrap anything specific to one mode in an if block, but the way it's currently structured is difficult to follow.
| if wrap: | ||
| shutil.copy2(os.path.join(seed_dir, 'after-wrap.kaem'), | ||
| os.path.join(self.target_dir, 'after.kaem')) | ||
| shutil.copy2(os.path.join(seed_dir, 'after.kaem'), | ||
| os.path.join(self.target_dir, 'after-wrapped.kaem')) |
There was a problem hiding this comment.
I tend to try to avoid having conditional logic in generator.py. Is there some reason this needs to be here, and can't occur within the live-bootstrap environment?
Ie, there would be conditional logic in after.kaem for wrap mode, by adding in some kind of WRAP=True into bootstrap.cfg, rather than replacing the kaem files like this.
| catm seed-full.kaem /steps/bootstrap.cfg /steps/env seed.kaem | ||
| if catm seed-full.kaem /steps/bootstrap.cfg /steps/env seed.kaem; then | ||
| else | ||
| replace --file /steps/env --output /steps/env --match-on /external/distfiles --replace-with /distfiles |
There was a problem hiding this comment.
Why is /distfiles being reintroduced?
There was a problem hiding this comment.
I believe the idea here is to just run the bootstrap directly from the checkout directory, bypassing rootfs.py or any manual preparation. Since distfiles are downloaded to ${checkout_dir}/distfiles, with no parent named "externals" guaranteed to exist, /external/distfiles will fail in this scenario, no matter what directory you map as the root.
The alternative is to download distfiles to ${checkout_dir}/external/distfiles (which I would somewhat prefer to this solution), or to just not support this use case at all, no matter how compelling it seems.
| if [ -d "/${d}" ] && ! [ -L "/${d}" ]; then | ||
| # Move the non symlink directory out of the way | ||
| mv "/${d}" "/${d}-saved" | ||
| fi |
There was a problem hiding this comment.
I'm reading the commit description where this was introduced but still cannot figure out why this was added.
If I'm understanding the commit, my best guess is because the entire git repository is being copied into target, but I'm not sure why that is desirable either (not a supported running method)
There was a problem hiding this comment.
Again, I suspect this is related to running without a target directory, i.e. straight from the checkout directory.
This adds support for wrap in three situations:
rootfs.py -wFor 2 and 3, wrap can be used by copying
seed/after-wrap.kaemtoseed/stage0-posix/after.kaem(symlinking works as well) and runningbootstrap-seeds/POSIX/${ARCH}/kaem-optional-seedfrom within theseed/stage0-posixdirectoryThis replaces #339