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

Jump to bottom

Haskell: configuration-common: updates to overrides #91312

Merged
merged 7 commits into from Jun 24, 2020
Merged

Haskell: configuration-common: updates to overrides #91312

merged 7 commits into from Jun 24, 2020

Conversation

Anton-Latukha
Copy link
Contributor

@Anton-Latukha Anton-Latukha commented Jun 22, 2020 •
edited

Motivation for this change

Reports and PRs are merging and closing, often overrides await a release.

  • xmobar: update patch description
  • statistics: free from overriding
  • rib: update override description
  • binary-instances: update override description
  • nixfmt: update override description
  • pantry: free from override and upstreamed patch
  • feed: free from override
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Anton-Latukha Anton-Latukha mentioned this pull request Jun 22, 2020
Closed
@Anton-Latukha Anton-Latukha force-pushed the upd-Haskell-configuration-common-01 branch from 25ff82f to f03d7b3 Compare June 22, 2020 18:55
@Anton-Latukha Anton-Latukha mentioned this pull request Jun 22, 2020
Closed
@Anton-Latukha Anton-Latukha changed the title Haskell: configuration-common: override updates Haskell: configuration-common: updates to overrides Jun 22, 2020
@maralorn
Copy link
Member

@Anton-Latukha Thx for this work!

Note that I have a slightly more sophisticated fix for hnix at #91251 so maybe you want to remove this from this PR or rebase it once the other one got merged.

Other changes look good to me, but haven‘t tested the builds.

@Anton-Latukha
Copy link
Contributor Author

Oh, thanks. You are right.

@Anton-Latukha Anton-Latukha force-pushed the upd-Haskell-configuration-common-01 branch from f03d7b3 to aef386c Compare June 22, 2020 21:27
@maralorn
Copy link
Member

maralorn commented Jun 22, 2020 •
edited

I like the improvement of the coments.

On a more general note we should definitely strive to have some standard of consistently telling our future selves when to touch an override again.

@Anton-Latukha
Copy link
Contributor Author

Anton-Latukha commented Jun 22, 2020 •
edited

I think someone in the past, several years ago and more (somehow I think about thoughtpolice or some other Uroboros type avatar) - had a scrip that cleared the list and tested clean builds of the broken/overridden packages.

It should be pretty simple to setup such crawler/walker.

@Anton-Latukha
Copy link
Contributor Author

Anton-Latukha commented Jun 22, 2020 •
edited

#91312 (comment)

Yes, IDK why others do not do such things, that is so obvious. Even for myself I made a function that inserts those # ISO-short-date: NOTE: , so I can indicate myself how long ago I made remark, it is pretty useful everywhere, good example is NixOS derivations list.

cdepillabout
cdepillabout reviewed Jun 23, 2020
View reviewed changes
pkgs/development/haskell-modules/configuration-common.nix Outdated
Comment on lines 1123 to 1119
# Jailbreak: https://github.com/jaor/xmobar/issues/463
# The test suite tries to mess with ALSA, which doesn't work in the build sandbox.
# > 0.33 => rm 464.patch
Copy link
Member

Choose a reason for hiding this comment

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

In addition to a message like > 0.33 => rm 464.patch, would you be able to open a PR upstream asking for a new release? (Or at least a Hackage revision...) And then link to that issue here?

It is nice to have an upstream issue where we can potentially communicate with the upstream maintainer about releases.

(The same goes for nix-fmt and possibly rib below.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

I thought about it, obviously.

I refuted it with that it is a responsibility of maintainers to make the release to fix builds, it is not our responsibility to push them to make releases.
Of course, we can foster them, and it would give us the link to check with the same util.

I thought of it just to basically have a CLI util that returns last Hackage version.

Also, I straightly adopted a practice to cycle through open reports "New/Next release". Made a release - close its report, and open new report for the next release, have those regularly named reports - that seems logical both from an organizational and downstream community perspective.

I would open upstream report requests regarding those entries, but again, it is really the main upstream maintainers responsibility to give a release to give the community the working project builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

This thing also can be semi-automated.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so this is what the Hackage revision system was made for.

Above, I asked for you to ask the maintainers to upload a new release, but in a lot of these cases, just a new revision of an existing release would be sufficient. I would expect package maintainers to know this and be able to make the call as to whether to do a new release or just bump deps in a revision.

As to whether or not we should be reporting upstream, I feel strongly that we should. Nixpkgs is often the very first place that versioning problems with various packages are found, since we automatically compile and distribute many packages from Hackage. I think it is important that we give back to the Haskell community by reporting versioning constraints that are too tight, so they can be fixed quickly.

Although I agree that this could be automated better. We would definitely welcome any tools to automate any part of the process of working with the Haskell stuff in nixpkgs!

@Anton-Latukha
xmobar: update patch description
6e36b3a 
M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha
statistics: clean-up
e9d03e9 
M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha
rib: update override description
535d4d8 
M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha
binary-instances: update override description
a03288a 
Previously mentioned issue closed as duplicate of an older one.

Issue waits upon QuickCheck release that includes a patch.

M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha
nixfmt: update override description
55df075 
M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha
pantry: free from override and upstreamed patch
61fe6c9 
M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha
feed: free from override
ed96ed8 
M  pkgs/development/haskell-modules/configuration-common.nix
@Anton-Latukha Anton-Latukha force-pushed the upd-Haskell-configuration-common-01 branch from aef386c to ed96ed8 Compare June 23, 2020 11:02
@Anton-Latukha
Copy link
Contributor Author

@GrahamcOfBorg build haskellPackages.xmobar haskellPackages.statistics haskellPackages.rib haskellPackages.nixfmt haskellPackages.pantry haskellPackages.feed

@maralorn
Copy link
Member

I am not sure, that we need the test on packages were you only changed the comments.^^

@Anton-Latukha
Copy link
Contributor Author

Yep, automatically populated the list. I used to updating regular packages on master. I probably made the bot too worried.

@cdepillabout
Copy link
Member

@GrahamcOfBorg build haskellPackages.xmobar
@GrahamcOfBorg build haskellPackages.statistics
@GrahamcOfBorg build haskellPackages.rib
@GrahamcOfBorg build haskellPackages.binary-instances
@GrahamcOfBorg build haskellPackages.nixfmt
@GrahamcOfBorg build haskellPackages.pantry
@GrahamcOfBorg build haskellPackages.feed

@cdepillabout
Copy link
Member

Looks like everything is building on at least Linux! Thanks for looking into this and fixing all these up!

@cdepillabout cdepillabout merged commit aea0a9c into NixOS:haskell-updates Jun 24, 2020
@Anton-Latukha Anton-Latukha deleted the upd-Haskell-configuration-common-01 branch June 24, 2020 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout cdepillabout left review comments

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501-1000 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Anton-Latukha @maralorn @cdepillabout