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

[NativeAOT-LLVM] Build target and host packages for WASM on the same platform #2133

Merged
merged 4 commits into from
Dec 19, 2022

Conversation

SingleAccretion
Copy link

This matches how the build for other artifacts is set up.

We must take care to disable building the corresponding "host" platform, otherwise the publishing process will be racy (since both builds will end up producing "host" packages of the same name).

Hopefully, this is the last fix needed for package publishing to work.

This matches how the build for other artifacts is set up.

We must take care to disable building the corresponding "host"
platform, otherwise the publishing process will be racy (since
both builds will end up producing "host" packages of the same
name).
@SingleAccretion SingleAccretion changed the title Build target and host packages for WASM on the same platform [NativeAOT-LLVM] Build target and host packages for WASM on the same platform Dec 18, 2022
@SingleAccretion
Copy link
Author

SingleAccretion commented Dec 18, 2022

Packages produced in the test run looked reasonable.

This reverts commit db3c8e4.
Comment on lines 42 to 43
- ${{ else }}:
- script: $(Build.SourcesDirectory)/src/tests/build$(scriptExt) nativeaot $(buildConfigUpper) ${{ parameters.archType }} ${{ parameters.testFilter }}
Copy link
Author

Choose a reason for hiding this comment

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

Miscellaneous YAML cleanup.

@SingleAccretion SingleAccretion marked this pull request as ready for review December 18, 2022 14:12
@SingleAccretion
Copy link
Author

@dotnet/nativeaot-llvm

@yowl
Copy link
Contributor

yowl commented Dec 18, 2022

This looks good to me.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

The `''` in `'tree nativeaot'` are significant.
@SingleAccretion
Copy link
Author

SingleAccretion commented Dec 18, 2022

Part of the change unintentionally made the CI run all of the Pri0 runtime tests. I have reverted that.

@jkotas jkotas merged commit 4209d23 into dotnet:feature/NativeAOT-LLVM Dec 19, 2022
@SingleAccretion SingleAccretion deleted the Package-Publish branch December 19, 2022 14:44
@yowl
Copy link
Contributor

yowl commented Dec 21, 2022

Was the loss of the WIndows x64 Release build an intentional effect of this PR?

@SingleAccretion
Copy link
Author

Yes, it was. What used to be built "on" the windows_x64 platform is now built "on" browser_wasm_win. Otherwise we would be in a situation where the win-x64 package would be produced twice (and only one of them would have the right content).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants