subject:"\[PATCH\] D133622\: \[clang\]\[test\] Disallow using the default module cache path in lit tests"

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-14 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

In D133622#3788218 , @bruno wrote:

>> I'm not sure how to deal with missing `env -u`.
>>
>> - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's 
>> interpretation of empty string for this variable. I'm not sure if the 
>> current behaviour (there will be no module cache in the cc1 at all) is 
>> intentional or useful.  Hesitant to change this behaviour.
>
> How about using `self.with_environment('CLANG_MODULE_CACHE_PATH', '')` so we 
> don't need to worry about using `env -u` to unset? My understand is that (1) 
> `getDefaultModuleCachePath` is the only place using 
> `CLANG_MODULE_CACHE_PATH`, and (2) `std::getenv` return nullptr if it's 
> empty, which will fallback to system provided path instead.

Where are you thinking we would call `self.with_environment` in this case? We 
explicitly do not want the system-provided path in most tests.  I think we 
would need to set it to `None`, since

> (2) `std::getenv` return nullptr if it's empty, which will fallback to system 
> provided path instead.

getenv returns empty string, not nullptr.

> Not sure it helps much but according to `option-X.test`, `system-aix` support 
> `unset`.

Heh, I'm worried we'll just hit an issue with a different platform (Windows?).  
If we can't find a better fix I guess I can at least attempt it and see what 
breaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

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

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits

bruno added a comment.

> I'm not sure how to deal with missing `env -u`.
>
> - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's 
> interpretation of empty string for this variable. I'm not sure if the current 
> behaviour (there will be no module cache in the cc1 at all) is intentional or 
> useful.  Hesitant to change this behaviour.

How about using `self.with_environment('CLANG_MODULE_CACHE_PATH', '')` so we 
don't need to worry about using `env -u` to unset? My understand is that (1) 
`getDefaultModuleCachePath` is the only place using `CLANG_MODULE_CACHE_PATH`, 
and (2) `std::getenv` return nullptr if it's empty, which will fallback to 
system provided path instead.

> - We could try to enumerate all the environments that don't support `env -u` 
> and disable these two tests on  those platforms.  So far it was just one AIX 
> bot, but I wouldn't be surprised if other less commonly used unixes have the 
> same issue.
>
> - We could make the command inscrutable, like `not env -u X true || env -u 
> ... real command ...` so that if `env -u X true` fails (presumably due to not 
> supporting `-u` option) we won't run the rest of the RUN line.

Not sure it helps much but according to `option-X.test`, `system-aix` support 
`unset`.

> If someone has a suggestion for a simple fix, I can try again.  But otherwise 
> I doubt it's worth putting much time into this.

Thanks for trying to improve this :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

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

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-12 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

Reverted due to failure on a bot 
https://lab.llvm.org/buildbot/#/builders/214/builds/3252

I'm not sure how to deal with missing `env -u`.

- We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's 
interpretation of empty string for this variable. I'm not sure if the current 
behaviour (there will be no module cache in the cc1 at all) is intentional or 
useful.  Hesitant to change this behaviour.
- We could try to enumerate all the environments that don't support `env -u` 
and disable these two tests on  those platforms.  So far it was just one AIX 
bot, but I wouldn't be surprised if other less commonly used unixes have the 
same issue.
- We could make the command inscrutable, like `not env -u X true || env -u ... 
real command ...` so that if `env -u X true` fails (presumably due to not 
supporting `-u` option) we won't run the rest of the RUN line.

If someone has a suggestion for a simple fix, I can try again.  But otherwise I 
doubt it's worth putting much time into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

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

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-12 Thread Ben Langmuir via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd96f526196ac: [clang][test] Disallow using the default 
module cache path in lit tests (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

Files:
  clang/test/Driver/modules-cache-path.m
  clang/test/Modules/driver.c
  llvm/utils/lit/lit/llvm/config.py


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced 
to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.
Index: clang/test/Modules/driver.c
===
--- clang/test/Modules/driver.c
+++ clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck 
-check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps 
%s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
 // RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s 
-### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
 
 // CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
Index: clang/test/Driver/modules-cache-path.m
===
--- clang/test/Driver/modules-cache-path.m
+++ clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-DEFAULT
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-DEFAULT
 // CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
 
 // RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.
Index: clang/test/Modules/driver.c
===
--- clang/test/Modules/driver.c
+++ clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
 // RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s -### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
 
 // CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
Index: clang/test/Driver/modules-cache-path.m
===
--- clang/test/Driver/modules-cache-path.m
+++ clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
 // CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
 
 // RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-09 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits

bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

Awesome!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

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

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-09 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir created this revision.
benlangmuir added reviewers: bruno, jansvoboda11, arphaman.
Herald added a subscriber: delcypher.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Make the default module cache path invalid when running lit tests so
that tests are forced to provide a cache path. This avoids accidentally
escaping to the system default location, and would have caught the
failure recently found in ClangScanDeps/multiple-commands.c.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133622

Files:
  clang/test/Driver/modules-cache-path.m
  clang/test/Modules/driver.c
  llvm/utils/lit/lit/llvm/config.py


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced 
to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.
Index: clang/test/Modules/driver.c
===
--- clang/test/Modules/driver.c
+++ clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck 
-check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps 
%s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
 // RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s 
-### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
 
 // CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
Index: clang/test/Driver/modules-cache-path.m
===
--- clang/test/Driver/modules-cache-path.m
+++ clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-DEFAULT
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-DEFAULT
 // CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
 
 // RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.
Index: clang/test/Modules/driver.c
===
--- clang/test/Modules/driver.c
+++ clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
 // RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s -### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
 
 // CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
Index: clang/test/Driver/modules-cache-path.m
===
--- clang/test/Driver/modules-cache-path.m
+++ clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
+// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
 // CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
 
 // RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

6 matches

Site Navigation

Mail list logo

Footer information