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 stj bultin #11056

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add stj bultin #11056

wants to merge 5 commits into from

Conversation

newmasterSG
Copy link

Fixes #9367

Context

System.Text.Json and System.Memory should be implicit build in roslyntaskfactory

Changes Made

-- Add runtime (Core or Framework) checker for test Doc
-- Add to default namespace STJ and Memory, when app runs by net core
-- Add ITestoutput for roslynTaskFactory tests

Testing

-- Problem with building test, because netstandart does not include STJ and Memory assemblies

Notes

Runtime version naming could change for .net core. Sometimes that's can be as Core, but for new version that's DotNet

Comment on lines 356 to 359
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework'">
<PackageReference Include="System.Text.Json" />
<PackageReference Include="System.Memory" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

I would not expect these references to be required. Are they?

Comment on lines +43 to +44
"System.Text.Json",
"System.Memory",
Copy link
Member

Choose a reason for hiding this comment

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

This adds the default namespaces, but I think you'll also need to add a reference to the "common assembly references" section below.

Choose a reason for hiding this comment

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

What about System.Text.Json.Nodes? I guess I could manually add the namespace myself, just wanted to ask because I would use it :)

Copy link
Author

Choose a reason for hiding this comment

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

I tried but that didn't worked, as I understood because need to their .dll files located in folder dotnet/ref which is standard folder for getting these assemblies

Copy link
Author

@newmasterSG newmasterSG Dec 5, 2024

Choose a reason for hiding this comment

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

I can create a new special folder where their dll can be located, however I wouldn't prefer this way

Copy link
Member

Choose a reason for hiding this comment

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

That shouldn't be necessary--you should be able to use the copies MSBuild itself will use at runtime as references (ideally we'd pass a ref assembly to the compiler as that's all it needs, but I wouldn't want to increase the size of MSBuild just to support these references).

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.

Add System.Text.Json and System.Memory as builtin implicit references
3 participants