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.

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

DefineConstants are getting overriden twice in Wasi.Build.Tests.csproj #108935

Open
carlossanlop opened this issue Oct 16, 2024 · 2 comments
Open

DefineConstants are getting overriden twice in Wasi.Build.Tests.csproj #108935

carlossanlop opened this issue Oct 16, 2024 · 2 comments
Labels
arch-wasm WebAssembly architecture area-Infrastructure-mono needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners os-browser Browser variant of arch-wasm os-wasi Related to WASI variant of arch-wasm test-bug Problem in test source code (most likely) untriaged New issue has not been triaged by the area owner

Comments

@carlossanlop
Copy link
Member

First here:

runtime/src/mono/wasi/Wasi.Build.Tests/Wasi.Build.Tests.csproj

Line 10 in 5d276d9

<DefineConstants Condition="'$(ContinuousIntegrationBuild)' != 'true'">TEST_DEBUG_CONFIG_ALSO</DefineConstants>

And then here:

runtime/src/mono/wasi/Wasi.Build.Tests/Wasi.Build.Tests.csproj

Line 24 in 5d276d9

<DefineConstants>TARGET_WASI</DefineConstants>

Unless there was a specific reason to override the default constants in the first case, and another reason to override everything again on the second line, I think they should be (respectively):

- <DefineConstants Condition="'$(ContinuousIntegrationBuild)' != 'true'">TEST_DEBUG_CONFIG_ALSO</DefineConstants>
+ <DefineConstants Condition="'$(ContinuousIntegrationBuild)' != 'true'">$(DefineConstants);TEST_DEBUG_CONFIG_ALSO</DefineConstants>

and

- <DefineConstants>TARGET_WASI</DefineConstants>
+ <DefineConstants>$(DefineConstants);TARGET_WASI</DefineConstants>

This was found in the release/9.0 branch.
@carlossanlop carlossanlop added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm os-wasi Related to WASI variant of arch-wasm test-bug Problem in test source code (most likely) labels Oct 16, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 16, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

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

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Oct 16, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @directhex, @matouskozak
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture area-Infrastructure-mono needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners os-browser Browser variant of arch-wasm os-wasi Related to WASI variant of arch-wasm test-bug Problem in test source code (most likely) untriaged New issue has not been triaged by the area owner
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@carlossanlop @jeffschwMSFT