Windows support using vcpkg [x64-windows-static-md] - #4
Merged
Conversation
MaxXSoft
reviewed
Oct 20, 2025
MaxXSoft
left a comment
MaxXSoft
left a comment
Owner
There was a problem hiding this comment.
Thanks for your contribution! Here are some suggestions I made.
| @@ -1,26 +1,70 @@ | |||
| use std::{env, path::PathBuf}; | |||
| use std::{env, path::{self, PathBuf}}; | |||
Owner
There was a problem hiding this comment.
Suggested change
| use std::{env, path::{self, PathBuf}}; | |
| use std::{env, path::PathBuf}; |
path::self can be removed.
Contributor
Author
There was a problem hiding this comment.
Thank you for your comments, I have addressed all of them (and tried clippy, let's see how the CI goes.)
| .emit_includes(true) | ||
| .find_package("zlib") | ||
| .unwrap(); | ||
| let zlib_include_dir = path::PathBuf::from(zlib.include_paths[0].clone()); |
Owner
There was a problem hiding this comment.
Suggested change
| let zlib_include_dir = path::PathBuf::from(zlib.include_paths[0].clone()); | |
| let zlib_include_dir = PathBuf::from(zlib.include_paths[0].clone()); |
Use PathBuf directly.
Comment on lines
+28
to
+29
| let mman_base_path = path::PathBuf::from(mman.include_paths[0].clone()); | ||
| let mman_include_dir = mman_base_path.join(path::PathBuf::from("mman")); |
Owner
There was a problem hiding this comment.
Suggested change
| let mman_base_path = path::PathBuf::from(mman.include_paths[0].clone()); | |
| let mman_include_dir = mman_base_path.join(path::PathBuf::from("mman")); | |
| let mman_base_path = PathBuf::from(mman.include_paths[0].clone()); | |
| let mman_include_dir = mman_base_path.join(PathBuf::from("mman")); |
| /// Creates a new hierarchy. | ||
| fn new(hier: &'a capi::fstHier) -> Self { | ||
| match hier.htyp as u32 { | ||
| match hier.htyp as c_int { |
Owner
There was a problem hiding this comment.
This will fail on Linux, please check the CI output.
I think the following modification is okay:
Suggested change
| match hier.htyp as c_int { | |
| match hier.htyp as capi::fstHierType { |
Owner
|
Thanks for the update! Clippy reported some other issues, but that's okay, I'll fix them later. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am adding windows support using vcpkg,
vcpkg package management program version 2025-02-11-bec4296bf5289dc9ce83b4f5095943e44162f9c2There are manual steps needed to be done by users, outlined in the readme.
I have verified that this works, at least
readfst.execan read my vst files