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

hurd: Fix st_dev name #3758

Closed
wants to merge 1 commit into from
Closed

hurd: Fix st_dev name #3758

wants to merge 1 commit into from

Conversation

sthibaul
Copy link
Contributor

The real GNU name is st_fsid, but the more commonly known name is st_dev, so better use it to avoid having to fix various rust package.

This also aligns on the existing struct stat field name.

@rustbot
Copy link
Collaborator

rustbot commented Jun 22, 2024

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

The real GNU name is st_fsid, but the more commonly known name is
st_dev, so better use it to avoid having to fix various rust package.

This also aligns on the existing struct stat field name, fixed by
07e57b2 ("hurd: Fix C API interface completion")
@JohnTitor
Copy link
Member

Thanks! But this crate just provides raw wrappers and changing a name may bring another confusion. I think it's better it remains as-is.

@JohnTitor JohnTitor closed this Jul 13, 2024
@sthibaul
Copy link
Contributor Author

sthibaul commented Jul 13, 2024

@JohnTitor : The current state is way more than confusing: it's not coherent between struct stat and struct stat64.

Long-term wise, keeping this incoherency will be a plague. We'd really much better stick to only the well-known name (which is provided by the C api too).

So far I have not seen a rust software use the st_fsid name, so let's act now to avoid seeing them pop up.

@sthibaul
Copy link
Contributor Author

So far I have not seen a rust software use the st_fsid name, so let's act now to avoid seeing them pop up.

@JohnTitor : On the other hand, I can quickly see ~45 rust software in debian that use the st_dev field name. Refusing to integrate this PR means imposing us the tedious work to go through all these to introduce an os-specific case, that this PR would avoid altogether. This is imposing us unnecessary work.

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

Successfully merging this pull request may close these issues.

3 participants