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

Replace SPM dependency with Swift Tool Support #59

Merged
merged 1 commit into from
Jun 23, 2020

Conversation

lakpa
Copy link
Contributor

@lakpa lakpa commented Jun 13, 2020

Signed-off-by: ldindu [email protected]

Issue number of the reported bug or feature request: #46

Describe your changes
Replacing Swift Package Manager dependency with Swift Tool Support to reduce the dependency footprint.

Testing performed
Not applicable

@marciniwanicki
Copy link
Contributor

Many thanks @lakpa for the PR. Looks very good!

The CI failure is probably not related to your changes. I will investigate it this week.

xcrun swift test --enable-code-coverage
646[283/283] Linking xcdiffPackageTests
6472020-06-13 00:45:39.895 xctest[4465:9761] The bundle “xcdiffPackageTests.xctest” couldn’t be loaded because it is damaged or missing necessary resources. Try reinstalling the bundle.
6482020-06-13 00:45:39.895 xctest[4465:9761] (dlopen_preflight(/Users/travis/build/bloomberg/xcdiff/.build/x86_64-apple-macosx/debug/xcdiffPackageTests.xctest/Contents/MacOS/xcdiffPackageTests): Library not loaded: /System/Library/Frameworks/CryptoKit.framework/Versions/A/CryptoKit
649  Referenced from: /Users/travis/build/bloomberg/xcdiff/.build/x86_64-apple-macosx/debug/libSwiftToolsSupport.dylib
650  Reason: image not found)
651error: Error Domain=NSCocoaErrorDomain Code=260 "The folder “codecov” doesn’t exist." UserInfo={NSFilePath=/Users/travis/build/bloomberg/xcdiff/.build/x86_64-apple-macosx/debug/codecov, NSUserStringVariant=(
652    Folder
653), NSUnderlyingError=0x7fa50b564050 {Error Domain=NSOSStatusErrorDomain Code=-43 "fnfErr: File not found"}}
654make: *** [test] Error 1
655error: .build/debug/xcdiff: Failed to load coverage: No such file or directory
656error: Could not load coverage information
657The command "make clean test ; ./Scripts/coverage.sh" exited with 1

@marciniwanicki
Copy link
Contributor

Thanks @lakpa, would you mind switching dependency to use SwiftToolsSupport-auto instead of SwiftToolsSupport?

Looks that Swift Tools Support (https://github.com/apple/swift-tools-support-core/blob/master/Package.swift) has 2 variations: dynamic SwiftToolsSupport and static SwiftToolsSupport-auto. To ease the installation process we'd probably prefer to use static. I wonder if that would help with the CI failure.

We can also try to point to the latest 0.1.5 version.

@lakpa
Copy link
Contributor Author

lakpa commented Jun 21, 2020

Thanks @lakpa, would you mind switching dependency to use SwiftToolsSupport-auto instead of SwiftToolsSupport?

Looks that Swift Tools Support (https://github.com/apple/swift-tools-support-core/blob/master/Package.swift) has 2 variations: dynamic SwiftToolsSupport and static SwiftToolsSupport-auto. To ease the installation process we'd probably prefer to use static. I wonder if that would help with the CI failure.

We can also try to point to the latest 0.1.5 version.

Sure, I will get it updated

@lakpa
Copy link
Contributor Author

lakpa commented Jun 21, 2020

It appears to be complaining as it couldn't CryptoKit.framework

Screenshot 2020-06-21 at 21 36 08

and that's available from macOS 10.15+ onwards

@@ -23,7 +23,7 @@ let package = Package(
),
],
dependencies: [
.package(url: "https://github.com/apple/swift-package-manager", .upToNextMajor(from: "0.5.0")),
.package(url: "https://github.com/apple/swift-tools-support-core.git", .upToNextMajor(from: "0.1.0")),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe if we bump the version to 0.1.5 we'll get the fix for this :)

swiftlang/swift-tools-support-core@f34f419#diff-8a8089dc1f441b3f42878901d077d13cR11

Thanks @lakpa !

"revision": "9abcc2260438177cecd7cf5185b144d13e74122b",
"version": "0.5.0"
"revision": "81e2cda46447b4d78c18e03b0d284d76950f7c2b",
"version": "0.1.5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah nvm it's already picked that version up - how odd :/

@codecov
Copy link

codecov bot commented Jun 22, 2020

Codecov Report

Merging #59 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #59   +/-   ##
=======================================
  Coverage   96.12%   96.12%           
=======================================
  Files          43       43           
  Lines        1598     1598           
=======================================
  Hits         1536     1536           
  Misses         62       62           
Impacted Files Coverage Δ
Sources/XCDiffCommand/CommandRunner.swift 95.49% <ø> (ø)
Sources/XCDiffCommand/Constants.swift 94.44% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4656838...9b31b7e. Read the comment docs.

@lakpa lakpa requested a review from kwridan June 22, 2020 22:55
@lakpa
Copy link
Contributor Author

lakpa commented Jun 22, 2020

Recent Xcode update change has fixed the failing CI issue, thanks @kwridan !!!

Copy link
Contributor

@marciniwanicki marciniwanicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks a lot @lakpa 👍

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