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

Improve handling of tarballs that don't consist of a single top-level directory #11195

Merged
merged 9 commits into from
Jul 29, 2024

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Jul 26, 2024

Motivation

When unpacking tarballs, we now only strip the top-level path component if the tarball consists of a single directory or a single non-executable file.

Fixes #4785: top-level directories are no longer merged into one.

Fixes #10983: top-level non-directories are no longer discarded.

Closes #9053.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Fixes NixOS#4785 (top-level directories are no longer merged into one).

Fixes NixOS#10983 (top-level non-directories are no longer discarded).
@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Jul 26, 2024
@roberth
Copy link
Member

roberth commented Jul 26, 2024

top-level non-directories are no longer discarded. Note that there still is a behavior change for tarballs that consist of a single executable file: to preserve the executable bit, these are returned as an executable file inside a directory.

I think this should also be the behavior for single non-executable files. Tarballs are expected to behave like directories, so we should probably always return a directory, even if it only has a single file.
Currently it's returning an empty directory - ie it's broken, so unless this is a regression, it seems we're free to pick the best behavior for this.

@edolstra
Copy link
Member Author

I think this should also be the behavior for single non-executable files.

Yeah, it looks like even Nix 2.3 returned an empty directory so I'll change it to dereference directories only.

Since this yielded an empty directory as far back as Nix 2.3, we don't
really need special handling for executables vs non-executables.
src/libfetchers/git-utils.hh Show resolved Hide resolved
@edolstra edolstra enabled auto-merge July 29, 2024 13:12
@edolstra edolstra merged commit 0b96c58 into NixOS:master Jul 29, 2024
12 checks passed
@edolstra edolstra deleted the tarball-roots branch July 29, 2024 14:59
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-07-29-nix-team-meeting-minutes-165/49970/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
3 participants