msvc-wine icon indicating copy to clipboard operation
msvc-wine copied to clipboard
verified publisher icon mstorsjo

Escape absolute paths with Z: also in response files

Open marcoesposito1988 opened this issue 2 years ago • 13 comments

Solves the issue discussed in #93

marcoesposito1988 avatar Nov 27 '23 08:11 marcoesposito1988

Can @huangqinjin have a look at this?

mstorsjo avatar Dec 10 '23 21:12 mstorsjo

@marcoesposito1988 could you please provide some rsp files here for reference? You can pass -d keeprsp to ninja to prevent rsp files being removed .

huangqinjin avatar Dec 14 '23 06:12 huangqinjin

@marcoesposito1988 could you please provide some rsp files here for reference? You can pass -d keeprsp to ninja to prevent rsp files being removed .

Here is an example:

-DBOOST_ALL_NO_LIB -DBOOST_UUID_FORCE_AUTO_LINK -DEIGEN_HAS_STD_RESULT_OF=0 -DEIGEN_MPL2_ONLY -DMYPROJECTCORE_DLL -DMYPROJECT_ENABLE_ASSERTIONS -DMYPROJECT_LOG_CAPTURE_ORIGIN -DNOMINMAX -DWIN32 -D_CRT_SECURE_NO_WARNINGS -D_MBCS -D_SCL_SECURE_NO_WARNINGS -D_SILENCE_CXX17_ADAPTOR_TYPEDEFS_DEPRECATION_WARNING -D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING -D_USE_MATH_DEFINES -D_WINDLL -I/home/marco/projects/myproject/myprojectwithplugins/cmake-build-msvc-release/MyProjectSuite/MyProjectLib/MyProjectCore/MyProjectCore_autogen/include -I/home/marco/projects/myproject/myprojectwithplugins/MyProjectSuite/Core/include -I/home/marco/projects/myproject/myprojectwithplugins/MyProjectSuite/Core/ext -external:I/mnt/windows/Dev/myproject/externals/externals-vs2017-x64/eigen/3.4.0 -external:I/mnt/windows/Dev/myproject/externals/externals-vs2017-x64/zlib/1.2.11/include -external:I/mnt/windows/Dev/myproject/externals/externals-vs2017-x64/boost/1.70.0 -external:I/mnt/windows/Dev/myproject/externals/externals-vs2017-x64/zstd/1.5.0/include -external:I/mnt/windows/Dev/myproject/externals/externals-vs2019-x64/minizip/3.0.9/include -external:I/mnt/windows/Dev/myproject/externals/externals-vs2017-x64/pcg-cpp/0.98.1.1-ffd522e7 -external:I/mnt/windows/Dev/myproject/externals/externals-vs2017-x64/date-tz/3.0.1/include -external:W0 /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MD /O2 /Ob2 /DNDEBUG -std:c++17 /W4 /WX /experimental:external /external:W3 /Oi /bigobj /EHsc /FC /MP /wd4068 /wd4251 /wd4800 /wd4324 /Z7 /diagnostics:caret -openmp

marcoesposito1988 avatar Dec 15 '23 23:12 marcoesposito1988

@marcoesposito1988 thanks. Could you please also provide an example of link.exe?

huangqinjin avatar Dec 16 '23 01:12 huangqinjin

@marcoesposito1988 thanks. Could you please also provide an example of link.exe?

Here:

MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/MyProjectCore_autogen/mocs_compilation.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Assert.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/ByteBuffer.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/ByteBufferStream.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/ByteSize.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Compression.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Configurable.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Cryptography.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/DateTime.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/DynamicLib.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Encoding.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Hashing.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Log.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Parameter.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Platform.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Properties.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/PropertiesIO.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Random.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/RandomEngine.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Signal.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/String.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Threading.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Timer.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Zip.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Log/ConsoleSink.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Log/FileSink.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Log/Sink.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Log/Worker.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/Directory.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/File.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/Filesystem.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/Path.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/TemporaryFile.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/TemporaryDirectory.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Filesystem/Url.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Circle.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Cuboid.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Ellipse.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Intersection.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Line.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/LineHelpersImpl.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Plane.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Polygon.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Rectangle.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Geometry/Triangle.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Resource/FilesystemRepository.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Resource/Resource.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Threading/PlaybackTimer.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Threading/StoppableThread.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Threading/ThreadPool.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/src/Utils/Version.cpp.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/ext/monocypher/monocypher.c.obj
MyProjectSuite/MyProjectLib/MyProjectCore/CMakeFiles/MyProjectCore.dir/MyProjectVersion.rc.res  Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/zlib/1.2.11/lib/zlib.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_thread-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_filesystem-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_date_time-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_system-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_chrono-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_regex-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_locale-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_program_options-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/zstd/1.5.0/lib/zstd_static.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2019-x64/minizip/3.0.9/lib/libminizip.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2019-x64/minizip/3.0.9/lib/bzip2.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2019-x64/minizip/3.0.9/lib/liblzma.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/date-tz/3.0.1/lib/Release/date-tz.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/boost/1.70.0/lib/libboost_atomic-vc141-mt-x64-1_70.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/zlib/1.2.11/lib/zlib.lib   Z:/mnt/windows/Dev/myProject/externals/externals-vs2017-x64/zstd/1.5.0/lib/zstd_static.lib   kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib

marcoesposito1988 avatar Dec 16 '23 01:12 marcoesposito1988

@marcoesposito1988 Thanks again.

  1. I think the regex can be simplified to s@(^|[[:blank:]])((/[^/[:blank:]]*){2,})@\1z:\2@g
  2. We care about BSD sed (e.g. macOS). So referring to https://stackoverflow.com/a/24276470, we should
  • Use -E instead of -r.
  • Handle -i flag like https://github.com/mstorsjo/msvc-wine/blob/539b98e5874bd596b308886811a7e4c02e25c82b/wrappers/cl#L42-L47
  1. Since link.exe options are in form of /OPTION:arg, and colon is illegal for Windows paths, colon can be added to [^/:[:blank:]]. Such that we don't wrongly convert /LIBPATH:/mnt to z:/LIBPATH:/mnt. @mstorsjo @marcoesposito1988 your opinion?

huangqinjin avatar Dec 16 '23 02:12 huangqinjin

@marcoesposito1988 Thanks again.

  1. I think the regex can be simplified to s@(^|[[:blank:]])((/[^/[:blank:]]*){2,})@\1z:\2@g

Hmm, why do you want the {2,} part here - why do we need at least two path segments before doing this rewrite? Thinking out loud, I guess that you want it so that you don't accidentally rewrite an option like /MD into z:/MD? But what about e.g. /Fo/absolute/path?

  1. Since link.exe options are in form of /OPTION:arg, and colon is illegal for Windows paths, colon can be added to [^/:[:blank:]]. Such that we don't wrongly convert /LIBPATH:/mnt to z:/LIBPATH:/mnt. @mstorsjo @marcoesposito1988 your opinion?

That's probably a good thing to do.

FWIW, in the existing rewrite for actual arguments, we have a bit of a heuristic for what to convert; if $(dirname $potentialPath) is an existing directory (and not /) we do convert it. Here it's probably not quite as nice to do such heuristics.

I'd like to see it written out, either here in the PR or perhaps rather as a code comment, what cases the regex is designed around and how it tries to handle them.

mstorsjo avatar Dec 21 '23 10:12 mstorsjo

 But what about e.g. /Fo/absolute/path?

To keep it simple and correct, I think we should restrict the rewritten to only link.exe (probably lib.exe as well). Because:

  1. The original issue is about link.exe.
  2. cl.exe accepts /Fo/absolute/path.

huangqinjin avatar Dec 21 '23 11:12 huangqinjin

But what about e.g. /Fo/absolute/path?

To keep it simple and correct, I think we should restrict the rewritten to only link.exe (probably lib.exe as well). Because:

  1. The original issue is about link.exe.
  2. cl.exe accepts /Fo/absolute/path.

That sounds reasonable - there's much more seldom any need for response files for compilation. It'd be good to clarify these design aspects in a code comment.

mstorsjo avatar Dec 21 '23 11:12 mstorsjo

Then we need a way to detect which tool is running in wine-msvc.sh.

huangqinjin avatar Dec 21 '23 11:12 huangqinjin

Then we need a way to detect which tool is running in wine-msvc.sh.

Ah, right. Or what if we just have patterns that primarily match linker commands, is there any risk of it breaking anything if it would be applied on cl.exe? Or is it just the case that we wouldn't end up handling all the tricky cases in cl.exe command lines?

mstorsjo avatar Dec 21 '23 11:12 mstorsjo

Then we need a way to detect which tool is running in wine-msvc.sh.

Ah, right. Or what if we just have patterns that primarily match linker commands, is there any risk of it breaking anything if it would be applied on cl.exe? Or is it just the case that we wouldn't end up handling all the tricky cases in cl.exe command lines?

We are rewriting /absolute/path in link.exe command line, which is hard to distinguish from /Fo/absolute/path in cl.exe command line. So using tool name is much simple.

huangqinjin avatar Dec 21 '23 12:12 huangqinjin

Then we need a way to detect which tool is running in wine-msvc.sh.

Ah, right. Or what if we just have patterns that primarily match linker commands, is there any risk of it breaking anything if it would be applied on cl.exe? Or is it just the case that we wouldn't end up handling all the tricky cases in cl.exe command lines?

We are rewriting /absolute/path in link.exe command line, which is hard to distinguish from /Fo/absolute/path in cl.exe command line. So using tool name is much simple.

Right, that's true - I agree.

mstorsjo avatar Dec 21 '23 12:12 mstorsjo