search

mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.33k stars 1.53k forks source link

linkers: Fix RSP syntax for linkers invoked with clang #11715

Open kasper93 opened 1 year ago

kasper93 commented 1 year ago

Fixes: #8981 Fixes: 2be074b1d445fcd30535bcf7518f8ce0738bcbf3

xclaesse commented 1 year ago

(Ignore my previous comments, I have a better understanding now)

What feels wrong is Compiler.rsp_file_syntax() returns self.linker.rsp_file_syntax(). See https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/compilers.py#L632 and https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/compilers.py#L1326. If linker and compiler can have different format, ninja should get the value for compiler and linker separately when they are direct. See generate_dynamic_link_rules() it does options = self._rsp_options(compiler). Shouldn't that be options = self._rsp_options(compiler.linker if compiler.linker.direct else compiler) or something like that?

kasper93 commented 1 year ago

What feels wrong is Compiler.rsp_file_syntax() returns self.linker.rsp_file_syntax().

I need to dig into it to understand, but looks like current logic considers only the linker input to be "rspable". I guess linker commands are commonly the longest one. I will look into it when I have time.

xclaesse commented 1 year ago

I guess linker commands are commonly the longest one.

That's the most likely indeed, but ninja backends calls self._rsp_options(compiler) in generate_compile_rule_for(), so that gets used to compile single objects as well, as far as I understand.

kasper93 commented 9 months ago

@xclaesse: I've updated the PR. I think is all I can do. I never intended to start refactoring half of codebase, but only fix an issue that prevents us from using meson with MSVC clang.

There is get_argument_syntax() and rsp_file_syntax() which ultimately means exactly the same thing, but refactoring all this requires in-depth look into every compiler/linker definition, which I'm not willing to do at this point. I fact 1.2.0 meson is not working at all for me, so it is even harder to work on it.

There is also can_linker_accept_rsp() which again does the same thing, because it could simply be merged with rsp_file_syntax() which would return None or something to signal that no rsp is supported.

kasper93 commented 4 weeks ago

Rebased. Let me know what can I do so we can fix this regression upstream.

Currently I have to use custom meson build for https://github.com/mpv-player/mpv/pull/14038 which is not great solution for us.

Thanks.

kasper93 commented 3 weeks ago

Could we merge this as a regression fix and delegate any improvement to future PRs?

benoit-pierre commented 2 weeks ago

Related #12252?

kasper93 commented 2 weeks ago

Related #12252?

Doesn't look like it is. In this case response file is used, but is generated with wrong escape sequence for given compiler/linker invocation.