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] Allow to use Nuget's cache for Nuget binary caching sources (fix #15169) #15512

Merged
merged 3 commits into from
Jan 25, 2021
Merged

Conversation

klalumiere
Copy link
Contributor

Describe the pull request

I also fixed a warning on Fix warning on clang version 10.0.0-4ubuntu1 so that I could compile toolsrc (the function is_date was not used anymore).

I should probably add documentation, but I wanted feedback before writing the doc. I fixed the issue I raise in #15169 by introducing an environment variable VCPKG_USE_NUGET_CACHE. This is the simplest solution I could think of, and it has the advantage of requiring a very small changeset/pull request.

I'm looking forward for your feedback. 🙂

  • Which triplets are supported/not supported? Have you updated the CI baseline? The same. No.

  • Does your PR follow the maintainer guide? Yes.

The warning was

```shell
../src/vcpkg/commands.porthistory.cpp:55:14: error: unused function 'is_date' [-Werror,-Wunused-function]
```
As the name suggests, this environment variable allow tu use Nuget
cache for Nuget binary caching sources.
@JackBoosY JackBoosY self-assigned this Jan 8, 2021
@JackBoosY JackBoosY requested a review from ras0219-msft January 8, 2021 01:39
@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 Jan 8, 2021
@JackBoosY
Copy link
Contributor

cc @ras0219 for review this PR.

@klalumiere
Copy link
Contributor Author

Hi @ras0219 !

Any news/feedback on the pull request? 🙂

Copy link
Contributor

@ras0219 ras0219 left a comment

Choose a reason for hiding this comment

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

Sorry for the long feedback loop!

This LGTM, though the new environment variable should be added:

  1. to docs/users/config-environment.md (cross-linked with binarycaching.md)
  2. to docs/users/binarycaching.md (cross-linked with config-environment.md)
  3. to help_topic_binary_caching() in binarycaching.cpp

Once docs are added, this should be merged! Thanks again for the great feature work :)

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:discussion labels Jan 25, 2021
@dan-shaw dan-shaw merged commit f60b947 into microsoft:master Jan 25, 2021
@klalumiere klalumiere deleted the fix-15169 branch January 25, 2021 14:37
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request 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
@senstar-nross
Copy link

@JackBoosY any chance a command-line option could be added to avoid having to set the ENV variable? Like --x-use-nuget-cache?

@JackBoosY
Copy link
Contributor

@JackBoosY any chance a command-line option could be added to avoid having to set the ENV variable? Like --x-use-nuget-cache?

You can open an issue with this question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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