-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] Fix AOT publish in paths with space on Windows #94166
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsConnected to investigation: #92399. This is only fix for Windows, so we could not change the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
@@ -951,7 +951,7 @@ private PrecompileArguments GetPrecompileArgumentsFor(ITaskItem assemblyItem, st | |||
if (isDedup) | |||
{ | |||
foreach (var aItem in _assembliesToCompile!) | |||
processArgs.Add(aItem.ItemSpec); | |||
processArgs.Add('"' + aItem.ItemSpec + '"'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
processArgs.Add('"' + aItem.ItemSpec + '"'); | |
processArgs.Add($"'{aItem.ItemSpec}'"); |
What is the remaining issue on linux/macos? |
That while a sample repro for them work, when the fix is applied to the repo, it fails. Please, see #92399 (comment) and the investigation PR for reference. Shortly: in Linux you can escape space either by putting the string into quotation OR by using backslash. If we try on a sample .c file both methods work. However, applying it in the code, that produces |
Co-authored-by: Ankit Jain <[email protected]>
Can you please add this to the issue tracking the linux case of this problem? |
/backport to release/8.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/8702511263 |
Connected to investigation: #92399.
This is only fix for Windows, so we could not change the
TmpPath
for other platforms yet.WasmDedup=true
we haderror : Can not open image C:\repos\wbt
because the pathC:\repos\wbt artifacts
was not put into quotations. This was a problem for all platforms and is fixed everywhere._AOTObjectFile
items should not only be separated by space but also put into quotations, otherwise a spaced path will be treated as 2 separate items.