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

Emit interceptor info correctly when invocation expr is on separate line #91107

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

layomia
Copy link
Contributor

@layomia layomia commented Aug 25, 2023

Addresses #90851. RC2 candidate.

@layomia layomia added this to the 8.0.0 milestone Aug 25, 2023
@layomia layomia requested a review from tarekgh August 25, 2023 04:43
@layomia layomia self-assigned this Aug 25, 2023
@ghost
Copy link

ghost commented Aug 25, 2023

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

Issue Details

Addresses #90851. RC2 candidate.

Author: layomia
Assignees: layomia
Labels:

area-Extensions-Configuration

Milestone: 8.0.0

@tarekgh
Copy link
Member

tarekgh commented Aug 25, 2023

Looks somehow this change affected the binding to the static properties on Mono?

https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-91107-merge-2933836994b843458b/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests/1/console.7dc38e07.log?helixlogtype=result

    Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests.ConfigurationBinderTests.GetCanReadStaticProperty [FAIL]
      Assert.Equal() Failure
      Expected: stuff
      Actual:   (null)
      Stack Trace:
        /_/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs(751,0): at Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests.ConfigurationBinderTests.GetCanReadStaticProperty()
        /_/src/mono/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.Mono.cs(22,0): at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(57,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
    Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests.ConfigurationBinderCollectionTests.DoesNotBindUninstantiatedISetWithUnsupportedKeys [SKIP]
        [Fact]
        public void GetCanReadStaticProperty()
        {
            var dic = new Dictionary<string, string>
            {
                {"StaticProperty", "stuff"},
            };
            var configurationBuilder = new ConfigurationBuilder();
            configurationBuilder.AddInMemoryCollection(dic);
            var config = configurationBuilder.Build();
            var options = new ComplexOptions();
            config.Bind(options);

            Assert.Equal("stuff", ComplexOptions.StaticProperty);
        }

@layomia
Copy link
Contributor Author

layomia commented Aug 25, 2023

@tarekgh hmm don't think so. We maintain the same binding behavior. Taking a look; might file an issue.

@layomia
Copy link
Contributor Author

layomia commented Aug 25, 2023

@tarekgh mono issues don't surface after rebasing on top of main.

Test failure appears related to #87783.

@layomia layomia merged commit a8a8a13 into dotnet:main Aug 25, 2023
103 of 105 checks passed
@layomia layomia modified the milestones: 8.0.0, 9.0.0 Aug 25, 2023
@layomia
Copy link
Contributor Author

layomia commented Aug 25, 2023

Needs backport to RC2 to consider fixed for 8.0.

@layomia
Copy link
Contributor Author

layomia commented Aug 25, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5980937731

@github-actions
Copy link
Contributor

@layomia backporting to release/8.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Emit interceptor info correctly when invocation expr is on separate line
Using index info to reconstruct a base tree...
A	src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs
M	src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.Options.cs
M	src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs
M	src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs
M	src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj
Auto-merging src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs
Auto-merging src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs
Auto-merging src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.Options.cs
CONFLICT (modify/delete): src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs deleted in HEAD and modified in Emit interceptor info correctly when invocation expr is on separate line. Version Emit interceptor info correctly when invocation expr is on separate line of src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Emit interceptor info correctly when invocation expr is on separate line
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@layomia an error occurred while backporting to release/8.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@layomia
Copy link
Contributor Author

layomia commented Aug 28, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6002565435

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants