Skip to content

[release/2.1] Detect package cache blocking source-built package uptake #711

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

Merged
merged 3 commits into from
Aug 27, 2018

Conversation

dagood
Copy link
Member

@dagood dagood commented Aug 23, 2018

Detect when a source-built package exists, but a different package exists in the package cache directory. NuGet restore short-circuits when the package id/version specified is already in the cache, so the source-built one won't be used by downstream repos.


This is for #606.

It actually spots an active problem in this branch (release/2.1):

Watch out! Source-built nupkg is not byte-for-byte identical to nupkg in cache: Newtonsoft.Json 9.0.1

I checked to make sure contents of the package cache are really from online. Related (although I don't think we realized the extent of the issue): #593.

Detect when a source-built package exists, but a different package exists in the package cache directory. NuGet restore short-circuits when the package id/version specified is already in the cache, so the source-built one won't be used by downstream repos.
@dagood dagood self-assigned this Aug 23, 2018
@dagood dagood requested review from dseefeld and crummel August 23, 2018 01:27
@@ -334,6 +344,8 @@
<PrebuiltPackages Include="$(PrebuiltPackagesPath)*.nupkg" />
<PrebuiltPackages Include="$(TarballPrebuiltPackagesPath)*.nupkg" Condition="'$(TarballPrebuiltPackagesPath)' != ''"/>

<PrebuiltPackages Include="$(CustomUsageCheckPackageDir)*.nupkg" Condition="'$(CustomUsageCheckPackageDir)' != ''" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get what's happening with CustomUsageCheckPackageDir here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, this is kind of out of nowhere here. I'll add a comment. The idea is it's used by FindSourceBuiltUsages to do a "custom usage check".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that, but why is it included in PrebuiltPackages?

Copy link
Member Author

@dagood dagood Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, good point... I was being lazy and using the existing target without much modification just out of convenience. I'll reorganize this to make sense.

I'll also try to make it run immediately after it detects the problem and focus the report the specific problematic package(s). If you just do what the message suggests and run this after the build completes, there's too much junk to practically sift through.

@dagood
Copy link
Member Author

dagood commented Aug 27, 2018

Now it outputs this in this situation (for every project after the place this problem was introduced):

  Checked cache for conflicts with source-built nupkgs. Took 00:00:00.0022916
  Finding project directories...
  Finding project.assets.json files...
  Reading usage info...
  Associating usage info with projects...
  Writing data to '/home/dagood/sb/source-build/bin/conflict-report/before-coreclr/usage.json'...
/home/dagood/sb/source-build/repos/dir.targets(202,5): warning : Detected packages in the cache that should be source-built, but aren't. See /home/dagood/sb/source-build/bin/conflict-report/before-coreclr/ for usage details. Newtonsoft.Json 9.0.1 [/home/dagood/sb/source-build/repos/coreclr.proj]

/home/dagood/sb/source-build/bin/conflict-report/before-javascriptservices/usage.json is:

{
  "UnusedPackages": [],
  "Usages": [
    {
      "ProjectDirectory": "/home/dagood/sb/source-build/src/common",
      "AssetsFile": "/home/dagood/sb/source-build/src/common/test/Microsoft.Extensions.CommandLineUtils.Tests/obj/project.assets.json",
      "PackageIdentity": "Newtonsoft.Json/9.0.1",
      "Annotation": null
    }
  ]
}

It does this for every project, since I think it's ok to let the build continue (in this case, at least), and I think it could be interesting to look at each later project's usages (if any).

@dagood
Copy link
Member Author

dagood commented Aug 27, 2018

@dotnet-bot test OSX10.12 Release
Usual ClosedChannelException flakiness.

@dagood
Copy link
Member Author

dagood commented Aug 27, 2018

Interesting, the conflict doesn't happen in the offline tarball build. Target "CheckSourceBuiltNupkgConflictUsages" skipped, due to false condition; ('@(ConflictingPackageInfos)' != '') was evaluated as ('' != ''). Makes sense, since it can't get the online one, and we specifically remove source-built packages from the tarball prebuilt dir.

@dagood dagood merged commit bb2b2d1 into dotnet:release/2.1 Aug 27, 2018
@dagood dagood deleted the overlap-alert branch August 27, 2018 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants