Deal with Windows path conventions (backslashes, .exe, etc.) #3350
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
|
The two commits about installation add useful warnings and shims to allow programs to be installed correctly (which should be becoming less common with Dune, etc. anyway). The problem part is the variable interpolation. This is indeed arising from files mentioned in |
AltGr
reviewed
Jun 12, 2018
src/client/opamAction.ml
Outdated
| let base' = | ||
| {base with c = OpamFilename.Base.add_extension base.c "exe"} in | ||
| if OpamFilename.exists (OpamFilename.create build_dir base'.c) then begin | ||
| OpamConsole.warning "Added .exe to %s; .install file should be fixed" (OpamFilename.Base.to_string base.c); |
There was a problem hiding this comment.
It's best in general to avoid displaying messages that only make sense to developpers/packagers to the end users, unless they have a clear way to report; but I am not sure how best to do this.
There was a problem hiding this comment.
Perhaps also noting that the package maintainer should be informed?
|
I had some thoughts on the
|
dra27
force-pushed
the
windows-filenames
branch
from
713209d
to
002801d
Compare
|
Refactored the interface for the warnings and altered so that running with |
Identifies Windows executables, DLLs and Unix shell scripts based on their content. This provides a primitive for identifying package installation problems (installing executable without .exe extension, expecting a #! script to work, etc.) Signed-off-by: David Allsopp <david.allsopp@metastack.com>
The install command doesn't make sense for Windows. OpamSystem.install altered to use copying instead and also various checks added to guard against package errors. In particular, .exe is automatically added to executables (with a warning) if exec is true and warnings are displayed if Cygwin-linked files or Unix shell-scripts are installed. Signed-off-by: David Allsopp <david.allsopp@metastack.com>
As with opam-installer, if a file cannot be found the .exe version is attempted. If this is found, then the file is installed using a .exe extension. A warning advising that the package should be fixed is also displayed! Signed-off-by: David Allsopp <david.allsopp@metastack.com>
Windows paths will routinely contain backslash characters, meaning that
opam's variable interpolation (OpamFilter.expand_interpolations_in_file)
will not work if the method is used to generate .config files.
The function now attempts to process the file using opam-file-format. If
this is successful, then variables expansions which occur inside quoted
and triple-quoted strings will have backslash and double-quote
characters escaped.
Thus, if the lib variable is C:\OPAM\system\lib then the following .in
file:
opam-version: "2.0"
variables {
lib: "%{lib}%/ocaml"
broken: %{lib}%
}
expands to:
opam-version: "2.0"
variables {
lib: "C:\\OPAM\\system\\lib/ocaml"
broken: C:\OPAM\system\lib
}
Signed-off-by: David Allsopp <david.allsopp@metastack.com>
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.
And the last portion of #3260 split off.
This borrows parts of
#3346and #3348 but also contains at least bit which needs further discussion.Further comment to follow.