Skip to content

[wasm] Make sure the TargetOS is capitalized #54927

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.

Sign up for GitHub

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

Jump to bottom
Merged
radekdoulik merged 2 commits into from
Aug 14, 2021
Merged

[wasm] Make sure the TargetOS is capitalized #54927

radekdoulik merged 2 commits into from
Aug 14, 2021

Conversation

@radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Jun 29, 2021

Fixes: #54926

The $(TargetOS) is used in target framework, which is case sensitive.
Make sure it is capitalized for Browser, so that build.cmd works when
used like:

./build.cmd -bl -os browser -subset libs

It fixes build errors like:

C:\r\src\libraries\shims\manual\System.forwards.cs(8,88): error CS0234: The type or namespace name 'ZLibException' does not exist in the namespace 'System.IO.Compression' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
C:\r\src\libraries\shims\manual\System.forwards.cs(9,77): error CS0234: The type or namespace name 'CookieVariant' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
C:\r\src\libraries\shims\manual\System.forwards.cs(10,77): error CS0234: The type or namespace name 'PathList' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]

@radekdoulik
[wasm] Make sure the TargetOS is capitalized
6f44a69 
Fixes: dotnet#54926

The $(TargetOS) is used in $(TargetFrameworks) and is case sensitive.
Make sure it is capitalized for Browser, so that build.cmd works when
used like:

    ./build.cmd -bl -os browser -subset libs

It fixes build errors like:

    C:\r\src\libraries\shims\manual\System.forwards.cs(8,88): error CS0234: The type or namespace name 'ZLibException' does not exist in the namespace 'System.IO.Compression' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
    C:\r\src\libraries\shims\manual\System.forwards.cs(9,77): error CS0234: The type or namespace name 'CookieVariant' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
    C:\r\src\libraries\shims\manual\System.forwards.cs(10,77): error CS0234: The type or namespace name 'PathList' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
@radekdoulik radekdoulik added arch-wasm WebAssembly architecture area-Build-mono labels Jun 29, 2021
@radekdoulik radekdoulik requested a review from lewing June 29, 2021 21:57
@ghost
Copy link

ghost commented Jun 29, 2021

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

Issue Details

Fixes: #54926

The $(TargetOS) is used in target framework, which is case sensitive.
Make sure it is capitalized for Browser, so that build.cmd works when
used like:

./build.cmd -bl -os browser -subset libs

It fixes build errors like:

C:\r\src\libraries\shims\manual\System.forwards.cs(8,88): error CS0234: The type or namespace name 'ZLibException' does not exist in the namespace 'System.IO.Compression' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
C:\r\src\libraries\shims\manual\System.forwards.cs(9,77): error CS0234: The type or namespace name 'CookieVariant' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
C:\r\src\libraries\shims\manual\System.forwards.cs(10,77): error CS0234: The type or namespace name 'PathList' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [C:\r\src\libraries\shims\manual\System.csproj]
Author: radekdoulik
Assignees: -
Labels:

arch-wasm, area-Build-mono
Milestone: -

lewing
lewing reviewed Jun 29, 2021
View reviewed changes
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

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

does Configuration have a similar problem?

@radical
Copy link
Member

radical commented Jun 29, 2021

What exactly was failing?

@radekdoulik
Copy link
Member Author

radekdoulik commented Jun 30, 2021 •
edited
Loading

does Configuration have a similar problem?

Configuration is fine

runtime/eng/build.ps1

Line 263 in 39d2ba3

$argumentsWithConfig = $arguments + " -configuration $((Get-Culture).TextInfo.ToTitleCase($config))";

@radekdoulik
Copy link
Member Author

radekdoulik commented Jun 30, 2021 •
edited
Loading

What exactly was failing?

System.csproj was failing, because some assemblies were not built. For example System.IO.Compression was not built with net6.0-Browser target framework.

radical
radical approved these changes Aug 11, 2021
View reviewed changes
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

I'm not sure where exactly the case difference trips up the build, but this avoids getting into the broken state on windows. So, 👍

@radekdoulik radekdoulik merged commit d95758f into dotnet:main Aug 14, 2021
@akoeplinger
Copy link
Member

akoeplinger commented Aug 16, 2021

Looks good to me but I'd have preferred something similar to what we do in build.sh where we do the remapping to the correct casing for all operating systems: https://github.com/radekdoulik/runtime/blob/b82cd2fb882f70e906ca6ee6858c1bdcdb407f15/eng/build.sh#L251-L284

@ghost ghost locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lewing lewing lewing left review comments

@radical radical radical approved these changes

Assignees

No one assigned

Labels

arch-wasm WebAssembly architecture area-Build-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[wasm] Windows build fails when with -os browser

4 participants

@radekdoulik @radical @akoeplinger @lewing