This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [libc++abi] Correctly quote source and target names in tests. NFC.
AbandonedPublic

Authored by ldionne on Dec 19 2020, 6:35 AM.

Details

Reviewers
Mordante
compnerd
curdeius
Group Reviewers
Restricted Project
Restricted Project
Summary

That fixes issues when using directories (source or build) containing e.g. spaces.
It *should* not break anything as it's just adding quotes where they were missing.

Diff Detail

Event Timeline

curdeius created this revision.Dec 19 2020, 6:35 AM
curdeius requested review of this revision.Dec 19 2020, 6:35 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 19 2020, 6:35 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
curdeius retitled this revision from [libc++] Correctly quote source and target names in tests. NFC. to [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..Dec 19 2020, 12:26 PM

One minor question, otherwise LGTM.

libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp
16

While you're at it, would it make sense to also guard %{cxx} and %{gdb}?

Ok. For the moment I just fixed what was failing with my current setup but will fix cxx and gdb too.

curdeius planned changes to this revision.Dec 20 2020, 9:00 AM
curdeius updated this revision to Diff 313181.Dec 21 2020, 12:42 PM
  • Quote {gdb}.
curdeius added inline comments.Dec 22 2020, 3:23 AM
libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp
16

FYI, cxx was already correctly quoted. I added quoting to gdb. There are other issues in other parts of LLVM tho (e.g. one cannot build clang in a build dir with spaces) but I'll handle that in different patches.

Mordante accepted this revision.Dec 22 2020, 11:50 AM

Thanks, LGTM.

compnerd requested changes to this revision.Jan 5 2021, 8:46 AM
compnerd added a subscriber: compnerd.
compnerd added inline comments.
libcxx/test/CMakeLists.txt
85

I believe that this doesn't work properly on Windows. The quoting rules on Windows are quite different. Using " instead of ' would work. However, depending on how this is used, CMake should be able to handle the quoting appropriately per platform.

libcxx/utils/libcxx/test/dsl.py
98

Please use pipes.quote for the argument quoting.

libcxx/utils/libcxx/test/format.py
73–74

Please use pipes.quote for the quoted parameters (and the same throughout the rest of the lit configuration).

This revision now requires changes to proceed.Jan 5 2021, 8:46 AM
ldionne commandeered this revision.Sep 8 2023, 8:53 AM
ldionne edited reviewers, added: curdeius; removed: ldionne.

[Github PR transition cleanup]

Abandoning since this would likely need to be re-done almost entirely if rebased.

Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2023, 8:53 AM
ldionne abandoned this revision.Sep 8 2023, 8:53 AM