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

Move .swiftmodule output directory #7103

Merged
merged 1 commit into from
Dec 7, 2023
Merged

Conversation

neonichu
Copy link
Contributor

While working on #7090, we noticed that any builds of Swift sources will currently find lots of unrelated module maps because of the structure of the build arena. We pass a -I to the top-level of the arena which leads to everything being recursively searched for modules, so the compiler will find module maps that aren't intended to be included at all.

Changing this so that the search paths won't include random module maps should actually solve various issues around seeing incomplete module maps, such as ones referring to ObjC compatibility headers that haven't been written, yet.

rdar://89082987

@neonichu neonichu self-assigned this Nov 16, 2023
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

Test Suite 'Selected tests' started at 2023-11-16 19:31:47.513Test Suite 'PackageToolTests' started at 2023-11-16 19:31:47.515Test Case 'PackageToolTests.testDumpSymbolGraphCompactFormatting' started at 2023-11-16 19:31:47.515/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:522: error: PackageToolTests.testDumpSymbolGraphCompactFormatting : XCTUnwrap failed: expected non-nil value of type "URL" -/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:522: error: PackageToolTests.testDumpSymbolGraphCompactFormatting : XCTUnwrap threw error "XCTestErrorWhileUnwrappingOptional()" -Test Case 'PackageToolTests.testDumpSymbolGraphCompactFormatting' failed (4.282 seconds)Test Suite 'PackageToolTests' failed at 2023-11-16 19:31:51.797Executed 1 test, with 2 failures (1 unexpected) in 4.282 (4.282) secondsTest Suite 'Selected tests' failed at 2023-11-16 19:31:51.797Executed 1 test, with 2 failures (1 unexpected) in 4.282 (4.282) seconds
Test Suite 'Selected tests' started at 2023-11-16 19:31:47.508Test Suite 'PackageToolTests' started at 2023-11-16 19:31:47.509Test Case 'PackageToolTests.testDumpSymbolGraphPrettyFormatting' started at 2023-11-16 19:31:47.510/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:533: error: PackageToolTests.testDumpSymbolGraphPrettyFormatting : XCTUnwrap failed: expected non-nil value of type "URL" -/home/build-user/swiftpm/Tests/CommandsTests/PackageToolTests.swift:533: error: PackageToolTests.testDumpSymbolGraphPrettyFormatting : XCTUnwrap threw error "XCTestErrorWhileUnwrappingOptional()" -Test Case 'PackageToolTests.testDumpSymbolGraphPrettyFormatting' failed (4.513 seconds)Test Suite 'PackageToolTests' failed at 2023-11-16 19:31:52.022Executed 1 test, with 2 failures (1 unexpected) in 4.513 (4.513) secondsTest Suite 'Selected tests' failed at 2023-11-16 19:31:52.023Executed 1 test, with 2 failures (1 unexpected) in 4.513 (4.513) seconds

@neonichu
Copy link
Contributor Author

Hm, #7106 suggests that these tests work on main, but I don't know why they would start failing here. Seems as if they are continuing to pass on macOS.

@neonichu
Copy link
Contributor Author

--- bootstrap: note: Installing /Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swiftpm-macosx-x86_64/x86_64-apple-macosx/release/PackageDescription.swiftmodule to /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/PackageDescription.swiftmodule
Traceback (most recent call last):
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 823, in <module>
    main()
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 65, in main
    args.func(args)
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 398, in install
    install_swiftpm(prefix, args)
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 438, in install_swiftpm
    install_dylib(args, "PackageDescription", dest, ["PackageDescription", "CompilerPluginSupport"])
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 460, in install_dylib
    install_binary(args, module + ".swiftmodule", install_dir)
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 467, in install_binary
    install_file(args, src, destination, destination_is_directory=destination_is_directory, ignored_patterns=ignored_patterns)
  File "/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Utilities/bootstrap", line 480, in install_file
    shutil.copy2(src, dest)
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/shutil.py", line 435, in copy2
    copyfile(src, dst, follow_symlinks=follow_symlinks)
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/shutil.py", line 264, in copyfile
    with open(src, 'rb') as fsrc, open(dst, 'wb') as fdst:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swiftpm-macosx-x86_64/x86_64-apple-macosx/release/PackageDescription.swiftmodule'

Install needs to be adjusted to new paths.

@neonichu
Copy link
Contributor Author

I'm unable to reproduce the Linux failures outside of CI :/

@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

@swift-ci please test windows

1 similar comment
@neonichu
Copy link
Contributor Author

@swift-ci please test windows

@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

Seems like the symbol graph command doesn't actually fail when it fails to do its job?

For example, it'll say

Couldn't load module 'Foo' in the current SDK and search paths.
Current visible modules:

but still exit successfully. These tests also don't log any output right now.

@neonichu
Copy link
Contributor Author

Ah no, the command works fine, but our harness around it completely ignores the exit code.

@neonichu
Copy link
Contributor Author

So I think I have to fix the harness first, then the testing code and after that I should be able to debug properly what is actually going wrong here.

@neonichu neonichu force-pushed the move-swiftmodule-output-directory branch from e439947 to 18d198a Compare November 29, 2023 21:21
@neonichu
Copy link
Contributor Author

Once #7141 has landed, we should be able to rebase this PR and hopefully get more information on how / why the dump symbol graph tests are failing on Linux CI.

@neonichu neonichu force-pushed the move-swiftmodule-output-directory branch from 18d198a to fe77713 Compare November 30, 2023 20:40
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

error: Failed to emit symbol graph for 'Bar': Couldn't load module 'Bar' in the current SDK and search paths.

This confirms my initial suspicion, but I am not sure I understand why there's a difference specifically on Linux CI here.

@neonichu neonichu force-pushed the move-swiftmodule-output-directory branch from fe77713 to d98c8c7 Compare November 30, 2023 22:25
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu neonichu force-pushed the move-swiftmodule-output-directory branch from d98c8c7 to 207a053 Compare November 30, 2023 22:43
@neonichu
Copy link
Contributor Author

@swift-ci please test

2 similar comments
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Dec 1, 2023

Looks like sourcekit-lsp needs changes to deal with the new modules directory

@neonichu
Copy link
Contributor Author

neonichu commented Dec 1, 2023

It doesn't seem as if the failures reproduce locally :/

While working on #7090, we noticed that any builds of Swift sources will currently find lots of unrelated module maps because of the structure of the build arena. We pass a `-I` to the top-level of the arena which leads to *everything* being recursively searched for modules, so the compiler will find module maps that aren't intended to be included at all.

Changing this so that the search paths won't include random module maps should actually solve various issues around seeing incomplete module maps, such as ones referring to ObjC compatibility headers that haven't been written, yet.

rdar://89082987
@neonichu neonichu force-pushed the move-swiftmodule-output-directory branch from 7582422 to d263869 Compare December 6, 2023 16:44
@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci please test

1 similar comment
@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

/home/build-user/sourcekit-lsp/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift:162: error: SwiftPMWorkspaceTests.testBasicSwiftArgs : failed - pattern ["-I", "/tmp/TemporaryDirectory.eLu8fe/pkg/.build/x86_64-unknown-linux-gnu/debug"] not found in arguments ["-module-name", "lib", "-incremental", "-emit-dependencies", "-emit-module", "-emit-module-path", "/tmp/TemporaryDirectory.eLu8fe/pkg/.build/x86_64-unknown-linux-gnu/debug/lib.swiftmodule", "-parse-as-library", "-c", "/tmp/TemporaryDirectory.eLu8fe/pkg/Sources/lib/a.swift", "-I", "/tmp/TemporaryDirectory.eLu8fe/pkg/.build/x86_64-unknown-linux-gnu/debug/Modules", "-target", "x86_64-unknown-linux-gnu", "-swift-version", "4.2", "-enable-batch-mode", "-index-store-path", "/tmp/TemporaryDirectory.eLu8fe/pkg/.build/x86_64-unknown-linux-gnu/debug/index/store", "-Onone", "-enable-testing", "-j36", "-DSWIFT_PACKAGE", "-DDEBUG", "-module-cache-path", "/tmp/TemporaryDirectory.eLu8fe/pkg/.build/x86_64-unknown-linux-gnu/debug/ModuleCache", "-parseable-output", "-parse-as-library", "-g", "-Xcc", "-fPIC", "-Xcc", "-g", "-Xcc", "-fno-omit-frame-pointer"]
Test Case 'SwiftPMWorkspaceTests.testBasicSwiftArgs' failed (0.518 seconds)

Hm, looks as if this was somehow using the old sourcekit-lsp code?

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

Yah, not sure what's going on, the log says it is using 8dc46c3, which is not a commit on my PR or on main ¯\_(ツ)_/¯

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

I guess I'll retry

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci please test

@MaxDesiatov
Copy link
Contributor

swiftlang/sourcekit-lsp#987
@swift-ci test windows

2 similar comments
@MaxDesiatov
Copy link
Contributor

swiftlang/sourcekit-lsp#987
@swift-ci test windows

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci test windows

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci test windows

1 similar comment
@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci test windows

@neonichu
Copy link
Contributor Author

neonichu commented Dec 6, 2023

swiftlang/sourcekit-lsp#987
@swift-ci test macOS

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

Seems like macOS CI is busted for now, so let's wait for that to get fixed.

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

swiftlang/sourcekit-lsp#987
@swift-ci test macOS

@neonichu neonichu merged commit 20b86fe into main Dec 7, 2023
5 checks passed
@neonichu neonichu deleted the move-swiftmodule-output-directory branch December 7, 2023 16:22
neonichu added a commit that referenced this pull request Dec 8, 2023
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this pull request May 29, 2024
When we detect that we’re running using a 5.10 toolchain, adjust the compiler arguments that we received from SwiftPM to drop any `/Modules` suffixes in the build directory, to account for the fact that 5.10 stores the modules one directory higher up (swiftlang/swift-package-manager#7103).
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.

3 participants