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

buildEnv (breaking texlive): allow very big lists to be passed #9757

Closed
abbradar opened this Issue Sep 9, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@abbradar
Member

abbradar commented Sep 9, 2015

See #287 (comment). I imagine this as a size check in buildEnv; when input list is too large, it's passed via file.

@vcunat vcunat changed the title from Allow very big lists to be passed to buildEnv to buildEnv: allow very big lists to be passed Sep 22, 2015

@vcunat

This comment has been minimized.

Show comment
Hide comment
@vcunat

vcunat Sep 22, 2015

Member

For better discoverability of this issue: texlive.combined.scheme-full gives Argument list too long.

Member

vcunat commented Sep 22, 2015

For better discoverability of this issue: texlive.combined.scheme-full gives Argument list too long.

@vcunat

This comment has been minimized.

Show comment
Hide comment
@vcunat

vcunat Sep 25, 2015

Member

For those who have texlive problems due to this, you can use the old texLiveFull in the meantime, if you really want all those thousands of packages.

Member

vcunat commented Sep 25, 2015

For those who have texlive problems due to this, you can use the old texLiveFull in the meantime, if you really want all those thousands of packages.

@vcunat vcunat changed the title from buildEnv: allow very big lists to be passed to buildEnv (breaking texlive): allow very big lists to be passed Sep 25, 2015

@Mathnerd314

This comment has been minimized.

Show comment
Hide comment
@Mathnerd314

Mathnerd314 Oct 13, 2015

Contributor

I was browsing the Nix manual and found passAsFile, this seems like an ideal case for it.

Contributor

Mathnerd314 commented Oct 13, 2015

I was browsing the Nix manual and found passAsFile, this seems like an ideal case for it.

@abbradar

This comment has been minimized.

Show comment
Hide comment
@abbradar

abbradar Oct 15, 2015

Member

@vcunat I've went ahead and implemented passing paths and postBuild as files (note that I don't know Perl at all, so my fix can be unidiomatic or ugly!) at abbradar@5a15be4 . We now have a problem with sed argument list being too long in your postBuild.

Member

abbradar commented Oct 15, 2015

@vcunat I've went ahead and implemented passing paths and postBuild as files (note that I don't know Perl at all, so my fix can be unidiomatic or ugly!) at abbradar@5a15be4 . We now have a problem with sed argument list being too long in your postBuild.

@vcunat

This comment has been minimized.

Show comment
Hide comment
@vcunat

vcunat Oct 15, 2015

Member

I don't know perl either. For the sed problem, I think this should be good:

diff --git a/pkgs/tools/typesetting/tex/texlive-new/combine.nix b/pkgs/tools/typesetting/tex/texlive-new/combine.nix
index 2c9119b..e69c6ec 100644
--- a/pkgs/tools/typesetting/tex/texlive-new/combine.nix
+++ b/pkgs/tools/typesetting/tex/texlive-new/combine.nix
@@ -82,21 +82,21 @@ in buildEnv {
     # updmap.cfg seems like not needing changes

     # now filter hyphenation patterns, in a hacky way ATM
-  ''
+  (let script =
+    writeText "hyphens.sed" (
+      lib.concatMapStrings (pkg: "/^\% from ${pkg.pname}/,/^\%/p;\n") pkgList.splitBin.wrong
+      + "1,/^\% from/p;" );
+  in ''
     (
-      local script='${
-        lib.concatMapStrings (pkg: "/^\% from ${pkg.pname}/,/^\%/p;\n")
-          pkgList.splitBin.wrong
-      } 1,/^\% from/p;'
       cd ./share/texmf/tex/generic/config/
       for fname in language.dat language.def; do
         [ -e $fname ] || continue;
         cnfOrig="$(realpath ./$fname)"
         rm ./$fname
-        cat "$cnfOrig" | sed -n "$script" > ./$fname
+        cat "$cnfOrig" | sed -n -f '${script}' > ./$fname
       done
     )
-  '' +
+  '') +

   # function to wrap created executables with required env vars
   ''
diff --git a/pkgs/tools/typesetting/tex/texlive-new/default.nix b/pkgs/tools/typesetting/tex/texlive-new/default.nix
index c8b2a41..a084c97 100644
--- a/pkgs/tools/typesetting/tex/texlive-new/default.nix
+++ b/pkgs/tools/typesetting/tex/texlive-new/default.nix
@@ -24,7 +24,7 @@
     * in case of any bugs or feature requests, file a github issue and /cc @vcunat
 */

-{ stdenv, lib, fetchurl, runCommand, buildEnv
+{ stdenv, lib, fetchurl, runCommand, writeText, buildEnv
 , callPackage, ghostscriptX, harfbuzz, poppler_min
 , makeWrapper, perl, python, ruby
 , useFixedHashes ? true
@@ -48,7 +48,8 @@ let

   # function for creating a working environment from a set of TL packages
   combine = import ./combine.nix {
-    inherit bin combinePkgs buildEnv fastUnique lib makeWrapper perl stdenv python ruby;
+    inherit bin combinePkgs buildEnv fastUnique lib makeWrapper writeText
+      perl stdenv python ruby;
   };

   # the set of TeX Live packages, collections, and schemes; using upstream naming
Member

vcunat commented Oct 15, 2015

I don't know perl either. For the sed problem, I think this should be good:

diff --git a/pkgs/tools/typesetting/tex/texlive-new/combine.nix b/pkgs/tools/typesetting/tex/texlive-new/combine.nix
index 2c9119b..e69c6ec 100644
--- a/pkgs/tools/typesetting/tex/texlive-new/combine.nix
+++ b/pkgs/tools/typesetting/tex/texlive-new/combine.nix
@@ -82,21 +82,21 @@ in buildEnv {
     # updmap.cfg seems like not needing changes

     # now filter hyphenation patterns, in a hacky way ATM
-  ''
+  (let script =
+    writeText "hyphens.sed" (
+      lib.concatMapStrings (pkg: "/^\% from ${pkg.pname}/,/^\%/p;\n") pkgList.splitBin.wrong
+      + "1,/^\% from/p;" );
+  in ''
     (
-      local script='${
-        lib.concatMapStrings (pkg: "/^\% from ${pkg.pname}/,/^\%/p;\n")
-          pkgList.splitBin.wrong
-      } 1,/^\% from/p;'
       cd ./share/texmf/tex/generic/config/
       for fname in language.dat language.def; do
         [ -e $fname ] || continue;
         cnfOrig="$(realpath ./$fname)"
         rm ./$fname
-        cat "$cnfOrig" | sed -n "$script" > ./$fname
+        cat "$cnfOrig" | sed -n -f '${script}' > ./$fname
       done
     )
-  '' +
+  '') +

   # function to wrap created executables with required env vars
   ''
diff --git a/pkgs/tools/typesetting/tex/texlive-new/default.nix b/pkgs/tools/typesetting/tex/texlive-new/default.nix
index c8b2a41..a084c97 100644
--- a/pkgs/tools/typesetting/tex/texlive-new/default.nix
+++ b/pkgs/tools/typesetting/tex/texlive-new/default.nix
@@ -24,7 +24,7 @@
     * in case of any bugs or feature requests, file a github issue and /cc @vcunat
 */

-{ stdenv, lib, fetchurl, runCommand, buildEnv
+{ stdenv, lib, fetchurl, runCommand, writeText, buildEnv
 , callPackage, ghostscriptX, harfbuzz, poppler_min
 , makeWrapper, perl, python, ruby
 , useFixedHashes ? true
@@ -48,7 +48,8 @@ let

   # function for creating a working environment from a set of TL packages
   combine = import ./combine.nix {
-    inherit bin combinePkgs buildEnv fastUnique lib makeWrapper perl stdenv python ruby;
+    inherit bin combinePkgs buildEnv fastUnique lib makeWrapper writeText
+      perl stdenv python ruby;
   };

   # the set of TeX Live packages, collections, and schemes; using upstream naming
@abbradar

This comment has been minimized.

Show comment
Hide comment
@abbradar

abbradar Oct 15, 2015

Member

Good news, with the patch I've been able to build texlive.combined.scheme-full! I'll put my fix to the review.

Member

abbradar commented Oct 15, 2015

Good news, with the patch I've been able to build texlive.combined.scheme-full! I'll put my fix to the review.

@abbradar

This comment has been minimized.

Show comment
Hide comment
@abbradar

abbradar Oct 15, 2015

Member

@vcunat: While we wait, maybe we can merge the sed-related patch? It would be needed anyway I believe.

Member

abbradar commented Oct 15, 2015

@vcunat: While we wait, maybe we can merge the sed-related patch? It would be needed anyway I believe.

@abbradar

This comment has been minimized.

Show comment
Hide comment
@abbradar

abbradar Oct 24, 2015

Member

@vcunat Now that #10506 is pushed into staging it would be nice to push the patch above -- would you?

Member

abbradar commented Oct 24, 2015

@vcunat Now that #10506 is pushed into staging it would be nice to push the patch above -- would you?

vcunat added a commit that referenced this issue Oct 24, 2015

@vcunat

This comment has been minimized.

Show comment
Hide comment
@vcunat

vcunat Nov 9, 2015

Member

All should be working fine in staging now.

Member

vcunat commented Nov 9, 2015

All should be working fine in staging now.

@vcunat vcunat closed this Nov 9, 2015

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