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: don't remove luatex from packages that require it. #33698

Merged
merged 1 commit into from Feb 23, 2018

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Jan 10, 2018

Fixes #31482.

Motivation for this change

Leaving other dependencies that we patch out into other collections, but keeping luatex since it is needed by at least texlive-scripts which is part of collection-basic (which is foundation of many other collections).

I'm not sure why it was not included in the first place--if it was closure size it appears that is not a significant concern, see #32661 (comment) for some numbers.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

PS I wish we had a place to dump things like Nix expressions that test "ensure this MWE still builds" or something. Soon, hopefully! :)

@FRidh
Copy link
Member

FRidh commented Jan 10, 2018

PS I wish we had a place to dump things like Nix expressions that test "ensure this MWE still builds" or something. Soon, hopefully! :)

Go for it! Still need to add some tests for Python constructs...

@FRidh FRidh requested a review from vcunat January 10, 2018 17:24
@dtzWill
Copy link
Member Author

dtzWill commented Jan 12, 2018

(I may do so eventually, but separately since I don't think I'll get to it anytime soon)

@dtzWill dtzWill changed the base branch from master to staging January 14, 2018 23:07
@dtzWill
Copy link
Member Author

dtzWill commented Jan 14, 2018

Moving to staging...

@dtzWill
Copy link
Member Author

dtzWill commented Jan 16, 2018

Ping! Would like to get this in before 18.03, with plenty of time for testing any unintended consequences. Please :).

@dtzWill
Copy link
Member Author

dtzWill commented Jan 31, 2018

cc @vcunat -- thoughts?

@dtzWill
Copy link
Member Author

dtzWill commented Feb 9, 2018

Ping!

@dtzWill
Copy link
Member Author

dtzWill commented Feb 19, 2018

@vcunat please acknowledge, even if only to say you can't review or need more time or whatever the case may be :).

@vcunat
Copy link
Member

vcunat commented Feb 19, 2018

@dtzWill I'm sorry. I haven't been really following texlive tickets lately. I'm spread over much nix* stuff and TeX happens to be relatively low on my priority list. It had been different when I created this framework, as I was using LaTeX a lot, I had more time for nix*, our texlive was composed of large packages being fully rebuilt on every stdenv change, etc. I think it would be better if there was someone else willing to take over the maintainership.

Closure size was certainly the motivation for this removal. Maybe the situation has changed since 2015, I don't know. Anyway, at a superficial look this seems OK, so feel free to merge (you probably know better than me from the brief look).

@dtzWill
Copy link
Member Author

dtzWill commented Feb 19, 2018

Thank you very much for your response @vcunat, and for all you do! 👍

Part of the motivation for this PR is that in absence of large amounts of maintainer time it seems best to follow upstream for minimal surprises or breakage due to our modifications (unless there is a pressing reason to do otherwise, which I was unsure was the case).

The rebuild frequency was not something I had considered--that is, if this adds dependencies to a number of packages how likely/frequently will large texlive packages need to be rebuilt?

That is not something I'm sure how to answer reasonably. Hmm.

@vcunat
Copy link
Member

vcunat commented Feb 19, 2018

Rebuild frequency isn't really relevant here. Almost all texlive are fixed-output derivations ;-)

@dtzWill
Copy link
Member Author

dtzWill commented Feb 19, 2018

Oh, I thought our patching might make large things depend on say, lua, when they didn't previously. Hmm, then maybe this is good? 😁

Anyone you'd like to take a second look before going ahead with this?

Thanks again!

@vcunat
Copy link
Member

vcunat commented Feb 19, 2018

Well, there are smaller (not gigabytes) compiled "parts" of packages that are added, and those do have dependencies on non-tex packages.

@dtzWill dtzWill merged commit b42b1c8 into NixOS:staging Feb 23, 2018
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