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

Support windows targets #11

Merged
merged 3 commits into from
Nov 15, 2024
Merged

Support windows targets #11

merged 3 commits into from
Nov 15, 2024

Conversation

emesare
Copy link
Contributor

@emesare emesare commented Oct 23, 2024

Tested on windows 10 and windows 11.

EDIT: It seems there is something else within the extraction that relies on windows symlinks directly which is an issue for non-admin users:

15:01:14  Caused by:
15:01:14    process didn't exit successfully: `C:/target\release\build\flatbuffers-build-a81ddc4b08ecd034\build-script-build` (exit code: 101)
15:01:14    --- stderr
15:01:14    thread 'main' panicked at C:\.cargo\git\checkouts\flatbuffers-build-8ef4ff77bea6d0f6\46e481f\build.rs:5:30:
15:01:14    failed to vendor flatc: failed to unpack `\\?\C:\tmp\.tmp6bPG6a\flatbuffers\flatbuffers-24.3.25\docs\source\CONTRIBUTING.md`
15:01:14  
15:01:15    Caused by:
15:01:15        A required privilege is not held by the client. (os error 1314) when symlinking ../../CONTRIBUTING.md to \\?\C:\tmp\.tmp6bPG6a\flatbuffers\flatbuffers-24.3.25\docs\source\CONTRIBUTING.md

@carlocorradini
Copy link

@rdelfin Any update?
I need this 😢

@rdelfin
Copy link
Owner

rdelfin commented Nov 15, 2024

oh! Really sorry, didn't see your PR! I'll take a look and prepare a release

Copy link
Owner

@rdelfin rdelfin left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to do this!

Comment on lines +315 to +316
#[cfg(windows)]
junction::create(output_path, symlink_path).map_err(Error::SymlinkCreationFailure)?;
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like the they're trying to add support on the standard library, but it's still nightly :/
rust-lang/rust#121709

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I have a requirement to run on rust 1.77 so I couldn't do that

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah absolutely, I don't want to turn this into a nightly-only crate

@rdelfin
Copy link
Owner

rdelfin commented Nov 15, 2024

Fixing clippy error in #12

@rdelfin rdelfin merged commit 700bd2e into rdelfin:main Nov 15, 2024
6 checks passed
@rdelfin
Copy link
Owner

rdelfin commented Nov 15, 2024

@carlocorradini: I can prepare a release, but before I do so, can you confirm latest main now works for you? If not, I can dig into why

@carlocorradini
Copy link

@rdelfin It works! Thanks! 🥳

@rdelfin
Copy link
Owner

rdelfin commented Nov 15, 2024

Glad to @carlocorradini. Check this project's crates.io and you should see a new version (0.2.1) with windows support

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.

3 participants