tectonic, biber-for-tectonic: wrap tectonic with biber-2.17, fix #88067 #273740

bryango · 2023-12-12T09:10:38Z

Note

This PR has undergone major design change. Please jump to tectonic, biber-for-tectonic: wrap tectonic with biber-2.17, fix #88067 #273740 (comment) and onwards for the current design.
It is easy to review this PR commit by commit: https://github.com/NixOS/nixpkgs/pull/273740/commits.

Description of changes

The version of biber is strongly coupled to the TeX-like system it works with. For example, the biber version required by tectonic is not found in nixpkgs, since the texlive bundle used by tectonic lags behind the one in nixpkgs. This has been a known issue. Previously, a user had to manually download (or pin nixpkgs to) a particular version of biber to use with the appropriate TeX system, in this case, tectonic.

In this PR, we introduce the biber-for-tectonic package as an alternative version of biber required by the current tectonic. At the same time, we make tectonic into a wrapper, which bundles tectonic-unwrapped with biber-for-tectonic. This fixes #88067.

Some nixpkgs code by @collares and @doronbehar is recycled for use in this PR.

Things done

Add a 👍 reaction to pull requests you find important.

doronbehar

I'm not sure I like this approach... It is a bit too overwhelming IMO. Why not simply create a biber2_17 attribute, that will overrideAttrs of biber, and we'll comment near it that tectonic is it's purpose?

I'm also not sure I want to wrap tectonic with biber, and I've expressed my opinion in 0889989 . However I do tend to support wrapping tectonic with the correct biber, as upstream fails to update the bundle for a while now, and the issue is indeed annoying in general.

The downside of wrapping Tectonic though, is that Tectonic may break again in the future when the Tectonic's TeX bundle will be updated but no new Tectonic version will be released, because we will find out about it too late, and then again we'd have to force Tectonic to use the newer Biber.

I was thinking, perhaps we can force Tectonic to use a web bundle that we know for sure is compatible with a certain Biber package? Then at least we know we will overcome the downside presented above.

bryango · 2023-12-13T08:59:25Z

Hi Doron! Thank you very much for the comment!

I'm not sure I like this approach... It is a bit too overwhelming IMO. Why not simply create a biber2_17 attribute, that will overrideAttrs of biber, and we'll comment near it that tectonic is it's purpose?

Yeah, that's perfectly fine! In fact, that's the first thing that I tried, but I later settled on the biberVersions abstraction because by doing this, when it comes to updating biber, everything can be done by updating biber/versions.nix, without touching any other file. We don't need to touch all-packages.nix or biber at all.

The downside of wrapping Tectonic though, is that Tectonic may break again in the future when the Tectonic's TeX bundle will be updated but no new Tectonic version will be released, because we will find out about it too late, and then again we'd have to force Tectonic to use the newer Biber.

Yes, I agree and I share all your concerns. The reason I chose to do this now is that the tectonic development is slowing down. This is bad news for tectonic but good news for downstream, since it would not be too difficult to keep up. Also since I am a keen user of both biber and tectonic, I have committed myself to be the maintainer (of the wrappers).

I was thinking, perhaps we can force Tectonic to use a web bundle that we know for sure is compatible with a certain Biber package? Then at least we know we will overcome the downside presented above.

That's brilliant, but I am not sure whether there is an API for that in tectonic...

bryango · 2023-12-13T12:14:48Z

I have simply the structure of biberVersions in biber/version.nix. Now there is no change of biber/default.nix.

However, we need to do biber.override { version = ...; } first and then .overrideAttrs, otherwise the meta.name attribute would be wrong. It seems that this is due to the wrapping of mkDerivation by buildPerlModule.

pkgs/tools/typesetting/biber/versions.nix

doronbehar

In addition to the below comments, if we'll settle down upon such a wrapper, maybe the --web-bundle option could allow us to stick to a specific bundle version which includes a biblatex version that supports our biber version we force here. Using this option needs testing though, and perhaps even documentation in the meta.longDescription.

pkgs/tools/typesetting/biber/versions.nix

pkgs/tools/typesetting/tectonic/wrapper.nix

doronbehar · 2023-12-13T13:14:59Z

but I later settled on the biberVersions abstraction because by doing this, when it comes to updating biber, everything can be done by updating biber/versions.nix, without touching any other file. We don't need to touch all-packages.nix or biber at all.

I agree that touching all-packages.nix is a headache :) Still you can create that derivation in a separate file, (use pkgs/by-name/bi/biber-for-tectonic to further ease yourself).

I was thinking, perhaps we can force Tectonic to use a web bundle that we know for sure is compatible with a certain Biber package? Then at least we know we will overcome the downside presented above.

That's brilliant, but I am not sure whether there is an API for that in tectonic...

See the main comment of the last review about the --web-bundle Tectonic option.

Yes, I agree and I share all your concerns. The reason I chose to do this now is that the tectonic development is slowing down. This is bad news for tectonic but good news for downstream, since it would not be too difficult to keep up. Also since I am a keen user of both biber and tectonic, I have committed myself to be the maintainer (of the wrappers).

Thanks for the link! I think it should be mentioned in the comments at the biber-for-tectonic/package.nix file that this thread strengthens our motivation to maintain tectonic as we do and that biber-for-tectonic package this way. In fact, if the --web-bundle option proves to work, perhaps we can even feel confident enough to force this wrapping for main tectonic as well, and not introduce a new tectonic-with-biber attribute.

There's also the middle-way betweeen your approach and the above, which would be to provide an unwrapped tectonic.unwrapped and keep the main tectonic derivation the wrapped one.

bryango · 2023-12-13T15:25:35Z

maybe the --web-bundle option could allow us to stick to a specific bundle version which includes a biblatex version that supports our biber version we force here. Using this option needs testing though, and perhaps even documentation in the meta.longDescription.

In fact, if the --web-bundle option proves to work, perhaps we can even feel confident enough to force this wrapping for main tectonic as well, and not introduce a new tectonic-with-biber attribute.

There's also the middle-way betweeen your approach and the above, which would be to provide an unwrapped tectonic.unwrapped and keep the main tectonic derivation the wrapped one.

This is interesting, but I need some time to take a look and see how to implement that!

I agree that touching all-packages.nix is a headache :) Still you can create that derivation in a separate file, (use pkgs/by-name/bi/biber-for-tectonic to further ease yourself).

This is cool! I didn't know this! But after some thought, I think this scheme places the biber override far away from tectonic and biber, which is somewhat cumbersome.

In the end, I decided to collapse biberVersions and move the biber-for-tectonic definition to tectonic/biber.nix. Please see whether it is okay this way!

pkgs/tools/typesetting/tectonic/wrapper.nix

pkgs/top-level/all-packages.nix

bryango · 2023-12-14T15:34:19Z

There is currently this upstream issue which prevents us to consistently override web-bundle for all cli interface. Currently, the default web bundle is hardcoded.

I am experimenting with patching tectonic to pin the hardcoded bundle; maybe that's too much? In any case, I haven't finished, but I will report back what I find.

Update: check out bryango/nixpkgs@master...biber-versions-ng for a proof of concept. Works fine for me but not sure about maintainability...

Update: I have submitted a PR upstream tectonic-typesetting/tectonic#1131 such that we can reliably pin the web bundle at build time.

doronbehar · 2023-12-18T09:34:21Z

Hey @bryango sorry for the long waiting. Well done on tectonic-typesetting/tectonic#1131 ! I really like the general idea, though I left a comment there as you probably noticed..

I think the best way to ensure reproducibility in Nixpkgs most cleanly would be to implement the following design:

tectonic-unwrapped is a derivation that includes some version of your upstream patch Allow overriding web bundle at _build time_ tectonic-typesetting/tectonic#1131 .
tectonic is nothing but a wrapping derivation for tectonic-unwrapped that includes both:
- ${biber-for-tectonic}/bin in it's $PATH
- The $TECTONIC_WEB_BUNDLE environment variable ensures the above biber is compatible with the bundle.
tectonic.passthru.unwrapped could be another way to access tectonic-unwrapped.
tectonic.passthru.biber could be another way to access biber-for-tectonic.

This way, users could use the attribute tectonic-unwrapped to play around with different bundles if they wish so, and be able to also play with the feature of your patch.

bryango · 2023-12-18T10:41:46Z

Hey @doronbehar thank you for the comment! The structure you proposed looks good to me. I will wait a bit for upstream response and see what we should do. If there is no response, I will start to implement it based on our discussions!

doronbehar · 2023-12-18T11:11:00Z

I'm afraid your work would be forgotten while we wait for upstream, and the issue will persist and most users won't get a proper fix as proposed here. We agree upon many of the changes and they aren't too entangled to the upstream PR IMO. Let's commit to Nixpkgs in the meantime the following commits:

maintainers: add bryango
tectonic: add bryango as maintainer
biber-for-tectonic: init at 2.17 # With you & I as maintainers
tectonic: redefine it as a wrapper derivation, using biber-for-tectonic

In the last commit, add a comment that mentions your upstream PR, and that ideally the web bundle of the wrapped tectonic should use a pinned texlive bundle, to ensure reproducibility... I think you had a good phrased comment in the current PR that you can start with.

doronbehar

I really prefer tectonic-unwrapped to be a clean Tectonic without patches. I also dislike the fact that you have to recompile tectonic just to change the URL of the bundle - I dislike it being a build time option and not a runtime option, since it takes a lot of resources to compile it...

I'm sorry to put too many obstacles in the way of this PR, but I want the solution to eventually become something that upstream may approve. See my comment at tectonic-typesetting/tectonic#1132 (comment) .

pkgs/tools/typesetting/tectonic/default.nix

doronbehar · 2023-12-20T12:11:53Z

pkgs/tools/typesetting/tectonic/tests.nix

+  notice = builtins.toFile "tectonic-offline-notice" ''
+    # To fetch tectonic's web bundle, the tests require internet access,
+    # which is not available in a build sandbox. To run the tests, try:
+    # `nix-build --no-sandbox --attr tectonic.passthru.tests`
+  '';


So this test is only meant for manual running? Not by Hydra? I don't think so. How about adding a VM based test? These can access the internet. See examples at nixos/tests/all-tests.nix.

I spent a night or two looking at the (sparsely documented) NixOS tests, and they are truly amazing beasts! However, it seems that although they do enjoy a network stack, they still do not have internet / "world wide web" access in a sandbox build. Fortunately, I do find out a way to achieve internet access: basically, we make the test as a "fetcher" like fetchurl. I have implemented this in #278410.

pkgs/tools/typesetting/tectonic/wrapper.nix

bryango · 2023-12-20T12:57:53Z

I really prefer tectonic-unwrapped to be a clean Tectonic without patches. I also dislike the fact that you have to recompile tectonic just to change the URL of the bundle - I dislike it being a build time option and not a runtime option, since it takes a lot of resources to compile it...

I totally understand the desire to be perfect, but I am afraid I am running out of energy to continue... And I am getting more and more occupied in my day job 😿 I will try to implement the remaining suggestions; the VM test is particularly interesting (I didn't know about that). For now, I will mark this PR as draft, but if you or anyone would like to build on the work here, feel free to do so!

doronbehar · 2023-12-20T13:00:39Z

It's OK @bryango , you have done very good job anyway. I will checkout out this PR locally and do prepare to merge the things that we agree upon. I'll leave the upstream patches aside for now.

bryango · 2023-12-20T13:09:56Z

It's OK @bryango , you have done very good job anyway. I will checkout out this PR locally and do prepare to merge the things that we agree upon. I'll leave the upstream patches aside for now.

Thank you very much! I learned quite a lot along the way. If there is anything confusing in the code that you would like me to clarify, feel free to ping me! I might code less, but I can explain.

The `tectonic` package depends on a version of `biber` that is generally different from the one in the nixpkgs `texlive` bundle. This package provides an override of biber suitable for use with tectonic. For biber<=2.17 on perl>=5.36.0 a patch is needed. This is recovered from a previous nixpkgs commit: NixOS@c784cdb Co-authored-by: Mauricio Collares <[email protected]> Co-authored-by: Doron Behar <[email protected]>

bryango

I think the current wrapper is the best solution for now. Nits: I would like Tex to be tex or TeX. Otherwise, I think it's good to go! I am running nixpkgs-review.

Also, I think it would be easier for other people to review by starting a new PR and closing this one. What do you think? This PR can then be a place to track the downstream changes if the --web-bundle update finally works out in the upstream.

Update: Result of nixpkgs-review pr 273740 run on x86_64-linux (details)

4 packages built:

biber-for-tectonic
biber-for-tectonic.devdoc
tectonic
tectonic-unwrapped

pkgs/tools/typesetting/tectonic/wrapper.nix

doronbehar · 2023-12-21T12:29:06Z

Will merge when CI is green.

Probably this was conditioned on stdenv.isLinux by mistake.

@bryango

The `tectonic` attribute is redefined to be a wrapper with a compatible version of biber, provided by `biber-for-tectonic`. The wrapper is partially recovered from a previous nixpkgs commit: NixOS@5aa8e9f Also: - Remove unneeded makeBinaryWrapper input in `tectonic-unwrapped`. - Add @bryango as a maintainer of both `tectonic-unwrapped` and `tectonic`. Co-authored-by: Doron Behar <[email protected]>

doronbehar · 2023-12-21T16:24:42Z

Fixed Darwin issue discovered by ofborg. Will wait for CI again..

doronbehar · 2023-12-21T16:29:37Z

@ofborg build tectonic

bryango · 2023-12-22T01:17:24Z

Wow awesome! Thank you very much for finishing this!

bryango mentioned this pull request Dec 12, 2023

Biber and BibLatex incompatibility #88067

Closed

ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 12, 2023

ofborg bot requested a review from ttuegel December 12, 2023 11:03

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Dec 12, 2023

doronbehar reviewed Dec 13, 2023

View reviewed changes

bryango force-pushed the biber-versions branch from f047666 to 00b92ba Compare December 13, 2023 12:08

bryango commented Dec 13, 2023

View reviewed changes

pkgs/tools/typesetting/biber/versions.nix Outdated Show resolved Hide resolved

bryango force-pushed the biber-versions branch from 00b92ba to 43d2040 Compare December 13, 2023 12:47

bryango changed the title ~~biber, biberVersions.biber_2_17, tectonic-with-biber: wrap tectonic with biber-2.17, fix #88067~~ biberVersions.biber-for-tectonic, tectonic-with-biber: wrap tectonic with biber-2.17, fix #88067 Dec 13, 2023

doronbehar reviewed Dec 13, 2023

View reviewed changes

bryango force-pushed the biber-versions branch from 4b0088b to a4340af Compare December 13, 2023 14:17

ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Dec 13, 2023

bryango force-pushed the biber-versions branch 3 times, most recently from 63ee25a to b9165ab Compare December 13, 2023 15:24

bryango force-pushed the biber-versions branch from b9165ab to 55d2e3f Compare December 14, 2023 03:04

bryango changed the title ~~biberVersions.biber-for-tectonic, tectonic-with-biber: wrap tectonic with biber-2.17, fix #88067~~ biber-for-tectonic, tectonic-with-biber: wrap tectonic with biber-2.17, fix #88067 Dec 14, 2023

doronbehar reviewed Dec 14, 2023

View reviewed changes

bryango force-pushed the biber-versions branch from 55d2e3f to 0bde4b6 Compare December 14, 2023 15:23

bryango mentioned this pull request Dec 15, 2023

Allow overriding web bundle at _build time_ tectonic-typesetting/tectonic#1131

Draft

bryango force-pushed the biber-versions branch from 0bde4b6 to a864d46 Compare December 15, 2023 12:50

ofborg bot requested review from lluchs and doronbehar December 19, 2023 19:24

bryango requested review from piegamesde and Janik-Haag as code owners December 20, 2023 08:58

bryango force-pushed the biber-versions branch from 105a6b5 to 0c51dff Compare December 20, 2023 09:11

doronbehar requested changes Dec 20, 2023

View reviewed changes

bryango marked this pull request as draft December 20, 2023 12:58

doronbehar force-pushed the biber-versions branch from 0c51dff to a132822 Compare December 20, 2023 15:14

doronbehar marked this pull request as ready for review December 20, 2023 15:15

ofborg bot requested a review from doronbehar December 21, 2023 00:26

bryango commented Dec 21, 2023

View reviewed changes

pkgs/tools/typesetting/tectonic/wrapper.nix Outdated Show resolved Hide resolved

pkgs/tools/typesetting/tectonic/wrapper.nix Outdated Show resolved Hide resolved

doronbehar force-pushed the biber-versions branch from a132822 to 9993964 Compare December 21, 2023 12:28

doronbehar approved these changes Dec 21, 2023

View reviewed changes

delroth added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Dec 21, 2023

doronbehar and others added 2 commits December 21, 2023 18:23

tectonic: symlink nextonic for all platforms

0a85473

Probably this was conditioned on stdenv.isLinux by mistake.

doronbehar force-pushed the biber-versions branch from 9993964 to 04f1b55 Compare December 21, 2023 16:24

delroth removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 21, 2023

ofborg bot requested a review from doronbehar December 21, 2023 18:25

doronbehar merged commit 55d199f into NixOS:master Dec 21, 2023
11 of 14 checks passed

bryango mentioned this pull request Jan 3, 2024

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

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tectonic, biber-for-tectonic: wrap tectonic with biber-2.17, fix #88067 #273740

tectonic, biber-for-tectonic: wrap tectonic with biber-2.17, fix #88067 #273740

bryango commented Dec 12, 2023 •

edited

Loading

doronbehar left a comment

bryango commented Dec 13, 2023

bryango commented Dec 13, 2023

doronbehar left a comment

doronbehar commented Dec 13, 2023

bryango commented Dec 13, 2023

bryango commented Dec 14, 2023 •

edited

Loading

doronbehar commented Dec 18, 2023

bryango commented Dec 18, 2023

doronbehar commented Dec 18, 2023

doronbehar left a comment

doronbehar Dec 20, 2023

bryango Jan 3, 2024

bryango commented Dec 20, 2023

doronbehar commented Dec 20, 2023

bryango commented Dec 20, 2023

bryango left a comment •

edited

Loading

doronbehar commented Dec 21, 2023

doronbehar commented Dec 21, 2023

doronbehar commented Dec 21, 2023

bryango commented Dec 22, 2023

tectonic, biber-for-tectonic: wrap tectonic with biber-2.17, fix #88067 #273740

tectonic, biber-for-tectonic: wrap tectonic with biber-2.17, fix #88067 #273740

Conversation

bryango commented Dec 12, 2023 • edited Loading

Note

Description of changes

Things done

doronbehar left a comment

Choose a reason for hiding this comment

bryango commented Dec 13, 2023

bryango commented Dec 13, 2023

doronbehar left a comment

Choose a reason for hiding this comment

doronbehar commented Dec 13, 2023

bryango commented Dec 13, 2023

bryango commented Dec 14, 2023 • edited Loading

doronbehar commented Dec 18, 2023

bryango commented Dec 18, 2023

doronbehar commented Dec 18, 2023

doronbehar left a comment

Choose a reason for hiding this comment

doronbehar Dec 20, 2023

Choose a reason for hiding this comment

bryango Jan 3, 2024

Choose a reason for hiding this comment

bryango commented Dec 20, 2023

doronbehar commented Dec 20, 2023

bryango commented Dec 20, 2023

bryango left a comment • edited Loading

Choose a reason for hiding this comment

doronbehar commented Dec 21, 2023

doronbehar commented Dec 21, 2023

doronbehar commented Dec 21, 2023

bryango commented Dec 22, 2023

bryango commented Dec 12, 2023 •

edited

Loading

bryango commented Dec 14, 2023 •

edited

Loading

bryango left a comment •

edited

Loading