[Index] Apply -index-ignore-system-modules to system clang modules

[kastiglione]

Conditionally disable indexing of system clang modules when using -index-ignore-system-modules.

The -index-ignore-system-modules flag currently prevents indexing of system Swift modules. This change expands the behavior to system clang modules too.

This change can be beneficial to builds that don't share a global indexstore across all compiles. For example in Bazel, each module can have its own unshared indexstore, and by using -index-ignore-system-modules the time and space needed for indexing can be reduced.

[kastiglione]

I'd like to add tests, any suggestions? The only current test using -index-ignore-system-modules is test/Index/Store/driver-multiple-inputs.swift. Maybe a separate test in test/Index/Store?

[kastiglione]

Ignoring whitespace: https://github.com/apple/swift/pull/32024/files?diff=unified&w=1

[nathawes]

I'd like to add tests, any suggestions? The only current test using -index-ignore-system-modules is test/Index/Store/driver-multiple-inputs.swift. Maybe a separate test in test/Index/Store?

Yeah adding a new test there makes sense. You need to use -Fsystem to import modules as system modules and should make sure we still list them as dependencies even if we don't output units for them. Something like the below:

// RUN: %empty-directory(%t)

// Make a basic clang framework to import
//
// RUN: %empty-directory(%t/MySystemFramework.framework/Headers)
// RUN: %empty-directory(%t/MySystemFramework.framework/Modules)
// RUN: echo 'void someSystemFunc(int arg);' > %t/MySystemFramework.framework/Headers/MySystemFramework.h
// RUN: echo 'framework module MySystemFramework { umbrella header "MySystemFramework.h" export * }' > %t/MySystemFramework.framework/Modules/module.modulemap

import MySystemFramework
someSystemFunc(2)


// Index this file with and without ignoring system frameworks
//
// RUN: %target-swiftc_driver -index-store-path %t/idx1 -o %t/file.o -Fsystem %t -typecheck %s
// RUN: %target-swiftc_driver -index-store-path %t/idx2 -o %t/file.o -index-ignore-system-modules -Fsystem %t -typecheck %s
// RUN: c-index-test core -print-unit %t/idx1 | %FileCheck --check-prefixes=ALLOWSYSTEM,BOTH %s
// RUN: c-index-test core -print-unit %t/idx2 | %FileCheck --check-prefixes=IGNORESYSTEM,BOTH %s


// We should always get a dependency on the system framework in the unit for this file's module.
//
// BOTH: DEPEND START
// BOTH: Unit | system | MySystemFramework |
// BOTH: DEPEND END

// We should get a unit for the system framework if not ignoring them.
//
// ALLOWSYSTEM: provider: clang
// ALLOWSYSTEM-NEXT: is-system: 1
// ALLOWSYSTEM-NEXT: is-module: 1
// ALLOWSYSTEM-NEXT: module-name: MySystemFramework

// But shouldn't if we are.
//
// IGNORESYSTEM-NOT: module-name: MySystemFramework

I think that should pass with your changes (it fails in my local build without them), but might need fixes in a few places.

[kastiglione]

I think that should pass with your changes (it fails in my local build without them), but might need fixes in a few places.

The test you provided passes with these changes. Nicely written test!

[kastiglione]

I named the file with a driver- prefix, but should it instead use a unit- prefix or something else?

[nathawes]

Yeah, "ignore-system-clang-modules" or similar is probably clearer for what it's really doing.

[kastiglione]

Renamed.

[nathawes]

@swift-ci please test

[nathawes]

LGTM!

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 4e227f8

[kastiglione]

Do I need to squash this PR? Not sure what to make of the failure.

13:17:35 + head_ghprbActualCommit=258ceda02fade5ba21cfad26b552803698f44cb5
13:17:35 + '[' 4e227f8a76deb1d189a0e62694124a11dbbe1792 == 258ceda02fade5ba21cfad26b552803698f44cb5 ']'
13:17:35 + echo 'Wrong sha 4e227f8a76deb1d189a0e62694124a11dbbe1792'
13:17:35 Wrong sha 4e227f8a76deb1d189a0e62694124a11dbbe1792
13:17:35 + echo ghprbActualCommit=258ceda02fade5ba21cfad26b552803698f44cb5
13:17:35 + echo '--- Automatically restarting the build, due to invalid sha ---'

[nathawes]

No, it's expected. The important bit is just after that:

11:17:35 + echo '--- Please ignore this build failure, downstream build will contain the result of this pull request ---'

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 4e227f8

[nathawes]

@swift-ci please test OS X Platform

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 258ceda

[nathawes]

@swift-ci please test OS X Platform