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.

Sign up for GitHub

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

ghc modules: use specific commit to fetch patches #70546

Merged
merged 1 commit into from Oct 6, 2019
Merged

ghc modules: use specific commit to fetch patches #70546

merged 1 commit into from Oct 6, 2019

Conversation

dylex
Copy link
Contributor

@dylex dylex commented Oct 6, 2019

rather than master to retrieve patches, some of which have gone stale

Motivation for this change

URLs for a few module patches used for building ghc 8.8 have gone stale since they refer to the master branch, which has since moved on and removed 8.8 stuff. This pins them at the last commit where they existed.

Note that there are other patches in this same file pulling from hvr/head.hackage master as well, which need fixing. However, those even refer to an old repo (github.com/hvr/head.hackage, which has moved to gitlab.haskell.org), and are still working for the moment, so I've left them alone. I can take a stab at updating them if that'd be helpful.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @peti

@dylex
ghc modules: use specific commit to fetch patches
324f0a7 
rather than master to retrieve patches, some of which have gone stale
@dylex dylex requested a review from basvandijk as a code owner October 6, 2019 23:16
@veprbl veprbl merged commit dbb7230 into NixOS:master Oct 6, 2019
veprbl added a commit that referenced this pull request Oct 7, 2019
@veprbl
ghc modules: use permanent urls
aded586 
This is a backport of dbb7230 ('ghc modules: use specific commit ...')

cc #70546
@veprbl veprbl added the 8.has: port to stable A PR already has a backport to the stable release. label Oct 7, 2019
peti added a commit that referenced this pull request Oct 7, 2019
@peti
Revert "ghc modules: use specific commit to fetch patches (#70546)"
3642089 
This reverts commit dbb7230.
@peti peti mentioned this pull request Oct 7, 2019
Merged
@peti
Copy link
Member

peti commented Oct 7, 2019

Please target these kind of changes to the haskell-updates branch. Oftentimes, that branch has various updates and if you bypass it, committing directly to master, then you'll create merge conflicts for haskell-updates, which in turn breaks the automatic tooling that's partially responsible for maintaining those updates.

Also, when patches from head.hackage disappear, then the reason is that they have become unnecessary, i.e. because a new version of the package in question has come out. The right response in that situation is to drop the override from Nixpkgs enturely and to use the proper solution, like, update the version of the package that didn't work before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veprbl veprbl veprbl approved these changes

@basvandijk basvandijk Awaiting requested review from basvandijk

Assignees
No one assigned
Labels
6.topic: haskell 6.topic: reproducible builds 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dylex @peti @veprbl