Change the version check invocation in install.ps1 #2708
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
When install.ps1 is executed inside MSYS (or cygwin I suspect) cmd is a shell script that invokes the real cmd. But in install.ps1 it tries to open this shell script instead of executing it. AFAICT the only reason to use cmd /c is to capture stderr because there is a bug with powersehll; but fzf prints the version information on stdout so we don't need to care about stderr. So we can just call the binary directly. This avoids the cmd issues.
|
@bkloster @jiangjianshan Hi, can you review this patch? |
|
FWIW Mozilla has decided to resolve this in a different way, so we no longer need this patch. I won't close the PR, but feel free to as this is an unusual execution environment. |
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.
When install.ps1 is executed inside MSYS (or cygwin I suspect) cmd
is a shell script that invokes the real cmd. But in install.ps1 it
tries to open this shell script instead of executing it.
AFAICT the only reason to use cmd /c is to capture stderr because
there is a bug with powersehll; but fzf prints the version information
on stdout so we don't need to care about stderr. So we can just call
the binary directly. This avoids the cmd issues.
This is desirable because 0.23.0 removed x86 windows binaries, meaning the ./install script no longer works if you are in an x86 shell. This affects Firefox developers in particular because our build environment tries to install fzf automatically but MozillaBuild is 32-bit causing it to fail.