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

Haskell packages not matching broken version should be allowed. #9287

Closed
wants to merge 1 commit into from
Closed

Haskell packages not matching broken version should be allowed. #9287

wants to merge 1 commit into from

Conversation

kquick
Copy link
Contributor

@kquick kquick commented Aug 16, 2015

As written, if a Haskell package does not match the broken version, the assert fails which aborts the evaluation. I observed this with cmdlib, which is marked as broken for "0.3.5", but the new version in hackage-packages.nix is "0.3.6". I believe that the attached captures the intent: broken packages versions are marked as broken and non-broken versions are accepted as-is.

@jagajaga
Copy link
Member

cc @peti

@peti
Copy link
Member

peti commented Aug 17, 2015

@kquick, the idea behind markBrokenVersion is that the assertion failures draws attention to the fact that an update on Hackage has occurred and that the override needs to be re-evaluated, i.e. removed or have its version updated. I usually use this function instead of markBroken for packages that have no upstream bug tracker, so it's hard to know when an issue has been fixed. So, from my point of view, the fact that there's an assertion failure is kind-of the only reason for that function to exist in the first place. 😁 It's a way to help us get obsolete cruft removed from our configure-xxx.nix files.

Does that make sense?

@peti peti added 0.kind: question Requests for a specific question to be answered 6.topic: haskell labels Aug 17, 2015
@kquick
Copy link
Contributor Author

kquick commented Aug 17, 2015

@peti: Ahh, nowI understand the purpose. Some thoughts on the matter:

  1. With the latest nix-channel unstable, this was causing an assertion for my local project that used cmdlib. It would be great if this was caught by hydra tests or something, but I understand it's difficult to guarantee that every single hackage package gets evaluated as part of the build.

  2. It's "obsolete" because the auto-generate hackage-packages got a new version from Hackage, right? But if someone is still using a broken version explicitly the broken annotation is still applicable. This is one area that nixpkgs doesn't really address: there is not really a per-package history, just a concept of "current", so from that perspective then yes, the broken annotation of the older version is no longer needed.

  3. This issue points out the friction between "maintainer" and "user". In this case, the assert helps the maintainer at the expense of the users. I'd suggest that this is the wrong direction, but I can certainly see the need for eliminating cruft from the maintainer's side.

If I understand correctly, the better patch would have been to drop the "markBroken" in the ghc7101 customizations specification. Perhaps a comment in lib.nix with essentially the text in your comment above would have helped lead me to this more correct conclusion.

Longer term, I could see the use for a script to compare current versions against older references that would help change the weighting in #3 towards user over maintainer, but I suppose that's something to put on someone's "free time" list. Anyone have one of those that's empty? :-)

@peti
Copy link
Member

peti commented Aug 17, 2015

It would be great if this was caught by hydra tests or something.

Yes, it would. Unfortunately, Hydra simply ignore packages that throw an assertion failure. I wish that weren't the case, but there's no hope changing that behavior in the foreseeable future because having an (mostly) assertion-failure-free Nixpkgs tree that evaluates on half a dozen platorms is an undertaking that would need a small army of volunteers to pull off. As it is now, we'd be swamped in useless error messages even more than we already are. So, well, Hydra is not going to help us with this at the moment.

It would help if the markBrokenVersion'ed package is used as a build input by another build -- in that case assertion errors are not ignored. As is happens, cmdlib is used only by other packages that are marked broken themselves, so this case didn't occur.

Generally speaking, I believe that markBrokenVersion doesn't accomplish what it was intended to do and that it should be removed altogether. I find myself not using it anymore anyway.

It's "obsolete" because the auto-generate hackage-packages got a new version from Hackage, right? But if someone is still using a broken version explicitly the broken annotation is still applicable.

I'm not sure whether I follow. What do you mean by "someone still using a broken version explicitly"?

This issue points out the friction between "maintainer" and "user". In this case, the assert helps the maintainer at the expense of the users.

Again, I don't believe that I understand your point. Why do you think that this override affect users negatively?

If I understand correctly, the better patch would have been to drop the "markBroken" in the ghc7101 customizations specification.

Yes, that's what I'd recommend doing.

I could see the use for a script to compare current versions against older references.

Yes, some automated way of catching those updates quickly once they occur would help improve the code base enormously!

@kquick
Copy link
Contributor Author

kquick commented Aug 17, 2015

This issue points out the friction between "maintainer" and "user". In this case, the assert helps the > > > maintainer at the expense of the users.
Again, I don't believe that I understand your point. Why do you think that this override affect users > negatively?

Basically just saying that from the "users" point of view, if the version supplied is no longer the known broken version that the build should or could simply be attempted with the optimistic perspective that it would succeed. This is in contrast to your position where the assert fails so that you as the maintainer know that the nix specifications need to be updated to return things to operational mode. However, that's also the optimistic perspective and there's no assurance that the new version isn't still broken.

Thanks for the explanation; I am closing this pull request since it's not the desired direction. Do you know what specifically was broken with cmdlib 0.3.5? I can submit a new pull request that either updates the "broken" check to match the 0.3.6 version or else remove the markBroken for cmdlib entirely.

@kquick kquick closed this Aug 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: question Requests for a specific question to be answered 6.topic: haskell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants