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.

Sign up for GitHub

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

Jump to bottom

pythonPackages.sphinxcontrib-tikz: init at 0.4.6 #57583

Merged
merged 1 commit into from
Jan 5, 2020
Merged

pythonPackages.sphinxcontrib-tikz: init at 0.4.6 #57583

merged 1 commit into from
Jan 5, 2020
+64 −0

Conversation

costrouc
Copy link
Member

Limited texlive package to minimal packages schema-small + standalone + pgfplots

Motivation for this change

I want tikz for my documentation (nothing beats tikz in editable graphics :) )

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.

Sorry, something went wrong.

@costrouc costrouc requested a review from FRidh as a code owner March 13, 2019 13:52
@costrouc
Copy link
Member Author

I have verified that this works locally with my documentation.

@costrouc
Copy link
Member Author

@GrahamcOfBorg build python2Packages.sphinxcontrib-tikz python3Packages.sphinxcontrib-tikz

@GrahamcOfBorg GrahamcOfBorg added 6.topic: python 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Mar 13, 2019
dotlambda
dotlambda reviewed Mar 14, 2019
View reviewed changes
pkgs/top-level/python-packages.nix
@@ -4556,6 +4556,10 @@ in {

sphinxcontrib-spelling = callPackage ../development/python-modules/sphinxcontrib-spelling { };

sphinxcontrib-tikz = callPackage ../development/python-modules/sphinxcontrib-tikz {
texLive = pkgs.texlive.combine { inherit (pkgs.texlive) scheme-small standalone pgfplots; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
texLive = pkgs.texlive.combine { inherit (pkgs.texlive) scheme-small standalone pgfplots; };
texlive = pkgs.texlive.combine { inherit (pkgs.texlive) scheme-small standalone pgfplots; };

dotlambda
dotlambda reviewed Mar 14, 2019
View reviewed changes
pkgs/development/python-modules/sphinxcontrib-tikz/default.nix Outdated
sha256 = "4f362b11e3c2bd17d5f0f07fec03917c16fc5bbcda6fe31ee137c547ed6b03a3";
};

propagatedBuildInputs = [ sphinx pdf2svg texLive ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TeXLive should not be propagated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...as well as pdf2svg

Copy link
Member Author

@costrouc costrouc Mar 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package will not work without pdf2svg and texlive. Curious how in general this is handled? I realize that both of those package have a lot of dependencies.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these aren't Python packages and should hence not be propagated. Instead, their path should be patched into the source using substituteAll.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an example of that? I've never seen it used. Like so?

nixpkgs/pkgs/development/python-modules/pygraphviz/default.nix

Line 17 in 1eca836

patches = [

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly 👍

@veprbl veprbl changed the title pythonPackages.sphinxcontrib-tikz: init at 0.4.6 [WIP] pythonPackages.sphinxcontrib-tikz: init at 0.4.6 Mar 22, 2019
@costrouc costrouc changed the title [WIP] pythonPackages.sphinxcontrib-tikz: init at 0.4.6 pythonPackages.sphinxcontrib-tikz: init at 0.4.6 Apr 14, 2019
@costrouc
Copy link
Member Author

@dotlambda I am not completely sure how to write the patch to include texlive. That is why I have not finished this PR.

@dotlambda
Copy link
Member

You get a checkout of the source code and look for all places where a texlive binary is called, let's say the code is Popen('pdflatex'). You replace this by Popen('@texlive@/bin/pdflatex'). Then you save the diff as e.g. binary-paths.patch and add

patches = [
  (substituteAll {
    src = ./binary-paths.patch;
    inherit texlive pdf2svg;
  })
];

@costrouc costrouc force-pushed the python-init-sphinxcontrib-tikz branch from 14775a0 to 4baba66 Compare July 23, 2019 16:55
@costrouc
pythonPackages.sphinxcontrib-tikz: init at 0.4.6

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
be4f844 
requires texlive.combined.scheme-full due to needing "standalone.cls"
from extras.
@costrouc costrouc force-pushed the python-init-sphinxcontrib-tikz branch from 4baba66 to be4f844 Compare July 23, 2019 17:09
@costrouc
Copy link
Member Author

@GrahamcOfBorg build python2Packages.sphinxcontrib-tikz python3Packages.sphinxcontrib-tikz

@costrouc
Copy link
Member Author

@dotlambda I have created the patch (that was fun to figure out how to do with git). I have checked that documentation that uses this package works properly. Ready for merge

@costrouc
Copy link
Member Author

Ping ready for merge

@ryantm ryantm merged commit 2e8fc97 into NixOS:master Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda

@ryantm ryantm

@FRidh FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@costrouc @dotlambda @ryantm @GrahamcOfBorg