mstorsjo / msvc-wine

Scripts for setting up and running MSVC in Wine on Linux
Other
684 stars 83 forks source link

Absolute paths in response files also need to be handled #93

Open marcoesposito1988 opened 1 year ago

marcoesposito1988 commented 1 year ago

The relatively low limit on command line length of the Windows tools can create problems when linking large libraries. link.exe was silently hanging in my case, and I spent hours pulling out my hair to understand why.

The typical way of dealing with this is using response files: the content of the command line is written to a file, and the path to this file is passed instead of the arguments as link.exe @Path/To/Response/File.rsp .... However, when cross-compiling with wine this doesn't work out of the box because Ninja thinks that we are on Linux and hence we don't need to do this by default.

A workaround when using Ninja is setting CMAKE_NINJA_FORCE_RESPONSE_FILE, which will do the correct thing. We then have another problem, which is that the absolute paths in the response file are not in "wine format" (beginning with Z:), because they are not intercepted by the wrapper script.

I think I found a solution, and I have been using it without problems so far. Shall I open a PR, or do you see a problem with this approach?

mstorsjo commented 1 year ago

The relatively low limit on command line length of the Windows tools can create problems when linking large libraries. link.exe was silently hanging in my case, and I spent hours pulling out my hair to understand why.

The typical way of dealing with this is using response files: the content of the command line is written to a file, and the path to this file is passed instead of the arguments as link.exe @Path/To/Response/File.rsp .... However, when cross-compiling with wine this doesn't work out of the box because Ninja thinks that we are on Linux and hence we don't need to do this by default.

A workaround when using Ninja is setting CMAKE_NINJA_FORCE_RESPONSE_FILE, which will do the correct thing. We then have another problem, which is that the absolute paths in the response file are not in "wine format" (beginning with Z:), because they are not intercepted by the wrapper script.

I think I found a solution, and I have been using it without problems so far. Shall I open a PR, or do you see a problem with this approach?

I think this sounds reasonable. Does @huangqinjin have any other opinions here?

I think it would be good if we would have some testing of this new codepath (by setting CMAKE_NINJA_FORCE_RESPONSE_FILE), perhaps it's enough to set that in test/test-cmake.sh?

Ideally I would maybe use something else than perl for the runtime rewriting, if it could be done with sed instead, that would feel more right to me, though. (Try to stick to the subset of sed that works identically across GNU and macOS.)

huangqinjin commented 1 year ago

I have some concern.

  1. The file path handling in command line options is not easy (and not exhaustive, no doubt) as you see https://github.com/mstorsjo/msvc-wine/blob/264e40f2d762e909b0f9cee5d6fa4e2da9fd61a4/wrappers/wine-msvc.sh#L27-L66 CMAKE_NINJA_FORCE_RESPONSE_FILE=TRUE makes cl.exe use @file as well as link.exe. So the best solution is to unify the handling of command line and response file.
  2. User can also provide a @file manually without using CMake. So I think modifying @file inplace is not a good idea.
  3. In case of CMake+Ninja, @file is generated by ninja on the fly and will be removed (unless -d keeprsp is passed to ninja) after command finishes. So it is okay to modify it. As I know those generated @file are under CMakeFiles, so matching @*CMakeFiles*.rsp is better in my opinion if we don't want to handle case 1 and 2 now.
  4. I also prefer sed if we now just want a simple solution to start dealing with @file.
mstorsjo commented 1 year ago

@huangqinjin Thanks for the valuable input here!

marcoesposito1988 commented 1 year ago

thanks @huangqinjin, really good feedback! I would then start working on points 3 and 4, if that makes sense for you all