Refactor tools.cpp to homogenize system tools policies #540
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
include/vcpkg/fwd/vcpkgpaths.h
Outdated
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| namespace vcpkg | |||
| { | |||
| struct DownloadManager; | |||
There was a problem hiding this comment.
Don't think this is the right fwd header for that?
Can you add an e2e test for that? (I think setting that env var and doing |
| [this]() { return this->fs.read_contents(this->xml_config, VCPKG_LINE_INFO); }); | ||
| } | ||
|
|
||
| virtual const Path& get_tool_path(StringView tool) const override { return get_tool_pathversion(tool).p; } |
There was a problem hiding this comment.
Can we make callers just do that now if we aren't going to special case it anymore?
It looks like before the special case was there so that we didn't have to try to get a version when we didn't know how to extract one...
There was a problem hiding this comment.
I like this more direct function because 90% of callers just want the path so they can proceed to build a command out of it.
If you would like to see get_tool_version() replaced with get_tool_pathversion().version while keeping get_tool_path(), I can do that.
There was a problem hiding this comment.
You could make it nonvirtual and in the interface if it's to make callers prettier.
There was a problem hiding this comment.
That would be fine by me -- I thought you hated having any code in the interface though? Or is that only for virtual member default implementations?
This PR subsumes #181.
This PR refactors the internal implementation of
ToolsCacheto achieve several goals:VcpkgPathsfrom theToolsCache/ToolsProviderinterface by injecting the specific data requiredVcpkgPathsto ToolsCache in thearchives.cppfunctionstools.cppVCPKG_FORCE_SYSTEM_BINARIES&VCPKG_FORCE_DOWNLOADED_BINARIESFetch tool without an entry
Fetch missing tool with special failure instructions
Successful fetch
CMake from system +
VCPKG_FORCE_SYSTEM_BINARIES/* todo: add more examples of error messages */