Skip to content

Write manifest "vcpkg install" output to stdout#15187

Merged
BillyONeal merged 2 commits intomicrosoft:masterfrom
PazerOP:pazerop/manifest-install-echo
Dec 21, 2020
Merged

Write manifest "vcpkg install" output to stdout#15187
BillyONeal merged 2 commits intomicrosoft:masterfrom
PazerOP:pazerop/manifest-install-echo

Conversation

@PazerOP
Copy link
Copy Markdown
Contributor

@PazerOP PazerOP commented Dec 18, 2020

First pull request to this project, so there might be things I'm overlooking.

  • What does your PR fix? Fixes #

In c902748, the output from the vcpkg install command run by manifest mode was written to a file. This had the side effect of no longer writing that output to stdout/err, which can be a very unfortunate change if your dependencies managed by vcpkg take a long time to install: you will sit there with the only output being "Running vcpkg install" at the very beginning, and "Running vcpkg install - done" at the very end. Since there is otherwise no progress indicator, I believe it beneficial to have that vcpkg install output visible in the console.

CMake's execute_process functionality has support for ECHO_OUTPUT_VARIABLE and ECHO_ERROR_VARIABLE, but unfortunately these only apply to the variable output methods, not the file output methods. This pull request stores the output of the vcpkg install command into a new variable _VCPKG_MANIFEST_INSTALL_LOGTEXT, and then after the vcpkg install command is finished, the output is written into the vcpkg-manifest-install.log file.

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    No other changes were made besides vcpkg.cmake. I believe this question is intended for users who are making changes to ports.

  • Does your PR follow the maintainer guide?
    I believe so, but again, my understanding is that this question is intended for people that are making changes to ports.

@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 18, 2020
`ECHO_OUTPUT_VARIABLE` and `ECHO_ERROR_VARIABLE` were only added in CMake 3.18. This change prevents those parameters from being added to the `execute_process` command when using older versions of CMake.
@ghost
Copy link
Copy Markdown

ghost commented Dec 18, 2020

CLA assistant check
All CLA requirements met.

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Dec 21, 2020
@BillyONeal BillyONeal merged commit 730187b into microsoft:master Dec 21, 2020
@PazerOP PazerOP deleted the pazerop/manifest-install-echo branch December 21, 2020 22:20
ryukw7 pushed a commit to ryukw7/vcpkg that referenced this pull request Dec 24, 2020
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:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants