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

libusb1: fix enableSystemd=false build #87200

Merged
merged 1 commit into from Sep 4, 2020
Merged

libusb1: fix enableSystemd=false build #87200

merged 1 commit into from Sep 4, 2020

Conversation

B4dM4n
Copy link
Contributor

@B4dM4n B4dM4n commented May 7, 2020

Motivation for this change

Without --disable-udev the build tries to use udev by default, which fails when systemd is not in buildInputs.

This also removes some old workarounds, which are fixed by upstream already and don't affect the build anymore.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS nix build -f . pkgsMusl.libusb1 pkgsStatic.libusb1 libusb1
    • macOS nix build '(with import ./. { system = "x86_64-darwin"; }; libusb1)'
    • 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.

bjornfor
bjornfor requested changes May 7, 2020
View reviewed changes
Copy link
Contributor

@bjornfor bjornfor left a comment

Choose a reason for hiding this comment

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

Please split in two commits: one that does the fix and another that does cleanup. Also, why is this PR removing the withStatic flag?

@B4dM4n
Copy link
Contributor Author

B4dM4n commented May 7, 2020

Please split in two commits: one that does the fix and another that does cleanup. Also, why is this PR removing the withStatic flag?

Removing the withStatic flag was a mistake. I thought it had a different use than additionally enabling static libraries.

After testing again with the withStatic flag I found that when building with withStatic=true and enableSystemd=true the libusb-1.0.la fixup actually alters the file, which no other configuration does.

There seem to be no packages that depend on this fixup. The only package which sets withStatic=true is stlink, but only for MacOS. Should I also remove this unused workaround?

@FRidh
Copy link
Member

FRidh commented May 8, 2020

withStatic is used in a different package set, e.g. pkgsStatic.libusb1.

@FRidh FRidh added this to WIP in Staging via automation May 8, 2020
Staging automation moved this from WIP to Ready May 9, 2020
@FRidh
Copy link
Member

FRidh commented Jul 2, 2020

please rebase

@B4dM4n
Copy link
Contributor Author

B4dM4n commented Jul 2, 2020

Rebase is done. After #88692 only the unneeded workaround is left.

@FRidh FRidh merged commit 60258be into NixOS:staging Sep 4, 2020
Staging automation moved this from Ready to Done Sep 4, 2020
@B4dM4n B4dM4n deleted the libusb-no-systemd branch September 4, 2020 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjornfor bjornfor bjornfor approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@B4dM4n @FRidh @bjornfor