[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG97dfaf4cd278: [modules] Fix error about the same module 
being defined in different .pcm files… (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/VFS/module-map-path.m

Index: clang/test/VFS/module-map-path.m
===
--- /dev/null
+++ clang/test/VFS/module-map-path.m
@@ -0,0 +1,110 @@
+// Test the module map path is consistent between clang invocations when using VFS overlays.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Pre-populate the module cache with the modules that don't use VFS overlays.
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks -I%t/include %t/prepopulate_module_cache.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Execute a compilation with VFS overlay. .pcm file path looks like /ModuleName-.pcm.
+//  corresponds to the compilation settings like language options.
+//  corresponds to the module map path. So if any of those change, we should use a different module.
+// But for VFS overlay we make an exception that it's not a part of  to reduce the number of built .pcm files.
+// Test that paths in overlays don't leak into  and don't cause using 2 .pcm files for the same module.
+// DEFINE: %{command} = %clang_cc1 -fsyntax-only -verify -F%t/Frameworks -I%t/include %t/test.m \
+// DEFINE:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@@g" %t/overlay.yaml.template > %t/external-names-default.yaml
+// RUN: %{command} -ivfsoverlay %t/external-names-default.yaml
+
+// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': true,@g" %t/overlay.yaml.template > %t/external-names-true.yaml
+// RUN: %{command} -ivfsoverlay %t/external-names-true.yaml
+
+// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': false,@g" %t/overlay.yaml.template > %t/external-names-false.yaml
+// RUN: %{command} -ivfsoverlay %t/external-names-false.yaml
+
+//--- prepopulate_module_cache.m
+#import 
+
+//--- test.m
+// At first import multi-path modules directly, so clang decides which .pcm file they should belong to.
+#import 
+#import 
+
+// Then import a module from the module cache and all its transitive dependencies.
+// Make sure the .pcm files loaded directly are the same as 'Redirecting' is referencing.
+#import 
+// expected-no-diagnostics
+
+
+//--- Frameworks/MultiPath.framework/Headers/MultiPath.h
+void multiPathFramework(void);
+
+//--- Frameworks/MultiPath.framework/Modules/module.modulemap
+framework module MultiPath {
+header "MultiPath.h"
+export *
+}
+
+
+//--- include/MultiPathHeader.h
+void multiPathHeader(void);
+
+//--- include/module.modulemap
+module MultiPathHeader {
+header "MultiPathHeader.h"
+export *
+}
+
+
+//--- Frameworks/Redirecting.framework/Headers/Redirecting.h
+#import 
+#import 
+
+//--- Frameworks/Redirecting.framework/Modules/module.modulemap
+framework module Redirecting {
+header "Redirecting.h"
+export *
+}
+
+
+//--- BuildTemporaries/MultiPath.h
+void multiPathFramework(void);
+//--- BuildTemporaries/module.modulemap
+framework module MultiPath {
+header "MultiPath.h"
+export *
+}
+//--- BuildTemporaries/header.h
+void multiPathHeader(void);
+//--- BuildTemporaries/include.modulemap
+module MultiPathHeader {
+header "MultiPathHeader.h"
+export *
+}
+
+//--- overlay.yaml.template
+{
+  'version': 0,
+  USE_EXTERNAL_NAMES_OPTION
+  'roots': [
+{ 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Headers', 'type': 'directory',
+  'contents': [
+{ 'name': 'MultiPath.h', 'type': 'file',
+  'external-contents': 'TMP_DIR/BuildTemporaries/MultiPath.h'}
+]},
+{ 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Modules', 'type': 'directory',
+  'contents': [
+{ 'name': 'module.modulemap', 'type': 'file',
+  'external-contents': 'TMP_DIR/BuildTemporaries/module.modulemap'}
+]},
+{ 'name': 'TMP_DIR/include', 'type': 'directory',
+  'contents': [
+{ 'name': 'MultiPathHeader.h', 'type': 'file',
+  'external-contents': 'TMP_DIR/BuildTemporaries/header.h'},
+{ 'name': 'module.modulemap', 'type': 'file',
+  'external-contents': 'TMP_DIR/BuildTemporaries/include.modulemap'}
+]}
+  ]
+}
+
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1327,7 +13

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/lib/Serialization/ASTWriter.cpp:1330
 AddPath(WritingModule->PresumedModuleMapFile.empty()
-? Map.getModuleMapFileForUniquing(WritingModule)->getName()
+? Map.getModuleMapFileForUniquing(WritingModule)
+  ->getNameAsRequested()

vsapsai wrote:
> jansvoboda11 wrote:
> > Can we canonicalize this also? It'd be useful in the scanner.
> I'm not sure about that. ASTReader has some complicated logic around reading 
> this value 
> https://github.com/llvm/llvm-project/blob/c1803d5366c794ecade4e4ccd0013690a1976d49/clang/lib/Serialization/ASTReader.cpp#L4005
>  So if we don't have a proven need for it with a test case, I wouldn't change 
> it.
Good point. I do have a use for it, but I think it'd be safer to do separately 
from this patch a qualify it in isolation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D156749#4565994 , @jansvoboda11 
wrote:

> I think your solution is the most pragmatic. If you're confident this doesn't 
> break anything internally, I say go for it. But I think it's good to be aware 
> of the pitfall I mentioned, and make sure the build system doesn't trigger 
> that.

As far as I have tested, it isn't breaking anything.




Comment at: clang/lib/Serialization/ASTWriter.cpp:1330
 AddPath(WritingModule->PresumedModuleMapFile.empty()
-? Map.getModuleMapFileForUniquing(WritingModule)->getName()
+? Map.getModuleMapFileForUniquing(WritingModule)
+  ->getNameAsRequested()

jansvoboda11 wrote:
> Can we canonicalize this also? It'd be useful in the scanner.
I'm not sure about that. ASTReader has some complicated logic around reading 
this value 
https://github.com/llvm/llvm-project/blob/c1803d5366c794ecade4e4ccd0013690a1976d49/clang/lib/Serialization/ASTReader.cpp#L4005
 So if we don't have a proven need for it with a test case, I wouldn't change 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 accepted this revision.
jansvoboda11 added a comment.

In D156749#4562469 , @vsapsai wrote:

> In D156749#4561803 , @jansvoboda11 
> wrote:
>
>> My suggestion is to use the actual real on-disk path. Not 
>> `FileEntryRef::getName()`, but something that always behaves as if 
>> `use-external-name` was set to `true`. I believe this would handle your 
>> VFS/VFS-use-external-name-true/VFS-use-external-name-false problem. It would 
>> also handle another pitfall: two compilations with distinct VFS overlays 
>> that redirect two different as-requested module map paths into the same 
>> on-disk path.
>
> Do you suggest doing it for the hashing or for ASTWriter or both? We are 
> already doing some module map path canonicalization (added a comment in 
> corresponding place) but it's not pure on-disk path, it takes into account 
> VFS.

I mainly meant in ``. And then make the PCM files VFS-agnostic, which 
would probably require us to do the same thing for all the paths we serialize 
there. But I don't know how feasible that is. Besides, the scanner relies on 
the virtual paths in PCM files.

I think your solution is the most pragmatic. If you're confident this doesn't 
break anything internally, I say go for it. But I think it's good to be aware 
of the pitfall I mentioned, and make sure the build system doesn't trigger that.




Comment at: clang/lib/Serialization/ASTWriter.cpp:1330
 AddPath(WritingModule->PresumedModuleMapFile.empty()
-? Map.getModuleMapFileForUniquing(WritingModule)->getName()
+? Map.getModuleMapFileForUniquing(WritingModule)
+  ->getNameAsRequested()

Can we canonicalize this also? It'd be useful in the scanner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D156749#4561803 , @jansvoboda11 
wrote:

> My suggestion is to use the actual real on-disk path. Not 
> `FileEntryRef::getName()`, but something that always behaves as if 
> `use-external-name` was set to `true`. I believe this would handle your 
> VFS/VFS-use-external-name-true/VFS-use-external-name-false problem. It would 
> also handle another pitfall: two compilations with distinct VFS overlays that 
> redirect two different as-requested module map paths into the same on-disk 
> path.

Do you suggest doing it for the hashing or for ASTWriter or both? We are 
already doing some module map path canonicalization (added a comment in 
corresponding place) but it's not pure on-disk path, it takes into account VFS.




Comment at: clang/lib/Lex/HeaderSearch.cpp:259-260
 // to lower-case in case we're on a case-insensitive file system.
 SmallString<128> CanonicalPath(ModuleMapPath);
 if (getModuleMap().canonicalizeModuleMapPath(CanonicalPath))
   return {};

Sort of canonicalization and obtaining real on-disk path we are doing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D156749#4552590 , @vsapsai wrote:

> In D156749#4551596 , @jansvoboda11 
> wrote:
>
>> Alternatively, we could keep VFS overlays out of the context hash but create 
>> `` from the on-disk real path of the defining module map and make the 
>> whole PCM VFS-agnostic. Then it'd be correct to import that PCM regardless 
>> of the specific VFS overlay setup, as long as all VFS queries of the 
>> importer resolve the same way they resolved within the instance that built 
>> the PCM. Maybe we can force the importer to recompile the PCM if that's not 
>> the case, similar to what we do for diagnostic options.
>
> I'm not sure I understand your proposal. Before this change we were 
> calculating hash from the on-disk real path of the defining module map. And 
> due to different VFS/no-VFS options defining module map is at different 
> locations on-disk.

My suggestion is to use the actual real on-disk path. Not 
`FileEntryRef::getName()`, but something that always behaves as if 
`use-external-name` was set to `true`. I believe this would handle your 
VFS/VFS-use-external-name-true/VFS-use-external-name-false problem. It would 
also handle another pitfall: two compilations with distinct VFS overlays that 
redirect two different as-requested module map paths into the same on-disk path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping @jansvoboda11 , @benlangmuir  in case you still have thoughts on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D156749#4551596 , @jansvoboda11 
wrote:

> What are the performance implications of making VFS overlays part of the 
> context hash?

I don't have specific measurements but for Xcode projects different targets 
have different VFS overlays. So adding VFS overlays to the context hash would 
prevent module sharing between targets. To use llvm+clang as an example it 
means different libraries will have different versions of the same module. For 
example, clangLex will have its version of llvmSupport and clangSerialization 
will have its version too. Actual impact depends on a project but for clang 
with 29 subdirectories in "clang/lib" an approximate estimate would be 10 times 
more modules to build. Note that clang build doesn't use VFS, so it won't be an 
actual impact on clang build but on projects with the size and structure 
similar to clang.

In D156749#4551596 , @jansvoboda11 
wrote:

>> And it is build system's responsibility to provide `-ivfsoverlay` options 
>> that don't cause observable differences.
>
> I wasn't aware of that. Do we document this anywhere? It surprises me that 
> we'd impose such restriction on the build system. This seems fairly easy to 
> accidentally violate and end up in the same situation - Clang instances with 
> different VFS overlays, identical context hashes and different canonical 
> module map paths for the same module.

There are no documents describing responsibilities  of different components, as 
far as I know. I haven't researched the issue extensively but I think only 
Xcode build system uses VFS overlays. And if you have only one client, it is a 
questionable use of time to formalize the interface. And the absence of formal 
contract allows to evolve different sides faster.

Yes, if a build system provides bad data to a compiler, it can cause errors. 
And there are no ways to force build systems not to do that. I don't think it 
is the compiler's responsibility to verify build system behavior. For example, 
if for incremental build not all impacted files are recompiled (given the 
dependencies are correct) - the compiler shouldn't track it themselves and 
recompile the required files.

In D156749#4551596 , @jansvoboda11 
wrote:

> Alternatively, we could keep VFS overlays out of the context hash but create 
> `` from the on-disk real path of the defining module map and make the 
> whole PCM VFS-agnostic. Then it'd be correct to import that PCM regardless of 
> the specific VFS overlay setup, as long as all VFS queries of the importer 
> resolve the same way they resolved within the instance that built the PCM. 
> Maybe we can force the importer to recompile the PCM if that's not the case, 
> similar to what we do for diagnostic options.

I'm not sure I understand your proposal. Before this change we were calculating 
hash from the on-disk real path of the defining module map. And due to 
different VFS/no-VFS options defining module map is at different locations 
on-disk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a subscriber: benlangmuir.
jansvoboda11 added a comment.

CC @benlangmuir, since we've talked about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

> And it is build system's responsibility to provide `-ivfsoverlay` options 
> that don't cause observable differences.

I wasn't aware of that. Do we document this anywhere? It surprises me that we'd 
impose such restriction on the build system. This seems fairly easy to 
accidentally violate and end up in the same situation - Clang instances with 
different VFS overlays, identical context hashes and different canonical module 
map paths for the same module.

What are the performance implications of making VFS overlays part of the 
context hash?

Alternatively, we could keep VFS overlays out of the context hash but create 
`` from the on-disk real path of the defining module map and make the 
whole PCM VFS-agnostic. Then it'd be correct to import that PCM regardless of 
the specific VFS overlay setup, as long as all VFS queries of the importer 
resolve the same way they resolved within the instance that built the PCM. 
Maybe we can force the importer to recompile the PCM if that's not the case, 
similar to what we do for diagnostic options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the quick review!

In D156749#4549226 , @ChuanqiXu wrote:

> Although there is a FIXME in the definition of `getNameAsRequested()`, it 
> looks not sense to require you to fix that. It might not be an over burden 
> for someone who  will be intended to fix this later. So LGTM.

I've discussed with @jansvoboda11 the future of 
`FileEntryRef::getNameAsRequested` and it is supposed to replace 
`FileEntryRef::getName` (eventually, not immediately), if I understand 
correctly. Given that, it is the move in the right direction regardless of 
FIXMEs. If I've misunderstood something, Jan can chime in and correct me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Chuanqi Xu via Phabricator via cfe-commits]

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

Although there is a FIXME in the definition of `getNameAsRequested()`, it looks 
not sense to require you to fix that. It might not be an over burden for 
someone who  will be intended to fix this later. So LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749

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

[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: ChuanqiXu, jansvoboda11.
Herald added a subscriber: ributzka.
Herald added a project: All.
vsapsai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix errors like

> module 'MultiPath' is defined in both 
> 'path/to/modules.cache/3JR48BPRU7BCG/MultiPath-1352QHUF8RNMU.pcm' and 
> 'path/to/modules.cache/3JR48BPRU7BCG/MultiPath-20HNSLLIUDDV1.pcm'

To avoid building extra identical modules `-ivfsoverlay` option is not a
part of the hash like "/3JR48BPRU7BCG/". And it is build system's
responsibility to provide `-ivfsoverlay` options that don't cause
observable differences.  We also need to make sure the hash like
"-1352QHUF8RNMU" is not affected by `-ivfsoverlay`. As this hash is
defined by the module map path, use the path prior to any VFS
remappings.

rdar://111921464


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156749

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/VFS/module-map-path.m

Index: clang/test/VFS/module-map-path.m
===
--- /dev/null
+++ clang/test/VFS/module-map-path.m
@@ -0,0 +1,110 @@
+// Test the module map path is consistent between clang invocations when using VFS overlays.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Pre-populate the module cache with the modules that don't use VFS overlays.
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks -I%t/include %t/prepopulate_module_cache.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Execute a compilation with VFS overlay. .pcm file path looks like /ModuleName-.pcm.
+//  corresponds to the compilation settings like language options.
+//  corresponds to the module map path. So if any of those change, we should use a different module.
+// But for VFS overlay we make an exception that it's not a part of  to reduce the number of built .pcm files.
+// Test that paths in overlays don't leak into  and don't cause using 2 .pcm files for the same module.
+// DEFINE: %{command} = %clang_cc1 -fsyntax-only -verify -F%t/Frameworks -I%t/include %t/test.m \
+// DEFINE:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@@g" %t/overlay.yaml.template > %t/external-names-default.yaml
+// RUN: %{command} -ivfsoverlay %t/external-names-default.yaml
+
+// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': true,@g" %t/overlay.yaml.template > %t/external-names-true.yaml
+// RUN: %{command} -ivfsoverlay %t/external-names-true.yaml
+
+// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': false,@g" %t/overlay.yaml.template > %t/external-names-false.yaml
+// RUN: %{command} -ivfsoverlay %t/external-names-false.yaml
+
+//--- prepopulate_module_cache.m
+#import 
+
+//--- test.m
+// At first import multi-path modules directly, so clang decides which .pcm file they should belong to.
+#import 
+#import 
+
+// Then import a module from the module cache and all its transitive dependencies.
+// Make sure the .pcm files loaded directly are the same as 'Redirecting' is referencing.
+#import 
+// expected-no-diagnostics
+
+
+//--- Frameworks/MultiPath.framework/Headers/MultiPath.h
+void multiPathFramework(void);
+
+//--- Frameworks/MultiPath.framework/Modules/module.modulemap
+framework module MultiPath {
+header "MultiPath.h"
+export *
+}
+
+
+//--- include/MultiPathHeader.h
+void multiPathHeader(void);
+
+//--- include/module.modulemap
+module MultiPathHeader {
+header "MultiPathHeader.h"
+export *
+}
+
+
+//--- Frameworks/Redirecting.framework/Headers/Redirecting.h
+#import 
+#import 
+
+//--- Frameworks/Redirecting.framework/Modules/module.modulemap
+framework module Redirecting {
+header "Redirecting.h"
+export *
+}
+
+
+//--- BuildTemporaries/MultiPath.h
+void multiPathFramework(void);
+//--- BuildTemporaries/module.modulemap
+framework module MultiPath {
+header "MultiPath.h"
+export *
+}
+//--- BuildTemporaries/header.h
+void multiPathHeader(void);
+//--- BuildTemporaries/include.modulemap
+module MultiPathHeader {
+header "MultiPathHeader.h"
+export *
+}
+
+//--- overlay.yaml.template
+{
+  'version': 0,
+  USE_EXTERNAL_NAMES_OPTION
+  'roots': [
+{ 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Headers', 'type': 'directory',
+  'contents': [
+{ 'name': 'MultiPath.h', 'type': 'file',
+  'external-contents': 'TMP_DIR/BuildTemporaries/MultiPath.h'}
+]},
+{ 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Modules', 'type': 'directory',
+  'contents': [
+{ 'name': 'module.modulemap', 'type': 'file',
+  'external-contents': 'TMP_DIR/BuildTemporaries/module.modulemap'}
+]},
+{