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

Change NuGet cache to be in repository root #7310

Closed
wants to merge 13 commits into from

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Jan 21, 2022

Related to #7187.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Jan 21, 2022

@elachlan This seems to work for CI. But I'm not sure if it will work well for local development. Haven't cloned the repository so I can't test.

Can we use something like https://stackoverflow.com/a/20014410/5108631? But we definitely want to revert the environment variable back to its original state after build.

Note that we can simply delete the environment variable, which will then have the effect from NuGet.config

@Youssef1313 Youssef1313 changed the title Change globalPackagesFolder Change NuGet cache to be in repository root Jan 21, 2022
@Youssef1313 Youssef1313 marked this pull request as ready for review January 21, 2022 09:14

[.nuget/**/*.cs]
# Instantiate argument exceptions correctly
dotnet_diagnostic.CA2208.severity = suggestion
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be dotnet_diagnostic.severity = suggestion?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm disabling CA2208 specifically so that you can clearly see what rules are violated by the referenced packages.

@elachlan
Copy link
Contributor

elachlan commented Jan 21, 2022

@Youssef1313 This doesn't work locally. I still get the standard errors. Thanks for your efforts though.

C:\Users\elachan\.nuget\packages\microsoft.codeanalysis.collections\4.0.0-4.21379.20\contentFiles\cs\nets
tandard2.0\ImmutableSegmentedList`1.cs(346,23): error CA2208: Call the ArgumentException constructor that contains a me
ssage and/or paramName parameter [D:\Development\msbuild\src\Framework\Microsoft.Build.Framework.csproj]

@Forgind I think this solution might work if we can fix that quirk with local build. Otherwise we can just move back to using editorconfigs.

Edit: the error is pointing to my user directory, not the cache you configured.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Jan 21, 2022

@elachlan Do you have NUGET_PACKAGES environment variable set to anything? The NUGET_PACKAGES has precedence over the path defined in NuGet.config

@elachlan
Copy link
Contributor

@elachlan Do you have NUGET_PACKAGES environment variable set to anything?

I just run "build" from the msbuild root via cmd prompt. As far as I can tell that env variable is not set.

@Youssef1313
Copy link
Member Author

@elachlan Do you have NUGET_PACKAGES environment variable set to anything?

I just run "build" from the msbuild root via cmd prompt. As far as I can tell that env variable is not set.

The environment variable is the only thing that can take precedence over NuGet.config. I think arcade has set it for your local environment?

[bool]$useGlobalNuGetCache = if (Test-Path variable:useGlobalNuGetCache) { $useGlobalNuGetCache } else { !$ci }

Here, useGlobalNuGetCache is likely set to !ci which is true for your local environment. Then:

function GetNuGetPackageCachePath() {
if ($env:NUGET_PACKAGES -eq $null) {
# Use local cache on CI to ensure deterministic build.
# Avoid using the http cache as workaround for https://github.com/NuGet/Home/issues/3116
# use global cache in dev builds to avoid cost of downloading packages.
# For directory normalization, see also: https://github.com/NuGet/Home/issues/7968
if ($useGlobalNuGetCache) {
$env:NUGET_PACKAGES = Join-Path $env:UserProfile '.nuget\packages\'
} else {
$env:NUGET_PACKAGES = Join-Path $RepoRoot '.packages\'
$env:RESTORENOCACHE = $true
}
}
return $env:NUGET_PACKAGES
}

@Youssef1313
Copy link
Member Author

I think we can just pass false for useGlobalNuGetCache from the build script and things should work smoothly?

@elachlan
Copy link
Contributor

We need to check with the team to confirm this is a good idea. They might dislike the idea of not using the global cache.

It might be simpler to just use the editorconfig in conjunction with the globalconfig as outlined in dotnet/roslyn#55992

@Youssef1313
Copy link
Member Author

Sure. I'm going to close for now. You can incorporate either using editorconfigs and globalconfigs, or using the local nuget cache approach in #7187 as you and the team see what's more suitable.

@Youssef1313 Youssef1313 deleted the patch-2 branch January 21, 2022 11:30
@elachlan
Copy link
Contributor

Sure. I'm going to close for now. You can incorporate either using editorconfigs and globalconfigs, or using the local nuget cache approach in #7187 as you and the team see what's more suitable.

Thank you so much for your help on this. It gives us a good idea on what is possible.

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.

2 participants