Skip to content
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

Avoid symlinking to /bin when building singularity containers (or warn on clashes) #93122

Open
cfhammill opened this issue Jul 14, 2020 · 10 comments
Labels
0.kind: enhancement Add something new 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md

Comments

@cfhammill
Copy link
Contributor

cfhammill commented Jul 14, 2020

Somewhat related to #93082.

Presently when building singularity images, the contents (derivations) paths of the container are traversed, and each file found in $content-n/bin are symlinked to /bin.

https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/singularity-tools/default.nix#L80

The relevant code is:

for c in ${toString contents} ; do
  for f in $c/bin/* ; do
    if [ ! -e bin/$(basename $f) ] ; then
      ln -s $f bin/
    fi
  done
done

In the case multiple contents have the same binary the first is used and subsequent are skipped.

I think it would be preferable if we used $PATH instead of symlinking to bin. To accomplish this we could instead traverse the contents appending their paths and write that to a file in .singularity.d/env setting SINGULARITY_PREPEND_PATH in e.g. env-nix.sh. This ensures the nix $PATHs precede the default PATH.

Alternatively, if this seems like too much work for not enough gain, issuing a warning in the above for loop when an executable is masked would be advantageous for users.

@jbedo what do you think?

@jbedo
Copy link
Contributor

jbedo commented Jul 14, 2020

I think this is a good idea, it's a cleaner solution. However, changing to path manipulation won't really change the masking situation, the order will still matter.

@cfhammill
Copy link
Contributor Author

thanks @jbedo! I realize the masking issue isn't fixed by this, I probably should have separated out the two issues. I'll try to put together a PR in the next couple days.

@veprbl veprbl added the 0.kind: enhancement Add something new label Jul 15, 2020
@stale
Copy link

stale bot commented Jan 11, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 11, 2021
@ShamrockLee
Copy link
Contributor

It is still important to me.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 1, 2021
@jbedo
Copy link
Contributor

jbedo commented Feb 1, 2021

@ShamrockLee which bit, the warning on masking or using path instead of symlinking?

@ShamrockLee
Copy link
Contributor

@ShamrockLee which bit, the warning on masking or using path instead of symlinking?

Using PATH.
I'm working on a singularity version of LnL7/nix-docker
I'm trying to make it accept environment variables (including PATH) by adding .singularity.d/env/90-environment.sh. It still doesn't seem to work.
PATH is nessesary for --fakeroot to work.

@ShamrockLee
Copy link
Contributor

ShamrockLee commented Feb 2, 2021

How about constructing a ${name}.def file on-the-fly during build time and sungularity build it?

@jbedo
Copy link
Contributor

jbedo commented Feb 3, 2021

I'm trying to make it accept environment variables (including PATH) by adding .singularity.d/env/90-environment.sh. It still doesn't seem to work.

Can you share your WIP so I can take a look?

@ShamrockLee
Copy link
Contributor

@jbedo Here.

https://github.com/ShamrockLee/nix-singularity

The container is built with default.nix, and a modified version of singularity-tool is in srcs/singularity-tool.nix.

The remaining part is the still the same as those in nix-docker, where they were forked.

@stale
Copy link

stale bot commented Aug 2, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: enhancement Add something new 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md
Projects
None yet
Development

No branches or pull requests

4 participants