[all-commits] [llvm/llvm-project] d2fa6b: [llvm-rc] Respect the executable specified in the ...

Martin Storsjö via All-commits all-commits at lists.llvm.org
Tue Mar 28 01:03:53 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: d2fa6b694c2052cef1ddd507f6569bc84e3bbe35
      https://github.com/llvm/llvm-project/commit/d2fa6b694c2052cef1ddd507f6569bc84e3bbe35
  Author: Martin Storsjö <martin at martin.st>
  Date:   2023-03-28 (Tue, 28 Mar 2023)

  Changed paths:
    M llvm/tools/llvm-rc/llvm-rc.cpp

  Log Message:
  -----------
  [llvm-rc] Respect the executable specified in the --preprocessor command

The arguments passed in this option were passed onto the child
process, but we still blindly used the clang binary that we had
found to sys::ExecuteAndWait as the intended executable to run.

If the user hasn't specified any custom --preprocessor command,
Args[0] is equal to the variable Clang.

This doesn't affect any tests, since the tests only print the
arguments it would try to execute (but not the first parameter to
sys::ExecuteAndWait), but there's no testes for executing it
(and validating that it did execute the right thing).

Differential Revision: https://reviews.llvm.org/D146793


  Commit: 282744a9ce18120dc0a6eceb02693b36980d9498
      https://github.com/llvm/llvm-project/commit/282744a9ce18120dc0a6eceb02693b36980d9498
  Author: Martin Storsjö <martin at martin.st>
  Date:   2023-03-28 (Tue, 28 Mar 2023)

  Changed paths:
    M llvm/tools/llvm-rc/llvm-rc.cpp

  Log Message:
  -----------
  [llvm-rc] Look for "clang-<major>" when locating a suitable preprocessor

In some cases, there's no adjacent executable named "clang" or
"clang-cl", but one name "clang-<major>". This logic doesn't
cover every possible deployment setup of course, but should
cover more fairly common/reasonable cases.

See
https://github.com/curl/curl-for-win/commit/caaae171ac43af5b883403714dafd42030d8de61#commitcomment-105808524
for discussion about a case where this would have been helpful.

Differential Revision: https://reviews.llvm.org/D146794


  Commit: 014e5c8d39c172a5b4bb1e1c75ced9bcb9ff2115
      https://github.com/llvm/llvm-project/commit/014e5c8d39c172a5b4bb1e1c75ced9bcb9ff2115
  Author: Martin Storsjö <martin at martin.st>
  Date:   2023-03-28 (Tue, 28 Mar 2023)

  Changed paths:
    M llvm/tools/llvm-rc/llvm-rc.cpp

  Log Message:
  -----------
  [llvm-rc] Fix the reference to the option for disabling preprocessing in a message

This was the original option name from the first iteration of the patch
that added the feature, but during review, a different name was suggested
and preferred - but the reference in the helpful message was missed.

Differential Revision: https://reviews.llvm.org/D146796


  Commit: dc41f387e357d87117b29e0da9e741be6884287c
      https://github.com/llvm/llvm-project/commit/dc41f387e357d87117b29e0da9e741be6884287c
  Author: Martin Storsjö <martin at martin.st>
  Date:   2023-03-28 (Tue, 28 Mar 2023)

  Changed paths:
    M llvm/tools/llvm-rc/llvm-rc.cpp

  Log Message:
  -----------
  [llvm-rc] Remove transitional preprocessing fallback logic

When preprocessing was integrated to llvm-rc in 2021, this was a
new requirement (previously one could execute llvm-rc without a
suitable preprocessing tool to be available).

As a transitional helper, llvm-rc fell back on skipping preprocessing
if no suitable tool was found (with a warning printed), but users
could pass an llvm-rc specific option to silence the warning, if they
explicitly want to run the tool without preprocessing.

Now 2 years later, remove the transitional helper - error out if
preprocessing failed. The option for disabling preprocessing remains.

Differential Revision: https://reviews.llvm.org/D146797


  Commit: 0f4c6b120f21d582ab7c5c4f2b2a475086c34938
      https://github.com/llvm/llvm-project/commit/0f4c6b120f21d582ab7c5c4f2b2a475086c34938
  Author: Martin Storsjö <martin at martin.st>
  Date:   2023-03-28 (Tue, 28 Mar 2023)

  Changed paths:
    M llvm/test/tools/llvm-rc/windres-preproc.test
    M llvm/tools/llvm-rc/llvm-rc.cpp

  Log Message:
  -----------
  [lvm-windres] Try to match GNU windres regarding handling of unescaped quotes

Some background context: GNU windres invokes the preprocessor in
a subprocess. Some windres options are passed through to the
preproocessor, e.g. -D options for predefining defines.
When GNU windres passes these options onwards, it takes the options
in exact the form they are received (in argv or similar) and
assembles them into a single preprocessor command string which gets
interpreted by a shell (IIRC via the popen() function, or similar).

When LLVM invokes subprocesses, it does so via APIs that take
properly split argument vectors, to avoid needing to worry about
shell quoting/escaping/unescaping. But in the case of LLVM windres,
we have to emulate the effect of the shell parsing done by popen().

Most of the relevant cases are already taken care of here, but this
patch fixes an uncommon case encountered in
https://github.com/llvm/llvm-project/issues/57334.
(This case is uncommon since it doesn't do what one would want to;
the quotes need to be escaped more to work as intended through
the popen() shell).

Differential Revision: https://reviews.llvm.org/D146848


Compare: https://github.com/llvm/llvm-project/compare/7cf203e73922...0f4c6b120f21


More information about the All-commits mailing list