Skip to content

libdatrie: depend on libiconv unconditionally #212758

Closed
SuperSandro2000 wants to merge 1 commit intoNixOS:stagingfrom
SuperSandro2000:libdatrie-iconv
Closed

libdatrie: depend on libiconv unconditionally #212758
SuperSandro2000 wants to merge 1 commit intoNixOS:stagingfrom
SuperSandro2000:libdatrie-iconv

Conversation

@SuperSandro2000
Copy link
Member

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@vcunat
Copy link
Member

vcunat commented Jan 26, 2023

It will be easier if we wait until the revert gets to staging (should happen within an hour automatically).

@vcunat
Copy link
Member

vcunat commented Jan 26, 2023

I also wasn't sure if the authors would instead prefer a quick fix on master, e.g. lib.optional (!stdenv.isLinux)

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 26, 2023
@vcunat
Copy link
Member

vcunat commented Jan 26, 2023

As it is now, git resolves this as conflictless merge that doesn't change anything 😄

@alyssais
Copy link
Member

I'd prefer this going to staging (thanks Sandro for making the PR!). It's not urgent.

libiconv is already defined per-platform.  The actual libiconv library
won't be built on platforms like Linux where it doesn't need to be, so
there's no need to maintain a separate platform list here.

Required to build for FreeBSD.
@alyssais
Copy link
Member

Rebased to pick up the revert.

@vcunat
Copy link
Member

vcunat commented Jan 26, 2023

Oh, race. I pushed 3d9ac1f directly.

@vcunat vcunat closed this Jan 26, 2023
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 26, 2023
@SuperSandro2000 SuperSandro2000 deleted the libdatrie-iconv branch January 27, 2023 12:20
@SuperSandro2000
Copy link
Member Author

It will be easier if we wait until the revert gets to staging (should happen within an hour automatically).

Yeah, I noticed that, too, while opening. I opened it as a to do for me to fix later. Should have marked it as draft tbh.

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

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants