Refactor ref src generators & pass ref assemblies by ViktorHofer · Pull Request #547 · dotnet/source-build-reference-packages

ViktorHofer · 2023-02-27T21:37:07Z

Fixes dotnet/source-build#3252
Fixes dotnet/source-build#3237

Refactor textonly & reference source generators & apply general clean-up
Pass the right reference assemblies to GenAPI invocation
Make GenAPI generate assembly attributes and typeforwards
Use PackageDownload instead of extra restore project
Use the default package cache instead of defining a separate one
Make the ref generator work with multi-assembly packages
Make the generators work cross-plat
Add "--exclude-package-dependencies" switch to ignore dependencies

andriipatsula · 2023-02-28T12:00:42Z

Hello @ViktorHofer , I tried to run the generator from a PR branch and got an error: "The "MSBuildInParallel" parameter is not supported by the "MSBuild" task loaded from assembly". And I took the latest commit.

Determining projects to restore...
Restored /root/.nuget/packages/microsoft.dotnet.arcade.sdk/8.0.0-beta.23120.1/tools/Tools.proj (in 18.59 sec).
Determining projects to restore...
Restored /repos/source-build-reference-packages/src/referencePackageSourceGenerator/ReferencePackageSourceGenerator.csproj (in 25.82 sec).
ReferencePackageSourceGenerator -> /repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/Debug/net8.0/ReferencePackageSourceGenerator.dll
/repos/source-build-reference-packages/src/referencePackageSourceGenerator/ReferencePackageSourceGenerator.csproj(37,14): error MSB4064: The "MSBuildInParallel" parameter is not supported by the "MSBuild" task loaded from assembly: Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a from the path: /repos/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Microsoft.Build.dll. Verify that the parameter exists on the task, the points to the correct assembly, and it is a settable public instance property.
/repos/source-build-reference-packages/src/referencePackageSourceGenerator/ReferencePackageSourceGenerator.csproj(31,5): error MSB4063: The "MSBuild" task could not be initialized with its input parameters

MichaelSimons · 2023-02-28T15:30:06Z

> ./generate.sh --pkg System.Threading.Tasks,4.3.0
  Determining projects to restore...
  All projects are up-to-date for restore.
/repos/source-build-reference-packages/src/referencePackageSourceGenerator/ReferencePackageSourceGenerator.csproj(9,5): error MSB4184: The expression "[MSBuild]::NormalizeDirectory('', '')" cannot be evaluated. Parameter "path" cannot have zero length.

andriipatsula · 2023-02-28T15:58:00Z

> ./generate.sh --pkg System.Threading.Tasks,4.3.0
  Determining projects to restore...
  All projects are up-to-date for restore.
/repos/source-build-reference-packages/src/referencePackageSourceGenerator/ReferencePackageSourceGenerator.csproj(9,5): error MSB4184: The expression "[MSBuild]::NormalizeDirectory('', '')" cannot be evaluated. Parameter "path" cannot have zero length.

Hm, I didn't notice that as I am using --dest parameter always. If you want to continue tests - please specify --dest parameter

ViktorHofer · 2023-02-28T16:10:38Z

Taking a look

andriipatsula · 2023-02-28T16:10:55Z

One more thing @MichaelSimons ,
I compare results of the SBRP generator from the main branch and from the PR. Package microsoft.codeanalysis.common,4.4.0

The version from main generates packages and direct dependencies and the version from the PR - direct and transitive dependencies.

What 'd be a desired behavior? The same as original version of SBRP?

From the main:

microsoft.codeanalysis.common
system.buffers
system.collections.immutable
system.memory
system.numerics.vectors
system.reflection.metadata
system.runtime.compilerservices.unsafe
system.text.encoding.codepages
system.threading.tasks.extensions

From the PR branch:

microsoft.codeanalysis.common
system.buffers
system.collections.immutable
system.memory
system.numerics.vectors
system.reflection.metadata
system.runtime.compilerservices.unsafe
system.text.encoding.codepages
system.threading.tasks.extensions
microsoft.codeanalysis.analyzers
microsoft.netcore.platforms
microsoft.netcore.targets
microsoft.win32.primitives
netstandard.library
system.appcontext
system.collections
system.collections.concurrent
system.console
system.diagnostics.debug
system.diagnostics.diagnosticsource
system.diagnostics.tools
system.diagnostics.tracing
system.globalization
system.globalization.calendars
system.globalization.extensions
system.io
system.io.compression
system.io.compression.zipfile
system.io.filesystem
system.io.filesystem.primitives
system.linq
system.linq.expressions
system.net.http
system.net.primitives
system.net.sockets
system.objectmodel
system.reflection
system.reflection.emit
system.reflection.emit.ilgeneration
system.reflection.emit.lightweight
system.reflection.extensions
system.reflection.primitives
system.reflection.typeextensions
system.resources.resourcemanager
system.runtime
system.runtime.extensions
system.runtime.handles
system.runtime.interopservices
system.runtime.interopservices.runtimeinformation
system.runtime.numerics
system.security.cryptography.algorithms
system.security.cryptography.cng
system.security.cryptography.csp
system.security.cryptography.encoding
system.security.cryptography.openssl
system.security.cryptography.primitives
system.security.cryptography.x509certificates
system.text.encoding
system.text.encoding.extensions
system.text.regularexpressions
system.threading
system.threading.tasks
system.threading.timer
system.xml.readerwriter
system.xml.xdocument

andriipatsula · 2023-02-28T16:15:47Z

Btw. Viktor introduced a new parameter --exclude-package-dependencies [true | false] - generates a SBRP package for a specified nuget package only. As for me, it's quite useful, especially for testing purposes.

MichaelSimons · 2023-02-28T16:19:53Z

What 'd be a desired behavior? The same as original version of SBRP?

Transitive. I am not sure why it didn't behave this way in the first place. It provides the best UX and doesn't require the user to figure out the graph and run generate multiple times.

ViktorHofer · 2023-03-01T07:47:38Z

@MichaelSimons the error above is now fixed. The generate.sh script always appended msbuild properties even if they weren't set which led to default properties and in this case paths not being honored. I changed the script to only append switches when they are provided (which is the desired behavior anyway).

ViktorHofer · 2023-03-01T10:03:30Z

With the current branch, the following dependency closure gets transformed to reference package source when invoking generate.sh --package microsoft.codeanalysis.common,4.4.0

Microsoft.CodeAnalysis.Common (v4.4.0)
Microsoft.CodeAnalysis.Analyzers (v3.3.3)
System.Collections.Immutable (v6.0.0)
System.Runtime.CompilerServices.Unsafe (v6.0.0)
System.Memory (v4.5.4)
System.Buffers (v4.5.1)
System.Diagnostics.Debug (v4.3.0)
Microsoft.NETCore.Platforms (v1.1.0)
Microsoft.NETCore.Targets (v1.1.0)
System.Runtime (v4.3.0)
System.Diagnostics.Tracing (v4.3.0)
System.Resources.ResourceManager (v4.3.0)
System.Globalization (v4.3.0)
System.Reflection (v4.3.0)
System.IO (v4.3.0)
System.Text.Encoding (v4.3.0)
System.Threading.Tasks (v4.3.0)
System.Reflection.Primitives (v4.3.0)
System.Threading (v4.3.0)
System.Runtime.CompilerServices.Unsafe (v4.5.3)
System.Numerics.Vectors (v4.5.0)
System.Runtime.Extensions (v4.3.0)
System.Runtime.InteropServices (v4.3.0)
System.Runtime.Handles (v4.3.0)
System.Numerics.Vectors (v4.4.0)
System.Memory (v4.5.5)
System.Reflection.Metadata (v5.0.0)
System.Collections.Immutable (v5.0.0)
System.Text.Encoding.CodePages (v6.0.0)
System.Threading.Tasks.Extensions (v4.5.4)
System.Collections (v4.3.0)

@andriipatsula's previous list was longer because we didn't filter out targeting packs before. That's now fixed. The remaining issue is that text only packages are considered during the reference package source traversal:

Microsoft.CodeAnalysis.Analyzers (v3.3.3)
Microsoft.NETCore.Platforms (v1.1.0)
Microsoft.NETCore.Targets (v1.1.0)

We could either ignore them (existing behavior) or pass them to the text only package generator. Alternatively, we could also merge both generators so that it can handle both types of packages. Text only packages could be identified by looking at the number of compile items.

andriipatsula · 2023-03-01T16:23:57Z

The SBRP generator LGTM. I run smoke tests and the tool behaves as expected. Thank you @ViktorHofer .

@MichaelSimons, could you please check the question related to text-only packages above:

We could either ignore them (existing behavior) or pass them to the text only package generator. Alternatively, we could also merge both generators so that it can handle both types of packages. Text only packages could be identified by looking at the number of compile items.
In general I wonder if we even need the strong distinction between text only and reference source packages in the generation. What was the reason that made us not include allowed content in reference source packages?

MichaelSimons · 2023-03-01T18:03:19Z

My main requirement is that text-only packages remain in a separate location of the repo. It is important that their is a distinction made between them and ref packages because the rules for consuming them are very different. There is a bar for adding new ones (e.g. it would be preferred that we truly source-build them). Because of that I do think it is preferable that you must take an explicit action to generate them.

I see value in having a single set of tools that would support either. My only request would be to do any additional refactoring separately as we would like to realize the immediate benefit of this PR.

What was the reason that made us not include allowed content in reference source packages? - I don't have the history here. I can speculate one reason was to avoid bloat from non-required content for the required usage scenarios. We are trying to reduce the size of this repo so bringing in a bunch of things like icons would work against this for no real value.

Regarding text-only packages referenced by reference packages, I think we should continue to ignore them as is the current behavior. We can reconsider this if it becomes a UX concern.

- Refactor textonly & reference source generators - Pass the right reference assemblies to GenAPI invocation - Make GenAPI generate assembly attributes and typeforwards - Use PackageDownload instead of extra restore project - Use the default package cache instead of defining a separate one - Make the ref generator work with multi-assembly packages - Make the generators work cross-plat - Add "--exclude-package-dependencies" switch to ignore dependencies

ViktorHofer · 2023-03-02T21:57:46Z

The PR should now be ready to be merged. I tested it with Microsoft.CodeAnalysis.Common + its dependencies and with System.Drawing.Common + dependency.

Text only packages are now skipped (with a message) but the code is already written so that we can easily trigger the text only package generator in the future (when such a package appears as a dependency), if we would want that.
I also noticed that the project file had the wrong casing and the targeting pack dependencies were missing. That's now fixed.
The generated Directory.Build.props file receives the PackageId as an input which is incorrect as it should get the assembly name instead. In most cases those are in sync but not in all. That said, this bug already exists in the current tooling in main.

ViktorHofer · 2023-03-02T23:11:54Z

@MichaelSimons would you mind taking another look and if everything's good, merge the PR in?

eng/Versions.props

generate.sh

src/referencePackageSourceGenerator/ReferencePackageSourceTask/GetPackageItems.cs

MichaelSimons · 2023-03-03T15:55:48Z

src/referencePackageSourceGenerator/ReferencePackageSourceTask/GenerateProject.cs

+        [Required]
+        public string ProjectTemplate { get; set; }
+
+        public override bool Execute()


IMO the readability of this method could be improved by breaking it apart a bit.

I left it as is as this was already a single method in main:

source-build-reference-packages/src/referencePackageSourceGenerator/Tasks/GenerateProjects.cs

Lines 32 to 225 in a3ee227

public override bool Execute()

{

DateTime startTime = DateTime.Now;

string assemblyVersionTemplate = File.ReadAllText("AssemblyVersionTemplate.txt");

string pkgProjectTemplate = File.ReadAllText("PackageProjectTemplate.xml");

string[] standardUsings = File.ReadAllLines("AssemblyStandardUsings.txt");

Log.LogMessage(MessageImportance.Low, "Start generating csproj files.");

// First, create project data for all passed in dlls

foreach (ITaskItem dll in PackageDlls)

{

var pd = ProjectData.GetOrCreateProjectData(TargetPackagesPath, dll.GetMetadata("FullPath"), SrcPath);

Log.LogMessage(MessageImportance.Low, $"Generating data for {pd.PackageName}/{pd.ShortTfm}");

}

// Second, setup dependencies between all generated project data

ProjectData.GenerateAllDependencies();

foreach (var pkgData in PackageData.GetAll())

{

string pkgProjectOutput = pkgProjectTemplate;

string packageReferenceIncludes = "\n";

string outputPathByTfm = "\n";

bool includesNetstandard21 = pkgData.ReferenceProjects.Any(dep => dep.ShortTfm == "netstandard2.1");

bool includesNetcoreapp30 = pkgData.ReferenceProjects.Any(dep => dep.ShortTfm == "netcoreapp3.0");

foreach (var dep in pkgData.ReferenceProjects)

{

string packageReferences = "";

string netStandardTag = "NETStandardImplicitPackageVersion";

if (GeneratorVersion == 1)

{

netStandardTag = "NETStandardLibraryPackageVersion";

}

if (dep.ShortTfm == "netstandard2.0" && !includesNetstandard21 && !includesNetcoreapp30)

{

packageReferences += $" <PackageReference Include=\"NETStandard.Library\" Version=\"$({netStandardTag})\" />\n";

}

foreach (var pkgRef in dep.GetPackageReferences())

{

string version = pkgRef.VersionRange.OriginalString;

// TODO: Generate a lookup table from source-build/PackageVersions.props. For now, there is only one...

if (pkgRef.Id == "NETStandard.Library")

{

if (!includesNetstandard21 && !includesNetcoreapp30)

{

packageReferences += $" <PackageReference Include=\"{pkgRef.Id}\" Version=\"$({netStandardTag})\" />\n";

}

}

else

{

packageReferences += $" <PackageReference Include=\"{pkgRef.Id}\" Version=\"{version}\" />\n";

}

}

if (dep.ShortTfm.StartsWith("net4"))

{

packageReferences += $" <PackageReference Include=\"Microsoft.NETFramework.ReferenceAssemblies.{dep.ShortTfm.Replace("net463","net462")}\" Version=\"1.0.2\" />\n";

}

foreach (var fwkRef in dep.GetFrameworkReferences())

{

if (fwkRef != "mscorlib")

{

packageReferences += $" <Reference Include=\"{fwkRef}\" />\n";

}

}

if (packageReferences != "")

{

packageReferenceIncludes += $" <ItemGroup Condition=\" '$(TargetFramework)' == '{dep.ShortTfm.Replace("net463","net462")}' \">\n";

packageReferenceIncludes += packageReferences;

packageReferenceIncludes += $" </ItemGroup>\n\n";

}

if (dep.SubPath == "lib")

{

outputPathByTfm += $" <PropertyGroup Condition=\" '$(TargetFramework)' == '{dep.ShortTfm.Replace("net463","net462")}' \">\n";

outputPathByTfm += $" <OutputPath>$(ArtifactsBinDir){pkgData.RelativePath}/{dep.SubPath}/</OutputPath>\n";

outputPathByTfm += $" </PropertyGroup>\n\n";

}

}

string targetFrameworks = pkgData.ReferenceProjects.Select(p => p.ShortTfm.Replace("net463","net462")).Distinct().Aggregate((tfmSet,tfm) => tfmSet + ";" + tfm);

string appStrongNameKeyFileName = GetStrongNameKeyFileName(pkgData.ReferenceProjects.First());

string keyFileTag = "";

if (GeneratorVersion == 1)

{

keyFileTag = $"\n <AssemblyOriginatorKeyFile>$(KeyFileDir){appStrongNameKeyFileName}.snk</AssemblyOriginatorKeyFile>";

}

// Don't generate StrongNameKeyId for MSFT key for V2 Generator

else if (appStrongNameKeyFileName != "MSFT")

{

string appStrongNameKeyId = GetStrongNameKeyId(pkgData.ReferenceProjects.First());

keyFileTag = $"\n <StrongNameKeyId>{appStrongNameKeyId}</StrongNameKeyId>";

}

string enableImplicitReferencesTag = "";

if (includesNetstandard21 || includesNetcoreapp30)

{

enableImplicitReferencesTag = "\n <DisableImplicitFrameworkReferences>false</DisableImplicitFrameworkReferences>";

}

pkgProjectOutput = pkgProjectOutput.Replace("#LowerCaseFileName#", pkgData.Id.ToLower());

pkgProjectOutput = pkgProjectOutput.Replace("#OutputPathByTfm#", outputPathByTfm);

pkgProjectOutput = pkgProjectOutput.Replace("#RelativePath#", pkgData.RelativePath);

pkgProjectOutput = pkgProjectOutput.Replace("#PackageReferences#", packageReferenceIncludes);

pkgProjectOutput = pkgProjectOutput.Replace("#TargetFrameworks#", targetFrameworks);

pkgProjectOutput = pkgProjectOutput.Replace("#KeyFileTag#", keyFileTag);

pkgProjectOutput = pkgProjectOutput.Replace("#EnableImplicitReferencesTag#", enableImplicitReferencesTag);

string projectOutputPath = pkgData.ProjectPath;

if (GeneratorVersion == 2) projectOutputPath = pkgData.V2ProjectPath;

Log.LogMessage(MessageImportance.High, $"Writing {projectOutputPath}");

Directory.CreateDirectory(Path.GetDirectoryName(projectOutputPath));

File.WriteAllText(projectOutputPath, pkgProjectOutput);

// Write out Directory.Build.props for V2

if (GeneratorVersion == 2)

{

var directoryBuildPropsText =

"<Project>\n" +

"\n" +

" <Import Project=\"$([MSBuild]::GetPathOfFileAbove(Directory.Build.props, $(MSBuildThisFileDirectory)..))\" />\n" +

"\n" +

" <PropertyGroup>\n" +

$" <AssemblyName>{pkgData.Id}</AssemblyName>\n" +

" </PropertyGroup>\n" +

"\n" +

"</Project>\n";

File.WriteAllText(pkgData.DirectoryBuildPropsPath, directoryBuildPropsText);

}

}

// Last, generate the AssemblyVersion and csproj files for each dll, inserting the dependencies

foreach (ProjectData pd in ProjectData.GetAllProjectData())

{

string assemblyVersionData = assemblyVersionTemplate;

string codeArtifactsPath = Path.GetDirectoryName(pd.ProjectFileName);

string pacakageReferenceIncludes = "";

foreach (var dep in pd.Dependencies)

{

pacakageReferenceIncludes += $"<PackageReference Include=\"{dep.Id}\" Version=\"{dep.Version}\" />\n";

}

List<string> usings = pd.GetAdditionalUsings();

usings.AddRange(standardUsings);

var usingsText = usings.Distinct().OrderBy(u => u).Aggregate((a, b) => a + "\n" + b);

assemblyVersionData = assemblyVersionData.Replace("#AssemblyUsings#", usingsText);

assemblyVersionData = assemblyVersionData.Replace("#FileName#", pd.PackageName);

assemblyVersionData = assemblyVersionData.Replace("#AssemblyVersion#", pd.GetAssemblyVersion());

assemblyVersionData = assemblyVersionData.Replace("#FileVersion#", pd.GetFileVersion());

assemblyVersionData = assemblyVersionData.Replace("#TypeForwards#", pd.GetTypeForwards());

// Some packages have a net463 TFM, which we can't work with. If both net462 and net463 is in a package,

// just eliminate net463, otherwise, rename it to net462

string outputFilename = pd.SourceFileName;

bool updateSrcText = true;

if (pd.SourceFileName.Contains("net463"))

{

Log.LogMessage(MessageImportance.High, $"WARNING: Encountered package with net463 TFM: {pd.SourceFileName}");

outputFilename = pd.SourceFileName.Replace("net463", "net462");

if (File.Exists(outputFilename))

{

File.Delete(pd.SourceFileName);

updateSrcText = false;

}

else

{

Directory.CreateDirectory(Path.GetDirectoryName(outputFilename));

File.Move(pd.SourceFileName, outputFilename);

}

Directory.Delete(Path.GetDirectoryName(pd.SourceFileName));

}

if (updateSrcText)

{

if (!File.Exists(outputFilename))

{

Log.LogError($"Source file '{outputFilename}' not found");

return false;

}

Log.LogMessage(MessageImportance.Low, $"Updating {outputFilename} with assembly version attributes");

Directory.CreateDirectory(codeArtifactsPath);

string srcText = File.ReadAllText(outputFilename);

srcText = srcText.Replace("#AssemblyVersionAttributes#", assemblyVersionData);

File.WriteAllText(outputFilename, srcText);

}

}

Log.LogMessage(MessageImportance.High, $"Done generating csproj files. Took {DateTime.Now - startTime}");

return true;

}

While I agree with the principle of factoring responsibilities into different methods, I would recommend to follow-up on that in a separate change. Also, this PR already adds ton of code comments and significantly reduces the code size because of simplification.

That's fair, with all the refactoring I lost sight of original code. I appreciate the effort you invested to improve the codebase - thanks.

MichaelSimons · 2023-03-03T16:05:15Z

How should I interpret the following errors when running ./generate.sh -pkg microsoft.build,16.5.0? I feel like this is something we need guidance on.

/root/.nuget/packages/microsoft.dotnet.genapi.task/8.0.100-preview.3.23152.1/build/Microsoft.DotNet.GenAPI.Task.targets(44,5): error CP1002: AssemblyLoadWarning: 'Could not resolve reference 'System.ServiceModel.Internals.dll' in any of the provided search directories.' [/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/microsoft.build/16.5.0/microsoft.build.16.5.0.csproj::TargetFramework=net472]
/root/.nuget/packages/microsoft.dotnet.genapi.task/8.0.100-preview.3.23152.1/build/Microsoft.DotNet.GenAPI.Task.targets(44,5): error CP1002: AssemblyLoadWarning: 'Could not resolve reference 'SMDiagnostics.dll' in any of the provided search directories.' [/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/microsoft.build/16.5.0/microsoft.build.16.5.0.csproj::TargetFramework=net472]
/root/.nuget/packages/microsoft.dotnet.genapi.task/8.0.100-preview.3.23152.1/build/Microsoft.DotNet.GenAPI.Task.targets(44,5): error CP1002: AssemblyLoadWarning: 'Could not resolve reference 'System.ServiceModel.Internals.dll' in any of the provided search directories.' [/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/system.runtime.serialization.primitives/4.1.1/system.runtime.serialization.primitives.4.1.1.csproj::TargetFramework=net46]
/root/.nuget/packages/microsoft.dotnet.genapi.task/8.0.100-preview.3.23152.1/build/Microsoft.DotNet.GenAPI.Task.targets(44,5): error CP1002: AssemblyLoadWarning: 'Could not resolve reference 'SMDiagnostics.dll' in any of the provided search directories.' [/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/system.runtime.serialization.primitives/4.1.1/system.runtime.serialization.primitives.4.1.1.csproj::TargetFramework=net46]
/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/system.runtime/4.1.0/system.runtime.4.1.0.csproj : error NU1605: Warning As Error: Detected package downgrade: System.Runtime from 4.3.0 to 4.1.0. Reference the package directly from the project to select a different version. 
/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/system.runtime/4.1.0/system.runtime.4.1.0.csproj : error NU1605:  system.runtime.4.1.0 -> NETStandard.Library 1.6.1 -> System.Runtime (>= 4.3.0) 
/repos/source-build-reference-packages/artifacts/bin/ReferencePackageSourceGenerator/system.runtime/4.1.0/system.runtime.4.1.0.csproj : error NU1605:  system.runtime.4.1.0 -> System.Runtime (>= 4.1.0)
    0 Warning(s)
    5 Error(s)

MichaelSimons · 2023-03-03T16:13:27Z

Another scenario: ./generate.sh -pkg microsoft.codeanalysis.csharp,4.0.1

/repos/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/NuGet.targets(132,5): error : '[4.0.1]' is not a valid version string. (Parameter 'value') [/repos/source-build-reference-packages/src/referencePackageSourceGenerator/ReferencePackageSourceGenerator.proj]

In this scenario, only the microsoft.codeanalysis.csharp,4.0.1 package way generated. It's dependency graph was not.

…atorRefactoring

ViktorHofer · 2023-03-05T18:38:33Z

@MichaelSimons thanks for the feedback so far 👍

How should I interpret the following errors when running ./generate.sh -pkg microsoft.build,16.5.0? I feel like this is something we need guidance on.

The ´AssemblyLoadWarning` warnings are reasonable and should stay. In the case of .NET Framework, reference assemblies are missing (intentionally AFAIK as those never got exposed in the reference assemblies pack). Those warnings were propagated to errors which I just fixed.

The package downgrade is intentional, as we are referencing a specific version of a package which might be a lower version than what other dependencies (targeting pack) might bring in. I just added a NoWarn for it.

In this scenario, only the microsoft.codeanalysis.csharp,4.0.1 package way generated. It's dependency graph was not.

Fixed. That package had fixed versions for its dependencies and the tooling / NuGet APIs handled that incorrectly.

…atorRefactoring

ViktorHofer requested review from MichaelSimons, crummel, mthalman and oleksandr-didyk as code owners February 27, 2023 21:37

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch from ce0e7d5 to ddaf0e1 Compare February 28, 2023 11:36

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch 6 times, most recently from 736f8b4 to d1b0db5 Compare February 28, 2023 14:00

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch 2 times, most recently from 4e21811 to 40f7b37 Compare March 1, 2023 07:45

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch 5 times, most recently from 2296a97 to 2f9e131 Compare March 1, 2023 09:52

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch from 2f9e131 to b1429b1 Compare March 1, 2023 15:41

andriipatsula approved these changes Mar 1, 2023

View reviewed changes

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch from b1429b1 to 8a2d49f Compare March 2, 2023 19:06

ViktorHofer mentioned this pull request Mar 2, 2023

UsingTask fails randomly on Ubuntu x64, only reboot helps dotnet/msbuild#8031

Closed

ViktorHofer force-pushed the ReferencePackageGeneratorRefactoring branch from bc52865 to f5264e1 Compare March 2, 2023 21:52

ViktorHofer added 2 commits March 2, 2023 23:35

Set AssemblyName & PackageId correctly

4c93156

Remove unused GeneratorVersion 1 logic

8733e71

oleksandr-didyk mentioned this pull request Mar 3, 2023

.NET 8: Tracking enable repo-level prebuilt detection dotnet/source-build#3017

Closed

27 tasks

MichaelSimons reviewed Mar 3, 2023

View reviewed changes

eng/Versions.props Outdated Show resolved Hide resolved

MichaelSimons reviewed Mar 3, 2023

View reviewed changes

ViktorHofer added 2 commits March 5, 2023 12:07

Bugfixes, CLI args, ...

3d24b60

Merge remote-tracking branch 'origin/main' into ReferencePackageGener…

27121d7

…atorRefactoring

Merge remote-tracking branch 'origin/main' into ReferencePackageGener…

a5eb8b0

…atorRefactoring

MichaelSimons approved these changes Mar 6, 2023

View reviewed changes

MichaelSimons merged commit 98e81d7 into dotnet:main Mar 6, 2023

mthalman mentioned this pull request Mar 6, 2023

Add Microsoft.Build.16.10.0 and Microsoft.Build.Tasks.Core.16.10.0 #559

Closed

ViktorHofer deleted the ReferencePackageGeneratorRefactoring branch March 6, 2023 16:38

	public override bool Execute()
	{
	DateTime startTime = DateTime.Now;
	string assemblyVersionTemplate = File.ReadAllText("AssemblyVersionTemplate.txt");
	string pkgProjectTemplate = File.ReadAllText("PackageProjectTemplate.xml");
	string[] standardUsings = File.ReadAllLines("AssemblyStandardUsings.txt");

	Log.LogMessage(MessageImportance.Low, "Start generating csproj files.");

	// First, create project data for all passed in dlls
	foreach (ITaskItem dll in PackageDlls)
	{
	var pd = ProjectData.GetOrCreateProjectData(TargetPackagesPath, dll.GetMetadata("FullPath"), SrcPath);
	Log.LogMessage(MessageImportance.Low, $"Generating data for {pd.PackageName}/{pd.ShortTfm}");
	}

	// Second, setup dependencies between all generated project data
	ProjectData.GenerateAllDependencies();

	foreach (var pkgData in PackageData.GetAll())
	{
	string pkgProjectOutput = pkgProjectTemplate;

	string packageReferenceIncludes = "\n";
	string outputPathByTfm = "\n";
	bool includesNetstandard21 = pkgData.ReferenceProjects.Any(dep => dep.ShortTfm == "netstandard2.1");
	bool includesNetcoreapp30 = pkgData.ReferenceProjects.Any(dep => dep.ShortTfm == "netcoreapp3.0");
	foreach (var dep in pkgData.ReferenceProjects)
	{
	string packageReferences = "";
	string netStandardTag = "NETStandardImplicitPackageVersion";
	if (GeneratorVersion == 1)
	{
	netStandardTag = "NETStandardLibraryPackageVersion";
	}

	if (dep.ShortTfm == "netstandard2.0" && !includesNetstandard21 && !includesNetcoreapp30)
	{
	packageReferences += $" <PackageReference Include=\"NETStandard.Library\" Version=\"$({netStandardTag})\" />\n";
	}
	foreach (var pkgRef in dep.GetPackageReferences())
	{
	string version = pkgRef.VersionRange.OriginalString;
	// TODO: Generate a lookup table from source-build/PackageVersions.props. For now, there is only one...
	if (pkgRef.Id == "NETStandard.Library")
	{
	if (!includesNetstandard21 && !includesNetcoreapp30)
	{
	packageReferences += $" <PackageReference Include=\"{pkgRef.Id}\" Version=\"$({netStandardTag})\" />\n";
	}
	}
	else
	{
	packageReferences += $" <PackageReference Include=\"{pkgRef.Id}\" Version=\"{version}\" />\n";
	}
	}
	if (dep.ShortTfm.StartsWith("net4"))
	{
	packageReferences += $" <PackageReference Include=\"Microsoft.NETFramework.ReferenceAssemblies.{dep.ShortTfm.Replace("net463","net462")}\" Version=\"1.0.2\" />\n";
	}
	foreach (var fwkRef in dep.GetFrameworkReferences())
	{
	if (fwkRef != "mscorlib")
	{
	packageReferences += $" <Reference Include=\"{fwkRef}\" />\n";
	}
	}
	if (packageReferences != "")
	{
	packageReferenceIncludes += $" <ItemGroup Condition=\" '$(TargetFramework)' == '{dep.ShortTfm.Replace("net463","net462")}' \">\n";
	packageReferenceIncludes += packageReferences;
	packageReferenceIncludes += $" </ItemGroup>\n\n";
	}
	if (dep.SubPath == "lib")
	{
	outputPathByTfm += $" <PropertyGroup Condition=\" '$(TargetFramework)' == '{dep.ShortTfm.Replace("net463","net462")}' \">\n";
	outputPathByTfm += $" <OutputPath>$(ArtifactsBinDir){pkgData.RelativePath}/{dep.SubPath}/</OutputPath>\n";
	outputPathByTfm += $" </PropertyGroup>\n\n";
	}
	}

	string targetFrameworks = pkgData.ReferenceProjects.Select(p => p.ShortTfm.Replace("net463","net462")).Distinct().Aggregate((tfmSet,tfm) => tfmSet + ";" + tfm);

	string appStrongNameKeyFileName = GetStrongNameKeyFileName(pkgData.ReferenceProjects.First());
	string keyFileTag = "";
	if (GeneratorVersion == 1)
	{
	keyFileTag = $"\n <AssemblyOriginatorKeyFile>$(KeyFileDir){appStrongNameKeyFileName}.snk</AssemblyOriginatorKeyFile>";
	}
	// Don't generate StrongNameKeyId for MSFT key for V2 Generator
	else if (appStrongNameKeyFileName != "MSFT")
	{
	string appStrongNameKeyId = GetStrongNameKeyId(pkgData.ReferenceProjects.First());
	keyFileTag = $"\n <StrongNameKeyId>{appStrongNameKeyId}</StrongNameKeyId>";
	}

	string enableImplicitReferencesTag = "";
	if (includesNetstandard21 \|\| includesNetcoreapp30)
	{
	enableImplicitReferencesTag = "\n <DisableImplicitFrameworkReferences>false</DisableImplicitFrameworkReferences>";
	}
	pkgProjectOutput = pkgProjectOutput.Replace("#LowerCaseFileName#", pkgData.Id.ToLower());
	pkgProjectOutput = pkgProjectOutput.Replace("#OutputPathByTfm#", outputPathByTfm);
	pkgProjectOutput = pkgProjectOutput.Replace("#RelativePath#", pkgData.RelativePath);
	pkgProjectOutput = pkgProjectOutput.Replace("#PackageReferences#", packageReferenceIncludes);
	pkgProjectOutput = pkgProjectOutput.Replace("#TargetFrameworks#", targetFrameworks);
	pkgProjectOutput = pkgProjectOutput.Replace("#KeyFileTag#", keyFileTag);
	pkgProjectOutput = pkgProjectOutput.Replace("#EnableImplicitReferencesTag#", enableImplicitReferencesTag);

	string projectOutputPath = pkgData.ProjectPath;
	if (GeneratorVersion == 2) projectOutputPath = pkgData.V2ProjectPath;
	Log.LogMessage(MessageImportance.High, $"Writing {projectOutputPath}");
	Directory.CreateDirectory(Path.GetDirectoryName(projectOutputPath));
	File.WriteAllText(projectOutputPath, pkgProjectOutput);

	// Write out Directory.Build.props for V2
	if (GeneratorVersion == 2)
	{
	var directoryBuildPropsText =
	"<Project>\n" +
	"\n" +
	" <Import Project=\"$([MSBuild]::GetPathOfFileAbove(Directory.Build.props, $(MSBuildThisFileDirectory)..))\" />\n" +
	"\n" +
	" <PropertyGroup>\n" +
	$" <AssemblyName>{pkgData.Id}</AssemblyName>\n" +
	" </PropertyGroup>\n" +
	"\n" +
	"</Project>\n";

	File.WriteAllText(pkgData.DirectoryBuildPropsPath, directoryBuildPropsText);
	}
	}
	// Last, generate the AssemblyVersion and csproj files for each dll, inserting the dependencies
	foreach (ProjectData pd in ProjectData.GetAllProjectData())
	{
	string assemblyVersionData = assemblyVersionTemplate;
	string codeArtifactsPath = Path.GetDirectoryName(pd.ProjectFileName);

	string pacakageReferenceIncludes = "";
	foreach (var dep in pd.Dependencies)
	{
	pacakageReferenceIncludes += $"<PackageReference Include=\"{dep.Id}\" Version=\"{dep.Version}\" />\n";
	}

	List<string> usings = pd.GetAdditionalUsings();
	usings.AddRange(standardUsings);
	var usingsText = usings.Distinct().OrderBy(u => u).Aggregate((a, b) => a + "\n" + b);

	assemblyVersionData = assemblyVersionData.Replace("#AssemblyUsings#", usingsText);
	assemblyVersionData = assemblyVersionData.Replace("#FileName#", pd.PackageName);
	assemblyVersionData = assemblyVersionData.Replace("#AssemblyVersion#", pd.GetAssemblyVersion());
	assemblyVersionData = assemblyVersionData.Replace("#FileVersion#", pd.GetFileVersion());
	assemblyVersionData = assemblyVersionData.Replace("#TypeForwards#", pd.GetTypeForwards());

	// Some packages have a net463 TFM, which we can't work with. If both net462 and net463 is in a package,
	// just eliminate net463, otherwise, rename it to net462
	string outputFilename = pd.SourceFileName;
	bool updateSrcText = true;
	if (pd.SourceFileName.Contains("net463"))
	{
	Log.LogMessage(MessageImportance.High, $"WARNING: Encountered package with net463 TFM: {pd.SourceFileName}");
	outputFilename = pd.SourceFileName.Replace("net463", "net462");
	if (File.Exists(outputFilename))
	{
	File.Delete(pd.SourceFileName);
	updateSrcText = false;
	}
	else
	{
	Directory.CreateDirectory(Path.GetDirectoryName(outputFilename));
	File.Move(pd.SourceFileName, outputFilename);
	}
	Directory.Delete(Path.GetDirectoryName(pd.SourceFileName));
	}

	if (updateSrcText)
	{
	if (!File.Exists(outputFilename))
	{
	Log.LogError($"Source file '{outputFilename}' not found");
	return false;
	}

	Log.LogMessage(MessageImportance.Low, $"Updating {outputFilename} with assembly version attributes");
	Directory.CreateDirectory(codeArtifactsPath);
	string srcText = File.ReadAllText(outputFilename);
	srcText = srcText.Replace("#AssemblyVersionAttributes#", assemblyVersionData);
	File.WriteAllText(outputFilename, srcText);
	}
	}
	Log.LogMessage(MessageImportance.High, $"Done generating csproj files. Took {DateTime.Now - startTime}");

	return true;
	}

Conversation

ViktorHofer commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andriipatsula commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelSimons commented Feb 28, 2023

Uh oh!

andriipatsula commented Feb 28, 2023

Uh oh!

ViktorHofer commented Feb 28, 2023

Uh oh!

andriipatsula commented Feb 28, 2023

From the main:

From the PR branch:

Uh oh!

andriipatsula commented Feb 28, 2023

Uh oh!

MichaelSimons commented Feb 28, 2023

Uh oh!

ViktorHofer commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ViktorHofer commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andriipatsula commented Mar 1, 2023

Uh oh!

MichaelSimons commented Mar 1, 2023

Uh oh!

ViktorHofer commented Mar 2, 2023

Uh oh!

ViktorHofer commented Mar 2, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelSimons Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

ViktorHofer Mar 5, 2023

Choose a reason for hiding this comment

Uh oh!

MichaelSimons Mar 6, 2023

Choose a reason for hiding this comment

Uh oh!

MichaelSimons commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelSimons commented Mar 3, 2023

Uh oh!

ViktorHofer commented Mar 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ViktorHofer commented Feb 27, 2023 •

edited

Loading

andriipatsula commented Feb 28, 2023 •

edited

Loading

ViktorHofer commented Mar 1, 2023 •

edited

Loading

ViktorHofer commented Mar 1, 2023 •

edited

Loading

MichaelSimons commented Mar 3, 2023 •

edited

Loading

ViktorHofer commented Mar 5, 2023 •

edited

Loading