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

tectonic: add online passthru.tests.biber-compatibility #278410

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

bryango
Copy link
Member

@bryango bryango commented Jan 3, 2024

Description of changes

Implement the (LaTeX + biber) compile test for the tectonic package wrapped with biber.

  • The test requires internet access to fetch Tectonic's web bundle on demand. This is achieved by (ab)using a fixed-output derivation, which is capable of internet access.
  • The tectonic.outPath is included in the test package name. This ensures that it is always triggered for rebuild when the main derivation changes.

I do have a few concerns about doing this:

  • How often will tectonic.passthru.tests be triggered by hydra? Every staging-next build would probably change tectonic's outPath, which will change the test's name according to this current design. I am not sure (1) whether this would trigger a test rebuild in hydra, and (2) if this happens, whether it would lead to too many test rebuilds. I don't want to accidentally DDoS the upstream server!
  • The test is basically realized as a customized fetcher, just like fetchurl. Is that legal in nixpkgs?

This PR follows from discussions in #273740. Note, however, that this PR is independent of the previously discussed web-bundle-locking proposal (which I am also working on upstream, but independent of this PR).

Update:

  • This PR is trivial to nixpkgs-review as it causes zero rebuild of the main packages.
  • Ofborg happy on x86_64-* and *-linux (log), still pending on aarch64-darwin.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@bryango bryango requested a review from doronbehar January 3, 2024 04:10
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jan 3, 2024
@bryango bryango force-pushed the tectonic-online-tests branch from 97ccc2d to 7d8a3b4 Compare January 3, 2024 05:28
@bryango bryango added the 8.has: tests This PR has tests label Jan 3, 2024
@bryango bryango force-pushed the tectonic-online-tests branch from 476f68a to 1ce6561 Compare January 3, 2024 15:15
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Nit.

@bryango bryango force-pushed the tectonic-online-tests branch from 1ce6561 to fe4860f Compare January 3, 2024 15:20
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Commit message: passthru.tests.biber-compatibility

@bryango bryango changed the title tectonic: add online passthru.tests.biber tectonic: add online passthru.tests.biber-compatibility Jan 3, 2024
@bryango
Copy link
Member Author

bryango commented Jan 3, 2024

Commit message: passthru.tests.biber-compatibility

Ah yes, of course hahaha totally missed it 😆 fixed.

@bryango bryango force-pushed the tectonic-online-tests branch from fe4860f to e3570b9 Compare January 3, 2024 15:25
@doronbehar
Copy link
Contributor

Great, should be good to go when CI is green. Ping me if I forget.

The test requires internet access to fetch tectonic's web bundle
on demand. This is achieved by abusing a fixed-output derivation,
which is capable of internet access.

The `tectonic.outPath` is included in the test package name. This
ensures that it is always triggered for rebuild when the main
derivation changes.

Co-authored-by: Doron Behar <[email protected]>
@bryango bryango force-pushed the tectonic-online-tests branch from e3570b9 to 58627fa Compare January 3, 2024 15:28
@bryango
Copy link
Member Author

bryango commented Jan 3, 2024

Great, should be good to go when CI is green. Ping me if I forget.

Splendid! Thank you! Just fixed buildInputs > nativeBuildInputs in some comments, waiting for CI.

@doronbehar doronbehar merged commit c34c6b8 into NixOS:master Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: tests This PR has tests 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants