treewide: convert patch series files to Nix expressions #96269
Conversation
deviant
requested review from
edolstra,
infinisil,
nbp and
ttuegel
as code owners
Quilt is in no way made redundant by the Nix expression language. Nix does not provide a workflow for managing patches: applying patches to an unpacked source tree, editing, and reordering patches. By all means, let's remove the patch series files, but let's not justify it with false statements. By the way, as a maintainer, the only thing that was a bigger pain for me than dealing with Quilt, was dealing with lots of patches without Quilt, so if anyone has suggestions there, I'm listening. |
I didn't say that. I said that the file format is redundant if using the Nix expression language, which offers the same syntactical constructs (and more). There's no reason why Quilt itself couldn't be modified to produce Nix expressions.
Have you checked out git-series? It offers a lot of the functionality provided by Quilt, but integrated into git (so you have full patchset history and diffing). It also has really nice support for rebasing patches (each patchset has a base commit, and the history of all rebases is retained). Applying patches/fixing rejects can be done normally with git itself. Of course, you will have projects that do not use git, and since you're probably more interested in the distribution aspects of patchsets, there are probably places where you cannot replace Quilt itself. But again, I'm talking about the format. I maintain a private overlay with a set of series.nix files for each of the pieces of software I have patches for, and while I don't have enough to justify using Quilt for the most part (and in the cases where there are conflicts/large numbers of patches/etc, I am generally working with the upstream repository), it's just a 1-1 translation between the two (indent everything by two spaces, stick square brackets around the ends, prefix with ./ to turn the paths into Nix paths, and add comments at the end of each line containing the commit message/mail subject to make things easier to browse through). One could easily imagine modifying Quilt itself to generate these. I hope this clarifies things. |
Nix expressions provide all the features that Quilt series files have, so using those instead is pointless. Also, lib.readPathsFromFile (the function used to read series files) has the following warning: > NOTE: This function is not performant and should be avoided. This also removes some orphaned series files, and unused references to copyPathsToStore (which were probably missed in previous commits where calls to lib.readPathsFromFile were removed)
> NOTE: This function is not performant and should be avoided. It's not used at all in-tree now, so we can remove it completely after any remaining users are given notice.
deviant
force-pushed
the
remove-quilt-series
branch
from
c39bd6a
to
423fc3f
Compare
|
@infinisil Done. |
|
(Is the editorconfig check new? Either way, it's not on lines I touched) |
lib.readPathsFromFileis slow, unidiomatic, and barely used throughout nixpkgs.It exists to read patch series files in the Quilt format, something made completely
redundant by the Nix expression language having full support for lists and comments.
This proposes to remove usage of these, opting to instead use Nix lists for the task,
which both removes indirection (patch series are now visible directly in the derivation),
and is more efficient. There were not many instances of
lib.readPathsFromFilebeingused, mostly localised to KDE software.
This also adds a warning notice to users of
lib.readPathsFromFile. I didn't modifyits documentation, and am unsure whether it should just be removed entirely. There
doesn't seem to be any documentation for process or deprecating things, so I just
took the most reasonable conservative approach I could.
By the way, is there a convenient way of testing if a particular commit changed the
derivation of an expression or not? It would be useful for refactors like this, where
there shouldn't be any difference between the two. nixpkgs-review just crashed.
I ended up checking each package manually; am I missing a simpler way?
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)