Skip to content
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

msvc, mingw: do not use cygpath by default #140

Closed
wants to merge 5 commits into from

Conversation

nojb
Copy link
Collaborator

@nojb nojb commented Jun 1, 2024

The default value of use_cygpath is Try, contrary to what the help string

" Do not use cygpath (default for msvc, mingw)";
says.

Fixes #139

@nojb nojb force-pushed the msvc_mingw_no_cygpath_by_default branch from f65f3ea to 45f89d2 Compare June 1, 2024 08:28
@nojb nojb force-pushed the msvc_mingw_no_cygpath_by_default branch from bf4b5a7 to 8372a50 Compare June 1, 2024 08:30
@nojb
Copy link
Collaborator Author

nojb commented Jun 1, 2024

The mingw build fails with this change, needs to be investigated.

@dra27
Copy link
Member

dra27 commented Jun 1, 2024

The mingw-w64 port under Cygwin definitely needs to do this to parse x86_64-w64-mingw32-gcc -print-search-dirs, but MSYS2 doesn't. cf. on Cygwin:

DRA@Thor ~
$ x86_64-w64-mingw32-gcc -print-search-dirs | fgrep libraries
libraries: =/usr/lib/gcc/x86_64-w64-mingw32/11/:/usr/lib/gcc/x86_64-w64-mingw32/11/../../../../x86_64-w64-mingw32/lib/x8./../x86_64-w64-mingw32/lib/../lib/:/usr/x86_64-w64-mingw32/sys-root/mingw/lib/x86_64-w64-mingw32/11/:/usr/x86_64-w64-mi2/11/../../../../x86_64-w64-mingw32/lib/:/usr/x86_64-w64-mingw32/sys-root/mingw/lib/

and on MSYS2:

DRA@Thor ~
$ gcc -print-search-dirs | fgrep libraries
libraries: =C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/;C:/msys64/mingw64/bin/../lib/gcc/;C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/../../../../x86_64-w64-mingw32/lib/x86_64-w64-mingw32/14.1.0/;C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/../../../../x86_64-w64-mingw32/lib/../lib/;C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/../../../x86_64-w64-mingw32/14.1.0/;C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/../../../../lib/;D:/a/msys64/mingw64/lib/x86_64-w64-mingw32/14.1.0/;D:/a/msys64/mingw64/lib/../lib/;C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/../../../../x86_64-w64-mingw32/lib/;C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/14.1.0/../../../;D:/a/msys64/mingw64/lib/

I haven't checked the history, but I think that the behaviour may have been different way back in the early days when the mingw port was accessed with gcc -mno-cygwin (and the system search dirs may have been hard-coded?), but I hadn't gone through the git history.

I think the best behaviour might be:

  • Don't use cygpath at all for the MSVC port (as here already)
  • For mingw, don't use cygpath as a default but do explicitly use it when parsing the search path for Cygwin only

I think all the pieces are in place for the second part, but for now a quick fix would just be to do the first (it's obviously unequivocally wrong to be calling cygpath for MSVC).

@nojb
Copy link
Collaborator Author

nojb commented Jun 1, 2024

Thanks for the quick review (on a weekend) :)

for now a quick fix would just be to do the first (it's obviously unequivocally wrong to be calling cygpath for MSVC).

OK, I did this.

@dra27
Copy link
Member

dra27 commented Jun 26, 2024

I'm still dithering over whether this is the right thing to do (versus updating the documentation!). What I'm wondering is whether there are any MSVC users out there (compiling in Cygwin - i.e. using Unix-like infrastructure) where they do end up passing Cygwin-style paths (e.g. for -I) and so where this has been quietly working?

@nojb
Copy link
Collaborator Author

nojb commented Jul 20, 2024

Makes sense; probably updating the documentation is the way to go. A bit underwater at the moment though, so closing this for the time being :) Thanks!

@nojb nojb closed this Jul 20, 2024
@nojb nojb deleted the msvc_mingw_no_cygpath_by_default branch July 20, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSVC flexlink appears to attempt cygpath
2 participants