-
Notifications
You must be signed in to change notification settings - Fork 362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various build process fixes for native Windows #2940
Merged
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
force-pushed
the
win-build
branch
3 times, most recently
from
May 15, 2017 19:55
3313e1c
to
912ed2c
Compare
If an error occurs launching a process, OPAM doesn't close the file handles (.err and .out) which on Windows results in error messages when trying to delete the logging directory (since on Windows you can't remove files which are still open).
If a file cannot be found, opam-installer will attempt to copy or delete the .exe file instead (and display a warning). opam.install therefore generated by configure.
Environment variable names are not case sensitive on Windows and PATH is by default returned as Path. Fix OpamStd.Env.get to use a case insensitive search on Windows (and similarly the internal function OpamSystem.env_var) and ensure that code which manipulates a list of environment variables allows for case-insensitive removal (opamState.add_to_env, etc.)
Windows builds of OPAM cannot rely on /bin/sh! Windows checks PATH environment variable instead.
rm -rf is only available if the user enables Cygwin or some other equivalent - converted to use cmd's rd command which is guaranteed to be available.
Detection of ocamlopt and .opt tools doesn't take into account .exe extensions on Windows. Added OpamStd.Sys.executable_name which appends .exe to a filename on Windows if it isn't already present. Use this to correct the detection of the ocaml-native and ocaml-native-tools global variables.
Directory separator should always be Filename.dir_sep, not a hard-coded "/"!
Small correction to use |
Thanks for taking the time to do all this! Only complaint I can find is a couple overly long lines ;) |
Oops 😊 - I don't think these were as heinous! Other PRs now all rebased - the inclusion of 352a51a means that the duplication of the |
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.
Last tranche (for now) from my windows-build branch. These are most of the less-invasive patches for Windows support. Combined with the rest of today's PRs, this gets the build system to the point that it can correctly compile and install opam using opam-installer (however, that opam is not capable of running opam init at this stage).