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

perlPackages.LaTeXML: use host perl and reenable tests except for epub #95812

Closed
wants to merge 2 commits into from
Closed

perlPackages.LaTeXML: use host perl and reenable tests except for epub #95812

wants to merge 2 commits into from

Conversation

xworld21
Copy link
Contributor

Use patchShebangs to replace #!.../bin/env perl with the host perl.

Motivation for this change

The current build uses the first perl in PATH, which makes latexml and the other executables in the package fail when other versions of perl are available.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

stigtsp
stigtsp approved these changes Sep 3, 2020
View reviewed changes
Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

Looks good to me

stigtsp
stigtsp requested changes Sep 4, 2020
View reviewed changes
Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

@xworld21 Can you change it so the PR contains only one commit?

stigtsp
stigtsp requested changes Sep 9, 2020
View reviewed changes
pkgs/top-level/perl-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/perl-packages.nix Outdated Show resolved Hide resolved
@matthuszagh
Copy link
Contributor

This PR is working for me, thanks for fixing this @xworld21!

@matthuszagh matthuszagh mentioned this pull request Sep 18, 2020
Closed
@xworld21 xworld21 changed the title perlPackages.LaTeXML: use host perl perlPackages.LaTeXML: use host perl and reenable tests except for epub Sep 27, 2020
@stigtsp
Copy link
Member

stigtsp commented Sep 27, 2020 •
edited

@xworld21 The PR looks good to me, but we need to wait for e34c4f7 to reach master from staging before we can merge, or alternatively you could PR this against staging if you prefer.

Currently this PR fails against perl-5.32.0 as it's missing Pod::Find provided by perlPackages.PodParser, which is fixed in e34c4f7:

builder for '/nix/store/2kgqq6fajmrjr7yf8qjm10vf6k5cnv9n-perl5.32.0-LaTeXML-0.8.4.drv' failed with exit code 2; last 10 log lines:
  t/93_formats.t       (Wstat: 9216 Tests: 37 Failed: 36)
    Failed tests:  2-37
    Non-zero exit status: 36
  t/95_complex_config.t (Wstat: 768 Tests: 4 Failed: 3)
    Failed tests:  2-4
    Non-zero exit status: 3
  Files=29, Tests=375, 232 wallclock secs ( 0.29 usr  0.10 sys + 219.82 cusr 10.78 csys = 230.99 CPU)
  Result: FAIL
  Failed 6/29 test programs. 50/375 subtests failed.
  make: *** [Makefile:1760: test_dynamic] Error 255
cannot build derivation '/nix/store/52dic58h68vgahj8z8bd6953272snamn-env.drv': 1 dependencies couldn't be built
[1 built (1 failed), 56 copied (66.5 MiB), 10.7 MiB DL]
error: build of '/nix/store/52dic58h68vgahj8z8bd6953272snamn-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/95812
1 package failed to build:
perl532Packages.LaTeXML

1 package built:
perl530Packages.LaTeXML

@stigtsp
Copy link
Member

stigtsp commented Oct 15, 2020

@xworld21 Can you rebase against master?

@SuperSandro2000 SuperSandro2000 marked this pull request as draft November 27, 2020 17:57
@SuperSandro2000
Copy link
Member

@xworld21 please rebase the PR and fix the merge conflict. I marked the PR wip to show that it needs some work.

xworld21 and others added 2 commits November 28, 2020 19:10
@xworld21
perlPackages.LaTeXML: use host perl
6220a33 
Use patchShebangs to replace #!.../bin/env perl with the host perl
@xworld21 @stigtsp
perlPackages.LaTeXML: reenable tests except for epub
8b11b50 
Co-authored-by: Stig P <stig@stig.io>
@xworld21
Copy link
Contributor Author

I have just updated the pull request, but I believe #105271 does a better job at it!

@SuperSandro2000
Copy link
Member

Closing in favor of #105271

@xworld21 xworld21 deleted the patch-1 branch February 16, 2021 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stigtsp stigtsp stigtsp requested changes

@aanderse aanderse Awaiting requested review from aanderse

@zakame zakame Awaiting requested review from zakame zakame will be requested when the pull request is marked ready for review zakame is a code owner

@dasJ dasJ Awaiting requested review from dasJ dasJ will be requested when the pull request is marked ready for review dasJ is a code owner

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@xworld21 @matthuszagh @stigtsp @SuperSandro2000