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

RFE: port to nix 0.29 #15

Closed
michel-slm opened this issue Oct 23, 2024 · 4 comments · Fixed by #16
Closed

RFE: port to nix 0.29 #15

michel-slm opened this issue Oct 23, 2024 · 4 comments · Fixed by #16

Comments

@michel-slm
Copy link

clircle 0.4.0+ mostly builds fine with Nix 0.27 and above in Fedora - we're still on 0.4.0 because bat has not been ported to use 0.5.0 yet

see https://koji.fedoraproject.org/koji/packageinfo?packageID=34719

The only change is to adjust for Nix >= 0.27 returning OwnedFd instead of raw pointers for some operations:
https://github.com/nix-rust/nix/blob/master/CHANGELOG.md#changed-2

which we currently work around:
https://src.fedoraproject.org/rpms/rust-clircle/pull-request/1#request_diff

This is obviously not the cleanest solution - ideally clircle is refactored to use OwnedFd instead of integers as appropriate - but we're in the middle of bootstrapping packages for EPEL 10 (the add-on extra packages repo for Enterprise Linux 10 distributions) and would rather not have to build and support older Nix crates there.

If an upcoming clircle release can be ported to use the latest Nix, we can try and get bat to use it and that way it will be worth it to upgrade to the latest clircle too.

@decathorpe
Copy link

Note that the initial patch we made was a bit broken - it caused the files to be closed before they were used. A followup change avoids that issue, though it looks like clircle will need some minor code changes to adapt to nix now returning OwnedFds.

This patch now works without test failures: https://src.fedoraproject.org/rpms/rust-clircle/blob/rawhide/f/clircle-handle_ownedfd_for_nix_ge_0_27.diff

@decathorpe
Copy link

Thank you! 🚀

@niklasmohrin
Copy link
Owner

Of course, thanks for reporting :) Release and PR to bat will probably come out later today

@niklasmohrin
Copy link
Owner

See sharkdp/bat#3113

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants