Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Destination.sdkPlatformFrameworkPaths() #5876

Merged
merged 1 commit into from Dec 8, 2022

Conversation

neonichu
Copy link
Member

@neonichu neonichu commented Nov 4, 2022

We should not ignore errors or an empty SDK platform path since that means XCTest imports and test execution might silently fail on macOS with no indication to what the problem is.

@neonichu neonichu self-assigned this Nov 4, 2022
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Linux self-hosted is failing with

<EXPR>:0: error: TestDiscoveryTests.testSubclassedTestClassTests : threw error "terminated(1): /home/build-user/swiftpm/.build/x86_64-unknown-linux-gnu/debug/swift-test --package-path /tmp/Miscellaneous_TestDiscovery_Subclass.1HY8Nx/Miscellaneous_TestDiscovery_Subclass --configuration debug output:
    /tmp/Miscellaneous_TestDiscovery_Subclass.1HY8Nx/Miscellaneous_TestDiscovery_Subclass/.build/x86_64-unknown-linux-gnu/debug/SubclassPackageTests.xctest: error while loading shared libraries: libswift_RegexParser.so: cannot open shared object file: No such file or directory

which seems like a broken toolchain potentially?

Windows is failing somewhere early in the process as well.

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from 6d50e07 to 2812780 Compare November 4, 2022 02:42
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from 2812780 to 6ccd668 Compare November 4, 2022 04:41
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from 6ccd668 to fa2a1d4 Compare November 4, 2022 05:59
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@MaxDesiatov
Copy link
Member

@swift-ci please smoke test macOS

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from fa2a1d4 to 42b8295 Compare November 4, 2022 16:33
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Of course things are passing after adding extra logging 🥲

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

[error]: Invalid manifest
/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/spm-tests-testSimpleAPI.qYJkUF/MyPkg/Package.swift:4:8: error: no such module 'PackageDescription'
import PackageDescription
       ^

Hm, not sure why we aren't seeing the new compilerCommandLine stuff I added.

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Hm, not sure why we aren't seeing the new compilerCommandLine stuff I added.

Ah, it is being set in the wrong place.

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from 42b8295 to 98e5054 Compare November 4, 2022 20:03
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from 98e5054 to b3f3f0b Compare November 4, 2022 20:30
@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Seems reasonable that this fails? We're looking for the PD libs in the installation directory before installing.

[error]: Invalid manifest (compiled with: ["/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc", "-vfsoverlay", "/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/TemporaryDirectory.dHFMod/vfs.yaml", "-L", "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/pm/ManifestAPI", "-lPackageDescription", "-Xlinker", "-rpath", "-Xlinker", "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/pm/ManifestAPI", "-target", "x86_64-apple-macosx10.15", "-sdk", "/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk", "-F", "/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks", "-I", "/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/lib", "-L", "/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/lib", "-swift-version", "5", "-I", "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/pm/ManifestAPI", "-sdk", "/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk", "-package-description-version", "5.7.0", "/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/spm-tests-testSimpleAPI.MrySgk/MyPkg/Package.swift", "-o", "/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/TemporaryDirectory.tJZRZ9/mypkg-manifest"])
/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/spm-tests-testSimpleAPI.MrySgk/MyPkg/Package.swift:4:8: error: no such module 'PackageDescription'
import PackageDescription
       ^

Not sure what changed to make us to do that.

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Probably we're ending up in the fallback case, instead of the testing case here:

// this tests if we are debugging / testing SwiftPM with SwiftPM
        if localFileSystem.exists(applicationPath.appending(component: "swift-package")) {
            return .init(
                manifestLibraryPath: applicationPath,
                pluginLibraryPath: applicationPath
            )
        }

        // we are using a SwiftPM outside a toolchain, use the compiler path to compute the location
        return .init(swiftCompilerPath: swiftCompilerPath)

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Not sure I understand how this ever works? applicationPath will be /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Xcode/Agents/xctest so we'll always use search paths relative to swiftCompilerPath when running a test that doesn't shell out.

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

Oh I see, this is why we're setting up a bit of a fake toolchain as part of bootstrap, so I think the issue is that we're directly pointing to the installed swiftc rather than to the symlink in the fake toolchain. I think what broke things is #4213, because with that we're using the installed swiftc directly.

@neonichu
Copy link
Member Author

neonichu commented Nov 4, 2022

@swift-ci please smoke test

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from add36ce to 81f89da Compare December 1, 2022 19:18
@neonichu
Copy link
Member Author

neonichu commented Dec 1, 2022

Dropped add36ce now, let's see if that brings back the issue.

@neonichu
Copy link
Member Author

neonichu commented Dec 1, 2022

@swift-ci please smoke test

@neonichu
Copy link
Member Author

neonichu commented Dec 1, 2022

This macOS failure looks unrelated:

/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swift-driver/Sources/SwiftDriver/Jobs/Planning.swift:200:31: error: variable binding in a condition requires an initializer\n    guard let dependencyGraph else {\n                              ^\nninja: build stopped: subcommand failed.\n'
Ninja invocation failed: 

@neonichu
Copy link
Member Author

neonichu commented Dec 1, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 2, 2022

[951/1087] Compiling SwiftDriver GeneratePCHJob.swift
/Users/ec2-user/jenkins/workspace/swift-package-manager-with-xcode-self-hosted-PR-osx/branch-main/swiftpm/.build/checkouts/swift-driver/Sources/SwiftDriver/Jobs/Planning.swift:200:31: error: variable binding in a condition requires an initializer
    guard let dependencyGraph else {
                              ^

Hmmm, not sure what this is about -- @artemcm any ideas?

@artemcm
Copy link
Contributor

artemcm commented Dec 2, 2022

@swift-ci please smoke test macOS

@artemcm
Copy link
Contributor

artemcm commented Dec 2, 2022

[951/1087] Compiling SwiftDriver GeneratePCHJob.swift
/Users/ec2-user/jenkins/workspace/swift-package-manager-with-xcode-self-hosted-PR-osx/branch-main/swiftpm/.build/checkouts/swift-driver/Sources/SwiftDriver/Jobs/Planning.swift:200:31: error: variable binding in a condition requires an initializer
    guard let dependencyGraph else {
                              ^

Hmmm, not sure what this is about -- @artemcm any ideas?

My bad. Fixed this here: apple/swift-driver#1233.

@neonichu
Copy link
Member Author

neonichu commented Dec 2, 2022

@swift-ci please smoke test macOS

2 similar comments
@neonichu
Copy link
Member Author

neonichu commented Dec 2, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 5, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 5, 2022

3 successful runs so far, I think we should do ~10 before making any further determination.

@neonichu
Copy link
Member Author

neonichu commented Dec 5, 2022

@swift-ci please smoke test macOS

5 similar comments
@neonichu
Copy link
Member Author

neonichu commented Dec 5, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 6, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 6, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 6, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 7, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 7, 2022

We are at 9 runs, so one more. Since we haven't seen the error again, it seems like it was stemming from some underlying tools failure rather than anything in SwiftPM itself.

@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from 81f89da to ffa2225 Compare December 7, 2022 23:23
@neonichu
Copy link
Member Author

neonichu commented Dec 7, 2022

@swift-ci please smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Dec 8, 2022

OK, this passed again. This should be sufficient signal to reenable the disabled tests. There's also a fix in here for one test which we need to bring in, as well as some other changes we should land. I'm going to split this PR into a couple individual changes.

We should not ignore errors or an empty SDK platform path since that means XCTest imports and test execution might silently fail on macOS with no indication to what the problem is.
@neonichu neonichu force-pushed the improve-sdkPlatformFrameworkPaths branch from ffa2225 to 46bf794 Compare December 8, 2022 06:39
@neonichu
Copy link
Member Author

neonichu commented Dec 8, 2022

Updating this PR to only change Destination.sdkPlatformFrameworkPaths()

@neonichu neonichu marked this pull request as ready for review December 8, 2022 06:40
@neonichu
Copy link
Member Author

neonichu commented Dec 8, 2022

@swift-ci please smoke test

@neonichu
Copy link
Member Author

neonichu commented Dec 8, 2022

Created #5958 and #5957 for the other changes that were previously part of this PR.

@neonichu neonichu merged commit b268125 into main Dec 8, 2022
@neonichu neonichu deleted the improve-sdkPlatformFrameworkPaths branch December 8, 2022 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants