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

pkgs: remove souper-2017-03-23 #61509

Merged
merged 1 commit into from May 18, 2019
Merged

Conversation

thoughtpolice
Copy link
Member

Motivation for this change

Souper is a very experimental package that's only useful for for
compiler hackers or people who are doing PhDs. The support/cost ratio of
keeping such a package probably isn't very good.

More importantly, this puts us one step closer to deleting
llvmPackages_4, as this is one of the only two remaining packages using
it. Deleting these older LLVM packages would be far more useful than the
value Souper currently provides.

Current versions of Souper support more recent LLVM versions anyway, so
this expression could be resurrected. But it might be better to place it
in the Nix User Archive (NUR), perhaps.

/cc @taktoa

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Souper is a very experimental package that's only useful for for
compiler hackers or people who are doing PhDs. The support/cost ratio of
keeping such a package probably isn't very good.

More importantly, this puts us one step closer to deleting
llvmPackages_4, as this is one of the only two remaining packages using
it. Deleting these older LLVM packages would be far more useful than the
value Souper currently provides.

Current versions of Souper support more recent LLVM versions anyway, so
this expression could be resurrected. But it might be better to place it
in the Nix User Archive (NUR), perhaps.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
@thoughtpolice thoughtpolice self-assigned this May 14, 2019
@thoughtpolice
Copy link
Member Author

@GrahamcOfBorg eval

@@ -5867,8 +5867,6 @@ in

soundkonverter = kdeApplications.callPackage ../applications/audio/soundkonverter {};

souper = callPackage ../development/compilers/souper { };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put a souper = throw "<reason for removal>";? I don't like removing things without any trace.

@taktoa
Copy link
Member

taktoa commented May 14, 2019

Unfortunately I don't have the time to maintain this, but perhaps @dtzWill would be interested? If not, feel free to nuke it, though I agree with @infinisil that we should leave a throw in there, just in case someone wants to write the package again and doesn't notice that I already did the bulk of the work involved.

@infinisil
Copy link
Member

Not only that, but if we decide to potentially break people's config's, it should at least tell them why we broke it. Without a throw, they'd just get a "no such attribute souper".

@thoughtpolice
Copy link
Member Author

thoughtpolice commented May 14, 2019

I mean, do we really need that for every single package removal? If this was a seriously well known package that fell by the wayside, was a woeful security nightmare or superseded by something else -- I would fully agree. But honestly -- how many users of Souper are there? Probably like 15, all of them work at a university, and likely ~1-2 of them use NixOS. And if they are using Souper, they probably want a version that's more than 2 years old! So the actual amount of users is effectively zero, I'd think.

I don't think this needs any kind of deprecation/throw in aliases or whatnot; it likely has been built by Hydra 50 times more than it has ever actually been downloaded, I would assume. It is literally not worth adding the line in the file for it.

@thoughtpolice
Copy link
Member Author

thoughtpolice commented May 14, 2019

I see no usefulness of deleting older LLVM packages. I find useful their presence in nixpkgs, unrelated to the two packages which need them.

Getting rid of older packages is essential in the long run for the sustainability of Nixpkgs. Perhaps I am the outlier here, but many people share this view of yours, and I don't agree with it:

  • Having packages built and shipped to users is effectively an endorsement that maintainers or other users will dedicate time to issues that arise with them. This clearly isn't always true for most packages, and even if it was, it's applied very un-evenly. This very ticket is doing bikeshedding over adding an alias with a deprecation warning for an extremely, extremely niche package that doesn't carry its weight and likely has zero actual users, if it ever had any noticeable number before. It is literally so insignificant that keeping around llvmPackages_4 could be considered a literal waste of physical energy wasted due to compilation, because one of its only needed dependent packages is completely unused in every way. It probably takes 10x longer to compile this one dependency than the unused thing that needs it, that's just a waste no matter how you cut it.

  • Every single package increases the "surface area" of the project and means that doing things like removing old APIs that might be in use, old dependencies, etc becomes far more difficult in the long run, regardless of their actual utility to any user. This looks good when you want to boast about "we support a huge number of packages!" and it's a nightmare when you need to fix a build error because a stdenv upgrade occurred, or someone updated a dependency and broke this thing, so now you want a private copy because the new version has an API change/regression, etc etc. The complexity never feels like much in isolation but accumulates significantly over time.
    Not only does deleting this code get rid of one package we have to maintain, these kinds of things often lead to several other simplifications, especially for complex expressions that must sustain multiple versions. It would be different if older LLVM versions were still under some support policy (Postgres has 5 major versions and we support them for 5 years, as upstream does, which seems viable), but these packages are effectively abandonware that have diverged from upstream.

  • Every additional packages literally adds time and compilation to all of our software builders, the size and memory use of evaluations, etc. Cutting down on this as much as we can would likely save hours -- cumulatively saving days -- of compilation time on Hydra. Hydra is a finite resource and we do not have an unlimited cheque to spin up 10,000 cloud instances forever. Compute resources for the build machines are probably are most vital resource to conserve for smooth usage of the cache.

The third point is really the thing I think few people appreciate in any way. Everyone believes that as long as a package still "compiles" it should be left alone, even when it is completely unmaintained and has nearly no users -- but this is effectively a way of externalizing the cost of package maintenance onto random people and the build farm, because "I don't maintain that package, but I guess 3 people on Earth use it once a year, so we should let Hydra use 10 cores to build it for 30 minutes, also I might need it once for 20 minutes a year from now" -- when all that time could be vastly spent compiling more useful packages and reducing churn/turnaround for detecting failures, etc. Combined with the fact Nix painlessly handles a huge problem in most toolchains (multiple versions, private library vendoring, etc), it's very "attractive" to go overboard here.


This also leads into your point about NUR and removing older clang -- if, tomorrow, you find a package in NUR or elsewhere that needs clang_35, well then, you should add clang_35 to NUR and use it from there, and add it to NUR. If you find a project and it only requires Clang 3.5, and we do not have this 3-5 year old package, and you want to include it upstream in Nixpkgs -- you should ask yourself first if it is actually worth adding upstream if that's the case, and think hard about why it has to use that old version, and why it can't be replaced/use a newer one, and why it's absolutely super important it exists in upstream (because it has 100,000,000 users all over the globe or whatever.) If you want to add it because there are 5 people including you who want to use it and the last commit to the project was 5 years ago and there are multiple alternatives -- it probably should not be in upstream, and it should be in NUR.

This also segues into a dynamic that happens all the time: even if a package takes 2 hours to compile and there's only 1 user in the entire project, the moment you suggest "remove it from upstream, move it to NUR, you can use it from there" the complaint soon becomes "well now I have to compile it, and I don't like that". In my opinion: that's just too damn bad. The needs of one person compiling an expression on their own machine once every 6 months for a one-off piece of software is vastly dwarfed by the needs of Nixpkgs as a project.

In the end, this stuff does help us listing off total package counts and treat old expressions like they're on display in a museum, which is nice for boasting about the project, but awful for sustainability and maintainability in the long run.

@thoughtpolice
Copy link
Member Author

And to be clear: not every package necessarily goes by the things I said before, obviously you have to use your own judgement.

In my opinion? llvmPackages_4 simply isn't worth the cost of keeping around -- maintenance cost, build time cost, etc -- for what it offers. The cost of a 10 line C program or whatever is vastly smaller than a multi-million line project to maintain, debug, ship to users, etc. I am also certainly guilty of being overzealous in shipping small, probably-not-very-useful packages upstream, but on the other hand most of these do not have extremely burdensome build time/maintenance requirements (and for others, I have it on a TODO to get rid of some of my older unmaintained ones, as well).

@thoughtpolice
Copy link
Member Author

thoughtpolice commented May 14, 2019

That is is closer to https://github.com/triton/triton stance: remove python2, GTK2 and let's try to live with it.

Please don't misrepresent my position: I've already said that not every package series should have this policy applied, for example PostgreSQL, which maintains 5 years of major versions and has an associated 5 packages, which I also help maintain. There's nothing inherently wrong with maintaining old packages, even for a long time, but there's no need to keep around things that are almost certainly useless.

Basically, GTK2 and Python2 also have vastly larger dependency and usage footprints, making them simply more useful to keep around in the long run. The associated maintenance cost is simply more justifiable. (Although, you're in for some bad news, because there's been active discussion recently about removing Python 2 or minimizing it/replacing it with Pypy). LLVM, on the other hand, a major package with many dependencies -- has nearly seven different major versions, and 3 of them have a total of 4 or 5 reverse dependencies total which are all probably pretty slim, and almost all of those packages are outdated pieces of software. It has a costly build cycle and a complex build expression which must be maintained. Some packages have more users than others, but in any case, this simply isn't a good use of maintainer resources, or compute time when they are barely used, and, IMO, we should seek to prune them where appropriate.

They're completely different categories of packages, and we do not need to keep literally everything that has been added in history to nixpkgs. Please try actually addressing the substance of my points and thinking over cases rather than blanket replying to them.

But the problem is such a Linux distro has 2 users no contributors besides the founders.

Trite retorts or whatnot aren't a response I care for, nor consider at all when actually talking about issues of this nature and writing patches for them.

If you have actual points that address the core complaints in my issues, then please lay them out. If you simply think it's nice that old packages sit there -- that's also fine! There's nothing really wrong with thinking that, but I will still actively seek to remove old packages lest actual justifiable technical reasons pop up, in all honesty, or I'm significantly outweighed on this position (but there is supporting momentum to remove old LLVM packages anyway, see #44372)

@infinisil
Copy link
Member

While I'm not entirely knowledgeable on clang/llvm, I'm with @thoughtpolice in that we should get rid of old/unused/unmaintained software over time. Additionally we have a huge advantage with nixpkgs + hydra in that users are able to just use an older nixpkgs version and use hydra's builds, years after packages have been removed.

@infinisil
Copy link
Member

We could even have

{
  foo = throw "foo has been removed due to it being outdated, " +
    "if you wish to have it anyways, " +
    "use nixpkgs version <rev> where this package is still available";
}

(where is the last nixpkgs commit that has a successfully cached build of the package)

@thoughtpolice
Copy link
Member Author

Something like that would be nice but we'd probably want something a little more automatic maybe? It would be awesome if this could just occur in some automatic way, though there's a lot of edge cases to think about (what if a previously used, ancient top-level attrname is reused?) Might be worth opening an issue about.

(Also, I hope I didn't sound snippy in this issue, I truly do think it's fine to have valid reasons to keep old packages, or even simply not want to remove them -- but I believe removing unneeded, costly things is ultimately in the best interests of everyone. After all, if we're all contributors, we have to clean our own house sometimes.)

@globin globin merged commit 316e381 into NixOS:master May 18, 2019
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