[Frontend] Lock the swiftmodule when rebuilding from the swiftinterface #36185
Conversation
|
@swift-ci Please smoke test |
There was a problem hiding this comment.
If we have a one-to-many situation where multiple frontends are building the same interface file into multiple binary modules at different paths, we will now spawn more threads and do this in parallel, vs. one-by-one.
I don't imagine being a problem because we are mostly building into the same location in the Module Cache, but do we know of any other scenarios where this could cause trouble?
|
@swift-ci Please smoke test macOS |
|
I think this version is more reliable in this scenario because if a different process was waiting on another one to rebuild the module for a different cache it would have been in vain. I'd like @nkcsgexi opinion on this, in case I missed something. |
|
@swift-ci Please smoke test macOS |
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
|
@swift-ci Please smoke test Windows |
When rebuilding a module interface from the textual interface, lock the destination path of the created swiftmodule instead of the source swiftinterface. The swiftinterface files are likely to be in the SDK and may be on a read-only filesystem. rdar://60247977
The APIJSON tests used the default module cache and generated different modules under the same name. This lead some tests to load a different module than intended. This was made more easily detectable with the lock on the swiftmodules instead of the swiftinterfaces.
|
The APIJSON tests were failing because they share a module cache and use the same name for different modules. They were writing to the the same swiftmodule file which then made some parallel tests read from the swiftmodule generated by another test. This was made more easily detectable with the lock on the swiftmodules instead of the swiftinterfaces. Using local module cache should be enough to avoid this. cc @cachemeifyoucan @swift-ci Please smoke test |
Thanks for fixing |
|
@swift-ci Please smoke test Windows |
| // RUN: echo 'public func foo() {}' > %t/sdk/Foo.swift | ||
| // RUN: %target-swift-frontend-typecheck -emit-module-interface-path %t/sdk/Foo.swiftinterface %t/sdk/Foo.swift -enable-library-evolution | ||
| // RUN: chmod a-w %t/sdk | ||
|
|
||
| // RUN: touch %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift | ||
| // RUN: echo 'import Foo' > %t/file-01.swift | ||
| // RUN: echo 'import Foo' > %t/file-02.swift | ||
| // RUN: echo 'import Foo' > %t/file-03.swift | ||
| // RUN: %target-swiftc_driver -j20 %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift -I %t -Xfrontend -Rmodule-interface-rebuild -module-cache-path %t/module-cache &> %t/result.txt | ||
| // RUN: %target-swiftc_driver -j20 %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift -I %t/sdk -Xfrontend -Rmodule-interface-rebuild -module-cache-path %t/module-cache &> %t/result.txt | ||
| // RUN: %FileCheck %s -check-prefix=CHECK-REBUILD < %t/result.txt | ||
|
|
||
| // RUN: %target-swiftc_driver -j20 %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift -I %t -Xfrontend -Rmodule-interface-rebuild -Xfrontend -disable-interface-lock -module-cache-path %t/module-cache-no-lock &> %t/result.txt | ||
| // RUN: %target-swiftc_driver -j20 %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift -I %t/sdk -Xfrontend -Rmodule-interface-rebuild -Xfrontend -disable-interface-lock -module-cache-path %t/module-cache-no-lock &> %t/result.txt | ||
|
|
||
| // RUN: %FileCheck %s -check-prefix=CHECK-REBUILD-NO-LOCK < %t/result.txt | ||
|
|
||
| // Reset the permissions | ||
| // RUN: chmod a+w %t/sdk |
There was a problem hiding this comment.
I think this test is brittle for incremental builds. It really needs a chmod a+x at the start of the test to reset %t/sdk to a known good state before running line 5: echo 'public func foo() {}' > %t/sdk/Foo.swift
When rebuilding a module interface from the textual interface, lock the destination path of the created swiftmodule instead of the source swiftinterface. The swiftinterface files are likely to be in the SDK and may be on a read-only filesystem.
rdar://60247977
I'm reenabling the test at the same time.