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

[vcpkg] Binaries recovered by Nuget are not cached locally, no matter the value of VCPKG_BINARY_SOURCES #15169

Closed
klalumiere opened this issue Dec 16, 2020 · 6 comments · Fixed by #15512
Assignees
Labels
category:question This issue is a question category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed

Comments

@klalumiere
Copy link
Contributor

klalumiere commented Dec 16, 2020

Describe the bug
This is somewhere between a new feature and a bug. I wanted to discuss the issue before trying to implement a fix.

I set VCPKG_BINARY_SOURCES to clear;default,readwrite;nuget,https://nuget.coveo.com/v3/index.json,read. I expected binaries to be fetched the first time from Nuget, and then to be fetched locally. However, they are always fetched from Nuget. This become a major issue for us since we have a large number of large dependencies. More details below.

Environment

  • OS: Windows and Ubuntu 20.04 (I tested both, but my latest tests were made on Ubuntu 20.04)
  • Compiler: msvc on Windows, clang version 10.0.0-4ubuntu1 on Ubuntu 20.04

To Reproduce

Consider the minimal /coveobaselib/vcpkg.json manifest

{
    "name": "coveobaselib",
    "version-string": "head",

    "dependencies": [
        {
            "name": "aws-sdk-cpp"
        }
    ]
}

Steps to reproduce the behavior:

  1. export VCPKG_BINARY_SOURCES="clear;default,readwrite;nuget,https://nuget.coveo.com/v3/index.json,read". I actually tried every permutations possible with default and nuget.
  2. cd /coveobaselib
  3. mkdir debug && cd debug
  4. cmake -G Ninja ..
  5. aws-sdk-cpp is downloaded from Nuget
  6. mkdir ../release && cd ../release. This could also be a directory anotherDebug where I test different compiler flags.
  7. aws-sdk-cpp is downloaded, again, from Nuget

Expected behavior
The second time (step 7), I expected aws-sdk-cpp to be cached locally, for instance in $HOME/.cache/vcpkg, and no query to the Nuget server to be made. Of course, in this toy example, it doesn't matter. However, in our case, we have more than 40 dependencies and some of them are large. This would be problematic for us to re-download them each time we clear our build directory.

Of course, the dependency aws-sdk-cpp do not need to be in $HOME/.cache/vcpkg, it only need to be local. For instance, I saw while browsing the code that Nuget also has a cache and that we purposely do not use it. Maybe we could use it? Another possible fix: the dependencies are downloaded in /vcpkg/packages. Maybe we could use them directly and not redownload them from Nuget if they are found? However, this would required "versioning" (preferably with the hash) the content of the folder vcpkg/packages. This will be required anyway if many repository manifests with different versions of the same package shares vcpkg/packages (for race conditions in particular).

In summary, to fix the issue, given VCPKG_BINARY_SOURCES="clear;source1,readwrite;nuget,https://nuget.coveo.com/v3/index.json,read;source2,readwrite", we could

A) Download the dependency from Nuget, copy (write) it to source1 only since it is before nuget in the list and it is in readwrite
B) Download the dependency from Nuget, copy (write) it to source2 only since it is after nuget in the list ("weird" for people that read from left to right, but I include it anyway) and it is in readwrite.
C) Download the dependency from Nuget, copy (write) it to every sources in readwrite.
D) Use Nuget cache.
E) Use the package from vcpkg/packages if it is available
F) Something else?

Looking forward for your feedback 🙂

@JackBoosY
Copy link
Contributor

@BillyONeal Can you please take a look?

Thanks.

@BillyONeal
Copy link
Member

If I understood @ras0219 / @ras0219-msft correctly when this feature was deployed, this is by design; I don't know if the nuget caches necessarily contain the same contents such that we could use one cache strategy to mint the other without the original sources. But I'm not positive.

@ras0219
Copy link
Contributor

ras0219 commented Dec 21, 2020

This behavior is by design (i.e. not a bug), but I think it's a very reasonable feature request to allow users to enable the nuget cache.

@klalumiere
Copy link
Contributor Author

@BillyONeal @ras0219

Hey! Thanks for taking the time to read the issue and answer 🙂 .

So you think in the 5 solutions I suggested (A) to E)), the best one (and an acceptable one) is to use Nuget cache? (i.e. D)). If you agree that this solution looks promising, I could start implementing this soon.

@BillyONeal
Copy link
Member

I have to defer to @ras0219 on that one myself

@JackBoosY JackBoosY added the category:question This issue is a question label Dec 22, 2020
@ras0219
Copy link
Contributor

ras0219 commented Dec 22, 2020

@klalumiere Yeah, exactly. I was thinking it would probably be appropriate to add a switch (or two) to the nuget and nugetconfig binary sources that remove the -NoCache and -DirectDownload flags.

I think for most users it probably makes sense to pass those flags by default still (treating NuGet as just a "dumb" download client with fancy authentication).

@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Dec 22, 2020
dan-shaw pushed a commit that referenced this issue Jan 25, 2021
 #15169) (#15512)

* Fix warning on clang version 10.0.0-4ubuntu1

The warning was

```shell
../src/vcpkg/commands.porthistory.cpp:55:14: error: unused function 'is_date' [-Werror,-Wunused-function]
```

* Add environment variable VCPKG_USE_NUGET_CACHE

As the name suggests, this environment variable allow tu use Nuget
cache for Nuget binary caching sources.

* Document NuGet's Cache environment variable
strega-nil pushed a commit to strega-nil/vcpkg that referenced this issue May 5, 2021
 microsoft#15169) (microsoft#15512)

* Fix warning on clang version 10.0.0-4ubuntu1

The warning was

```shell
../src/vcpkg/commands.porthistory.cpp:55:14: error: unused function 'is_date' [-Werror,-Wunused-function]
```

* Add environment variable VCPKG_USE_NUGET_CACHE

As the name suggests, this environment variable allow tu use Nuget
cache for Nuget binary caching sources.

* Document NuGet's Cache environment variable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:question This issue is a question category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants