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

texlive.combine: allow dependencies for python scripts #55933

Closed
wants to merge 2 commits into from
Closed

texlive.combine: allow dependencies for python scripts #55933

wants to merge 2 commits into from

Conversation

romildo
Copy link
Contributor

@romildo romildo commented Feb 17, 2019

Motivation for this change

Some Python scripts from texlive depend on some Python modules and up to now there was no way of specifying them, given them a broken state.

This PR allows to specify those dependencies, which are taken into account when wrapping the Python scripts.

It is a much better fix for #10885.

Tested with:

$ nix-shell -p 'with import ./. {}; (texlive.combine {inherit (texlive) scheme-small pygmentex pythontex;})' --pure --run pygmentex

cc @vcunat

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@veprbl
Copy link
Member

veprbl commented Feb 17, 2019

Thank you for looking into this. The problem is a bit more general than just the python case here. For example we have the same issue but with perl in #35537. I think the correct solution should be using the buildPythonApplication because it implements various quirks of python packaging and we don't want to reimplement that in texlive. I think the simplest solution here would be to just drop pygmentex from texlive and then pygmentex will be provided in the only one way via your original expression.

@veprbl veprbl added the 6.topic: TeX Issues regarding texlive and TeX in general label Feb 17, 2019
@romildo
Copy link
Contributor Author

romildo commented Feb 17, 2019

just drop pygmentex from texlive

But there is the accompanying latex style file, which needs to be visible by latex and friends.

@veprbl
Copy link
Member

veprbl commented Feb 18, 2019

@romildo I guess those can be also provided by your expression if it mimics a texlive package (see #17748). Ideally, that custom expression should then be injected back into texlive, though, AFAIK, there is no mechanism for doing that at the moment.

@romildo
Copy link
Contributor Author

romildo commented Feb 18, 2019

@veprbl I have tried your suggestion to provide the complete pygmentex package, with binary, style file, and demo.

For that purpose I changed the pygmentex nix expression to the following:

{ stdenv, fetchFromBitbucket, python2Packages }:

python2Packages.buildPythonApplication rec {
  pname = "pygmentex";
  version = "0.8";
  tlType = "run";

  src = fetchFromBitbucket {
    owner = "romildo";
    repo = pname;
    rev = version;
    sha256 = "07dnv7hgppy15bda2kcbrlvfqzl6lhza80klc7133dwg8q92hm6m";
  };

  pythonPath = with python2Packages; [ pygments chardet ];

  dontBuild = true;

  doCheck = false;

  installPhase = ''
    mkdir -p $out/bin
    cp -a pygmentex.py $out/bin

    mkdir -p $out/tex/latex/pygmentex
    cp -a pygmentex.sty $out/tex/latex/pygmentex

    mkdir -p $out/doc/latex/pygmentex
    cp -a README demo.* blueshade.png Factorial.java $out/doc/latex/pygmentex
  '';

  meta = with stdenv.lib; {
    homepage = https://www.ctan.org/pkg/pygmentex;
    description = "Auxiliary tool for typesetting code listings in LaTeX documents using Pygments";
    longDescription = ''
      PygmenTeX is a Python-based LaTeX package that can be used for
      typesetting code listings in a LaTeX document using Pygments.

      Pygments is a generic syntax highlighter for general use in all kinds of
      software such as forum systems, wikis or other applications that need to
      prettify source code.

      This package installs just the script needed to process code listings
      snippets extracted from the a LaTeX document by the pygmentex LaTeX
      package. In order to use it effectivelly the texlive package pygmentex
      also has to be installed. This can be done by adding pygmentex to
      texlive.combine.
    '';
    license = licenses.lppl13c;
    platforms = platforms.unix;
    maintainers = with maintainers; [ romildo ];
  };
}

To test I used nix-shell:

$ nix-shell -p pygmentex 'with import ./. {}; (texlive.combine {pygmentex = {pkgs = [pygmentex];}; inherit (texlive) scheme-small mdframed needspace tcolorbox environ trimspaces efbox lipsum cm-super;})' --pure

$ cd /var/tmp/pygmentex/
$ rm pygmentex.sty pygmentex.py
$ pdflatex demo
$ pygmentex.py demo.snippets
$ pdflatex demo

It works.

Some downsides:

  • pygmentex has to be mentioned twice: to obtain access to the binary, and to make the style file visible to texlive programs; this last one is not too elegant
  • the pygmentex package is not the one provided with texlive

I think I still would prefer the solution presented by this PR, which wraps the python script setting $PYTHONPATH to include the locations where the needed dependencies can be found. $PYTHONPATH belongs to the specification of Python and is supported. It makes things simple for users in this case.

@veprbl
Copy link
Member

veprbl commented Feb 19, 2019

@romildo I think this is a step in the right direction. If you could submit this as PR, I would gladly review it.

As for this PR, I'm not yet convinced that this is the best solution. We are looking to fix not only pygmentex here, but few others. We already have a mature infrastructure for providing wrappers for scripted languages (commonly engaged via buildPythonApplication, buildPerlPackage, etc.), it would be undesirable to duplicate that logic in already quite complicated texlive.combine. For instance we would want to extend this implementation to correctly handle dependencies of our dependencies (e.g. allegedly pygments can import from docutils at runtime). It seems like we also need support Perl in our solution. I think it would be nice to look for a solution that can handle these tasks gracefully before we go ahead with this.

  • the pygmentex package is not the one provided with texlive

Is this because you fetch src from your repo, so the source differs? What is the downside of that?

@romildo
Copy link
Contributor Author

romildo commented Feb 19, 2019

@romildo I think this is a step in the right direction. If you could submit this as PR, I would gladly review it.

The new PR: #56063

@romildo
Copy link
Contributor Author

romildo commented Feb 24, 2019

Is this because you fetch src from your repo, so the source differs? What is the downside of that?

Yes. Pygmentex would not update together with texlive when a new texlive is available. But this is minor.

@veprbl
Copy link
Member

veprbl commented Feb 27, 2019

@romildo I don't see a problem with that. You are the upstream, so we probably get the best version of pygmentex here. The way we package texlive, from snapshots of a rolling release, there isn't really much guarantee about versions of things in the first place, I think. But we could, of course, change pygmentex.src to use it's value from texlive.

@veprbl
Copy link
Member

veprbl commented Mar 7, 2019

During a routine bump of texlive I found out how to override texlive.pygmentex. This is already done for biber:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/typesetting/tex/texlive/bin.nix#L287

If you want to tie pygmentex'es version to texlive you can do it in a way similar to 4e103fb

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

3 participants