[vcpkg-artifacts] Find git using vcpkg fetch git.
#569
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I wrote this forwarder to what ce's documentation said: --registry to point to the registry. But that command on the ce side just accepts the first parameter as the path to the registry, so the mapping was wrong. Also moves handling of --debug into the common bits.
…ing messages to stderr rather than stdout. Note that error messages still go to stdout. Get the path to git by running `vcpkg fetch`
BillyONeal
force-pushed
the
git-install-sources
branch
from
ae99184 to
95f1414
Compare
ras0219-msft
approved these changes
Jun 6, 2022
ce/ce/vcpkg.ts
Outdated
| subproc.on('error', (err) => { reject(err); }); | ||
| subproc.on('close', (code, signal) => { | ||
| if (code === 0) { accept(result); } | ||
| accept(undefined); |
There was a problem hiding this comment.
Should this be
Suggested change
| accept(undefined); | |
| reject("vcpkg returned nonzero exit code: " + result) |
or just reject(result)?
|
|
||
| protected: | ||
| MessageSink() = default; | ||
| ~MessageSink() = default; |
There was a problem hiding this comment.
Suggested change
| ~MessageSink() = default; | |
| virtual ~MessageSink() = default; |
Agreed nobody should ever be deleting it through the interface, but it seems good practice anyway.
There was a problem hiding this comment.
Being protected already gives it the right behavior here.
| virtual void print(Color c, StringView sv) override { msg::write_unlocalized_text_to_stdout(c, sv); } | ||
| }; | ||
|
|
||
| StdOutMessageSink stdout_sink_instance; |
There was a problem hiding this comment.
Suggested change
| StdOutMessageSink stdout_sink_instance; | |
| static StdOutMessageSink stdout_sink_instance; |
There was a problem hiding this comment.
Resolved by moving into an unnamed namespace. Ditto below.
| virtual void print(Color c, StringView sv) override { msg::write_unlocalized_text_to_stderr(c, sv); } | ||
| }; | ||
|
|
||
| StdErrMessageSink stderr_sink_instance; |
There was a problem hiding this comment.
Suggested change
| StdErrMessageSink stderr_sink_instance; | |
| static StdErrMessageSink stderr_sink_instance; |
|
Also added some smoke test instructions. I still need to figure out a good way to get CMake talking to rush to make this something seamless we can put into the e2e-tests. |
ras0219-msft
approved these changes
Jun 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--x-from-scripttofetchto make some attempt to make stdout only the resulting path so that scripts can embed fetching paths:VCPKG_COMMANDto find a vcpkg to launch, and to callvcpkg fetch gitto get git's path.