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

[RFC 0009] Nix rapid release #9

Closed
wants to merge 8 commits into from
Closed

Conversation

shlevy
Copy link
Member

@shlevy shlevy commented Apr 4, 2017

@shlevy
Copy link
Member Author

shlevy commented Apr 4, 2017

@edolstra

@shlevy
Copy link
Member Author

shlevy commented Apr 4, 2017

(I really wish I had commit access here so I could request reviewers etc....)

@vcunat
Copy link
Member

vcunat commented Apr 4, 2017

👍 for the principle at least (I haven't read details yet).

@shlevy shlevy force-pushed the nix-rapid-release branch from c38e290 to 2151699 Compare April 4, 2017 14:17
@Profpatsch
Copy link
Member

+1, one addition: Release nix 1.12 as nix 2.0.0 and do semver from there.

@shlevy
Copy link
Member Author

shlevy commented Apr 4, 2017

@Profpatsch good idea, pushed.

@vcunat
Copy link
Member

vcunat commented Apr 4, 2017

The release number policy seems a mostly independent issue to me, and I wouldn't like to get blocked on that. For example, version 2.0.0 might suggest backward-incompatible changes in the nix language, but that's not really planned AFAIK.

@shlevy
Copy link
Member Author

shlevy commented Apr 4, 2017

It's not completely independent, but I'm not averse to changing it if that's the only concern.

@domenkozar
Copy link
Member

I'm very much in favor of this :)

@Profpatsch
Copy link
Member

For example, version 2.0.0 might suggest backward-incompatible changes in the nix language, but that's not really planned AFAIK.

Completely revamping the CLI interface (which is a major interface change) and rewrite of major parts in C++ (trimming 40MB from the package manager size) should be enough for a 2.0 I think.
Also there hasn’t been a release in over a year, so there’s probably lots of breaking changes apart from that.

@shlevy
Copy link
Member Author

shlevy commented Apr 4, 2017

The old interface still exists, FWIW.

@shlevy
Copy link
Member Author

shlevy commented Apr 4, 2017

@domenkozar Co-author?

@shlevy shlevy requested a review from edolstra April 4, 2017 15:33
@zimbatm zimbatm changed the title Add nix-rapid-release rfc [RFC 009] Add nix-rapid-release rfc Apr 4, 2017
@shlevy
Copy link
Member Author

shlevy commented Apr 7, 2017

@zimbatm This RFC clearly has community support, but no one has stepped up to be a coauthor. Frankly I think that requirement just creates more work than needed and requires RFC authors to be pests 😦 . Any chance we can remove it?

@Ekleog
Copy link
Member

Ekleog commented Apr 7, 2017

I don't really think 3 days' notice is enough to gather all counter-arguments that could potentially arise, but I guess that's orthogonal to the coauthor requirement (that said, if noone is stepping up to coauthor, given the really basic requirements for it, maybe it's not that much wanted by the community? -- anyway this discussion about removing coauthor should go somewhere, like in comments in a RFC to remove the requirement)

Then, here are two issues with this, to me. First, I think words like "Personally" should not appear in a RFC. The final text should be the One True Source of Wisdom once accepted, so personal opinion should not matter. But the real issue is that this RFC is too extreme in my opinion.

Stability of a program as defined by "ability to make a release" greatly varies depending on the project. Having master always building and passing the test suite is not an issue but clearly not enough, as test suites always let things through. I think that having a commit on master always meaning a release is just too much, for it means every PR has to be completely bug-free before being released.

That said, I think having a faster release cycle would indeed be useful. But there is a need to actually do something consistent, that lets bugs introduced by new PRs be discovered by the testers running off master branch (or even by manual functional testing, just before a release). An intermediate solution may be to simply tag all issues with a milestone, and as soon as all the issues tagged with the next milestone are solved it can be released.

This, assuming not too many features are tagged for the next milestone, means a fast release cycle (hopefully one new feature per release at most, serious issues can even get a bugfix release by just setting them as the only issue for the next milestone), better tracking of what still needs to be done for next release, yet still allows bugs to be discovered on master before the release, and also to be fixed before it by making the release depend on them.

What do you think about it?

@shlevy
Copy link
Member Author

shlevy commented Apr 7, 2017

@Ekleog

I don't really think 3 days' notice is enough to gather all counter-arguments that could potentially arise, but I guess that's orthogonal to the coauthor requirement

Yeah, I wasn't expecting everyone to chime in in 3 days! Just that a bunch of people have expressed lots of support for this one. I don't think it's really a reflection of how much interest there really is, but maybe I'm wrong there.

anyway this discussion about removing coauthor should go somewhere, like in comments in a RFC to remove the requirement

Yeah, I wanted to open up an issue but this repo has them disabled 😮 Not sure this really merits an RFC... And who would coauthor it? 😁

First, I think words like "Personally" should not appear in a RFC. The final text should be the One True Source of Wisdom once accepted, so personal opinion should not matter.

Fair point, I'll remove that bit.

Having master always building and passing the test suite is not an issue but clearly not enough, as test suites always let things through.

This is always true and why we have bug fixes and PR authors should be careful to manuall test and think of corner cases. I don't think we get significant extra test coverage by having long release cycles.

I think that having a commit on master always meaning a release is just too much, for it means every PR has to be completely bug-free before being released.

That's removed now, if there has been a big feature merge recently we can hold off until testing can happen if need be.

That said, I think having a faster release cycle would indeed be useful. But there is a need to actually do something consistent, that lets bugs introduced by new PRs be discovered by the testers running off master branch (or even by manual functional testing, just before a release).

I'm not sure what this means/how it differs from my idea

An intermediate solution may be to simply tag all issues with a milestone, and as soon as all the issues tagged with the next milestone are solved it can be released.

In my experience these just grow, and aren't maintained well.

What do you think about it?

Personally I think just a delay/testing window manually imposed when a change is worrisome is a better approach.

@Ekleog
Copy link
Member

Ekleog commented Apr 7, 2017 via email

@edolstra edolstra changed the title [RFC 009] Add nix-rapid-release rfc [RFC 009] Nix rapid release Apr 10, 2017
@zimbatm
Copy link
Member

zimbatm commented Apr 11, 2017

@shlevy the co-author is more interesting for long RFCs, it's to avoid having 1000 comments on typo and such when the important bit is the content. At the end of day it's just a process to help us.

I feel like this RFC is a reaction to 1.12 taking ages to be released. This is probably better solved by defining what's left to do to get 1.12 out of the door no?

Who is the release manager for nix, @edolstra? What are the steps required to get it released?

@shlevy
Copy link
Member Author

shlevy commented Apr 11, 2017

@zimbatm It's not just about 1.12, it's been the norm for all previous nix releases and makes it difficult to solve problems that involve upstreaming features.

@Ericson2314
Copy link
Member

@shlevy I really ought not to be coauthor as I've never contributed to Nix itself and will be traveling in May, but if no one steps up I'll do it. It's very important that this happen.

@zimbatm
Copy link
Member

zimbatm commented Apr 11, 2017

@shlevy since this is a process-type RFC it should probably also identify who is in charge of release management. Who has the power to do it and who is volunteering to do it, what are the responsibilities, ... Otherwise the RFC will be discussed and there will be nobody to implement it.

@shlevy
Copy link
Member Author

shlevy commented Apr 11, 2017

@zimbatm I'm happy to do it, and I would not be surprised if there are others if not me, but only @edolstra can decide that.

@zimbatm zimbatm changed the title [RFC 009] Nix rapid release [RFC 0009] Nix rapid release Apr 13, 2017
@shlevy
Copy link
Member Author

shlevy commented Apr 19, 2017

@edolstra Can you chime in here? This one seems to need your goahead

@shlevy
Copy link
Member Author

shlevy commented Apr 27, 2017

@edolstra can we expect a response either way from you here?

@aneeshusa
Copy link

Big +1 from me on this RFC, especially having master more "release-ready" at all times - I work on other projects where this makes it much nicer to develop. I also think the RFC process can help with larger changes to Nix itself and help avoid regressions.
For Nix 1.12 (or 2.0) specifically, I would consider fixing NixOS/nix#1356 necessary before release.

@Ericson2314
Copy link
Member

Ericson2314 commented Jun 3, 2017

I will say, if #14 is passed, I will have a much more enjoyable time implementing it were this RFC policy.

@Ericson2314
Copy link
Member

Ericson2314 commented Jun 7, 2017

One compromise position might be to have a staging branch for DB schema transitions.

@Ekleog
Copy link
Member

Ekleog commented Jun 13, 2017

OK, so re-reading this thread, everyone seems to agree this RFC is needed, except me saying sometimes a bit of time between development of a new feature and a release may help.

Having re-read the RFC, I still think it's much better than the status quo, and would like to withdraw my negative comments (the thing I think I was opposed to was having a release on each master commit, the current state of the RFC looks great to me).

With this move made there seem to be no longer any negative comment.

Could we move forward, then? (I'm thinking having long-lived RFCs where everyone agrees is not a good thing for future RFCs to come up: it reduces the motivation of people to actually make them)

PS: I'm assuming @edolstra's not answering this thread is an implicit way to tell the community “you made the RFC process, now use it and stop relying on me so much”. Which would mean we should go forward and land this RFC if we actually want it implemented. If I'm wrong, please, by all means say it.

@shlevy
Copy link
Member Author

shlevy commented Jun 14, 2017

@Ekleog I don't think that's a fair way to interpret @edolstra's lack of response here. I've been explicitly chastised for adding release tags to the nix repo without his say up until now and he is the primary active developer on the project. I don't see a way to proceed with this issue in particular without his say-so. And it would take 2 minutes for him to leave a comment suggesting that we should go ahead based on community consensus here.

@domenkozar
Copy link
Member

Imho, the right way to go is to have a release manager, the same way we did for NixOS.

@shlevy
Copy link
Member Author

shlevy commented Jun 14, 2017

Happy to do that or find someone to do it, but that process itself will require at least initial signoff from @edolstra

@Ekleog
Copy link
Member

Ekleog commented Jun 14, 2017

@shlevy You are right about the 2 minutes required to leave a comment suggesting we should go ahead based on community consensus, but what stuns me is that he actually changed the PR title on Apr 10 (meaning he noticed and read it), yet left no comment... Obviously, I may be wrongly interpreting it, and you have much more experience than me :)

@Ericson2314
Copy link
Member

I offered a few changes, mainly making the first two bullets stricter. What do you all think?

@vcunat
Copy link
Member

vcunat commented Jun 17, 2017

I'm afraid the efforts are doomed unless/until Eelco shows some support. (I don't know if that might be helped by someone pledging some effort to make the releases happen.)

@shlevy
Copy link
Member Author

shlevy commented Jun 18, 2017

I'm willing to commit to reasonable efforts (including ensuring test coverage, making a timeline, tracking down bugs, etc.) to get the next release out and set us up to move on to a rapid release model if approved.

@shlevy
Copy link
Member Author

shlevy commented Jun 25, 2017

@edolstra Can you please let us know what further info/changes/commitments would be necessary to enable to you make a decision on this?

@Ekleog
Copy link
Member

Ekleog commented Jun 30, 2017

Given nixpkgs has to be compatible with stable nix (per policy), not having nix 1.12 blocks a number of potential changes (like the use of encryption as per RFC #5 when it's accepted, or nixos-prepare-root that would make for a cleaner implementation of #12, for the two that I can think of).

So having nix 1.12 released and then trying to release a bit more often would most likely help, even though obviously it doesn't mean to let bugs sink in a release, and this process of releasing often while not letting partially written modifications in is (I think) described here.

EDIT: This was initially an answer to @0xABAB's post from 2017-07-01, which appears to have been deleted since then. I will not repost it here, given he apparently wants it to not be readable, but this would make my comment unreadable without the details.

@0xABAB
Copy link

0xABAB commented Jul 2, 2017

@edolstra does vastly more work (apparently 9 times the number of commits of the second committer) on Nix. As such, while it would be cool if he would like to modify his way of working, we can't actually make him do this; without @edolstra there defacto is no Nix (but nixpkgs would still be viable).

I think if @edolstra doesn't ever respond, the only option is to fork.

I am not saying we should fork, because overall nix works fairly well and seems to improve over time.

This RFC is a bit like Australia and the UK voting about whether the US should invade Syria.

The way this should have been handled was sending an e-mail to @edolstra (assuming he doesn't mind those from @shlevy) asking whether he thinks it would be a good idea and if the most important stakeholder was on board (that's what the RFC process prescribes even) then it would have been a good time to ask what the rest of the community thinks about it (if at all), since defacto @edolstra's opinion is the only one which matters, due to the fact of him being the owner of the repository.

The only reason for asking the community would be to decrease the risk of alienating the community, but the risk of that is so incredibly low in that case that it doesn't seem useful to even ask.

@zimbatm
Copy link
Member

zimbatm commented Jul 2, 2017

I still think this RFC is a reaction to 1.12 taking ages to be released. And @edolstra is not communicating which makes it hard to know what to do next. I can only guess but I'm assuming there are two major reasons not to do the release:

  • Not enough testing. The tool is not stable enough.
  • The new UX is not ready. It's a big change which has deep implications for the future so understandably one would want to make sure it's in the right shape.

Based on those assumptions I would say that the best path forward is to (1) find and fix all remaining stability issues. (2) make a nix 1.12 release where the nix dispatcher is disabled. That would let us have the release while still maturing the nix dispatcher.

@vcunat
Copy link
Member

vcunat commented Jul 3, 2017

  • (UX not ready): I don't think that's a reason to stop the release. The old nix-foo interface still works OK on 1.12pre* and we can explicitly say that the new nix command is experimental and possible to change completely.

@vcunat
Copy link
Member

vcunat commented Jul 3, 2017

  • (not enough testing): look at Hydra:

    Hydra 0.1.1234.abcdef (using nix-1.12pre5413_b4b1f452).

    I think that's quite some testing by itself, at least for some parts of nix. It has one specific configuration, but some potential problems, e.g. recent setuid/setgid "failures" in builders, are detected that way. I've been running 1.12 on my machine where I do most of nix* stuff, as another small point.

@shlevy shlevy closed this Jan 19, 2018
Ericson2314 added a commit to Ericson2314/nix-rfcs that referenced this pull request Apr 9, 2020
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.

9 participants