Skip to content

Fixes #138 #139

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fixes #138 #139

wants to merge 5 commits into from

Conversation

Darkrael
Copy link

This fixes #138.

Important note: Due to bindgen now generating "unsafe extern" blocks instead of "extern" blocks, the MSRV is now 1.82, where support for this was added.

…nsafe extern" blocks instead of "extern" blocks, the MSRV is now 1.82, where support for this was added.
@otavio
Copy link
Member

otavio commented Mar 27, 2025

It seems that newer versions have fixed this difference. Take a look at the pull request below:

rust-lang/libc#3625

Is it possible for you to experiment with that?

@Darkrael
Copy link
Author

Hey, thanks for the quick response. As far as i can see, this only changes the type of a single filed of the stat field to be the same as some constants to make some operation between them possible. This does not change the size of the last 4 parameters of the struct.
The newest version of rust libc still has the same binary layout and i've tested it with the newest version just now an have the same problems.
Changing this in rust libc would break all uses that rely on the struct representing stat64, so i don't think that they will do that.

@Darkrael
Copy link
Author

I should clarify, that the problem is, that the size of st_size, st_atime, st_mtime and st_ctime are defined as 64 bit types in rust libc, but libarchive returns a stat struct where these are 32-bit types (on windows). I have the 64-bit version of libarchive installed and this is still true and seems to be intentional. For reference, i have libarchive:x64-windows-static-md 3.7.7 installed via vcpkg.

@otavio
Copy link
Member

otavio commented Mar 27, 2025

So, is it possible for it to adjust the GitHub Actions workflow and fix the formatting of the code?

@Darkrael
Copy link
Author

I'm unsure what you are requesting. The Github Actions seem to be failing before my commit (like in the newest master commit 9a7c715) already. Looking at why they are failing they seem to be unrelated to my changes and i'm not sure how i should change them. The code style of my changes should be correct as per standard rustfmt formatting. The rust 1.65 test seem to be failing due to another crate, but will be broken by this merge due to using a new bindgen (like i've noted). This can probably be fixed by generating the bindings using an older version of bindgen. But I'm not sure why these checks even exist and if this old rust version should still be supported.
If you want me to help with anything specific, i can do that but i need to know what exactly you are requesting.

@otavio
Copy link
Member

otavio commented Mar 28, 2025

I really appreciate your offer to help. After reviewing the issues, it appears that they are quite unrelated. I'm considering merging this request as is. If you can assist us in ironing out the previous versions, that would be very, very helpful.

As you noted, I believe that the style issues arise because the workflow is using the Rust nightly compiler instead of the stable version, which causes the formatting to differ. I think it would be wise to stick with the stable version and avoid nightly features for code formatting.

Regarding the old version of Rust support, it is not critical to support such an old version, but I prefer to avoid breaking support for older versions when possible. Wouldn't it be nice if we could maintain compatibility with lower versions? Not necessarily version 1.65, but version 1.75 or similar would be great, as it would facilitate easier upgrades for embedded devices and other users. How do you think we should proceed? Would you prefer to match this as it is and work on resolving issues in another pull request, or would you like to rework this one?

@Darkrael
Copy link
Author

Darkrael commented Mar 31, 2025

I've locked the tokio version to 1.38, which has an MSRV of 1.63 and also downgrades mio to be compatible with 1.65.
Additionally, i've added a CLI option to the bindgen command to target a specific rust version, therefore generating compatible code and avoiding unsafe extern blocks without needing to pull an old bindgen version.

As far as the failing nightly actions, i think it's due to the -Zprofile flag being removed in rust 1.84 (see rust-lang/rust#131829). From what i can see, there is no direct replacement, but -Cinstrument-coverage is mentioned as being similar.

Could you please rerun the actions to see if there are further conflicts?

@Darkrael
Copy link
Author

Hey, sorry for not getting back sooner, but i had a lot to do. I've locked the tokio-utils crate to an older version that has support for rust 1.65 and tested it in the rust docker container for that version. It seems to run fine now. I've also added a simple check in an integration test that checks the stat object given by the ArchiveIterator. It's not a precise that but this should catch an incorrect type being parsed from the FFI (like the one before my code changes).
Please rerun the tests to see if this fixed the actions.

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

Successfully merging this pull request may close these issues.

Incorrect stat struct used on windows leading to incorrect metadata
2 participants