singularity
singularity copied to clipboard
`runscript` wiped during build
Version of Singularity
4.2.1
Describe the bug
When building an image from a sandbox folder the contained runscript is emptied and replaced by a default script stating it was not defined.
To Reproduce
singularity build --sandbox fmriprep_v24.1.0/ docker://nipreps/fmriprep:latestsingularity build fmriprep-v24.1.0.sif fmriprep_v24.1.0/singularity exec fmriprep-v24.1.0.sif cat /.singularity.d/runscript
Additional context
The issue appeared due to https://github.com/sylabs/singularity/pull/3151 which moved the makeBaseEnv after the extraction of the image. That function is supposed to create default files but overwrites existing ones at https://github.com/apptainer/apptainer/blob/259c4621881193f3591b7e3dfd278d744b472be1/internal/pkg/build/sources/base_environment.go#L159-L161
I don't see why TRUNCATE is used there as previously the folder should be empty so no files exist to truncate. Only when/after using an existing image files can exist, can't they?
The comment at the start reads
If the file already exists ensure it has requested permissions as OpenFile won't set on an existing file and some docker containers have hosts or resolv.conf without write perm.
I don't see how it follows that e.g. the hosts file should be cleared in that case.
Thanks for the report on this. Yes, it looks like this is a combination of the change in #3151 to fix a different issue, with some non-obvious existing behaviour that wasn't picked up in review.
We'll look into a fix and adding a test that would catch this. I'm slightlty surprised we don't have anything in the e2e tests already that would have picked it up.
I'm quite sure that simply not replacing the existing file(s) is the right solution. My assumption is that the makeBaseEnv was always called before populating the folder/sandbox with preexisting files.
This means that those base-env files would have been overwritten by the extracted/copied files. So not truncating them when doing makeBaseEnv after the extraction would yield the same (previous) behavior.
Only doubt I have is about that seemingly docker-related permission fix: There seems to be at least one case where the overwriting behavior is desired. It just isn't clear to me when and why exactly?
apptainer/apptainer#2561 is where the corresponding problem was first reported.
I reported it here too because the current solution was developed after an exchange here and later ported to apptainer.
My initial approach was to remove pre-created files that would be overwritten by the extraction: https://github.com/Flamefire/singularity/commit/ce2d67096cf55de2305978a88b516578bc8bb63f
@dtrudg noted
Relying on scanning the output of unsquashfs is brittle [...] In other code paths, where we build e.g. from an OCI source, rather than a local image file, the directories are created after image extraction, which avoids this issue.
That is, of course, correct:
(cp *OCIConveyorPacker) Pack(ctx context.Context) does unpackRootfs then insertBaseEnv
The "local packer" code is quite different though as it does the makeBaseEnv first in (cp *LocalConveyorPacker) Get and the returns a packer that works on top of that. Possibly intended for code-reuse.
Given the new code after #3151 I expect that packing from a local SIF, squashfs or ext3 file would show the same behavior.
There's a PR with a fix now at #3360 / #3362
could this be related to my problem? https://github.com/sylabs/singularity/discussions/3329
could this be related to my problem? #3329
Yes - that's almost certainly the same underlying issue.