Allow native Windows to use Cygwin tools #3348
Merged
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
dra27
added a commit
to dra27/opam
that referenced
this pull request
May 10, 2018
|
Wow, this is another heavy piece of work ! LGTM :) |
dra27
force-pushed
the
windows-cygwin-tools
branch
from
90132f0
to
f77f215
Compare
|
Assuming the CI passes, rebased to remove the duplicated commit and ready to merge. |
dra27
added a commit
to dra27/opam
that referenced
this pull request
Jun 10, 2018
This was referenced Oct 4, 2018
Various Cygwin tools (notably tar and rsync) don't recognise Windows bashslash-style paths and need to have the path converted using the cygpath utility. Add two functions to OpamSystem to support this: 1. OpamSystem.is_cygwin_variant uses cygcheck to determine if a given program is the Cygwin-compiled version (cygcheck shows DLLs linked, so the presence of cygwin1.dll is taken to imply a Cygwin-based command). Note that for a Cygwin build of OPAM (and indeed for any other Unix-like build), this function always returns false. For a native Cygwin build, the user should specify Cygwin-style paths. 2. OpamSystem.apply_cygpath converts its argument to a Cygwin-style path using the cygpath utility Signed-off-by: David Allsopp <david.allsopp@metastack.com>
Cygwin tar needs paths to be translated. With this patch, opam init can succesfully download and initialise ~/.opam using wget. Signed-off-by: David Allsopp <david.allsopp@metastack.com>
file:// URLs need to be translated to use Cygwin PATHs.
Patch the rsync/local backend to use Cygpath when calling rsync. Enables the backend. Signed-off-by: David Allsopp <david.allsopp@metastack.com>
Invoking processes on Windows is fundamentally different from Unix since Windows processes receive a single-string command line, rather than an argv array. Windows has a set of conventions for quoting these (totally independent of the Command Processor cmd.exe and unambiguously allowing any argv array to be encoded) which OCaml already follows in the Unix module. Cygwin, for various reasons, does not follow these conventions and various different shims are required, particularly to avoid Cygwin's globbing operations. Armed with the ability to call Cygwin executables, OpamProcess is also to locate a script processor, meaning it can handle #! scripts directly. This is less error prone than trying to run them using sh -c which on Cygwin has even more complex escaping rules which have to be navigated. Signed-off-by: David Allsopp <david.allsopp@metastack.com>
This preferred mode of invocation of Cygwin programs is intended to keep the command line length down, but it's causing problems for ocamlbuild. Signed-off-by: David Allsopp <david.allsopp@metastack.com>
dra27
force-pushed
the
windows-cygwin-tools
branch
from
f77f215
to
40e14ff
Compare
|
Assuming a CI pass with this rebase, is there anything to prevent this one being merged to master? (it's definitely not for 2.0, obviously!) |
|
No, ok for me. Thanks a lot for this work! |
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.
Yet another bit from #3260.
This also borrows a commit from #3346, so it will need rebasing before merge.This set of patches address two fundamental issues:
#!comments) and also ensures that command line arguments passed to Cygwin executables are correctly encoded/escaped, since they don't follow the same rules as native Windows executables. Note that this has nothing to do with shell escaping: this is dealing with the conversion of arguments as a string list to a single string (which is how Windows passes the command line) by the calling process being recovered as the same string array by the invoked process.