Bring back "Lock scratch directory during tool execution" #7291

neonichu · 2024-01-23T21:10:05Z

This reverts commit e92df2d and brings back the changes from #7269.

neonichu · 2024-01-23T21:10:14Z

neonichu · 2024-01-23T21:19:12Z

Ah yes, that would do it:

Test Case 'PackageToolTests.testInitEmpty' started at 2024-01-23 21:14:29.174
<EXPR>:0: error: PackageToolTests.testInitEmpty : threw error "executionFailure(underlying: terminated(1): /home/build-user/swiftpm/.build/x86_64-unknown-linux-gnu/debug/swift-package --package-path /tmp/spm-tests-testInitEmpty.EZMbQ4/Foo init --type empty output:
    Another instance of SwiftPM is already running using '/home/build-user/swiftpm/.build', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)
    , stdout: "", stderr: "Another instance of SwiftPM is already running using \'/home/build-user/swiftpm/.build\', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)\n")"
Test Case 'PackageToolTests.testInitEmpty' failed (1.794 seconds)

So looks like the self-hosted CI jobs are kind of inherently broken because they share a single build directory during parallel test execution.

neonichu · 2024-01-23T22:14:14Z

Same thing is also broken on smoke tests...

neonichu · 2024-01-25T01:05:19Z

OK, the issue here is actually kind of interesting:

the test runs swift package init --package-path $TMPDIR/Foo
$TMPDIR is a subdirectory of the SwiftPM checkout in CI
The way --package-path is implemented is that it only changes the current working directory
SwiftPM will upwards search for the first directory that has a package manifest, which in this case is the SwiftPM checkout which leads to multiple tests locking the same .build

I'm not 100% sure what this means for the implementation:

we could change the locking such that it only happens for commands mutating state in .build, but I think over time this will have a high chance of forgetting about this and not locking when necessary
we could change --package-path to not do the recursive upwards search -- it does seem a bit unexpected that it does that but the documentation suggests that it works this way, so maybe not something that we want to change
we could decide this is correct and change the tests such that they don't end up concurrently locking the same build arena

neonichu · 2024-01-29T18:48:53Z

We ended up going with a modified approach 2, commands can now opt-out of the upwards search and package init will.

neonichu · 2024-01-29T18:49:05Z

@swift-ci please test

neonichu · 2024-01-29T18:49:52Z

@swift-ci please test windows

neonichu · 2024-01-29T19:50:12Z

Hm, seems like Linux self-hosted is probably hanging?

neonichu · 2024-01-29T21:41:01Z

@swift-ci please test linux

neonichu · 2024-01-29T22:14:43Z

OK, this wasn't actually a solution, several subcommands can still fail:

Test Suite 'PackageToolTests' started at 2024-01-29 21:48:28.511
Test Case 'PackageToolTests.testArchiveSource' started at 2024-01-29 21:48:28.511
/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:1635: error: PackageToolTests.testArchiveSource : XCTAssertTrue failed - actual: "Another instance of SwiftPM is already running using '/home/build-user/swiftpm/.build', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)
"
Test Case 'PackageToolTests.testArchiveSource' failed (8.155 seconds)
Test Suite 'PackageToolTests' failed at 2024-01-29 21:48:36.665
	 Executed 1 test, with 1 failure (0 unexpected) in 8.155 (8.155) seconds
Test Suite 'Selected tests' failed at 2024-01-29 21:48:36.665
	 Executed 1 test, with 1 failure (0 unexpected) in 8.155 (8.155) seconds

Test Suite 'Selected tests' started at 2024-01-29 21:48:41.131
Test Suite 'PackageToolTests' started at 2024-01-29 21:48:41.132
Test Case 'PackageToolTests.testNoParameters' started at 2024-01-29 21:48:41.132
<EXPR>:0: error: PackageToolTests.testNoParameters : threw error "executionFailure(underlying: terminated(1): /home/build-user/swiftpm/.build/x86_64-unknown-linux-gnu/debug/swift-package output:
    Another instance of SwiftPM is already running using '/home/build-user/swiftpm/.build', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)
    , stdout: "", stderr: "Another instance of SwiftPM is already running using \'/home/build-user/swiftpm/.build\', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)\n")"
Test Case 'PackageToolTests.testNoParameters' failed (1.05 seconds)
Test Suite 'PackageToolTests' failed at 2024-01-29 21:48:42.182
	 Executed 1 test, with 1 failure (1 unexpected) in 1.05 (1.05) seconds
Test Suite 'Selected tests' failed at 2024-01-29 21:48:42.182
	 Executed 1 test, with 1 failure (1 unexpected) in 1.05 (1.05) seconds

Test Suite 'Selected tests' started at 2024-01-29 21:48:46.709
Test Suite 'PackageToolTests' started at 2024-01-29 21:48:46.710
Test Case 'PackageToolTests.testUnknownOption' started at 2024-01-29 21:48:46.710
/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:94: error: PackageToolTests.testUnknownOption : XCTAssertTrue failed - unexpected failure matching 'Another instance of SwiftPM is already running using '/home/build-user/swiftpm/.build', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)
' against pattern contains("error: Unknown option \'--foo\'")
Test Case 'PackageToolTests.testUnknownOption' failed (1.005 seconds)
Test Suite 'PackageToolTests' failed at 2024-01-29 21:48:47.715
	 Executed 1 test, with 1 failure (0 unexpected) in 1.005 (1.005) seconds
Test Suite 'Selected tests' failed at 2024-01-29 21:48:47.715
	 Executed 1 test, with 1 failure (0 unexpected) in 1.005 (1.005) seconds

Test Suite 'Selected tests' started at 2024-01-29 21:48:46.711
Test Suite 'PackageToolTests' started at 2024-01-29 21:48:46.713
Test Case 'PackageToolTests.testUsage' started at 2024-01-29 21:48:46.713
/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:54: error: PackageToolTests.testUsage : XCTAssertTrue failed - unexpected failure matching 'Another instance of SwiftPM is already running using '/home/build-user/swiftpm/.build', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)
' against pattern contains("Usage: swift package")
Test Case 'PackageToolTests.testUsage' failed (1.424 seconds)
Test Suite 'PackageToolTests' failed at 2024-01-29 21:48:48.137
	 Executed 1 test, with 1 failure (0 unexpected) in 1.424 (1.424) seconds
Test Suite 'Selected tests' failed at 2024-01-29 21:48:48.137
	 Executed 1 test, with 1 failure (0 unexpected) in 1.424 (1.424) seconds

Test Suite 'Selected tests' started at 2024-01-29 21:48:45.334
Test Suite 'PackageToolTests' started at 2024-01-29 21:48:45.336
Test Case 'PackageToolTests.testPlugin' started at 2024-01-29 21:48:45.336
/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:88: error: PackageToolTests.testPlugin : XCTAssertTrue failed - unexpected failure matching 'Another instance of SwiftPM is already running using '/home/build-user/swiftpm/.build', waiting until that process has finished execution...error: unableToAquireLock(errno: 11)
' against pattern contains("error: Missing expected plugin command")
Test Case 'PackageToolTests.testPlugin' failed (1.258 seconds)

Though I'm not sure I understand why we only see these failures on Linux?

neonichu · 2024-01-30T01:24:44Z

I think the only option we have is locking based on whether a tool actually uses getActiveWorkspace() because at this point it needs to have a "real" .build

Sources/CoreCommands/SwiftTool.swift

neonichu · 2024-01-31T23:04:31Z

@swift-ci please test

neonichu · 2024-01-31T23:20:50Z

@swift-ci please test

neonichu · 2024-01-31T23:23:22Z

@swift-ci please test windows

neonichu · 2024-02-01T03:38:13Z

@swift-ci please test windows

neonichu · 2024-02-01T17:28:07Z

@swift-ci please test windows

neonichu · 2024-02-01T20:17:14Z

[2024-02-01 19:12:55] Finished building 'C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\sourcekit-lsp' to 'T:\16' for arch 'x64' in 00:01:04.3181050

[2024-02-01 19:12:55] Building 'C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\tools\swift-inspect' to 'T:\x64\swift-inspect' for arch 'x64'...
Build timed out (after 60 minutes). Marking the build as aborted.
Build was aborted

neonichu · 2024-02-01T20:17:27Z

@swift-ci please test windows

neonichu · 2024-02-01T20:53:31Z

@swift-ci please test windows

neonichu · 2024-02-02T00:00:32Z

@swift-ci please test windows

neonichu · 2024-02-02T01:59:50Z

It looks as if "swift-inspect" is being built with swift build in the Windows job, so this could be a real issue with the locking code on Windows. Not sure how to proceed.

neonichu · 2024-02-02T18:15:09Z

Ah, maybe the current implementation is specific to how flock works, because I am taking the lock twice which seems to work fine if it's the same process doing it.

neonichu · 2024-02-02T18:51:26Z

@swift-ci please test windows

neonichu · 2024-02-02T18:53:29Z

@swift-ci please test windows

neonichu · 2024-02-02T19:06:18Z

@swift-ci please test windows

neonichu · 2024-02-02T19:15:24Z

@swift-ci please test windows

neonichu · 2024-02-02T21:55:17Z

Ah, maybe the current implementation is specific to how flock works, because I am taking the lock twice which seems to work fine if it's the same process doing it.

Seems like this was indeed the issues exposed by Windows CI, fixed now.

neonichu · 2024-02-02T21:55:32Z

@swift-ci please test macOS

neonichu · 2024-02-02T21:55:37Z

@swift-ci please test linux

Sources/CoreCommands/SwiftTool.swift

neonichu · 2024-02-05T20:22:40Z

@swift-ci please test

neonichu · 2024-02-05T20:22:49Z

@swift-ci please test windows

This reverts commit e92df2d and brings back the changes from #7269. resolves #6643 rdar://113964179

neonichu · 2024-02-05T23:49:28Z

@swift-ci please test

neonichu · 2024-02-05T23:49:39Z

@swift-ci please test windows

neonichu self-assigned this Jan 23, 2024

neonichu force-pushed the lock-scratch-directory-2 branch from a7130fb to 7ff2cae Compare January 29, 2024 18:48

neonichu marked this pull request as ready for review January 29, 2024 18:49

neonichu requested review from tomerd and MaxDesiatov as code owners January 29, 2024 18:49

neonichu requested a review from bnbarham January 29, 2024 18:49

neonichu force-pushed the lock-scratch-directory-2 branch from 7ff2cae to 0e77a24 Compare January 29, 2024 21:40

neonichu mentioned this pull request Jan 31, 2024

Build can overlap with resolve packages resulting in corrupted dependencies checkouts #6643

Open

neonichu force-pushed the lock-scratch-directory-2 branch 2 times, most recently from 05f5991 to 17ede9e Compare January 31, 2024 23:02

neonichu commented Jan 31, 2024

View reviewed changes

Sources/CoreCommands/SwiftTool.swift Show resolved Hide resolved

neonichu commented Jan 31, 2024

View reviewed changes

Sources/CoreCommands/SwiftTool.swift Outdated Show resolved Hide resolved

neonichu commented Jan 31, 2024

View reviewed changes

Sources/CoreCommands/SwiftTool.swift Outdated Show resolved Hide resolved

neonichu force-pushed the lock-scratch-directory-2 branch from 17ede9e to d308cd6 Compare January 31, 2024 23:19

neonichu force-pushed the lock-scratch-directory-2 branch from d308cd6 to 247c5fa Compare February 2, 2024 18:51

MaxDesiatov reviewed Feb 5, 2024

View reviewed changes

Sources/CoreCommands/SwiftTool.swift Outdated Show resolved Hide resolved

neonichu force-pushed the lock-scratch-directory-2 branch from 247c5fa to 189c5c8 Compare February 5, 2024 20:20

Bring back "Lock scratch directory during tool execution"

1d4dec4

This reverts commit e92df2d and brings back the changes from #7269. resolves #6643 rdar://113964179

neonichu force-pushed the lock-scratch-directory-2 branch from 189c5c8 to 1d4dec4 Compare February 5, 2024 23:49

MaxDesiatov approved these changes Feb 6, 2024

View reviewed changes

neonichu merged commit 6bcbf11 into main Feb 6, 2024

neonichu deleted the lock-scratch-directory-2 branch February 6, 2024 19:08

fxwx23 mentioned this pull request Nov 18, 2024

SwiftPM6: Fix to skip locking package build directory mikeger/XcodeSelectiveTesting#38

Merged

Bring back "Lock scratch directory during tool execution" #7291

Bring back "Lock scratch directory during tool execution" #7291

Uh oh!

Conversation

neonichu commented Jan 23, 2024

Uh oh!

neonichu commented Jan 23, 2024

Uh oh!

neonichu commented Jan 23, 2024

Uh oh!

neonichu commented Jan 23, 2024

Uh oh!

neonichu commented Jan 25, 2024

Uh oh!

neonichu commented Jan 29, 2024

Uh oh!

neonichu commented Jan 29, 2024

Uh oh!

neonichu commented Jan 29, 2024

Uh oh!

neonichu commented Jan 29, 2024

Uh oh!

neonichu commented Jan 29, 2024

Uh oh!

neonichu commented Jan 29, 2024

Uh oh!

neonichu commented Jan 30, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neonichu commented Jan 31, 2024

Uh oh!

neonichu commented Jan 31, 2024

Uh oh!

neonichu commented Jan 31, 2024

Uh oh!

neonichu commented Feb 1, 2024

Uh oh!

neonichu commented Feb 1, 2024

Uh oh!

neonichu commented Feb 1, 2024

Uh oh!

neonichu commented Feb 1, 2024

Uh oh!

neonichu commented Feb 1, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

neonichu commented Feb 2, 2024

Uh oh!

Uh oh!

neonichu commented Feb 5, 2024

Uh oh!

neonichu commented Feb 5, 2024

Uh oh!

neonichu commented Feb 5, 2024

Uh oh!

neonichu commented Feb 5, 2024

Uh oh!

Uh oh!