Skip to content

Fix build on NetBSD. #2005

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

Merged
merged 1 commit into from
May 9, 2025
Merged

Fix build on NetBSD. #2005

merged 1 commit into from
May 9, 2025

Conversation

0-wiz-0
Copy link
Contributor

@0-wiz-0 0-wiz-0 commented May 9, 2025

rustix::fs 1.x has unified the nanonseconds member across operating systems.

Fixes #2004

@0323pin
Copy link

0323pin commented May 9, 2025

@0-wiz-0 thanks for this 👍

Copy link
Member

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

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

Thanks! Per #2004 (comment), I accidentally introduced this bug in #1949 by merging that update PR without adapting gix-index to the changes in rustix.

I have verified that the changes in this PR restore the ability to build on NetBSD, and that I am able to run commands like gix clone with the resulting binary.

A small change, to the commit message only

Although commits that adapt to changes in updated dependencies are most often not conventional (as only some commits need be), in this case the commit is fixing a bug that has appeared in a published version of the gix-index crate.

So I think the commit message, which is currently "Fix build on NetBSD.", will have to start with fix: in order to facilitate proper versioning and changelog entry generation. (It could be anything like "fix: Build on NetBSD." or "fix: Fix build on NetBSD" or some other variation if preferred.)

Since you've checked the box to allow maintainers to push to your branch, I can make such a change and force-push it. I'd be happy to do that, but I figured I'd check first, since you might prefer to do it.

@Byron
Copy link
Member

Byron commented May 9, 2025

Thanks a lot for contributing the fix, and thanks @EliahKagan for reviewing and validating it!

On another note, now that I read the feedback about the commit message, I just want to share that I have been operating on the implicit rules that

  • if I can push into the branch and
  • the commit isn't verified/signed

then I can rewrite the commits by rebasing and/or make changes to assure changelog and version generation will be desirable.

Maybe this is something we should make explicit in the contribution guidelines so that in future you can assume consent in that regard.

rustix::fs 1.x has unified the nanonseconds member across operating systems.
@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 9, 2025

@EliahKagan: I've changed the commit message as suggested - feel free to edit it if you want to adapt it even more!

Copy link
Member

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

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

Thanks--looks good!

@EliahKagan EliahKagan merged commit 33c4d6b into GitoxideLabs:main May 9, 2025
22 checks passed
@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 9, 2025

Thank you for merging this so quickly!
Do you know when the next release is planned?
We'll have to convince some projects to switch to a fixed version of gitoxide to fix their NetBSD build.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 10, 2025
@EliahKagan
Copy link
Member

EliahKagan commented May 10, 2025

Since there have been no SemVer-breaking changes to gix-index between its most recent v0.40.0 release and here, and this change is non-SemVer-breaking, it seems to me that it may be enough to release gix-index v0.40.1. Projects that specify gix-index v0.40.0 in the usual way should then automatically select gix-index v0.40.1 when built, unless --locked is passed.

@Byron Is this workable?

@Byron
Copy link
Member

Byron commented May 10, 2025

Thanks for the ping - it's indeed no problem to publish the fix, so it should arrive automatically downstream.

@EliahKagan
Copy link
Member

As shown in this gist, I've verified that installation works on the main branch, and when installing from crates.io, since the crate releases associated with the fix here. (That was yesterday, before zip was yanked as described in #2013. This and #2004 are unrelated to #2013, except in that #2013 would make it hard to verify this.)

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 this pull request may close these issues.

no field st_mtimensec on type rustix::fs::Stat
4 participants