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

boehm-gc: remove sys_select patch #53622

Merged
merged 1 commit into from Jan 31, 2019
Merged

boehm-gc: remove sys_select patch #53622

merged 1 commit into from Jan 31, 2019

Conversation

dylex
Copy link
Contributor

@dylex dylex commented Jan 8, 2019

Motivation for this change

This boehm-gc patch referenced a live git repo, from which the patch was removed in
https://gitweb.gentoo.org/proj/musl.git/commit/dev-libs?id=ea1d7393a2fc76acc9fa4f1cd5b8d88590a3a4eb
This adds a fixed hash reference to the origin of this patch. However, given the triviality of this patch, it might make more sense to include it directly. It's also possible the patch is no longer necessary (I haven't tried, yet).

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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@alyssais
Copy link
Member

alyssais commented Jan 8, 2019

The patch is indeed no longer necessary, as far as I can tell, hence it’s removal from Gentoo. You beat me to a PR, but I’ve been using an unpatched version for a few days now.

Could you please remove the patch and change the base of this PR to staging?

@dylex dylex changed the base branch from master to staging January 8, 2019 15:26
@dylex dylex changed the title boehm-gc: fix sys_select patch url git hash boehm-gc: remove sys_select patch Jan 8, 2019
@dylex
Copy link
Contributor Author

dylex commented Jan 16, 2019

It looks like @veprbl instead made my original change in ac7f4c0. Not sure if you still want to remove it instead.

@Mic92 Mic92 requested a review from dtzWill January 16, 2019 21:02
@alyssais
Copy link
Member

Still worth removing IMO.

@alyssais
Copy link
Member

Side note: GitHub doesn't produce notifications for force pushes, so I didn't get a notification that this PR had been updated. A simple "done" comment is nice in this case to let people following know that something has changed.

@dtzWill
Copy link
Member

dtzWill commented Jan 16, 2019

LGTM!

@alyssais alyssais merged commit 69466ba into NixOS:staging Jan 31, 2019
@alyssais
Copy link
Member

Sorted out the merge conflict myself here just to get it over the line.

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

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@FRidh FRidh Awaiting requested review from FRidh

@infinisil infinisil Awaiting requested review from infinisil

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@nbp nbp Awaiting requested review from nbp

@dtzWill dtzWill Awaiting requested review from dtzWill

Assignees
No one assigned
Labels
2.status: merge conflict 6.topic: fetch 6.topic: nixos 6.topic: python 6.topic: stdenv Standard environment 6.topic: xfce The Xfce Desktop Environment 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dylex @alyssais @dtzWill @GrahamcOfBorg