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

Add inline array marshaller sample to our custom marshalling sample. #6064

Merged
merged 5 commits into from
Aug 24, 2023

Conversation

jkoritzinsky
Copy link
Member

This won't build successfully until the InlineArray feature is out of preview, as the project file doesn't set LangVersion to Preview.

Fixes dotnet/runtime#86572

@jkoritzinsky jkoritzinsky requested a review from a team as a code owner August 14, 2023 18:21
@BillWagner
Copy link
Member

Hi @jkoritzinsky

You can add the <LangVersion>preview</LangVersion> element in the csproj. We've got tooling that removes it when a new release goes GA.

It's one of the tools that helps us keep up to date as we write docs for preview features, and then reset for the next version.

@jkoritzinsky
Copy link
Member Author

@BillWagner any ideas on how to get this building with a .NET 8 SDK since it needs to build with the VS host (due to the C++ project)?

@BillWagner
Copy link
Member

@jkoritzinsky

any ideas on how to get this building with a .NET 8 SDK since it needs to build with the VS host (due to the C++ project)?

Add a snippets5000.json file, like here: https://github.com/dotnet/samples/blob/main/framework/wcf/Basic/Ajax/ComplexTypeAjaxService/CS/snippets.5000.json

That specifies using Visual Studio as the host.

@jkoritzinsky
Copy link
Member Author

I did that, but now it's complaining that there's no .NET 8 SDK available.

@BillWagner
Copy link
Member

I did that, but now it's complaining that there's no .NET 8 SDK available.

@jkoritzinsky

It looks like you're the first to need .NET 8 in this repo. Update this line with the .NET installer channel: https://github.com/dotnet/samples/blob/main/.github/workflows/build-validation.yml#L18

Then you should be fine. It should match the channel in the dotnet/docs repo: https://github.com/dotnet/docs/blob/main/.github/workflows/snippets5000.yml#L18

@jkoritzinsky
Copy link
Member Author

I already did that and I'm still getting the same error. If I don't use the visual studio host, it correctly finds the .NET 8 SDK, but it doesn't seem to work with the VS host.

@BillWagner
Copy link
Member

adding @adegeo

I've exhausted my knowledge of how the tooling works. He may know.

@adegeo
Copy link
Contributor

adegeo commented Aug 17, 2023

I don't think this would work with Visual Studio. Doesn't Visual Studio use it's own copies of .NET? The latest versions of Visual Studio probably don't install .NET 8 preview as part of the base image. You'll need to remove the snippets config file and just use the normal dotnet host with the now modified build-validation file that installs .NET 8.

cl.exe should be in path still, since visual studio is in path, correct? Also, there is the support in the snippets system to run custom commands for compiling, if it just doesn't seem to work.

@jkoritzinsky
Copy link
Member Author

The issue is that the .NET SDK isn't hooked up to find the VC++ targets, so the imports in the C++ project fail.

@adegeo
Copy link
Contributor

adegeo commented Aug 18, 2023

@jkoritzinsky We may have to just merge without the testing framework as this may just be outside of the realm we support. Question, though, the readme indicates that you can do dotnet build, are they supposed to also do something else to get the C++ target?

@jkoritzinsky
Copy link
Member Author

If you're in a VS command prompt as the instructions say, the targets will be found.

@jkoritzinsky
Copy link
Member Author

@adegeo if you're okay with it, I'm okay merging this without the testing framework. We can also wait until .NET 8 releases, whichever you prefer. Once .NET 8 comes out and the VS version on the testing machines is updated, then the tests will start passing.

@BillWagner
Copy link
Member

Merging based on preview builds.

@BillWagner BillWagner merged commit 45be6ca into dotnet:main Aug 24, 2023
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.

Create custom InlineArray marshaller for LibraryImport generator
5 participants