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
caml_expand_command_line behaves incorrectly #7473
Comments
Comment author: @protz For the record, I'm trying to add a new option to my tool of the form: krml -drop Hacl.Spec.* and I'm just trying to find a way to pass the argument "as-is" without anyone trying to be smart about shell-expansion... if there's a way to bypass caml_expand_command_line I'd be very happy about it. perhaps we should just drop caml_expand_command_line altogether... it seems like a surprising behavior |
Comment author: @xavierleroy We have two issues / questions here. 1- Globbing of file name patterns not working. Looks bad indeed, and I'm too lazy to get access to a Windows machine right now. What happens if you put a directory name? As in ".\camlprog.exe ./Hack.Spec.*" 2- Turning globbing off. Currently there is no way to do that. 2a- Why globbing, you ask? I believe(d) this is the behavior of a great many Windows command-line tools, including the MSVC compiler, and Cygwin commands when started from a bash shell. But maybe things have changed in the last 20 years... 2b- Should we give a way to turn globbing off? Perhaps. A flag in OCAMLRUNPARAM would do, for example, but is not terribly friendly. |
Comment author: @protz Note: I managed to reproduce on 4.03.0 and 4.04.0 versions, that I built myself. 1- Your suggestion performs proper path expansion. 2a- Granted; I just always start my commands from a cygwin shell so I always relied on bash's shell expansion feature, and never felt the need to have OCaml do it. 2b- unclear... perhaps I'm the only one who wants that sort of options? Hacl.Spec.* denotes the set of F* modules that start with that given prefix... |
Comment author: @xavierleroy Thanks for the additional test and info. I strongly suspect bad string surgery on lines 367-370 of byterun/win32.c. |
Comment author: @protz Some more info from Redmond. I asked an expert, and:
|
Comment author: @xavierleroy So, just by linking with a Microsoft-provided library, all the bugs become theirs? Great! thanks for the tip! Just need to check that setargv.obj is available in Mingw as well as in MSVC... |
Comment author: @protz Submitted a pull request for a quickfix that seems to work for me. Otherwise, yes, the more future-proof version is to use the .obj provided by MSVC... but this may prevent David Allsopp from building with circa 1997 versions of MSVC. Feel free to discard the pull request if you want to go with the cleaner version, it didn't take me too long to fix :) |
Comment author: @dra27 @protz the globbing expansion via setargv.obj in MSVCRT is as old the hills, you will not break my museum :p @xLeroy - your memory of Cygwin is correct, unless noglob is set in CYGWIN, it does a lot more work to expand wildcards when a Cygwin program is invoked from outside Cygwin (i.e. from cmd) and it's always and continues to be the case that Windows processes are expected to do globbing, not the shell. It would be good if there were a way to prevent the expansion being done (or opt into it?) - we could do that in the same way as Microsoft by adding some kind of setenv.cmo/setenv.cmx or we could consider something less inelegant (like providing the original argv somewhere!)... |
Comment author: @xavierleroy Fixed on trunk by merge of commit [561b846]. |
Original bug ID: 7473
Reporter: @protz
Status: resolved (set by @xavierleroy on 2017-02-06T18:58:31Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 4.04.0
Target version: 4.05.0 +dev/beta1/beta2/beta3/rc1
Fixed in version: 4.05.0 +dev/beta1/beta2/beta3/rc1
Category: platform support (windows, cross-compilation, etc)
Bug description
Consider this:
This is the native windows toolchain, compiled with x86_64-w64-mingw32.
This is the contents of the directory:
Note that I'm running this within a Windows shell to rule out Cygwin behavior.
Note also:
The text was updated successfully, but these errors were encountered: