-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Detect multiple conflicting assemblies being passed to crossgen #117786
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
Conversation
…enerate a warning on stderr (Issue dotnet#114513)
@@ -253,6 +253,22 @@ public int Run() | |||
} | |||
} | |||
|
|||
Dictionary<string, AssemblyNameInfo> allAssemblyNames = new (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is too late to detect duplicates.
crossgen, ilc and ilverify are all on first reference wins plan for references and ignore duplicates.
The duplicates are eliminated very early during command line parsing at
runtime/src/coreclr/tools/Common/CommandLineHelpers.cs
Lines 347 to 358 in 0621e64
if (dictionary.TryGetValue(simpleName, out string otherFullFileName)) | |
{ | |
if (strict) | |
{ | |
throw new CommandLineException("Multiple input files matching same simple name " + | |
fullFileName + " " + otherFullFileName); | |
} | |
} | |
else | |
{ | |
dictionary.Add(simpleName, fullFileName); | |
} |
strict
flag is true for input assemblies and it is false for references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the policy for duplicates is likely going to break some of the folks who are invoking crossgen manually. Manual invoked crossgen command line often references using wildcards, like -r somepath\*.dll -r someotherpath\*.dll
and the different paths tend to have duplicates that are harmless. These folks will see wall of warnings with this change.
We do not officially support invoking crossgen manually. You won't run into this with PublishReadyToRun
that's our officially supported way to invoke crossgen. I am not sure whether this is worth fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for investigating this. I'll figure out whether it's worth doing the work to actually detect this scenario robustly and generate warnings for it.
{ | ||
// Now check whether the full name is an exact match. If it's not, this indicates that the user accidentally provided two versions of the same | ||
// assembly during compilation, and the results will probably not be what they want, so we should generate a warning message. | ||
if (firstModule.FullName != moduleName.FullName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Identical full names do not imply that assembly content is identical. It is very common for two assemblies to have identical full name but have different content. For example., assemblies from given nuget package that target different TFMs have identical full name.
If we want to change how we treat duplicate references, we should disallow or warn on any duplicated simple names, similar how the strict
flag works for input assemblies.
Should fix #114513