这是indexloc提供的服务，不要输入任何密码
Skip to content

Add a lint CI #21

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2025
Merged

Add a lint CI #21

merged 1 commit into from
Jun 5, 2025

Conversation

@kou
Copy link
Member

@kou kou commented Jun 4, 2025

This upgrades SwiftLint to the latest version (0.59.1) from 0.53.0. So some format fixes are included.

Closes #20.

@kou
Copy link
Member Author

kou commented Jun 4, 2025

@abandy Could you take a look at this?

raulcd
raulcd reviewed Jun 4, 2025
View reviewed changes
Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

This looks good to me but it seems to timeout on the fork. I am unsure why it would require more than 10 minutes to build the environment. Maybe it's only the first time?
https://github.com/kou/arrow-swift/actions/runs/15434589578/job/43438396587

@kou
Add a lint CI
f6cd499 
This upgrades SwiftLint to the latest version (0.59.1) from 0.53.0. So
some format fixes are included.

Closes apache#20.
@kou
Copy link
Member Author

kou commented Jun 4, 2025

Oh, sorry. I missed it.

I increased timeout to 15min from 10min.

Maybe it's only the first time?

Yes. We need to build SwiftLint that is written by Swift.

@kou
Copy link
Member Author

kou commented Jun 4, 2025

https://github.com/kou/arrow-swift/actions/runs/15435171137/job/43440064721

raulcd
raulcd approved these changes Jun 4, 2025
View reviewed changes
Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

It's green now. I'll wait for @abandy to take a look or will merge in a couple days if no-one has done it

amoeba
amoeba reviewed Jun 4, 2025
View reviewed changes
@amoeba
Copy link
Member

amoeba commented Jun 4, 2025

I tried running the hook in this PR locally on Linux and macOS (aarch64) and SwiftLint takes a bit of work to get running, I failed on both systems for different reasons. @kou I assume you have this working in your PR because you built it once and cached it? What steps did you use?

@kou
Copy link
Member Author

kou commented Jun 4, 2025

Oh, could you share the results you got?

I just executed nice pre-commit run --show-diff-on-failure --color=always --all-files like the workflow does.

@amoeba
Copy link
Member

amoeba commented Jun 4, 2025

On macOS, I get a very cryptic,

❯ nice pre-commit run --show-diff-on-failure --color=always --all-files
[INFO] Installing environment for https://github.com/realm/SwiftLint.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/usr/bin/swift', 'build', '--package-path', '/Users/bryce/.cache/pre-commit/repozxdjlrih', '-c', 'release', '--build-path', '/Users/bryce/.cache/pre-commit/repozxdjlrih/swift_env-default/.build')
return code: 1
stdout: (none)
stderr:
    error: 'repozxdjlrih': Invalid manifest (compiled with: ["/Library/Developer/CommandLineTools/usr/bin/swiftc", "-vfsoverlay", "/var/folders/hr/w0c05_fn7x5b8nyslfp8tfsw0000gn/T/TemporaryDirectory.NZQCcp/vfs.yaml", "-L", "/Library/Developer/CommandLineTools/usr/lib/swift/pm/ManifestAPI", "-lPackageDescription", "-Xlinker", "-rpath", "-Xlinker", "/Library/Developer/CommandLineTools/usr/lib/swift/pm/ManifestAPI", "-target", "arm64-apple-macosx14.0", "-F", "/Library/Developer/CommandLineTools/Library/Developer/Frameworks", "-sdk", "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk", "-swift-version", "5", "-I", "/Library/Developer/CommandLineTools/usr/lib/swift/pm/ManifestAPI", "-sdk", "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk", "-package-description-version", "5.9.0", "/Users/bryce/.cache/pre-commit/repozxdjlrih/Package.swift", "-o", "/var/folders/hr/w0c05_fn7x5b8nyslfp8tfsw0000gn/T/TemporaryDirectory.IrnSdW/repozxdjlrih-manifest"])
    error: link command failed with exit code 1 (use -v to see invocation)
    Undefined symbols for architecture arm64:
      "PackageDescription.Package.__allocating_init(name: Swift.String, defaultLocalization: PackageDescription.LanguageTag?, platforms: [PackageDescription.SupportedPlatform]?, pkgConfig: Swift.String?, providers: [PackageDescription.SystemPackageProvider]?, products: [PackageDescription.Product], dependencies: [PackageDescription.Package.Dependency], targets: [PackageDescription.Target], swiftLanguageVersions: [PackageDescription.SwiftVersion]?, cLanguageStandard: PackageDescription.CLanguageStandard?, cxxLanguageStandard: PackageDescription.CXXLanguageStandard?) -> PackageDescription.Package", referenced from:
          _main in Package-1.o
    ld: symbol(s) not found for architecture arm64
    clang: error: linker command failed with exit code 1 (use -v to see invocation)
    error: 'repozxdjlrih': Invalid manifest (compiled with: ["/Library/Developer/CommandLineTools/usr/bin/swiftc", "-vfsoverlay", "/var/folders/hr/w0c05_fn7x5b8nyslfp8tfsw0000gn/T/TemporaryDirectory.aKZjft/vfs.yaml", "-L", "/Library/Developer/CommandLineTools/usr/lib/swift/pm/ManifestAPI", "-lPackageDescription", "-Xlinker", "-rpath", "-Xlinker", "/Library/Developer/CommandLineTools/usr/lib/swift/pm/ManifestAPI", "-target", "arm64-apple-macosx14.0", "-F", "/Library/Developer/CommandLineTools/Library/Developer/Frameworks", "-sdk", "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk", "-swift-version", "5", "-I", "/Library/Developer/CommandLineTools/usr/lib/swift/pm/ManifestAPI", "-sdk", "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk", "-package-description-version", "5.9.0", "/Users/bryce/.cache/pre-commit/repozxdjlrih/Package.swift", "-o", "/var/folders/hr/w0c05_fn7x5b8nyslfp8tfsw0000gn/T/TemporaryDirectory.bOAVKI/repozxdjlrih-manifest"])
    error: link command failed with exit code 1 (use -v to see invocation)
    Undefined symbols for architecture arm64:
      "PackageDescription.Package.__allocating_init(name: Swift.String, defaultLocalization: PackageDescription.LanguageTag?, platforms: [PackageDescription.SupportedPlatform]?, pkgConfig: Swift.String?, providers: [PackageDescription.SystemPackageProvider]?, products: [PackageDescription.Product], dependencies: [PackageDescription.Package.Dependency], targets: [PackageDescription.Target], swiftLanguageVersions: [PackageDescription.SwiftVersion]?, cLanguageStandard: PackageDescription.CLanguageStandard?, cxxLanguageStandard: PackageDescription.CXXLanguageStandard?) -> PackageDescription.Package", referenced from:
          _main in Package-1.o
    ld: symbol(s) not found for architecture arm64
    clang: error: linker command failed with exit code 1 (use -v to see invocation)
Check the log at /Users/bryce/.cache/pre-commit/pre-commit.log

and when I try to install swiftlint from brew, I get,

swiftlint: A full installation of Xcode.app 8.0 is required to compile
this software. Installing just the Command Line Tools is not sufficient.

Xcode can be installed from the App Store.
Error: swiftlint: An unsatisfied requirement failed this build.

@amoeba
Copy link
Member

amoeba commented Jun 4, 2025

I think the above are related and makes sense. The ubuntu runner must ship with a standard Swift toolchain so this works on CI. On a Mac, you can't just use the CommandLineTools to run run pre-commit hook, you have to have a full XCode toolchain.

+1 to this PR as-is.

@abandy
Copy link
Contributor

abandy commented Jun 5, 2025

Lgtm! I tried it locally on my mac and it works as expected. (swiftlint has worked since we setup the linting when swift was in the main arrow repo so I probably am benefitting from prior setup).

@kou kou merged commit 0a91054 into apache:main Jun 5, 2025
@kou kou deleted the lint branch June 5, 2025 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amoeba amoeba amoeba left review comments

@raulcd raulcd raulcd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a lint CI

4 participants

@kou @amoeba @abandy @raulcd