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

stdenv: don't preserve ownership information in defaultUnpack hook #359873

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

rski
Copy link
Contributor

@rski rski commented Nov 28, 2024

-p preserves permission,mode,timestamps.

When trying to copy with permission on NFS, it can fail;

The closest documentation I found on this was:

https://access.redhat.com/solutions/725513

One suggestion is to change the netapp configuration, but another that should work for everyone is to use the cp command presented in this patch.

With this, builds on nfs work for me. I however cannot say that I'm certain that removing the permission preservation won't cause any other troubles elsewhere, e.g. by perhaps introducing some sort of non-determinism.

See #244331

Example errors include:

@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/lpmhj0fh1ww4zby35m4lfvnq0sm60jfd-source
cp: preserving permissions for 'source/.gitignore': Operation not supported
cp: preserving permissions for 'source/README.md': Operation not supported
do not know how to unpack source archive /nix/store/lpmhj0fh1ww4zby35m4lfvnq0sm60jfd-source

and

      > Cannot copy /nix/store/49v7a7ali88cgdkfn1l9i1shz0a9lmxf-source to source: destination already exists!
       > Did you specify two "srcs" with the same "name"?
       > do not know how to unpack source archive /nix/store/49v7a7ali88cgdkfn1l9i1shz0a9lmxf-source

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

-p preserves permission,mode,timestamps.

When trying to copy with permission on NFS, it can fail;

The closest documentation I found on this was:

https://access.redhat.com/solutions/725513

One suggestion is to change the netapp configuration, but another that
should work for everyone is to use the cp command presented in this
patch.

With this, builds on nfs work for me. I however cannot say that I'm
certain that removing the permission preservation won't cause any other
troubles elsewhere, e.g. by perhaps introducing some sort of
non-determinism.
@jsoo1
Copy link
Contributor

jsoo1 commented Nov 28, 2024

One request: can you add an example error message to the PR description or commit message, please?

@rski
Copy link
Contributor Author

rski commented Nov 28, 2024

One request: can you add an example error message to the PR description or commit message, please?

done

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 29, 2024
Copy link
Contributor

@tobim tobim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain your use-case for this change? Are you actually creating build sandboxes on NFS or do you need this for manually running unpackPhase while working on certain packages?

pkgs/stdenv/generic/setup.sh Show resolved Hide resolved
@rski
Copy link
Contributor Author

rski commented Nov 29, 2024

doesn't seem to actually work, I'll close this until i figure out something that does :(

@rski rski closed this Nov 29, 2024
@rski
Copy link
Contributor Author

rski commented Nov 29, 2024

Can you explain your use-case for this change? Are you actually creating build sandboxes on NFS or do you need this for manually running unpackPhase while working on certain packages?

My entire nix store is on nfs. So whenever I run nix build and something doesn't hit the cache, defaultUnpack might be called which results in this. Usually it's due to running home-manager with overrides or extra derivations of my own

@philiptaron
Copy link
Contributor

philiptaron commented Nov 29, 2024

As a distributed filesystems engineer, the root cause for this failure you're experiencing almost certainly resides somewhere between your NFS server and its configuration and the mount options on your NFS client. An NFS server, properly mounted, should absolutely be able to hold all the permissions that cp -p is copying, and doubly so for the Nix store, which is world-readable without making use of the sticky bit or setuid/setgid bits.

In particular, you're likely experiencing a problem caused by attempting to set ownership (chown/chgrp) without having the permissions to do so.

@tobim
Copy link
Contributor

tobim commented Nov 29, 2024

Maybe your source has some acls that cannot be set at the destination? Does this SO answer match your problem: https://serverfault.com/a/281292?

@aelbarkani
Copy link

I have the same problem with Ceph storage (both CephFS and RBD), so it's not only NFS I think.

@aelbarkani
Copy link

aelbarkani commented Nov 29, 2024

btw @rski tried the commands you proposed in this PR and it seems to be working in my environment (OpenShift containers with SELinux enabled and high constraints on security wtih Ceph volumes mounted, CephFS in this case):

# 1000990000 @ workspaceaf516e039e6f4aff-665db5bb48-rjcwg in /projects/project on git:init x [19:29:01] 
$ cp -pr --reflink=auto -- pyproject.toml pyproject.toml.test
cp: Operation not supported (os error 95)

# 1000990000 @ workspaceaf516e039e6f4aff-665db5bb48-rjcwg in /projects/project on git:init x [19:29:26] C:1
$ cp -pr --reflink=auto pyproject.toml pyproject.toml.test 
cp: Operation not supported (os error 95)

# 1000990000 @ workspaceaf516e039e6f4aff-665db5bb48-rjcwg in /projects/project on git:init x [19:30:06] C:1
$ cp -pr --reflink=auto -- pyproject.toml pyproject.toml.test
cp: Operation not supported (os error 95)

# 1000990000 @ workspaceaf516e039e6f4aff-665db5bb48-rjcwg in /projects/project on git:init x [19:30:12] C:1
$ cp -pr -- pyproject.toml pyproject.toml.test 
cp: Operation not supported (os error 95)

# 1000990000 @ workspaceaf516e039e6f4aff-665db5bb48-rjcwg in /projects/project on git:init x [19:30:17] C:1
$ cp -p -- pyproject.toml pyproject.toml.test 
cp: Operation not supported (os error 95)

# 1000990000 @ workspaceaf516e039e6f4aff-665db5bb48-rjcwg in /projects/project on git:init x [19:30:21] C:1
$ cp -r --preserve=mode,timestamps --reflink=auto -- pyproject.toml pyproject.toml.test

I'm going to try to reproduce in a Nix flake.

@philiptaron
Copy link
Contributor

From the GNU coreutils manual:

Using --preserve with no attributes listed is equivalent to --preserve=mode,ownership,timestamps.

Thus, what this patch drops is --preserve=ownership.

Since the contents in the Nix store do not need ownership (they are owned by root), I think this is a fine patch to take.

@philiptaron philiptaron reopened this Nov 29, 2024
@philiptaron philiptaron changed the title pkgs/stdenv: fix defaultUnpack on nfs stdenv: don't preserve ownership information in defaultUnpack hook Nov 29, 2024
@philiptaron
Copy link
Contributor

I plan on merging this sometime today unless there are big objections.

@philiptaron philiptaron merged commit 595b424 into NixOS:staging Nov 29, 2024
63 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants