-
Notifications
You must be signed in to change notification settings - Fork 55
msys-2.0.dl to cygwin1.dll switch patches #334
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
base: msys2-3.6.7
Are you sure you want to change the base?
Changes from all commits
6b394a8
d2fdc8a
1cc96c7
f4de5f4
5a976d7
3e941b8
4445374
a9ea83f
9482d18
88b485b
86ad1e0
2ea6dbc
fd5ce30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -356,36 +356,21 @@ create_child (char **argv) | |
| printf ("create_child: %s\n", one_line.buf); | ||
|
|
||
| SetConsoleCtrlHandler (NULL, 0); | ||
| /* Commit message for this code was: | ||
| "* strace.cc (create_child): Set CYGWIN=noglob when starting new process so that | ||
|
|
||
| Cygwin will leave already-parsed the command line alonw." | ||
|
|
||
| I can see no reason for it and it badly breaks the ability to use | ||
| strace.exe to investigate calling a Cygwin program from a Windows | ||
| program, for example: | ||
| strace mingw32-make.exe | ||
| .. where mingw32-make.exe finds sh.exe and uses it as the shell. | ||
| The reason it badly breaks this use-case is because dcrt0.cc depends | ||
| on globbing to happen to parse commandlines from Windows programs; | ||
| irrespective of whether they contain any glob patterns or not. | ||
|
|
||
| See quoted () comment: | ||
| "This must have been run from a Windows shell, so preserve | ||
| quotes for globify to play with later." | ||
|
|
||
| const char *cygwin_env = getenv ("MSYS"); | ||
|
|
||
| #if 0 | ||
| const char *cygwin_env = getenv ("CYGWIN"); | ||
| const char *space; | ||
|
|
||
| if (cygwin_env && strlen (cygwin_env) <= 256) // sanity check | ||
| if (cygwin_env && strlen (cygwin_env) <= 256) /* sanity check */ | ||
| space = " "; | ||
| else | ||
| space = cygwin_env = ""; | ||
| char *newenv = (char *) malloc (sizeof ("MSYS=noglob") | ||
| char *newenv = (char *) malloc (sizeof ("CYGWIN=noglob") | ||
| + strlen (space) + strlen (cygwin_env)); | ||
| sprintf (newenv, "MSYS=noglob%s%s", space, cygwin_env); | ||
| sprintf (newenv, "CYGWIN=noglob%s%s", space, cygwin_env); | ||
| _putenv (newenv); | ||
| */ | ||
| #endif | ||
|
|
||
|
Comment on lines
-359
to
+373
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a lot of dead code. It would make more sense to investigate properly what the original commit tried to do, why it was wrong, and then to drop that part altogether. But that should most likely come as a separate Merge Request.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do this for reduce |
||
| ret = CreateProcess (0, one_line.buf, /* command line */ | ||
| NULL, /* Security */ | ||
| NULL, /* thread */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up until this point, I see the keying factor to be the presence of the
MSYSand theCYGWINenvironment variable. It is not quite clear what this MR wants to do if both are set, but it is clear that it wants to trigger the MSYS2 vs Cygwin code paths depending on the presence of either environment variable.This hunk, however, tells a very different story (and one that I could understand much better): whether to run in MSYS2 or in Cygwin mode is keyed by the name of the DLL.
It sounds a bit fraught with peril to have both mechanisms. It would probably be a lot more desirable to key only on one of the two, not on both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temporary patch that bootstrap from "msys-2.0.dll" to "cygwin1.dll", once the bootstrap finished. we can then revert this patch directly.