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

Inconsistent packages.lock.json after updating global.json from 3.1.405 to 5.0.102 #10456

Open
breidikl opened this issue Jan 14, 2021 · 9 comments
Assignees
Labels
Area:RestoreRepeatableBuild The lock file features Category:Quality Week Issues that should be considered for quality week Functionality:Restore Priority:2 Issues for the current backlog. Resolution:External This issue appears to be External to nuget Type:Learning

Comments

@breidikl
Copy link

Details about Problem

Depending on what is installed on the machine, running restore on the same project will result in different packages.lock.json files.

Specifically we use the following command on our builds:

dotnet.exe restore --use-lock-file --no-dependencies /p:ImportProjectExtensionProps=false /p:ImportProjectExtensionTargets=false /nodeReuse:false /maxcpucount:1 --lock-file-path "\packages.lock.json" --locked-mode

If the build fails with NU1004 (and we're not in an automated build environment), we'll re-run restore without --locked-mode to update the lock file and rebuild.

The problem is that the lock files are failing and being updated for differences in what is installed on the machine. For example, on our dev environments with Visual Studio installed, the lock file is smaller while in our more minimal environments without VS are adding additional sections similar to the following:

11 + "Microsoft.NETFramework.ReferenceAssemblies": {
12 + "type": "Direct",
13 + "requested": "[1.0.0, )",
14 + "resolved": "1.0.0",
15 + "contentHash": "7D2TMufjGiowmt0E941kVoTIS+GTNzaPopuzM1/1LSaJAdJdBrVP0SkZW7AgDd0a2U1DjsIeaKG1wxGVBNLDMw==",
16 + "dependencies": {
17 + "Microsoft.NETFramework.ReferenceAssemblies.net472": "1.0.0"
18 + }
19 + },
92 + "Microsoft.NETFramework.ReferenceAssemblies.net472": {
93 + "type": "Transitive",
94 + "resolved": "1.0.0",
95 + "contentHash": "VI0f22cH/xOlXLUVY7t6vr2Gi7cg0vzwpspNGz5isRusyspZblF7jJMw49E8mF1m071JyeVmZVkIK9c5kbiyFA=="
96 + },

Using the new lock file in the other environment then fails and updating the lock file in that environment then removes the new sections. There is also variability in the dev environments based on the features being used.

This problem did not occur when using sdk version 3.1.405 in global.json (when we would get a consistent lock file just based on the project), and only started after changing the sdk version to 5.0.102.

@nkolev92
Copy link
Member

The extra package reference is brought by the SDK in https://github.com/dotnet/sdk/blob/61e32e28afb1ab622e96625aa80cebc78373554f/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets#L359-L377.

Can you try setting AutomaticallyUseReferenceAssemblyPackages to false?

Not sure why it's being added at this point, but we can dig more if setting the property fixes it.

@breidikl
Copy link
Author

breidikl commented Jan 20, 2021 via email

@nkolev92
Copy link
Member

Sounds great.

We can move this to SDK team so they can help us figure out this changed.

cc @dsplaisted @sfoslund @wli3

@dsplaisted
Copy link

We started automatically referencing the reference assembly packages as needed in the .NET 5 SDK: dotnet/sdk#10981

These references are only supposed to be added when the corresponding targeting pack is not installed in Program Files. So I would expect the build to fail for the 3.1 SDK if the targeting pack was not installed, and I would expect the lock files to be the same with the 5.0 SDK if both environments have the targeting pack installed.

@breidikl
Copy link
Author

In most of the examples, these were for projects targeting down level full framework versions (i.e. 4.6.2, 4.7, etc...).

We do have packages containing the relevant targeting packs that are correctly found in our build environment via shared logic,, though that doesn't seem to be a consideration for how the lock files are being updated (and the available targeting packs installed to Program Files varies across dev machines with an even more minimal setup on lab/cloud machines). Our main concern is that we want deterministic lock files based on the source state of the project.

Turning off the the functionality via AutomaticallyUseReferenceAssemblyPackages returns us to the earlier behavior. If there were another option to always add the reference version to the lock file (regardless of the Program Files installed state), that would also work for us.

@nkolev92
Copy link
Member

@dsplaisted

Why not use PackageDownload instead?
It wouldn't affect the lock file.

Worth understanding why:

Specifically focusing on @breidikl's comment:

We do have packages containing the relevant targeting packs that are correctly found in our build environment via shared logic,, though that doesn't seem to be a consideration for how the lock files are being updated (and the available targeting packs installed to Program Files varies across dev machines with an even more minimal setup on lab/cloud machines).

@dsplaisted
Copy link

The .NET Framework targeting pack packages can be referenced manually, either in an SDK-style project or in a classic project. So the NuGet packages have .targets files that need to be imported which tell MSBuild where to find the reference assemblies. Also, the Microsoft.NETFramework.ReferenceAssemblies package has version-specific dependencies on the packages which deliver the reference assemblies for each framework.

If you want to use these packages and want stable lock files, you should be able to simply add an explicit package reference to the Microsoft.NETFramework.ReferenceAssemblies package.

@nkolev92 nkolev92 added Area:RestoreRepeatableBuild The lock file features Functionality:Restore Resolution:External This issue appears to be External to nuget Type:Learning labels Jan 21, 2021
@nkolev92
Copy link
Member

nkolev92 commented Feb 9, 2021

Related to #9195.

We discussed this during our sync and considered a few alternatives. Here's some of those notes + my personal answers to the 2 ideas/2 open questions.

  • Reference assembly packages automatically being referenced – not working well with lock file
    • Should it be ignored by lock file because IsImplicitlyDefined = true?
      • @nkolev92: Probably not. An implicitly defined reference will still affect the transitive dependencies of a project. Completely ignoring a package would mean trying to ignore the impact of that package on the package graph, which is chalenging.
    • Should we avoid implicitly adding these package references when using a lock file, and instead generate an error saying you can add an explicit package reference.
      • @nkolev92: I think this might be the compromise to provide customers with a repeatable experience.

I want to reiterate a point brought in the above comments.
It is expected that the build will fail if the SDK ever determined it needs to add a targeting pack.

@breidikl What do you think about the above approach of the SDK not adding the targeting pack when using lock files? Would you prefer to add the targeting pack reference yourself instead?

@nkolev92 nkolev92 added this to the Sprint 183 - 2021.02.01 milestone Feb 9, 2021
@breidikl
Copy link
Author

@nkolev92 Our ultimate goal is for our builds to be deterministic and repeatable based on the state of our sources, regardless of what is installed on the machine (and eventually regardless of platform being used to do the builds). Having the ability to audit the list of required packages from the source state (contained in the lock file) is also helpful.

In our case we already have custom logic that resolves out targeting pack content to packages available in our environments, which means we don't need to list them explicitly in the lock file (handled via some package boostrapping that's not specific to msbuild/dotnet). So for now we've disabled AutomaticallyUseReferenceAssemblyPackages, which has unblocked folks in our org that wanted to target .Net 5.

Failing and asking users to explicitly add reference packages when needed could also work for our case, assuming that would be reflected in the lock file and doesn't vary machine to machine for the same sources. Having an option to automatically add the all the references based on targets regardless of the install state would also satisfy our concerns.

Adding more implicit packages not listed in the lock files would not be desirable from our perspective. We're still working around the issue described at #7724 to add in the correct list of implicit runtime identifiers packages for our environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:RestoreRepeatableBuild The lock file features Category:Quality Week Issues that should be considered for quality week Functionality:Restore Priority:2 Issues for the current backlog. Resolution:External This issue appears to be External to nuget Type:Learning
Projects
None yet
Development

No branches or pull requests

4 participants