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] Implementation of --x-binarysource=nuget (and friends) #12058

Merged

Conversation

ras0219
Copy link
Contributor

@ras0219 ras0219 commented Jun 22, 2020

This PR adds --x-binarysource=nuget, --x-binarysource=nugetconfig, and --x-binarysource=interactive.

Along the way, it adds bulk prefetch of packages (needed for good network performance) and updates our private copy of nuget.exe to v5.5.1 for improved Azure DevOps support.

This completes the Backend #2 and Configuration items of #11204.

Remaining work before marking as ready to merge:

  • Improve generated descriptions of NuGet packages

@MVoz
Copy link
Contributor

MVoz commented Jun 22, 2020

that's the good news

@ras0219 ras0219 marked this pull request as ready for review June 22, 2020 20:48
@@ -53,14 +54,14 @@ namespace vcpkg::Help
{"owns", command_topic_fn<Commands::Owns::COMMAND_STRUCTURE>},
{"remove", command_topic_fn<Remove::COMMAND_STRUCTURE>},
{"search", command_topic_fn<Commands::Search::COMMAND_STRUCTURE>},
{"binarycaching", help_topic_binary_caching},
Copy link
Contributor

Choose a reason for hiding this comment

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

Could probably be in alphabetical order

@JackBoosY JackBoosY self-assigned this Jun 23, 2020
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

This all looks good, the only thing I'm 'request changes' over are the missing unit tests for parts that should be easy to test:

  • XmlSerializer / XmlWriter
  • Strings::find_first_of
  • reformat_version

(I made other test suggestions herein but I don't necessarily consider them easy enough to test to 'Request Changes' over, but seeing tests added for them would make me happy; and all other code review comments herein are nitpicks)

toolsrc/src/vcpkg/base/strings.cpp Show resolved Hide resolved
toolsrc/src/vcpkg/base/system.process.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/base/system.process.cpp Show resolved Hide resolved
toolsrc/include/vcpkg/base/system.process.h Show resolved Hide resolved
toolsrc/src/vcpkg/binarycaching.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/binarycaching.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/binarycaching.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/binarycaching.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/binarycaching.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/binarycaching.cpp Outdated Show resolved Hide resolved
@ras0219-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ras0219 ras0219 requested a review from BillyONeal June 26, 2020 18:59
@ras0219-msft ras0219-msft merged commit 91e798a into microsoft:master Jun 26, 2020
@ras0219-msft
Copy link
Contributor

Thanks everyone for the Code Review!

strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
…soft#12058)

* [vcpkg] Initial implementation of --x-binarysource=nuget

* [vcpkg] Remove double-double quoting of CMake arguments

* [vcpkg] Update nuget.exe to 5.5.1 to support Azure DevOps Artifacts

* [vcpkg] Enable batching of NuGet server calls with prefetch(). Add `interactive` binarysource.

* [vcpkg] Add `nugetconfig` binary source

* [vcpkg] Short circuit querying remote NuGet servers once all refs are found

* [vcpkg] Add experimental help for binary caching

* [vcpkg] Improved NuGet cache package descriptions and version formatting

* [vcpkg] Rename `CmdLineBuilder::build()` to extract()

* [vcpkg-help] Ascii-betize help topics

* [vcpkg] Add tests for cmdlinebuilder. Improve handling of quotes and slashes.

* [vcpkg] Addressing code review comments

* [vcpkg] Add tests for vcpkg::reformat_version()

* [vcpkg] Added test for vcpkg::generate_nuspec()

* [vcpkg] Add tests for vcpkg::XmlSerializer

* [vcpkg] Addressed code review comment

* [vcpkg] Add test for vcpkg::Strings::find_first_of

* [vcpkg] Fix machine-specific paths in test for vcpkg::generate_nuspec()

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
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.

6 participants