I suspect many straightforward approaches will place values on this list in tension/conflict, so much of its value (if it has any) may come from what it rules out. Process changes that reduce individual/global workload and spread it around better are hopefully welcome, but I also think there are some real risks, like:
expending effort just to relocate the clogs (i.e., from collective indecision over a PR to collective indecision over how long we should give a previously-stellar-but-now-burned-out code-owner to address rule on the PR before giving someone else their turf)
converting tasks people are doing with enthusiasm but not consistency/efficiency into a bureaucratic process that they avoid, or into a regular notification-nag that keeps them from focusing on other work
You are right, I didn’t mean to question the theoretic usefulness of the labels, that is their information value.
I meant to question the practical usefulness in social context, that is their informational value + the ability to interpret them.
This is why I labeled them provocatively as cryptographic: You need decoder knowledge (like many do).
It could also be:
impact/S, impact/M, …
maintainer-advantage
You might notice the differenc in stance. It makes them automatically more human friendly.
I sometimes feel labels might have been abused as stateful data store for the ofborg because they seem to change in arbitrary non-intuitive fashions.
I also had a discussion with @timokau on this about marvin labels (“LastMile” / “LastMeter”). That discussion supports where I’m coming from and where I’m aiming at.
But it’s a secondary (even terciary) hurdle, if existing labels only would be used more actively.
+1 to seeing if there could be improvements to the review process.
I sort of think we should just auto approve r-ryantm PR’s that are older than a week old. They’re basically the best tested PRs in existence + I think they’re cognitively expensive to review compared to their risk.
I like the idea. @ryantm, why not? If those are mostly good why not inverse the odds? (and be a bit pushy on the maintainers, who’d else review them on unstable)
I look at every r-ryantm PR and read all the comments on them, and I think probably about 5 to 15% of them are bad in some way. The issues include: dependency changes, runtime breakages, important reverse dependencies failing, Darwin/ARM failures, needing to be updated as part of a group, already part of another more comprehensive PR, etc.
I think it really does take human knowledge and intuition to decide if they are okay to merge.
I could see this changing if we had more package level tests. For example, I’d expect it to be safe to auto-merge one where the package and all its reverse dependencies had tests and the tests passed.
Personally, for all the packages I maintain, I’ve added sanity checks so I’d be able to stay confident that @r-ryantm’s PRs haven’t broken anything (e.g this).
I once sketched in my head a policy we could adopt, where packages could hold a meta field that will enable PRs automatically merge by ofborg once they pass it’s tests along with @r-ryantm’s reverse dependencies tests. Thus maintainers would hopefully be encouraged to add this meta field as long is there’s an appropriate check phase of some sort.
RFC 50 wanted to address that but it got stall because of disagreements upon the kind of policy the bot would implement.
I don’t know. This gives upstream maintainers more or less the power to directly push into nixpkgs by creating a tag. It only requires one malicious actor or compromised GitHub account to push a backdoor or trojan into nixpkgs.
When reviewing PRs I try to (where humanly possible) do a diff between upstream versions to quickly glance if there are no fishy things going on. I also run binaries in a very restrictive bubblewrap sandbox to check if they attempt require strange things (I also plan to automatically extract a list of accessed paths, attempted connections, etc.).
I think there should be a human in the loop, so that the chance to catch something malicious (or just a stupid error) is increased.
IMO this is one of the tasks of a Linux distribution, to ensure that the users get vetted, working, packages. If that is not doable anymore, then perhaps we are packaging more than we can manage.
I completely agree that it would be great if maintainers could merge PRs against their packages. I think this speeds up the process and will keep more maintainers around, since they are more empowered to actually maintain their packages.
I think the fact we package mostly free software, means developers of packages we ship won’t do such a thing. Though I still agree that this is a big risk, as even if the developer is trusted, their infrastructure could be compromised or perhaps the developer’s credentials to the hosting system.
Yea I guess you are right. To my view, this all comes down to my view to our way of managing human resources…
This is exactly what I’m trying to do on my pr to nix-review. But when I think about it, your approach is superior. Wouldn’t this call for a mass PR to try to add a --help check to every package? - and merge those where it’s successful?
The ship has already sailed on this to some extent.
99% of the Haskell packages in nixpkgs are updated automatically with no (or little) human interaction.
I’m not necessarily saying that this is good or bad, just that the Haskell package set in nixpkgs is not checked to the same standards you’re describing. One may argue that it not good to have different standards for different parts of nixpkgs.
Someone is still looking at breakages. I think this is the nixpkgs sweet spot. If the packages have a enough logic to validate that they work “under normal” conditions as part of the build, then there’s not much reason for human involvement.
For python packages, I generally recommend people to include unit tests, and import the module. This gives good assurance that the package is in a usable state.
EDIT: Since python packages aren’t directly imported from a package repository, it’s still necessary for a human to review that nothing malicious is being introduced into the code, but I would consider this “minimal” human interaction.
Agreed. The same is true for many Rust, Go, and Node projects, that pull in so many dependencies that it becomes infeasible to review everything. So, even if the application is vetted, the transitive dependencies are just ack’ed more or less automatically.
I think in the end we (as a field) would probably want heuristics or perhaps models that recognize suspicious patterns and could point people to it. But I don’t think we are there yet. I guess another technical solution would be to sandbox applications as much as possible.
In general, if we’d have automatic merges across the whole package set, I would be scared as hell to deploy NixOS/nixpkgs inside an organization or for a customer. It opens quite a large surface for general or targeted attacks [1]. I think there are at least (for me) two preferable options:
Accept that (outside CVEs) software is sometimes a few revisions behind. It is better to have slightly older software that is somewhat vetted than software that is automatically updates but anything could happen; or
accept that it is maybe not feasible to incorporate every possible package in nixpkgs and focus on a smaller set of packages that are needed by a large set of people. Then have niches covered by (groups of) people who maintain their own package sets/flakes.
[1] Of course, at this point you also have to trust other reviewers and committers. But it is an additional layer of defense.
I’m not necessarily saying that this is good or bad, just that the Haskell package set in nixpkgs is not checked to the same standards you’re describing. One may argue that it not good to have different standards for different parts of nixpkgs.
It might be a good idea to maintain some parts of Nixpkgs to a higher standard than others, but then I guess we need documentation which part has which standard.
(I do not believe in human skimming of upstream diffs of many central packages like freetype, definitely not all of the core packages; if some packages are heroically reviewed up to tracing syscalls and should not be merged until such a check is done, this is definitely worth documenting — a new meta entry for maintenance standards?)
In general, if we’d have automatic merges across the whole package set, I would be scared as hell to deploy NixOS/nixpkgs inside an organization or for a customer. It opens quite a large surface for general or targeted attacks [1]. I think there are at least (for me) two preferable options:
I … do not believe we are even close to having any procedures capable to avoid a targeted attack by an attacker capable to make one upstream release and keep it up and out of security news for a few of weeks. (Note that r-ryantm tracks other repositories, so there is some delay here, and then we definitely need one week window for «wait, no»). A targeted attack surely will be an easily remotely exploitable weakness on par with median submissions to Underhanded Code Contests buried in a related bugfix. It takes time to find it by manual and automatic analysis, and it does not affect the behaviour immediately upon launch…
.