-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
gettext: adopt and update #299647
gettext: adopt and update #299647
Conversation
- finalAttrs design pattern - reword comment - use lib.* functions - put LDFLAGS under env - get rid of nested with in meta - set meta.mainProgram - remmove vrthra and add AndersonTorres as maintainer
Controlled by enableLibiconv (default as (!stdenv.isLinux && !stdenv.hostPlatform.isCygwin)).
"Canonicalizing" them to be read in ASCIIbetical order.
Drop CoreServices in order to avoid infinite recursion.
Removing the assumption it was built with iconv support.
@ofborg build pkgsMusl.gettext |
enableParallelChecking = false; # fails sometimes | ||
|
||
setupHooks = [ | ||
../../../build-support/setup-hooks/role.bash |
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.
Looks like it won't pass the by-name check.
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 am cogitating to write a dummy package that installs those scripts at Nix Store.
@infinisil apologies in advance. I am collecting some work on updates at gettext. However, this expression requires a file from The easy and dirty solution is make a copy of this file. I am thinking about creating a Nix package that installs all these |
I wouldn't put all the setup hooks in a single derivation. There's already various derivations defining specific setup hooks, I'd follow that pattern. By putting them in separate derivations, you can add them to e.g. I'm very glad to see this being improved, this is exactly the sort of thing the self-contained restriction of |
I’m pretty sure this will be needed for the clang 18 update on Darwin. The current version of gettext is replacing |
] | ||
++ lib.optionals enableCoreServices [ | ||
# prevent infinite recursion for the darwin stdenv | ||
./003-revert-avoid-crash-on-macos-14.patch | ||
]; |
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.
] | |
++ lib.optionals enableCoreServices [ | |
# prevent infinite recursion for the darwin stdenv | |
./003-revert-avoid-crash-on-macos-14.patch | |
]; | |
]; | |
propagatedBuildInputs = lib.optionals enableCoreServices [ | |
# prevent infinite recursion for the darwin stdenv | |
./003-revert-avoid-crash-on-macos-14.patch | |
]; |
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.
When autoreconfPhase
is run with the new gettext, m4/intlmacosx.m4
is updated, adding CoreServices as a dependency of the original package.
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.
After rebuilding the stdenv with this change, there is one downside. It requires adding CoreServices and its propagated inputs the Darwin stdenv’s allowedRequisites
, which differ depending on the SDK.
Is it possible to add a dev
output to gettext, so CoreServices is propagated from it instead of out
?
I used the following (with CoreServices as a regular build input) to successfully rebuild the Darwin stdenv. libpsl is an example of a package that needs CoreServices propagated to it. It also built successfully.
I also split the hook into two hooks. The one copied into $out
sets up GETTEXTDATADIRS
while the other ensures that libintl
is linked.
# Two setup hooks are needed. One is for making sure gettext can find its data files at runtime, and the other
# is to make sure `libintl` is linked even on non-glibc targets. This is the latter hook. The other is below.
setupHooks = [
../../../build-support/setup-hooks/role.bash
./gettext-setup-hook-dev.sh
];
postInstall = ''
mkdir -p "$out/nix-support"
cat ${../../../build-support/setup-hooks/role.bash} ${./gettext-setup-hook.sh} > "$out/nix-support/setup-hook"
'';
Edit: CoreServices
needs to be in propagatedBuildInputs
for it to be picked up by overrideSDK
. Otherwise, anything that needs a non-default SDK and uses gettext won’t build (e.g., Wine).
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 will wait your modifications before migrating this, since I don't and I won't have a Darwin machine.
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.
Modifications to the existing gettext package? I can open a PR (or PRs, depending) for those.
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.
The migration to by-name will be a little bit harder.
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.
Because of the hooks? I cheated with libiconv-darwin (by reexporting them from libiconvReal and using those), but I don’t think that will be possible here.
Description of changes
Picking the work from Wegank at #279197
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.