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

We have a ton of Erlang interpreters and even more top-level attributes for them #24047

Closed
copumpkin opened this issue Mar 19, 2017 · 10 comments
Closed

Comments

@copumpkin
Copy link
Member

development/interpreters/erlang contains the following:

  • R14
  • R16
  • R16B02-8-basho
  • R17
  • R18
  • R19

And all-packages.nix contains the following attributes calling out to those files:

  • erlangR16
  • erlangR16_odbc
  • erlang_basho_R16B02
  • erlang_basho_R16B02_odbc
  • erlangR17
  • erlangR17_odbc
  • erlangR17_javac
  • erlangR17_odbc_javac
  • erlangR18
  • erlangR18_odbc
  • erlangR18_javac
  • erlangR18_odbc_javac
  • erlangR19
  • erlangR19_odbc
  • erlangR19_javac
  • erlangR19_odbc_javac
  • erlang
  • erlang_odbc
  • erlang_javac
  • erlang_odbc_javac

Passing in various attributes. Do we really need all of those? I'm upgrading the Darwin stdenv to LLVM 4.0 and Clang is fairly strict so I need to patch Erlang, and I'm balking at having to patch all of these separately. R14 even appears to be unreferenced.

Not saying they should all necessarily be deleted, but I want them to be deliberately kept around rather than nixpkgs accreting all sorts of versions of things. Across nixpkgs we generally try to not keep around older versions of tools and libraries unless it's very common to depend on older versions of them, mostly because it's a maintenance nightmare to keep a ton of versions lying around (like what I'm running into now).

cc all maintainers of the various expressions: @the-kenny @mdaiter @sjmackenzie @couchemar

@butterflya
Copy link
Contributor

butterflya commented Mar 19, 2017

As you say, the problem is a lack of a policy.

A policy which can be automated is to determine a set of different distributions to use as a baseline (for example CentOS <current version - 2 major versions>) in the case of a singleton set and then keep all the versions which are compatible with the operating systems in the earlier mentioned set.

Above is my proposal for a policy, but feel free to suggest others or to even open an issue specifically for this. I am in favor of a policy where a machine can decide the answer such that we eliminate discussion in the project regarding specific packages; if at some later point in time there might be a reason to discuss the policy, because of a changed environment, then that would be of later concern.

@mdaiter
Copy link
Contributor

mdaiter commented Mar 19, 2017

As the maintainer of the basho fork of Erlang, I can attest that Riak, Riak CS and Stanchion officially list that specific fork of the interpreter to function properly.
There is a massive shift between R17 and R18 that breaks and deprecates large portions of the os namespace functionality in terms of API naming and arity of functions. We should be hesitant when removing either of these.
I haven't personally seen a package need R15 or R14 specifically.

Maybe a general game plan for the time being would be to review all packages in 'top-level/all-packages.nix' and see which packages specifically need R14. If no packages require it, then we can safely remove it. We can do this for R15 as well, so on so forth.

I would be extremely hesitant when removing specific versions of the interpreter. The community created those packages for a reason, and we don't want to remove these packages unless that reason is null and void.

@copumpkin
Copy link
Member Author

copumpkin commented Mar 19, 2017

@butterflya I'd love for this sort of thing to be automatically enforceable but I don't know how to do it and it's still wasting me a bunch of time right now so I'd like to do something about the short term 😄 either way your points seem fine but it gets harder in the case of nixpkgs when we have all sorts of variants of packages that can happily coexist if we want them to.

@mdaiter yeah, I won't presume to know how the erlang ecosystem works. However, a lot of nixpkgs comes into being by people who don't quite understand how things work copying and pasting things around, so it's hard to judge when I see a big combinatorial explosion of odbc/java/erlang versions whether that was intentional for the right reasons or someone just thought "that's how it's done when you add a new version".

Either way, point taken about things being used. The policy I'd aim for is that if another nixpkgs package uses an interpreter version (and can't be upgraded to a newer one), then we keep the interpreter, otherwise it goes away. That's I think how we generally approach it elsewhere, even though I don't think anyone's written down such a policy.

So:

  • R14.nix seems completely unreferenced in any Nix expression in nixpkgs
  • erlangR16 is used by couchdb
  • erlangR16_odbc doesn't appear to be referenced anywhere in any other expression
  • erlang_basho_R16B02 is used as you say by Riak and Stanchion
  • erlang_basho_R16B02_odbc is unused
  • erlangR17 is used by yaws
  • erlangR17_odbc, erlangR17_javac, and erlangR17_odbc_javac are unused
  • erlangR18 is the default value of erlang (and thus used all over the place)
  • erlangR18_odbc, erlangR18_javac, and erlangR18_odbc_java are aliases for the unversioned counterparts, but neither the versioned nor the unversioned counterparts seem referenced anywhere
  • erlangR19 and the usual three variants appear unused.

My inclination would be as follows:

  1. Kill R14.nix with fire since nobody even has any way to notice today (this is already done)
  2. Kill unused variants of older versions. This would be R16_odbc, basho_R16B02_odc, erlangR17_odbc, erlangR17_javac, and erlangR17_odbc_javac. For now I'll consider R18 and R19 "current".
  3. Ideally leave a comment trying to dissuade future people from blindly copying the odbc/javac combinations into top-level attributes, as each of them boils down to a fair amount of build work for Hydra and folks trying to test changes, and seem to largely be unreferenced except perhaps in (invisible to us) local deployments. I'd like the policy to be to justify keeping all the combinations in a comment rather than to default to keeping them around.

Thoughts? I'm happy to delete the things I'm proposing and to add relevant comments if y'all are okay with it.

@copumpkin
Copy link
Member Author

Deleted the R14 file in de0849d. I'll leave the attributes in place until I get some input from more people.

@LnL7
Copy link
Member

LnL7 commented Mar 19, 2017

I drafted some cleanup in #17240 but never finished it.

As for the versions, I think we should remove all the versions with overrides unless there's a good reason not to. People can use .override if they need it. We should probably also use lowPrio for the older versions.

@butterflya
Copy link
Contributor

I think it also depends on the details of the patch. Why can't there be one program which generates the configuration for all versions?

@couchemar
Copy link
Contributor

erlangR16 is used by couchdb
erlangR17 is used by yaws

Is it hard restriction? Can we switch to erlang R19 (18?) in this expressions?

@mdaiter
Copy link
Contributor

mdaiter commented Mar 20, 2017

@copumpkin I just compiled yaws on Erlang R18, and all seems to work. Furthermore, the yaws project page states that the project can be compiled with any version of Erlang past R16B01. Therefore, we could remove R17 if we switch over yaws to R18.

A PR is open here: #24122

@mdaiter
Copy link
Contributor

mdaiter commented Mar 20, 2017

@copumpkin just opened a PR for upgrading CouchDB to R17 here: #24123. Therefore, if we hold off on deleting R17, we could delete R16.

@mdaiter
Copy link
Contributor

mdaiter commented Mar 21, 2017

@copumpkin just opened a PR for switching the Riak interpreter here: #24157. This appears to be one of the last packages using R16. If we accept this, we could delete the R16 interpreter.

LnL7 added a commit to LnL7/nixpkgs that referenced this issue Jun 23, 2017
Fixes NixOS#24047

All of the variants are still available in the beam.interpreters attrset.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants