-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Make "localizations" sane again #37012
Conversation
a433d81
to
dee8a2c
Compare
@GrahamcOfBorg eval |
Failure on aarch64-linux (full log) Attempted: gettext, libiconv Partial log (click to expand)
|
Failure on aarch64-linux (full log) Attempted: gettext, libiconv Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: gettext, libiconv Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: gettext, libiconv Partial log (click to expand)
|
Something like this is definitely the right step! Have you seen the |
I still want to find a way to build |
Yep removing 'libiconv' and 'libintl' from GCC will make GCC much more "normal". The problem is that lots of packages assume that you are building with GCC and so the '-liconv' or '-lintl' are not given. We shouldn't have to add specific libraries in a setup-hook under normal circumstances but this seems like an okay case given the Linux world's long legacy of GCC dependency. Any ideas on what the failure on Linux is? I suspect GCC doesn't like being given "-liconv" or "-lintl" even though it should have them baked in.
Right now libiconv is provided I think as long as libc != glibc. I think that should cover Musl but haven't tested it. |
Teehee, looks like you found the problem? 😁 |
This is great, very much approve :1:. |
@GrahamcOfBorg build libelf |
No attempt on x86_64-linux The following builds were skipped because they don't evaluate on x86_64-linux: libelf No log is available. |
No attempt on aarch64-linux The following builds were skipped because they don't evaluate on aarch64-linux: libelf No log is available. |
@GrahamcOfBorg build gcc |
No attempt on x86_64-linux The following builds were skipped because they don't evaluate on x86_64-linux: gcc No log is available. |
No attempt on aarch64-linux The following builds were skipped because they don't evaluate on aarch64-linux: gcc No log is available. |
@GrahamcOfBorg eval |
831cf11
to
fe1f573
Compare
Failure on x86_64-linux (full log) Attempted: gettext, libiconv The following builds were skipped because they don't evaluate on x86_64-linux: darwin.libiconv Partial log (click to expand)
|
Failure on aarch64-linux (full log) Attempted: gettext, libiconv The following builds were skipped because they don't evaluate on aarch64-linux: darwin.libiconv Partial log (click to expand)
|
@GrahamcOfBorg build girara irssi harfbuzz |
Failure on x86_64-darwin (full log) Attempted: darwin.libiconv, gettext, libiconv, streamripper The following builds were skipped because they don't evaluate on x86_64-darwin: pcmciaUtils Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: gettext, libiconv The following builds were skipped because they don't evaluate on x86_64-linux: darwin.libiconv Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: girara, irssi, harfbuzz Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: girara, irssi, harfbuzz Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: gettext, libiconv The following builds were skipped because they don't evaluate on aarch64-linux: darwin.libiconv Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Attempted: darwin.libiconv, gettext, libiconv Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: gettext, libiconv, pcmciaUtils, streamripper The following builds were skipped because they don't evaluate on x86_64-linux: darwin.libiconv Partial log (click to expand)
|
Failure on aarch64-linux (full log) Attempted: girara, irssi, harfbuzz Partial log (click to expand)
|
Great! |
|
||
propagatedBuildInputs = [ zlib libffi libiconv ] | ||
++ libintlOrEmpty; | ||
propagatedBuildInputs = [ zlib libffi ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it's right or wrong, but noticed this (while investigating problems w/musl and missing libiconv)--
what's the rationale for removing libiconv from propagated build inputs here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can have it propagate if thia breaks things. The rationale is that things that depend on glib shouldnt need to know about libiconv (esp. setup hooks). Not sure if it breaks glib though?
@matthewbauer @dtzWill @Ericson2314 should staging be usable yet? |
I don't know. I didn't intend to break it, but if this breaks it I think it's worth it. |
@Ericson2314 Looks like there is some breakage, but hopefully it's not too bad: gobject-introspection is now failing: https://hydra.nixos.org/build/71894082/nixlog/1
Any idea why it's running the libiconv setup-hook in gobject-introspection (it only needs libintl)? |
If you nix shell with |
That job linked above seems to be passing now? So hopefully it's fixed? 😁 |
Yeah, just switch to the "propagatedBuildInputs" fixed it (looks like nativeBuildInputs idea is not going to work). We still have some failing though, if anyone wants to take a look at them: https://hydra.nixos.org/build/71917155/nixlog/1 |
Hmm, we'll see. Do you know if there are any non-Darwin failures? My builders are offline for a few days so I don't have my usual ability to test on musl/etc O:). |
None that I have found so far. |
# libiconv must be listed in load flags on non-Glibc | ||
# it doesn't hurt to have it in Glibc either though | ||
iconvLdflags() { | ||
export NIX_LDFLAGS="$NIX_LDFLAGS -liconv" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we ensure that we don't keep adding this multiple times? e.g. check that -liconv
is not already in $NIX_LDFLAGS
before rexporting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could but even then we wouldn't know if the package itself passes -liconv. I think it's safe to pass it twice to gcc and clang. I don't get any warnings at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, multiple -l
flags are usually no issues, and might even be preferable with cyclic dependencies.
I was just confused as it ended up four times in my ghc derivation. And I got four error messages. At first I though it would have to be in four different places.
env-vars
43:declare -x NIX_TARGET_LDFLAGS=" -L/nix/store/0pnvlr2ghaigvlwn2zg4nialw3sx5xmn-libiconv-1.15-x86_64-pc-mingw32/lib -liconv -liconv -L/nix/store/0pnvlr2ghaigvlwn2zg4nialw3sx5xmn-libiconv-1.15-x86_64-pc-mingw32/lib -liconv -liconv"
ghc-d0d02e2/libffi/build/x86_64-pc-mingw32/libtool
11829:postdeps="-liconv -liconv -liconv -liconv -lmingw32 -lgcc_s -lgcc -lmoldname -lmingwex -lmsvcrt -ladvapi32 -lshell32 -luser32 -lkernel32 -lmingw32 -lgcc_s -lgcc -lmoldname -lmingwex -lmsvcrt"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's probably packages propagating libiconv incorrectly. There are quite a few that do this still & probably need to be fixed. I think that's where the duplicates are coming from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this get cross support with the "role" stuff? I thought it did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully- I'm not sure if anyone has used it yet in cross compile. In theory at least though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK yeah that was indeed done. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Motivation for this change
This is a fairly big PR, so I'll list out what it does:
I'm interested in comments on whether this is a good idea!
/cc @Ericson2314 @LnL7 @copumpkin @NixOS/darwin-maintainers