Restrict supported tarball formats to actual Tarballs#10918
Merged
edolstra merged 1 commit intoNixOS:masterfrom Jun 17, 2024
Merged
Restrict supported tarball formats to actual Tarballs#10918edolstra merged 1 commit intoNixOS:masterfrom
edolstra merged 1 commit intoNixOS:masterfrom
Conversation
The documentation is clear about the supported formats (with at least `builtins.fetchTarball`). The way the code was written previously it supported all the formats that libarchive supported. That is a surprisingly large amount of formats that are likely not on the radar of the Nix developers and users. Before people end up relying on this (or if they do) it is better to break it now before it becomes a widespread "feature". Zip file support has been retained as (at least to my knowledge) historically that has been used to fetch nixpkgs in some shell expressions *many* years back. Fixes NixOS#10917
andir
force-pushed
the
restrict-tarfile-formats
branch
from
e20317f to
5a9e1c0
Compare
roberth
approved these changes
Jun 15, 2024
Member
roberth
left a comment
roberth
left a comment
There was a problem hiding this comment.
Niche formats are a risk for reproducibility; for instance a bugfix or a "new" feature like executable bit support have potential to change source hashes, so it should definitely not be all.
We should only want low risk mature formats.
Any additions other than zip and tar should come with a lengthy piece of research proving those properties.
👍 for me and I hope we don't need any other formats.
Member
|
I think this is likely to cause regressions no matter how you cut it. The thing we should actually do here is to figure out how to ensure all the archive formats don't change behaviour over versions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The documentation is clear about the supported formats (with at least
builtins.fetchTarball). The way the code was written previously it supported all the formats that libarchive supported. That is a surprisingly large amount of formats that are likely not on the radar of the Nix developers and users. Before people end up relying on this (or if they do) it is better to break it now before it becomes a widespread "feature".Zip file support has been retained as (at least to my knowledge)
historically that has been used to fetch nixpkgs in some shell
expressions many years back.
Fixes #10917
Context
See #10917
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.