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

[wasm] Process LinkerArg for wasm targets #107194

Merged
merged 2 commits into from
Sep 15, 2024
Merged

Conversation

lewing
Copy link
Member

@lewing lewing commented Aug 30, 2024

Putting this up for discussion rather than a final form, I think it makes sense to use common names

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Aug 30, 2024

CustomLinkerArg is an internal property in the NAOT targets. LinkerArg is its public counterpart.

@lewing
Copy link
Member Author

lewing commented Aug 31, 2024

CustomLinkerArg is an internal property in the NAOT targets. LinkerArg is its public counterpart.

I was working off the assumption that the use of CustomLinkerArg in wit-bindgen was correct, should that be changed to LinkerArg?

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Aug 31, 2024

I was working off the assumption that the use of CustomLinkerArg in wit-bindgen was correct, should that be changed to LinkerArg?

Yes.

@lewing lewing changed the title [wasi] Process CustomLinkerArg for wasi [wasi] Process LinkerArg for wasi Aug 31, 2024
@lewing lewing requested a review from maraf September 1, 2024 08:24
@lewing
Copy link
Member Author

lewing commented Sep 1, 2024

It looks like only iOS consumes LinkerArgs right now @maraf can you take a look across the platforms?

@maraf
Copy link
Member

maraf commented Sep 2, 2024

The CustomLinkerArg is use exclusively in NativeAOT. The LinkerArg is used in Apple, Android and LibraryMode and is added to CustomLinkerArg collection in NativeAOT https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets#L352

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture os-wasi Related to WASI variant of arch-wasm labels Sep 2, 2024
Copy link
Contributor

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

@lewing
Copy link
Member Author

lewing commented Sep 10, 2024

The CustomLinkerArg is use exclusively in NativeAOT. The LinkerArg is used in Apple, Android and LibraryMode and is added to CustomLinkerArg collection in NativeAOT https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets#L352

I just meant I missed the android use in a quick scan, thanks for the full reply. This patch just sticks it in wasi targets, it should really be in the shared bits or at least added to both.

@lewing lewing marked this pull request as ready for review September 10, 2024 16:41
@lewing lewing changed the title [wasi] Process LinkerArg for wasi [wasm] Process LinkerArg for wasm targets Sep 10, 2024
@lewing lewing added the os-browser Browser variant of arch-wasm label Sep 10, 2024
@lewing lewing merged commit 4c10eff into dotnet:main Sep 15, 2024
36 checks passed
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants