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

6.0 dotnet/runtime: Microsoft.DotNet.Compatibility.ValidatePackage fails, "System.MissingMethodException: Method not found: '...'" #2501

Open
dagood opened this issue Oct 12, 2021 · 29 comments
Labels
area-build Improvements in source-build's own build process

Comments

@dagood
Copy link
Member

dagood commented Oct 12, 2021

I hit this in dotnet/runtime in the online tarball build on 2021-10-11. The conditions aren't clear, but so far it seems like flakiness:

/home/dagood/sb/6.0-tb-msbuild/src/runtime.b1aedd5a54d0c54ec12afe98a41979fb053998a5/artifacts/source-build/self/package-cache/microsoft.dotnet.compatibility/1.0.0-rc.2.21419.17/build/Microsoft.NET.Compatibility.Common.targets(26,5):
  error MSB4018: The "Microsoft.DotNet.Compatibility.ValidatePackage" task failed unexpectedly.
System.MissingMethodException: Method not found: 'Int32 Microsoft.CodeAnalysis.ISymbol.get_MetadataToken()'.
   at Microsoft.CodeAnalysis.CSharp.Symbols.AssemblySymbol.CreateISymbol()
   at Microsoft.CodeAnalysis.CSharp.Symbol.get_ISymbol()
   at Microsoft.CodeAnalysis.CSharp.Symbols.SymbolExtensions.GetPublicSymbol[TISymbol](Symbol symbol)
   at Microsoft.CodeAnalysis.CSharp.Symbols.SymbolExtensions.GetPublicSymbol(Symbol symbol)
   at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.CommonGetAssemblyOrModuleSymbol(MetadataReference reference)
   at Microsoft.CodeAnalysis.Compilation.GetAssemblyOrModuleSymbol(MetadataReference reference)
   at Microsoft.DotNet.ApiCompatibility.AssemblySymbolLoader.LoadAssembly(String name, Stream stream) in /_/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs:line 166
   at Microsoft.DotNet.PackageValidation.ApiCompatRunner.RunApiCompat() in /_/src/Compatibility/Microsoft.DotNet.PackageValidation/ApiCompatRunner.cs:line 47
   at Microsoft.DotNet.PackageValidation.CompatibleFrameworkInPackageValidator.Validate(Package package) in /_/src/Compatibility/Microsoft.DotNet.PackageValidation/CompatibleFrameworkInPackageValidator.cs:line 62
   at Microsoft.DotNet.Compatibility.ValidatePackage.ExecuteCore() in /_/src/Compatibility/Microsoft.DotNet.Compatibility/ValidatePackage.cs:line 65
   at Microsoft.NET.Build.Tasks.TaskBase.Execute() in /_/src/Tasks/Common/TaskBase.cs:line 38
   at Microsoft.DotNet.Compatibility.ValidatePackage.Execute() in /_/src/Compatibility/Microsoft.DotNet.Compatibility/ValidatePackage.cs:line 45
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask) [/home/dagood/sb/6.0-tb-msbuild/src/runtime.b1aedd5a54d0c54ec12afe98a41979fb053998a5/artifacts/source-build/self/src/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj]

After I hit the error, I reran ./build.sh --online in the tarball, and it passed on the second attempt.

Here's the task's subtree in the binlog, with a diff vs. the same tree in a "good" build: https://gist.github.com/dagood/b2bfb711265cd2b7138d740f2060877c

The only difference except the error itself is that the "bad" version has this line:
NoWarn = ;1701;1702;1705;1591,CS8969;NU5104;NU5104;NU5105
And the "good" version has this line... just repeating NU5104 two more times:
NoWarn = ;1701;1702;1705;1591,CS8969;NU5104;NU5104;NU5104;NU5104;NU5105

This suggests to me there must be some subtle MSBuild evaluation difference. There are some differences in the binlogs--search for $project Microsoft.Extensions.Caching.Abstractions.csproj Build netstandard2.0 net461, pick the second one (the src/ project, not ref/), and go to the evaluation node (click the id:1264 / id:1255 link on the project node), copy the subtree, and diff:
https://gist.github.com/dagood/e4abb7b2bd55ce2b0519e3978c9909e3
(Included lots of diff context lines.)

I'm not sure what the significance is, though. (GitHub isn't letting me upload files right now, so I'll try to attach the binlogs later.)

My build was based on dotnet/installer@18d694a877.


@eerhardt also hit this (on 2021-10-12):

I didn't change what SHA I was on for any repos. I just made some .csproj changes in the runtime, ./build.sh --online, got the above error, ./build.sh --online again and was successful
for installer, I'm on src/installer.e0343e851a6dd66bbae5abc2509a446c02847e5d


I searched for the task name in dotnet/source-build and I see that @omajid hit this before, here: #2477 (comment) (2021-09-26). I don't see any diagnosis in the thread though--I guess it wasn't related to the main topic in the end:

I tried replacing runtime.linux-x64.microsoft.netcore.ilasm and runtime.linux-x64.microsoft.netcore.ildasm with versions downloaded from https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6/nuget/v3/index.json but it leads to errors in the build:

dotnet-28be3e9a006d90d8c6e87d4353b77882829df718-x64-bootstrap/src/runtime.826f81a11ad17f415668fe1cb934bdaf00d36ea2/artifacts/source-build/self/package-cache/microsoft.dotnet.compatibility/1.0.0-rc.2.21419.17/build/Microsoft.NET.Compatibility.Common.targets(26,5): error MSB4018: The "Microsoft.DotNet.Compatibility.ValidatePackage" task failed unexpectedly. [dotnet-28be3e9a006d90d8c6e87d4353b77882829df718-x64-bootstrap/src/runtime.826f81a11ad17f415668fe1cb934bdaf00d36ea2/artifacts/source-build/self/src/src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj]                                                                             
dotnet-28be3e9a006d90d8c6e87d4353b77882829df718-x64-bootstrap/src/runtime.826f81a11ad17f415668fe1cb934bdaf00d36ea2/artifacts/source-build/self/package-cache/microsoft.dotnet.compatibility/1.0.0-rc.2.21419.17/build/Microsoft.NET.Compatibility.Common.targets(26,5): error MSB4018: System.MissingMethodException: Method not found: 'Int32 Microsoft.CodeAnalysis.ISymbol.get_MetadataToken()'. [dotnet-28be3e9a006d90d8c6e87d4353b77882829df718-x64-bootstrap/src/runtime.826f81a11ad17f415668fe1cb934bdaf00d36ea2/artifacts/source-build/self/src/src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj]  

Is this a known issue? Any idea how to work around this?


Based on the random hits and retry seeming to always work when attempted, it seems like flakiness. The source of flakiness isn't clear though... it doesn't make sense for this build task to be nondeterministic, and the inputs appear effectively identical in the binlog.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dseefeld dseefeld added area-build Improvements in source-build's own build process and removed untriaged labels Oct 14, 2021
@dagood
Copy link
Member Author

dagood commented Oct 22, 2021

I hit this (or very similar) again, also went away with retry:

The "Microsoft.DotNet.Compatibility.ValidatePackage" task failed unexpectedly.
System.TypeLoadException: Method 'CompareSourceLocations' in type 'Microsoft.CodeAnalysis.CSharp.CSharpCompilation' from assembly 'Microsoft.CodeAnalysis.CSharp, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' does not have an implementation.
   at Microsoft.DotNet.ApiCompatibility.AssemblySymbolLoader..ctor(Boolean resolveAssemblyReferences)
   at Microsoft.DotNet.PackageValidation.ApiCompatRunner.GetAssemblySymbolFromStream(Stream assemblyStream, MetadataInformation assemblyInformation, Boolean& resolvedReferences) in /tarball/src/sdk.4d19f430e05e0c0771b6abf86be9bf137f2cc242/artifacts/source-build/self/src/src/Compatibility/Microsoft.DotNet.PackageValidation/ApiCompatRunner.cs:line 113
   at Microsoft.DotNet.PackageValidation.ApiCompatRunner.RunApiCompat() in /tarball/src/sdk.4d19f430e05e0c0771b6abf86be9bf137f2cc242/artifacts/source-build/self/src/src/Compatibility/Microsoft.DotNet.PackageValidation/ApiCompatRunner.cs:line 50
   at Microsoft.DotNet.PackageValidation.CompatibleFrameworkInPackageValidator.Validate(Package package) in /tarball/src/sdk.4d19f430e05e0c0771b6abf86be9bf137f2cc242/artifacts/source-build/self/src/src/Compatibility/Microsoft.DotNet.PackageValidation/CompatibleFrameworkInPackageValidator.cs:line 62
   at Microsoft.DotNet.Compatibility.ValidatePackage.ExecuteCore() in /tarball/src/sdk.4d19f430e05e0c0771b6abf86be9bf137f2cc242/artifacts/source-build/self/src/src/Compatibility/Microsoft.DotNet.Compatibility/ValidatePackage.cs:line 92
   at Microsoft.NET.Build.Tasks.TaskBase.Execute() in /tarball/src/sdk.4d19f430e05e0c0771b6abf86be9bf137f2cc242/artifacts/source-build/self/src/src/Tasks/Common/TaskBase.cs:line 38
   at Microsoft.DotNet.Compatibility.ValidatePackage.Execute() in /tarball/src/sdk.4d19f430e05e0c0771b6abf86be9bf137f2cc242/artifacts/source-build/self/src/src/Compatibility/Microsoft.DotNet.Compatibility/ValidatePackage.cs:line 49
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)

@omajid
Copy link
Member

omajid commented Oct 22, 2021

FWIW, I am running into this on pretty much every build on my end. @tmds suggested adding a /p:EnablePackageValidation=false in src/runtime.*/eng/SourceBuild.props and that's been my workaround since.

@dagood
Copy link
Member Author

dagood commented Oct 22, 2021

Hmm. A few more bits of info, then: both builds were on a "real" Fedora 33 machine with an i7-6700 and SSD. (I've gotten in trouble before for having a faster-than-CI build machine, so just writing that down in case it's somehow that kind of race.)

@tmds
Copy link
Member

tmds commented Oct 26, 2021

I think it makes sense to skip package validation on source-build.

Afaik package validation happens against a previous version of the package, so disabling it may even get rid of some prebuilts.

@dagood @dseefeld wdyt?

@dagood
Copy link
Member Author

dagood commented Oct 26, 2021

Sounds good to me. I'll start this off with an issue to discuss the failure itself (which probably indicates some problem!) and a PR to "officially" disable it during source-build.

@dagood
Copy link
Member Author

dagood commented Oct 26, 2021

Ah, found dotnet/runtime#59908. This is apparently a known issue that has been fixed in main:

@safern @ViktorHofer is there a plan to get this into release/6.0 (or has it been)? We've been hitting this frequently in the 6.0 dev process. As an alternative, it seems like we can remove ValidatePackage entirely from source-build. (Depending on the Roslyn versions involved, maybe it's impossible to properly fix in 6.0?)

@ViktorHofer
Copy link
Member

I will let @safern speak to that as getting this into 6.0 would require both the ework in dotnet/sdk and the changes that you listed above to be ported.

@safern
Copy link
Member

safern commented Oct 26, 2021

@dagood I think we can port the fix on the SDK to release/6.0. This is the fix: dotnet/sdk#22277

@marcpopMSFT @mmitche you think this is something we can do for 6.0 or should we have to wait for the first servicing release?

@dagood maybe you can apply that patch on source-build in the meantime?

@dagood
Copy link
Member Author

dagood commented Oct 26, 2021

maybe you can apply that patch on source-build in the meantime?

From a source-build perspective, it seems safer to me to disable the validation in dotnet/runtime rather than port a fix from main that changes a DLL that ships in the SDK.

(Given the very limited amount of source-build tests, I think it's safest to wait for the port to be in the official dotnet/sdk servicing branch, so we are making sure the Microsoft-built SDK doesn't hit snags with the port during its more extensive set of tests.)

Or does the validation task do something important in the dotnet/runtime build beyond being a guardrail for devs?

@safern
Copy link
Member

safern commented Oct 26, 2021

Or does the validation task do something important in the dotnet/runtime build beyond being a guardrail for devs?

It is a guardrail for devs, so I think disabling on source-build shouldn't affect anything, specially since it will be disabled on the 6.0 source-build so I don't expect the package validation task to catch any issues in servicing due to packages not changing nor new apis added, etc. Plus we are covered with the dotnet/runtime build.

You can just set this property to false on source-build: https://github.com/dotnet/runtime/blob/release/6.0/eng/packaging.targets#L4

@dagood
Copy link
Member Author

dagood commented Oct 26, 2021

@dagood
Copy link
Member Author

dagood commented Oct 27, 2021

We're planning on leaving out this fix for the 6.0.100 source-built SDK tarball, for 6.0.100 release stability.

It sounds like @omajid has already set up the workaround in some places, and I prepared a patch for dotnet/runtime to fix it: dotnet/installer#12526. These fixes can be pulled into more build processes if this problem gets hit. It's just a little too late to include those in dotnet/installer for 6.0.100.

This can be fixed in 6.0.101/next servicing release. dotnet/installer#12526 can be merged to add a patch, or ideally dotnet/runtime will fix it directly with one or both of these:

@safern
Copy link
Member

safern commented Oct 27, 2021

@dagood I think we would also need to port the SDK fix to the SDK 6.0 release branch if we don't want to disable the validation in dotnet/runtime.

@dagood
Copy link
Member Author

dagood commented Oct 27, 2021

I think we would also need to port the SDK fix to the SDK 6.0 release branch if we don't want to disable the validation in dotnet/runtime.

Yeah, I meant for https://github.com/dotnet/runtime/issues/60883 to encompass that. I didn't realize that essentially all that happened in dotnet/runtime was a version uptake, so I suppose it would have made more sense to file in dotnet/sdk. I've transferred it. (New link: dotnet/sdk#22353.)

@tmds
Copy link
Member

tmds commented Oct 27, 2021

@dagood can you, or someone else, comment on the need to do package validation as part of source-build?

@MichaelSimons
Copy link
Member

@dagood can you, or someone else, comment on the need to do package validation as part of source-build?

I believe this was covered earlier in #2501 (comment).

It is a guardrail for devs, so I think disabling on source-build shouldn't affect anything...we are covered with the dotnet/runtime build.

@tmds
Copy link
Member

tmds commented Oct 27, 2021

Thanks. I had missed that comment. It matches with my earlier input.

I got confused because this PR describes it as a workaround.

I think we can/should keep it disabled for source-build.

@dagood
Copy link
Member Author

dagood commented Oct 27, 2021

Yeah, I'd still call it a workaround because validation should work when enabled, and upstream is tracking a fix as a bug for servicing. But on the other hand, the value of enabling it is theoretical (that is, AFAIK we haven't seen it catching anything or been in a situation where we'd expect it to catch something).

@eerhardt
Copy link
Member

I think we can/should keep it disabled for source-build.

What are the expected usages for source-build? If I wanted to develop/prototype a new feature across the entire stack (runtime, roslyn, sdk, etc), is that a possible scenario for source-build?

I ask this because we are choosing to turn the dev checks we have in the build off. (Another example is roslyn analyzers.) But if devs want to develop in this environment, they really want these checks in place to catch mistakes.

So I guess as long as the only usage of source-build is to "package a version of the product that has already been verified elsewhere", then turning these checks off seems OK.

@safern
Copy link
Member

safern commented Oct 27, 2021

Good point, @eerhardt. As of package validation, if someone is bringing up a new platform using source-build and wants to add an implementation for an OOB package for that new platform, that is where package validation should whelp the dev author the package correctly and make sure that platform specific implementation meets the contract. I don't think is critical but I'd like to service the fix we did in the SDK for 6.0 anyway, so why not take advantage of it and use it in source-build?

@tmds
Copy link
Member

tmds commented Oct 28, 2021

To me it's similar to source-build not running any functional tests by default. It's a trade-off between what you gain vs what it costs you. The cost may be extra build time, potential to fail the build due to false negative, and the additional dependencies that are needed.

@eerhardt @safern I leave the trading off to you.

I'm interested in learning this: does the validation introduce extra dependent packages that are used to validate against? Or what is used as the baseline to check against?

@ViktorHofer
Copy link
Member

I'm interested in learning this: does the validation introduce extra dependent packages that are used to validate against? Or what is used as the baseline to check against?

The package baseline validation is disabled in source build (via the DisablePackaheBaselineValidation switch) as it would require many more pre-buildts (a previous stable version for every 6.0 package).

That said, PackageValidation offers other validation and protection which we would like to keep enabled even on sourcebuild. I share the sentiment of my colleagues that it would be good to service this during the next window.

To me it's similar to source-build not running any functional tests by default. It's a trade-off between what you gain vs what it costs you. The cost may be extra build time, potential to fail the build due to false negative, and the additional dependencies that are needed.

If we could enable a set of functional tests that don't require pre-builts / external dependencies, then we would probably do so. Making the existing tests work on source build is just not a high priority today but I would expect source build for .NET to become steadily more important over the next years.

@tmds
Copy link
Member

tmds commented Oct 28, 2021

The package baseline validation is disabled in source build

This addresses something I was concerned about.

That said, PackageValidation offers other validation and protection

For my education, what does it validate? and what does it use as a baseline to validate against?

@ViktorHofer
Copy link
Member

For my education, what does it validate

At the moment these: https://github.com/dotnet/sdk/blob/683fb42d2b49c0e1a61b8663d640f4c6dbe94bcb/src/Compatibility/Microsoft.DotNet.Compatibility/ValidatePackage.cs#L102-L103. It checks if tfms and runtime assets in a package are compatible with each other, i.e. a netstandard2.0 asset and a net6.0 asset. This is something that we often got wrong in the past so it's quite useful to have protection for.

and what does it use as a baseline to validate against?

The protection mentioned above doesn't need a baseline. The package baseline validation which is disabled in sourcebuild uses the previous stable package and checks for breaking changes (and other things) in the live built package.

@safern and @Anipik as the PackageValidation's authors can speak in more detail to that.

@safern
Copy link
Member

safern commented Oct 28, 2021

@tmds we just published the v1 of the package validation docs. It would be great as you are interested on the type of validation it does if you could look at it and gives feedback if it was clear enough to solve your questions and we could tweak the docs.

https://docs.microsoft.com/dotnet/compatibility/package-validation

@dagood dagood removed their assignment Jan 3, 2022
@MichaelSimons MichaelSimons moved this to 6.0 BackLog in .NET Source Build Feb 1, 2022
@MichaelSimons MichaelSimons moved this from 6.0 BackLog to 7.0 Backlog in .NET Source Build Apr 28, 2022
@MichaelSimons MichaelSimons moved this from 7.0 Backlog to 8.0 Backlog in .NET Source Build Sep 22, 2022
@MichaelSimons
Copy link
Member

With the Unified Build plan and the introduction of the VMR, it is expected that devs would implement cross-cutting features within the VMR.

@MichaelSimons MichaelSimons moved this from Needs Review to Backlog in .NET Source Build Jul 6, 2023
@ViktorHofer
Copy link
Member

@MichaelSimons what work does this issue track?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-build Improvements in source-build's own build process
Projects
Status: Backlog
Development

No branches or pull requests

8 participants