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

fetchzip: get write permission on unpacked directory #105845

Merged
merged 1 commit into from Dec 5, 2020

Conversation

lukegb
Copy link
Contributor

@lukegb lukegb commented Dec 4, 2020

Motivation for this change

This is a workaround for NixOS/nix#4295, which caused single-user Linux
Nix installations using sandboxed builds to start failing to build
fetchzip derivations after 4a5c493.

In short: removing write permissions for the entire directory is great,
except we then can't rename(2) it to the final Nix store path out of the
sandbox, because we don't have write permission on the directory and
thus cannot update the ".." directory entry.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @abathur @bhipple @MetaDark @fzakaria

This is a workaround for NixOS/nix#4295, which caused single-user Linux
Nix installations using sandboxed builds to start failing to build
fetchzip derivations after 4a5c493.

In short: removing write permissions for the entire directory is great,
except we then can't rename(2) it to the final Nix store path out of the
sandbox, because we don't have write permission on the directory and
thus cannot update the ".." directory entry.
@lukegb
Copy link
Contributor Author

lukegb commented Dec 4, 2020

Result of nixpkgs-review pr 105845 run on x86_64-linux 1

(This one on NixOS)

@ofborg ofborg bot added the 6.topic: fetch label Dec 4, 2020
@lukegb
Copy link
Contributor Author

lukegb commented Dec 4, 2020

Result of nixpkgs-review pr 105845 run on x86_64-linux 1

(This one on Ubuntu 20.04 with a single-user Nix install)

@abathur
Copy link
Member

abathur commented Dec 4, 2020

@lukegb the review links are to https://github.com/Mic92/nixpkgs-review

@lukegb
Copy link
Contributor Author

lukegb commented Dec 4, 2020

Yeah, they would be. Basically, nixpkgs-review didn't rebuild anything because "nothing" changed.

@fzakaria
Copy link
Contributor

fzakaria commented Dec 4, 2020

Amazing! Thanks for finding the regression.

Does nixpkgs or Nix itself have any single-user testing setup?
I wonder if there's something we can do to stop this from regressing again.

@bhipple bhipple merged commit b65e82c into NixOS:master Dec 5, 2020
@lukegb lukegb deleted the singleuser-fetchzip branch December 5, 2022 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants