Skip to content

[csharp] Verify codegen #738

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

Merged
merged 9 commits into from
Nov 14, 2023

Conversation

jsturtevant
Copy link
Collaborator

@jsturtevant jsturtevant commented Nov 7, 2023

The code gen is running but isn't being verified. This builds the source in the same way as the other languages.

It fixes the stub code so it uses the stub functions not just creating a class so that the code generated can compile. It also disables a few tests that are failing, to be fixed later

It uses the CI work enabled in #718, so will rebase after that is in.

Part of #713

@jsturtevant
Copy link
Collaborator Author

There is not enough space on the disk.

Seems like the builds for dotnet take up lots of space 🤔

@jsturtevant jsturtevant requested review from silesmo and yowl November 7, 2023 16:24
@jsturtevant
Copy link
Collaborator Author

after doing a bit of clean up, getting real errors:

D:\a\wit-bindgen\wit-bindgen\target\codegen-tests\guest-csharp-strings\the_world\Wit.imports.foo.foo.StringsInterop.cs(80,23): error CS0111: Type 'StringsInterop.ReturnArea' already defines a member called 'GetUTF8String' with the same parameter types [D:\a\wit-bindgen\wit-bindgen\target\codegen-tests\guest-csharp-strings\the_world\the-world.csproj]

Signed-off-by: James Sturtevant <[email protected]>
Signed-off-by: James Sturtevant <[email protected]>
@yowl
Copy link
Collaborator

yowl commented Nov 11, 2023

There is not enough space on the disk.

Seems like the builds for dotnet take up lots of space 🤔

Yes, its unfortunate. Do we need to ask for more space?

Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant
Copy link
Collaborator Author

The only option for getting more space is moving to the Enterprise or custom runners :-/ Interestingly it seems that some times we get a drive with 100GB+ and that's why it passed once.

I think we have a few options:

  • Clean up the project after build - Trying this out first
  • use the C: drive for build outputs - not technically supported but seems that other projects have done this (Running out of disk space on Windows 2019 actions/runner-images#1341 (comment))
  • refactor the project builds so they happen on different jobs - I think this is probably the right choice longer time which will also speed up CI and can look at it later

@tschneidereit
Copy link
Member

I would very much recommend using C: for output. Enough projects are depending on that that I have a hard time seeing GH just break it. And even if they do, we can solve it then instead of preemptively trying to solve a problem we might never have :)

Signed-off-by: James Sturtevant <[email protected]>
Signed-off-by: James Sturtevant <[email protected]>
Signed-off-by: James Sturtevant <[email protected]>
Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant jsturtevant marked this pull request as ready for review November 13, 2023 21:57
@jsturtevant
Copy link
Collaborator Author

Was able to use dotnet clean to keep storage manageable and is consistently passing.

/assign @yowl @silesmo

Copy link
Collaborator

@yowl yowl left a comment

Choose a reason for hiding this comment

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

Thanks, this will help a lot.

dir.join("nuget.config"),
r#"<?xml version="1.0" encoding="utf-8"?>
<configuration>
<config>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this is copied from the runtime version. I should have removed this really as I don't think there is much benefit and it may improve the CI time without it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its fine though and we can address them both at the same time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there a bit of duplicate code and this will become more challenging with a different project settings required for mono work. Let's follow up with a clean up?

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