[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
This revision was automatically updated to reflect the committed changes.
DavidSpickett marked an inline comment as done.
Closed by commit rGfe5912249efa: [clang][Driver] Fix tool path priority test
failures (authored by DavidSpickett).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83055/new/
https://reviews.llvm.org/D83055
Files:
clang/test/Driver/program-path-priority.c
clang/test/lit.cfg.py
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
config.substitutions.append(
('%src_include_dir', config.clang_src_dir + '/include'))
+config.substitutions.append(
+('%target_triple', config.target_triple))
# Propagate path to symbolizer for ASan/MSan.
llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- clang/test/Driver/program-path-priority.c
+++ clang/test/Driver/program-path-priority.c
@@ -13,6 +13,11 @@
/// so only name priority is accounted for, unless we fail to find
/// anything at all in the prefix.
+/// Note: All matches are expected to be at the end of file paths.
+/// So we match " on the end to account for build systems that
+/// put the name of the compiler in the build path.
+/// E.g. /build/gcc_X.Y.Z/0/...
+
/// Symlink clang to a new dir which will be its
/// "program path" for these tests
// RUN: rm -rf %t && mkdir -p %t
@@ -21,14 +26,18 @@
/// No gccs at all, nothing is found
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s
-// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
-// NO_NOTREAL_GCC-NOT: /gcc
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc"
+/// Some systems will have "gcc-x.y.z" so for this first check
+/// make sure we don't find "gcc" or "gcc-x.y.z". If we do find either
+/// then there is no point continuing as this copy of clang is not
+/// isolated as we expected.
+// NO_NOTREAL_GCC-NOT: {{/gcc[^/]*"}}
/// -gcc in program path is found
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
-// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc"
/// -gcc on the PATH is found
// RUN: mkdir -p %t/env
@@ -36,74 +45,89 @@
// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
-// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc"
/// -gcc in program path is preferred to one on the PATH
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
-// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
-// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc"
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc"
/// On program path, -gcc is preferred to plain gcc
// RUN: touch %t/gcc && chmod +x %t/gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
-// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
-// NOTREAL_GCC_PREFERRED-NOT: /gcc
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc"
+// NOTREAL_GCC_PREFERRED-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc in program path
// RUN: rm %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
-// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc on the PATH
// RUN: rm %t/gcc
// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
-// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc"
+
+/// We cannot trust clang --version, or cmake's LLVM_DEFAULT_TARGET_TRIPLE
+/// to give us the one and only default triple.
+/// Can't trust cmake because on Darwin, triples have a verison appended to them.
+/// (and clang uses the versioned string to search)
+/// Can't trust --version because it will pad 3 item triples to 4 e.g.
+/// powerpc64le-linux-gn
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
miyuki added a comment. I have reproduced the type 1 failure and I confirm that this patch fixes the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
stevewan accepted this revision. stevewan added a comment. This revision is now accepted and ready to land. This LGTM, but since I'm not most familiar with the type 1 failure in description, let's see if other reviewers have further comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
DavidSpickett marked 2 inline comments as done. DavidSpickett added inline comments. Comment at: clang/test/Driver/program-path-priority.c:117 +/// Check file exists first in case $DEFAULT_TRIPLE == %target_triple +// RUN: file -E %t/$DEFAULT_TRIPLE-gcc 2>&1 > /dev/null && \ +// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix || true stevewan wrote: > DavidSpickett wrote: > > stevewan wrote: > > > Maybe I'm not seeing something obvious here, but I'm not aware of the > > > `file -E` usage, and on Linux I got `file: invalid option -- 'E'`. > > I was looking for a way to mimic [! -f ], which worked on the command > > line but not in a test. From my system's file: > > -E On filesystem errors (file not found etc), instead of handling > > the error as regu‐ > > lar output as POSIX mandates and keep going, issue an error > > message and exit. > > > > I didn't realise it was non-standard, so I've switched to mv || true with > > the error redirected so it won't confuse the verbose output. > I believe having just `mv || true` works, but I did like the idea of > pre-checking for the existence of the file, and the test is more robust with > the check. Is there a reason we wouldn't use `test -f` here? Honestly it slipped my mind that [ ] was the same as test. Updated to test && move || true which you're right is clearer. (one will always fail if the two triples are the same but at least we're only hiding the error we expect, rather than all errors mv could have) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
DavidSpickett updated this revision to Diff 277344.
DavidSpickett marked an inline comment as done.
DavidSpickett added a comment.
- Use more standard 'test -f' command before 'mv's
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83055/new/
https://reviews.llvm.org/D83055
Files:
clang/test/Driver/program-path-priority.c
clang/test/lit.cfg.py
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
config.substitutions.append(
('%src_include_dir', config.clang_src_dir + '/include'))
+config.substitutions.append(
+('%target_triple', config.target_triple))
# Propagate path to symbolizer for ASan/MSan.
llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- clang/test/Driver/program-path-priority.c
+++ clang/test/Driver/program-path-priority.c
@@ -13,6 +13,11 @@
/// so only name priority is accounted for, unless we fail to find
/// anything at all in the prefix.
+/// Note: All matches are expected to be at the end of file paths.
+/// So we match " on the end to account for build systems that
+/// put the name of the compiler in the build path.
+/// E.g. /build/gcc_X.Y.Z/0/...
+
/// Symlink clang to a new dir which will be its
/// "program path" for these tests
// RUN: rm -rf %t && mkdir -p %t
@@ -21,14 +26,18 @@
/// No gccs at all, nothing is found
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s
-// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
-// NO_NOTREAL_GCC-NOT: /gcc
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc"
+/// Some systems will have "gcc-x.y.z" so for this first check
+/// make sure we don't find "gcc" or "gcc-x.y.z". If we do find either
+/// then there is no point continuing as this copy of clang is not
+/// isolated as we expected.
+// NO_NOTREAL_GCC-NOT: {{/gcc[^/]*"}}
/// -gcc in program path is found
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
-// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc"
/// -gcc on the PATH is found
// RUN: mkdir -p %t/env
@@ -36,74 +45,89 @@
// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
-// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc"
/// -gcc in program path is preferred to one on the PATH
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
-// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
-// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc"
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc"
/// On program path, -gcc is preferred to plain gcc
// RUN: touch %t/gcc && chmod +x %t/gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
-// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
-// NOTREAL_GCC_PREFERRED-NOT: /gcc
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc"
+// NOTREAL_GCC_PREFERRED-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc in program path
// RUN: rm %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
-// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc on the PATH
// RUN: rm %t/gcc
// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
-// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc"
+
+/// We cannot trust clang --version, or cmake's LLVM_DEFAULT_TARGET_TRIPLE
+/// to give us the one and only default triple.
+/// Can't trust cmake because on Darwin, triples have a verison appended to them.
+/// (and clang uses the versioned string to search)
+/// Can't trust --version because it will pad 3 item triples to 4 e.g.
+/// powerpc64le-linux-gnu -> powerpc64le-unknown-linux-gnu
+/// (and clang use
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
stevewan added inline comments. Comment at: clang/test/Driver/program-path-priority.c:117 +/// Check file exists first in case $DEFAULT_TRIPLE == %target_triple +// RUN: file -E %t/$DEFAULT_TRIPLE-gcc 2>&1 > /dev/null && \ +// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix || true DavidSpickett wrote: > stevewan wrote: > > Maybe I'm not seeing something obvious here, but I'm not aware of the `file > > -E` usage, and on Linux I got `file: invalid option -- 'E'`. > I was looking for a way to mimic [! -f ], which worked on the command > line but not in a test. From my system's file: > -E On filesystem errors (file not found etc), instead of handling > the error as regu‐ > lar output as POSIX mandates and keep going, issue an error > message and exit. > > I didn't realise it was non-standard, so I've switched to mv || true with the > error redirected so it won't confuse the verbose output. I believe having just `mv || true` works, but I did like the idea of pre-checking for the existence of the file, and the test is more robust with the check. Is there a reason we wouldn't use `test -f` here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
DavidSpickett marked 2 inline comments as done. DavidSpickett added inline comments. Comment at: clang/test/Driver/program-path-priority.c:117 +/// Check file exists first in case $DEFAULT_TRIPLE == %target_triple +// RUN: file -E %t/$DEFAULT_TRIPLE-gcc 2>&1 > /dev/null && \ +// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix || true stevewan wrote: > Maybe I'm not seeing something obvious here, but I'm not aware of the `file > -E` usage, and on Linux I got `file: invalid option -- 'E'`. I was looking for a way to mimic [! -f ], which worked on the command line but not in a test. From my system's file: -E On filesystem errors (file not found etc), instead of handling the error as regu‐ lar output as POSIX mandates and keep going, issue an error message and exit. I didn't realise it was non-standard, so I've switched to mv || true with the error redirected so it won't confuse the verbose output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
DavidSpickett updated this revision to Diff 276952.
DavidSpickett added a comment.
- mv with stderr redirected instead of using non standard file -E flag
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83055/new/
https://reviews.llvm.org/D83055
Files:
clang/test/Driver/program-path-priority.c
clang/test/lit.cfg.py
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
config.substitutions.append(
('%src_include_dir', config.clang_src_dir + '/include'))
+config.substitutions.append(
+('%target_triple', config.target_triple))
# Propagate path to symbolizer for ASan/MSan.
llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- clang/test/Driver/program-path-priority.c
+++ clang/test/Driver/program-path-priority.c
@@ -13,6 +13,11 @@
/// so only name priority is accounted for, unless we fail to find
/// anything at all in the prefix.
+/// Note: All matches are expected to be at the end of file paths.
+/// So we match " on the end to account for build systems that
+/// put the name of the compiler in the build path.
+/// E.g. /build/gcc_X.Y.Z/0/...
+
/// Symlink clang to a new dir which will be its
/// "program path" for these tests
// RUN: rm -rf %t && mkdir -p %t
@@ -21,14 +26,18 @@
/// No gccs at all, nothing is found
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s
-// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
-// NO_NOTREAL_GCC-NOT: /gcc
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc"
+/// Some systems will have "gcc-x.y.z" so for this first check
+/// make sure we don't find "gcc" or "gcc-x.y.z". If we do find either
+/// then there is no point continuing as this copy of clang is not
+/// isolated as we expected.
+// NO_NOTREAL_GCC-NOT: {{/gcc[^/]*"}}
/// -gcc in program path is found
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
-// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc"
/// -gcc on the PATH is found
// RUN: mkdir -p %t/env
@@ -36,74 +45,87 @@
// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
-// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc"
/// -gcc in program path is preferred to one on the PATH
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
-// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
-// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc"
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc"
/// On program path, -gcc is preferred to plain gcc
// RUN: touch %t/gcc && chmod +x %t/gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
-// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
-// NOTREAL_GCC_PREFERRED-NOT: /gcc
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc"
+// NOTREAL_GCC_PREFERRED-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc in program path
// RUN: rm %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
-// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc on the PATH
// RUN: rm %t/gcc
// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
-// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc"
+
+/// We cannot trust clang --version, or cmake's LLVM_DEFAULT_TARGET_TRIPLE
+/// to give us the one and only default triple.
+/// Can't trust cmake because on Darwin, triples have a verison appended to them.
+/// (and clang uses the versioned string to search)
+/// Can't trust --version because it will pad 3 item triples to 4 e.g.
+/// powerpc64le-linux-gnu -> powerpc64le-unknown-linux-gnu
+/// (and clang uses the former to search)
+///
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
stevewan added inline comments. Comment at: clang/test/Driver/program-path-priority.c:117 +/// Check file exists first in case $DEFAULT_TRIPLE == %target_triple +// RUN: file -E %t/$DEFAULT_TRIPLE-gcc 2>&1 > /dev/null && \ +// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix || true Maybe I'm not seeing something obvious here, but I'm not aware of the `file -E` usage, and on Linux I got `file: invalid option -- 'E'`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
DavidSpickett updated this revision to Diff 276671.
DavidSpickett added a comment.
- Updated rm/mv command lines to account for $DEFAULT_TRIPLE being the same as
%target_triple.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83055/new/
https://reviews.llvm.org/D83055
Files:
clang/test/Driver/program-path-priority.c
clang/test/lit.cfg.py
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
config.substitutions.append(
('%src_include_dir', config.clang_src_dir + '/include'))
+config.substitutions.append(
+('%target_triple', config.target_triple))
# Propagate path to symbolizer for ASan/MSan.
llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- clang/test/Driver/program-path-priority.c
+++ clang/test/Driver/program-path-priority.c
@@ -13,6 +13,11 @@
/// so only name priority is accounted for, unless we fail to find
/// anything at all in the prefix.
+/// Note: All matches are expected to be at the end of file paths.
+/// So we match " on the end to account for build systems that
+/// put the name of the compiler in the build path.
+/// E.g. /build/gcc_X.Y.Z/0/...
+
/// Symlink clang to a new dir which will be its
/// "program path" for these tests
// RUN: rm -rf %t && mkdir -p %t
@@ -21,14 +26,18 @@
/// No gccs at all, nothing is found
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s
-// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
-// NO_NOTREAL_GCC-NOT: /gcc
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc"
+/// Some systems will have "gcc-x.y.z" so for this first check
+/// make sure we don't find "gcc" or "gcc-x.y.z". If we do find either
+/// then there is no point continuing as this copy of clang is not
+/// isolated as we expected.
+// NO_NOTREAL_GCC-NOT: {{/gcc[^/]*"}}
/// -gcc in program path is found
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
-// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc"
/// -gcc on the PATH is found
// RUN: mkdir -p %t/env
@@ -36,74 +45,89 @@
// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
-// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc"
/// -gcc in program path is preferred to one on the PATH
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
-// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
-// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc"
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc"
/// On program path, -gcc is preferred to plain gcc
// RUN: touch %t/gcc && chmod +x %t/gcc
// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
-// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
-// NOTREAL_GCC_PREFERRED-NOT: /gcc
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc"
+// NOTREAL_GCC_PREFERRED-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc in program path
// RUN: rm %t/notreal-none-elf-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
-// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc"
/// -gcc on the PATH is preferred to gcc on the PATH
// RUN: rm %t/gcc
// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
-// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc"
+
+/// We cannot trust clang --version, or cmake's LLVM_DEFAULT_TARGET_TRIPLE
+/// to give us the one and only default triple.
+/// Can't trust cmake because on Darwin, triples have a verison appended to them.
+/// (and clang uses the versioned string to search)
+/// Can't trust --version because it will pad 3 item triples to 4 e.g.
+/// powerpc64le-linux-gnu -> powerpc64le-unknown-linux-gnu
+/// (and clang uses t
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
MaskRay added inline comments. Comment at: clang/test/Driver/program-path-priority.c:117 // RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix +// RUN: mv %t/%target_triple-gcc %t/prefix // RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc If $DEFAULT_TRIPLE == %target_triple, this mv command will fail. The same applies to the rm below. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83055: [clang][Driver] Fix tool path priority test failures
DavidSpickett added a comment. For the record my logic here is that I could change the tool names to use the same name as --version. However that will break whatever mips toolchain needed this code in the first place. (https://reviews.llvm.org/D13340?id=36227#inline-108606) This default triple lookup code had some support to be removed anyway, so I'd rather patch up this test first before doing so. So that it's easier to roll back if there are still users of it. Having tests of the existing behaviour is a good thing and I don't think writing to both potential default triples undermines that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83055/new/ https://reviews.llvm.org/D83055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
