[PATCH] D119144: [tests][Driver] Pass an empty sysroot for `DEFAULT_SYSROOT` builds

[Carlo Cabrera via Phabricator via cfe-commits]

carlocab created this revision.
carlocab added a reviewer: abidh.
carlocab added a project: clang.
Herald added a subscriber: ki.stfu.
carlocab requested review of this revision.
Herald added a subscriber: cfe-commits.

The `baremetal-sysroot` test fails when the toolchain is configured with
`DEFAULT_SYSROOT`. So, to emulate not having passed one at all, let's
pass an empty sysroot instead.

This simply follows D79694 , D66834 
, and D51972 
. D51972  in 
particular was
meant to address a failing test for behaviour when `--sysroot` is not
passed and `DEFAULT_SYSROOT` is set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119144

Files:
  clang/test/Driver/baremetal-sysroot.cpp


Index: clang/test/Driver/baremetal-sysroot.cpp
===
--- clang/test/Driver/baremetal-sysroot.cpp
+++ clang/test/Driver/baremetal-sysroot.cpp
@@ -10,7 +10,7 @@
 // RUN: ln -s %clang %T/baremetal_default_sysroot/bin/clang
 
 // RUN: %T/baremetal_default_sysroot/bin/clang -no-canonical-prefixes %s -### 
-o %t.o 2>&1 \
-// RUN: -target armv6m-none-eabi \
+// RUN: -target armv6m-none-eabi --sysroot= \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
 // CHECK-V6M-C: "{{.*}}clang{{.*}}" "-cc1" "-triple" 
"thumbv6m-none-unknown-eabi"
 // CHECK-V6M-C-SAME: "-internal-isystem" 
"{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"


Index: clang/test/Driver/baremetal-sysroot.cpp
===
--- clang/test/Driver/baremetal-sysroot.cpp
+++ clang/test/Driver/baremetal-sysroot.cpp
@@ -10,7 +10,7 @@
 // RUN: ln -s %clang %T/baremetal_default_sysroot/bin/clang
 
 // RUN: %T/baremetal_default_sysroot/bin/clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target armv6m-none-eabi \
+// RUN: -target armv6m-none-eabi --sysroot= \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
 // CHECK-V6M-C: "{{.*}}clang{{.*}}" "-cc1" "-triple" "thumbv6m-none-unknown-eabi"
 // CHECK-V6M-C-SAME: "-internal-isystem" "{{.*}}/baremetal_default_sysroot{{[/\\]+}}bin{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}armv6m-none-eabi{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Carlo Cabrera via Phabricator via cfe-commits]

carlocab added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2136
+} else {
+sdk = "macosx";
+}

Since we’re already doing a bunch of triple checks above, it probably doesn’t 
hurt to check that we really are targeting macOS here.

This may not help with any real use cases, but, in the past, some tests get 
broken when LLVM is configured with `-DDEFAULT_SYSROOT`, and this change is 
likely to break those tests in the same way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Carlo Cabrera via Phabricator via cfe-commits]

carlocab added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2147
+default:
+  // We shouldn't get here, unless the target OS doesn't have an SDK.
+  break;

`llvm_unreachable`? Or `assert`, at least.



Comment at: clang/test/Driver/darwin-sdkroot.c:1
-// Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// REQUIRES: system-darwin
+

I'm not sure this is the right move here, since there are a number of other 
test cases in this file that don't require `system-darwin`. We probably do want 
those tests to run on more buildbots to make sure problems with them are caught 
sooner rather than later.

Maybe we can check that `/usr/bin/xcrun` is executable instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136315/new/

https://reviews.llvm.org/D136315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119144: [tests][Driver] Pass an empty sysroot for `DEFAULT_SYSROOT` builds

[Carlo Cabrera via Phabricator via cfe-commits]

carlocab added a comment.

Thanks Petr. I don’t have commit access, so I’d appreciate it if you could 
commit this for me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119144/new/

https://reviews.llvm.org/D119144

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119144: [tests][Driver] Pass an empty sysroot for `DEFAULT_SYSROOT` builds

[Carlo Cabrera via Phabricator via cfe-commits]

carlocab added a comment.
Herald added a project: All.

Per suggestion on Discord, here’s my git author line:

Carlo Cabrera 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119144/new/

https://reviews.llvm.org/D119144

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits