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

Gettext.sh #13072

Closed
wants to merge 2 commits into from
Closed

Gettext.sh #13072

wants to merge 2 commits into from

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Feb 17, 2016

#13053 exposed even more impurity that this PR fixes.

This fixes issues like:

/nix/store/05fbja9p8wcdw36l7754wpvg9biiwm7v-git-2.7.0/libexec/git-core/git-rebase: line 62: gettext: command not found
/nix/store/05fbja9p8wcdw36l7754wpvg9biiwm7v-git-2.7.0/libexec/git-core/git-rebase: line 569: gettext: command not found

I also included an upgrade to gettext since it does a mass-rebuild

Bash tools like `git rebase` use gettext.sh. But it doesn't find gettext
unless the user has installed it himself.
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @lancelotsix, @jagajaga and @durko to be potential reviewers

@zimbatm
Copy link
Member Author

zimbatm commented Feb 17, 2016

Still compiling locally and needs testing

@vcunat
Copy link
Member

vcunat commented Feb 17, 2016

BTW, the news for this bump contains: Remove dependency on expat.

@vcunat
Copy link
Member

vcunat commented Feb 17, 2016

This one looks mergeable directly to master, I think:

diff --git a/pkgs/development/libraries/gettext/absolute-paths.diff b/pkgs/development/libraries/gettext/absolute-paths.diff
new file mode 100644
index 0000000..6d5cf1c
--- /dev/null
+++ b/pkgs/development/libraries/gettext/absolute-paths.diff
@@ -0,0 +1,21 @@
+diff --git a/gettext-runtime/src/gettext.sh.in b/gettext-runtime/src/gettext.sh.in
+index 1dfa3bb..d6ef8a8 100644
+--- a/gettext-runtime/src/gettext.sh.in
++++ b/gettext-runtime/src/gettext.sh.in
+@@ -86,14 +86,14 @@ fi
+ # looks up the translation of MSGID and substitutes shell variables in the
+ # result.
+ eval_gettext () {
+-  gettext "$1" | (export PATH `envsubst --variables "$1"`; envsubst "$1")
++  @out@/bin/gettext "$1" | (export PATH `envsubst --variables "$1"`; envsubst "$1")
+ }
+ 
+ # eval_ngettext MSGID MSGID-PLURAL COUNT
+ # looks up the translation of MSGID / MSGID-PLURAL for COUNT and substitutes
+ # shell variables in the result.
+ eval_ngettext () {
+-  ngettext "$1" "$2" "$3" | (export PATH `envsubst --variables "$1 $2"`; envsubst "$1 $2")
++  @out@/bin/ngettext "$1" "$2" "$3" | (export PATH `envsubst --variables "$1 $2"`; envsubst "$1 $2")
+ }
+ 
+ # Note: This use of envsubst is much safer than using the shell built-in 'eval'
diff --git a/pkgs/development/libraries/gettext/default.nix b/pkgs/development/libraries/gettext/default.nix
index 3d7cfc0..1443626 100644
--- a/pkgs/development/libraries/gettext/default.nix
+++ b/pkgs/development/libraries/gettext/default.nix
@@ -7,6 +7,7 @@ stdenv.mkDerivation (rec {
     url = "mirror://gnu/gettext/${name}.tar.gz";
     sha256 = "0pb9vp4ifymvdmc31ks3xxcnfqgzj8shll39czmk8c1splclqjzd";
   };
+  patches = [ ./absolute-paths.diff ];

   outputs = [ "out" "doc" ];

@@ -28,7 +29,8 @@ stdenv.mkDerivation (rec {
         "gt_cv_func_CFLocaleCopyCurrent=no"
       ]);

-  patchPhase = ''
+  postPatch = ''
+   substituteAllInPlace gettext-runtime/src/gettext.sh.in
    substituteInPlace gettext-tools/projects/KDE/trigger --replace "/bin/pwd" pwd
    substituteInPlace gettext-tools/projects/GNOME/trigger --replace "/bin/pwd" pwd
    substituteInPlace gettext-tools/src/project-id --replace "/bin/pwd" pwd

I'll probably do it in a while. The update seems more risky without testing, now that we want a smooth rebuild with patched glibc.

@zimbatm
Copy link
Member Author

zimbatm commented Feb 17, 2016

aha, I was wondering how to produce a patch with variable $out.

Do you know why gettext/expat.nix exists at all ? If it's not needed anymore then it could be removed as well. gettext.sh in that version is broken because of the makeWrapper.

Since expat has been replaced by libxml2, don't we also have to provide that dependency ? They seem to provide their own version but it might be better to use our own.

@vcunat
Copy link
Member

vcunat commented Feb 17, 2016

gtk3 seems to need a xml-enabled gettext. I assume we will just replace gettext by libxml2 in that case... but this exactly was a signal not to hurry with the update into the next master evaluation.

vcunat added a commit that referenced this pull request Feb 17, 2016
@vcunat
Copy link
Member

vcunat commented Feb 17, 2016

I pushed that patch to master before we start rebuilding it on Hydra.

@vcunat
Copy link
Member

vcunat commented Feb 17, 2016

We should test the update, resolve libxml etc. stuff and stage later.

@zimbatm
Copy link
Member Author

zimbatm commented Feb 18, 2016

cool, I'll keep this PR just for the gettext update then

@jagajaga
Copy link
Member

Ping?

@zimbatm
Copy link
Member Author

zimbatm commented Feb 20, 2016

Closing in favor of #13145

@zimbatm zimbatm closed this Feb 20, 2016
@zimbatm zimbatm deleted the gettext.sh branch February 20, 2016 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants