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

Update to Preview7 SDK #106333

Merged
merged 9 commits into from
Aug 14, 2024
Merged

Update to Preview7 SDK #106333

merged 9 commits into from
Aug 14, 2024

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Aug 13, 2024

Changes are similar to the update to preview 6 : 4a8aca8

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 13, 2024
@ericstj
Copy link
Member Author

ericstj commented Aug 13, 2024

@steveharter @LakshanF - does this failure make sense when picking up Preview7 SDK?

  [Trimming Tests] Running test: /__w/1/s/src/libraries/System.ObjectModel/tests/TrimmingTests/TypeConverterAttributeStringArgCtorTest.cs ...
  MONO_WASM: Runtime instantiation of this attribute is not allowed.
     at System.ComponentModel.DefaultValueAttribute.get_Value()
     at TypeConverterAttributeTest.Program.Main(String[] args)
     at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
     at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
EXEC : error : Runtime instantiation of this attribute is not allowed. [/__w/1/s/src/libraries/System.ObjectModel/tests/TrimmingTests/System.ObjectModel.TrimmingTests.proj]
##[error]EXEC(0,0): error : (NETCORE_ENGINEERING_TELEMETRY=Build) Runtime instantiation of this attribute is not allowed.

I see you've been doing work here with #105766 so I don't want to jump to assumptions on the fix.

var attribute = new DefaultValueAttribute(typeof(string), "Hello, world!");

https://dev.azure.com/dnceng-public/public/_build/results?buildId=774284&view=logs&j=7b499086-9930-5591-220e-2ec7c40eb990&t=016d048d-94ae-5266-5657-3d7f9fdefeae&l=551

@steveharter steveharter added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 13, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

@steveharter
Copy link
Member

@steveharter @LakshanF - does this failure make sense when picking up Preview7 SDK?

Yes that is a known issue with that SDK test.

@ericstj
Copy link
Member Author

ericstj commented Aug 13, 2024

Seems like this is broken due to dotnet/sdk@8ca5983

I think we can just add _DefaultValueAttributeSupport to the test project. @LakshanF

@ericstj
Copy link
Member Author

ericstj commented Aug 13, 2024

Actually there are many more test failures due to the SDK disabling support for DefaultValueAttribute:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=774282&view=ms.vss-test-web.build-test-results-tab&runId=19803006&paneView=debug&resultId=109463

We might need more changes here. Also my first attempt at this, doesn't work because the property doesn't make it into the generated projects.

@agocke
Copy link
Member

agocke commented Aug 13, 2024

  • @sbomer who might have more context on this change

@sbomer
Copy link
Member

sbomer commented Aug 13, 2024

These tests look like they're explicitly testing DefaultValueAttribute support that's incompatible with trimming and probably should be disabled when trimming. Looks like @ericstj's latest change is working to set the feature switch as a workaround, and it just needs to be done for a few more tests.

@ericstj
Copy link
Member Author

ericstj commented Aug 13, 2024

I have some fixes for most of the tests. I'm letting this run to get more data before pushing. Please stay tuned.

Throw only when type constructor is used. Fix tests to condition on this.
These tests serialize types that use DefaultValueAttribute which is
disabled.
@ericstj
Copy link
Member Author

ericstj commented Aug 13, 2024

Crud, I missed one. Have the fix for that, but seeing if I'll get the other platforms complete before submitting.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants