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

Generate Link dll name with case matching winmd definition #2932

Closed
youyuanwu opened this issue Mar 15, 2024 · 5 comments
Closed

Generate Link dll name with case matching winmd definition #2932

youyuanwu opened this issue Mar 15, 2024 · 5 comments
Labels
question Further information is requested

Comments

@youyuanwu
Copy link

Suggestion

Given the function in winmd definition:

[DllImport("FabricCommon", ExactSpelling = true, PreserveSig = false)]
public unsafe static extern HRESULT FabricDecryptText([In][Const] PWSTR encryptedText, [In] FABRIC_X509_STORE_LOCATION certStoreLocation, [Out][RetVal] IFabricStringResult* decryptedText);

windows-bindgen generates the following code for linking:

    #[link(name = "fabriccommon")]
    extern "system" {
        pub fn FabricDecryptText(
        ) -> ::windows_core::HRESULT;
    }

The link name has lower case "fabriccommon" not matching the case "FabricCommon". This request is to ask to make the name the same by default without changing the casing or add an option to configure this.
The desired code should look like this:

    #[link(name = "FabricCommon")]
    extern "system" {
        pub fn FabricDecryptText(
        ) -> ::windows_core::HRESULT;
    }

On windows there is this does not affect dll look up, but we (sf rust) use the same binding on linux, and the linker is case sensitive and cannot locate the lib. The current work around is to do a string replacement after code generation like this:

https://github.com/Azure/service-fabric-rs/blob/2fef8f1fe56135ddd3e6eea027ba59e34b0d7ad2/CMakeLists.txt#L43

@riverar
Copy link
Collaborator

riverar commented Mar 17, 2024

I'd prefer we leave this as-is. This will continue to be a problem regardless of what we do here, due to the case-sensitive filesystem in use on a target we don't support.

Related: microsoft/win32metadata#648, #638

@riverar
Copy link
Collaborator

riverar commented Mar 17, 2024

Perhaps a middle ground option here would be to add a bindgen configuration flag. Although it's not clear how useful that would be for service-fabric as it appears they do their own gen.

@kennykerr
Copy link
Collaborator

One option would be to preserve case when not targeting Windows. Something like #2934 but as @riverar points out I'm unclear what that project is doing with code gen so I'm not sure whether that will suffice. Are you moving away from your own code gen? Is that what this issue is about?

@kennykerr kennykerr added question Further information is requested and removed enhancement New feature or request labels Mar 18, 2024
@youyuanwu
Copy link
Author

The custom code gen is irrelevant for this issue. It is auto generating some experimental bindings on top of the windows-bindgen produced code.
Looks like the merged PR above can resolve this issue. I will try once it is released.

@kennykerr
Copy link
Collaborator

Feel free to take it for a spin by pointing your Cargo dependency to the git repo.

https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-git-repositories

youyuanwu added a commit to Azure/service-fabric-rs that referenced this issue Apr 22, 2024
Updated all windows versions in Cargo toml files to 0.56.
Regenerated com crate by running the generate_rust cmake target.
The generated code size reduced significantly.
Dll link name fix from windows-rs is included:
microsoft/windows-rs#2932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants