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

Changes to enable the build on Linux #40

Merged
merged 12 commits into from
Sep 6, 2024
Merged

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Sep 1, 2024

This PR enables dotnet build on linux. dotnet test still fails and CI is not enabled for linux.

I removed the PackageSourceMapping as it seemed to be causing a build failure on linux, interesting to see if the Windows build is ok without it.

Edit:
Fixed up the tests and added Linux to the build matrix. Renamed the artifacts to include the os name to avoid conflicts.

@jsturtevant is there anything else I should do for Linux ?

@yowl yowl marked this pull request as ready for review September 1, 2024 01:30
@yowl yowl changed the title Changes to enable the buld on Linux Changes to enable the build on Linux Sep 1, 2024
.github/workflows/build.yml Outdated Show resolved Hide resolved
@@ -67,7 +67,8 @@
<PrebuiltWasmToolsTarget Include="aarch64-macos" Rid="osx-arm64" Ext=".tar.gz" />
<PrebuiltWasmToolsTarget Include="x86_64-linux" Rid="linux-x64" Ext=".tar.gz" />
<PrebuiltWasmToolsTarget Include="x86_64-macos" Rid="osx-x64" Ext=".tar.gz" />
<PrebuiltWasmToolsTarget Include="x86_64-windows" Rid="win-x64" Ext=".zip" ExeExt=".exe" />
<!-- tar on non-Windows often cannot handle zip archives -->
<PrebuiltWasmToolsTarget Include="x86_64-windows" Rid="win-x64" Ext=".zip" ExeExt=".exe" Condition="$([MSBuild]::IsOSPlatform('Windows'))" />
</ItemGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

This will have the result that, if built and published on Linux, there wouldn't be an .exe in the resulting nuget package.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could either document that it must publish on Windows and/or open an issue to make it more robust?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, added #41 and referenced in the code.

src/WitBindgen/WitBindgen.csproj Outdated Show resolved Hide resolved
test/E2ETest/testapps/nuget.config Outdated Show resolved Hide resolved
@yowl yowl closed this Sep 4, 2024
@yowl yowl reopened this Sep 4, 2024
@jsturtevant jsturtevant merged commit 81d9da1 into bytecodealliance:main Sep 6, 2024
4 checks passed
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.

2 participants