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.

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

Pin SwiftLint #4258

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Pin SwiftLint #4258

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0a821c3
Installs SwiftLint through SPM.
JayShortway Sep 10, 2024
cc2ce28
CI no longer installs SwiftLint via Homebrew.
JayShortway Sep 10, 2024
0565f33
Removes SwiftLint from the Brewfile.
JayShortway Sep 10, 2024
dfea1ec
Adds SwiftLint as an Xcode Package Dependency and uses it as a Build …
JayShortway Sep 10, 2024
4892ba5
pre-commit.sh is adjusted to use SwiftLint as a Swift package plugin
JayShortway Sep 10, 2024
43f2fc8
pre-commit.sh correctly prints SwiftLint version
JayShortway Sep 10, 2024
1235b90
Cleans up the output from pre-commit.sh printing the SwiftLint version
JayShortway Sep 10, 2024
12cf9eb
CI uses SwiftLint as a swift package plugin.
JayShortway Sep 10, 2024
6022e17
Adds swift to dependabot.
JayShortway Sep 10, 2024
5b75763
Revert "Adds SwiftLint as an Xcode Package Dependency and uses it as …
JayShortway Sep 10, 2024
cf1aa39
swiftlint.sh calls swiftlint as a swift package plugin.
JayShortway Sep 10, 2024
eb8e20c
Reverts unintended changes to the Brewfile.
JayShortway Sep 10, 2024
8f0256a
Fixes swiftlint.sh using xcrun.
JayShortway Sep 11, 2024
854cd35
Updates Package.resolved in the xcodeproj.
JayShortway Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 14 additions & 25 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,6 @@ commands:
install_mint:
type: boolean
default: false
install_swiftlint:
type: boolean
default: true
steps:
- install-bundle-dependencies:
directory: << parameters.directory >>
Expand All @@ -183,16 +180,10 @@ commands:
steps:
- install-brew-dependency:
dependency_name: "mint"
- when:
condition: << parameters.install_swiftlint >>
steps:
- install-brew-dependency:
dependency_name: "swiftlint"
- run: brew tap robotsandpencils/made
- save_cache:
key: homebrew-cache-{{ checksum "Brewfile.lock.json" }}-{{ arch }}
paths:
- /usr/local/Cellar/swiftlint/
- /usr/local/Cellar/xcbeautify/
- /Users/$USER/Library/Caches/Homebrew/

Expand Down Expand Up @@ -490,8 +481,7 @@ jobs:

steps:
- checkout
- install-dependencies:
install_swiftlint: false
- install-dependencies
- update-spm-installation-commit
- run:
name: SPM RevenueCatUI Tests
Expand Down Expand Up @@ -519,8 +509,7 @@ jobs:

steps:
- checkout
- install-dependencies:
install_swiftlint: false
- install-dependencies
- update-spm-installation-commit
- run:
name: SPM RevenueCatUI Release Build
Expand Down Expand Up @@ -552,8 +541,7 @@ jobs:

steps:
- checkout
- install-dependencies:
install_swiftlint: false
- install-dependencies
- update-spm-installation-commit
- run:
name: SPM RevenueCatUI Release Build
Expand Down Expand Up @@ -632,8 +620,7 @@ jobs:

steps:
- checkout
- install-dependencies:
install_swiftlint: false
- install-dependencies
- run:
name: Run tests
command: bundle exec fastlane test_ios
Expand All @@ -660,8 +647,7 @@ jobs:

steps:
- checkout
- install-dependencies:
install_swiftlint: false
- install-dependencies
- update-spm-installation-commit
- run:
name: Run tests
Expand Down Expand Up @@ -693,8 +679,7 @@ jobs:
steps:
- checkout
- set-maximum-duration
- install-dependencies:
install_swiftlint: false
- install-dependencies
- update-spm-installation-commit
- run:
name: Run tests
Expand Down Expand Up @@ -773,7 +758,6 @@ jobs:
- install-dependencies:
install_xcbeautify: false
install_mint: true
install_swiftlint: false
- install-xcbeautify-for-xcode14
- update-spm-installation-commit
- install-runtime:
Expand Down Expand Up @@ -807,7 +791,6 @@ jobs:
- install-dependencies:
install_xcbeautify: false
install_mint: true
install_swiftlint: false
- macos/install-rosetta
- install-xcbeautify-for-xcode14
- update-spm-installation-commit
Expand Down Expand Up @@ -1106,8 +1089,14 @@ jobs:
- run:
name: Run fastlane swiftlint lane
command: |
bundle exec fastlane run swiftlint raise_if_swiftlint_error:true strict:true \
reporter:junit output_file:fastlane/test_output/swiftlint/junit.xml
# FIXME This doesn't work because somehow swift package plugins run for every target/module/scheme?
# Only the last of those runs will save the output file, causing issues to be lost.
swift package plugin \
--allow-writing-to-directory fastlane/test_output/swiftlint \
swiftlint \
--strict \
--reporter junit \
--output fastlane/test_output/swiftlint/junit.xml
Comment on lines +1094 to +1099
Copy link
Member Author

@JayShortway JayShortway Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This doesn't work because somehow swift package plugins run for every module. Only the last of those runs will save the output file, causing issues to be lost.

Copy link
Member Author

Choose a reason for hiding this comment

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

Running it with verbose output tells me it's running for each module, which map to the targets specified in our Package.swift:

info: Finished running in module 'RevenueCat_CustomEntitlementComputation'
info: Finished running in module 'RevenueCatUITests'
info: Finished running in module 'RevenueCatUI'
info: Finished running in module 'RevenueCat'
info: Finished running in module 'ReceiptParserTests'
info: Finished running in module 'ReceiptParser'

I think this is swift package that's running the plugin command for each target. It seems to be happening outside of swiftlint's control. Not sure yet how to make swift package confine itself to a single target.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a convention to support a --target argument. SwiftLint's command plugin supports it partly. It passes the --target argument on to the actual swiftlint program, which doesn't recognize it and errors out.

There’s a fork that fixes this issue. I’ve asked if they can propose their fix upstream.

Copy link
Member Author

Choose a reason for hiding this comment

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

0.57.1 includes a fix for this.

- store_test_results:
path: fastlane/test_output
- store_artifacts:
Expand Down
4 changes: 4 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ updates:
directory: "/" # Location of package manifests
schedule:
interval: "daily"
- package-ecosystem: "swift" # See documentation for possible values
directory: "/" # Location of package manifests
schedule:
interval: "daily"
1 change: 0 additions & 1 deletion Brewfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ tap "homebrew/cask"
tap "homebrew/core"
tap "robotsandpencils/made"

brew "swiftlint"
brew "xcbeautify"
brew "xcodes"
44 changes: 0 additions & 44 deletions Brewfile.lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,50 +15,6 @@
}
},
"brew": {
"swiftlint": {
"version": "0.53.0",
"bottle": {
"rebuild": 0,
"root_url": "https://ghcr.io/v2/homebrew/core",
"files": {
"arm64_sonoma": {
"cellar": ":any_skip_relocation",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:240ccda9de55d948d0c635798079074099bfcb73ffda41428900fdc748aeea7b",
"sha256": "240ccda9de55d948d0c635798079074099bfcb73ffda41428900fdc748aeea7b"
},
"arm64_ventura": {
"cellar": ":any_skip_relocation",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:7b7ceb7896c6833965cc4eac9001255d8adde6c5432045d5a8ab6aea8a9e81d9",
"sha256": "7b7ceb7896c6833965cc4eac9001255d8adde6c5432045d5a8ab6aea8a9e81d9"
},
"arm64_monterey": {
"cellar": ":any_skip_relocation",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:78c2a4c3f4a2f6847b484527b0f0f916da71e3ee29e49890fd44b63fe7b38e26",
"sha256": "78c2a4c3f4a2f6847b484527b0f0f916da71e3ee29e49890fd44b63fe7b38e26"
},
"sonoma": {
"cellar": ":any_skip_relocation",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:abdca78dd8a8bd268053b3be195fe891bb74aef5502ab3a6b871ae0c6bb04540",
"sha256": "abdca78dd8a8bd268053b3be195fe891bb74aef5502ab3a6b871ae0c6bb04540"
},
"ventura": {
"cellar": ":any_skip_relocation",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:be711c707bf3b49fa0dd6e2ae576b309aad620f9b56a2c6e7b1ac5cf35cf652a",
"sha256": "be711c707bf3b49fa0dd6e2ae576b309aad620f9b56a2c6e7b1ac5cf35cf652a"
},
"monterey": {
"cellar": ":any_skip_relocation",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:13487d68a971dbe035019364e19d70641af2a18c06e52925d238685b384a7979",
"sha256": "13487d68a971dbe035019364e19d70641af2a18c06e52925d238685b384a7979"
},
"x86_64_linux": {
"cellar": "/home/linuxbrew/.linuxbrew/Cellar",
"url": "https://ghcr.io/v2/homebrew/core/swiftlint/blobs/sha256:fbbc56fccfcfcd34564feb7325567e2ff3638d3c609396a5c4aa13311c7b26e0",
"sha256": "fbbc56fccfcfcd34564feb7325567e2ff3638d3c609396a5c4aa13311c7b26e0"
}
}
}
},
"xcbeautify": {
"version": "1.0.0",
"bottle": {
Expand Down
9 changes: 9 additions & 0 deletions Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@
"revision" : "26ed3a2b4a2df47917ca9b790a57f91285b923fb",
"version" : "1.12.0"
}
},
{
"identity" : "swiftlintplugins",
"kind" : "remoteSourceControl",
"location" : "https://github.com/SimplyDanny/SwiftLintPlugins",
"state" : {
"revision" : "7c80ce6f142164b0201871e580b021d1b2c69804",
"version" : "0.57.0"
}
}
],
"version" : 2
Expand Down
3 changes: 2 additions & 1 deletion Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ let shouldIncludeDocCPlugin = environmentVariables["INCLUDE_DOCC_PLUGIN"] == "tr
var dependencies: [Package.Dependency] = [
.package(url: "[email protected]:Quick/Nimble.git", from: "10.0.0"),
// SST requires iOS 13 starting from version 1.13.0
.package(url: "[email protected]:pointfreeco/swift-snapshot-testing.git", .upToNextMinor(from: "1.12.0"))
.package(url: "[email protected]:pointfreeco/swift-snapshot-testing.git", .upToNextMinor(from: "1.12.0")),
.package(url: "https://github.com/SimplyDanny/SwiftLintPlugins", exact: "0.57.0")
]
if shouldIncludeDocCPlugin {
// Versions 1.4.0 and 1.4.1 are failing to compile, so we are pinning it to 1.3.0 for now
Expand Down
9 changes: 9 additions & 0 deletions RevenueCat.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@
"revision" : "26ed3a2b4a2df47917ca9b790a57f91285b923fb",
"version" : "1.12.0"
}
},
{
"identity" : "swiftlintplugins",
"kind" : "remoteSourceControl",
"location" : "https://github.com/SimplyDanny/SwiftLintPlugins",
"state" : {
"revision" : "7c80ce6f142164b0201871e580b021d1b2c69804",
"version" : "0.57.0"
}
}
],
"version" : 2
Expand Down
27 changes: 8 additions & 19 deletions scripts/pre-commit.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
#!/bin/bash

# Figure out where swiftlint is
HOMEBREW_BINARY_DESTINATION="/opt/homebrew/bin"
SWIFT_LINT="${HOMEBREW_BINARY_DESTINATION}/swiftlint"

if ! test -d $HOMEBREW_BINARY_DESTINATION; then
# X86_64 macs have this destination
SWIFT_LINT="/usr/local/bin/swiftlint"
fi
SWIFT_LINT="swift package plugin swiftlint"

# Start timer
START_DATE=$(date +"%s")
Expand Down Expand Up @@ -57,18 +51,13 @@ verify_no_included_apikeys() {
done
}

if [[ -e "${SWIFT_LINT}" ]]; then
echo "SwiftLint version: $(${SWIFT_LINT} version)"
# Run only if not merging
if ! git rev-parse -q --verify MERGE_HEAD; then
# Run for just staged files
while IFS= read -r -d '' file; do
run_swiftlint "${file}"
done < <(git diff --cached --name-only -z)
fi
else
echo "${SWIFT_LINT} is not installed. Please install it via: fastlane setup_dev"
exit -1
echo "SwiftLint version: $(${SWIFT_LINT} --version | head -n 1)"
# Run only if not merging
if ! git rev-parse -q --verify MERGE_HEAD; then
# Run for just staged files
while IFS= read -r -d '' file; do
run_swiftlint "${file}"
done < <(git diff --cached --name-only -z)
fi

END_DATE=$(date +"%s")
Expand Down
30 changes: 1 addition & 29 deletions scripts/swiftlint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,3 @@
#!/usr/bin/env bash

# Arm64 macs have this destination.
HOMEBREW_BINARY_DESTINATION="/opt/homebrew/bin"
if ! test -d $HOMEBREW_BINARY_DESTINATION; then
# X86_64 macs have this destination
HOMEBREW_BINARY_DESTINATION="/usr/local/bin"
fi

echo "Adding homebrew bin folder to PATH (${HOMEBREW_BINARY_DESTINATION})"
PATH="${HOMEBREW_BINARY_DESTINATION}:${PATH}"

if which swiftlint >/dev/null; then
script_path="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
source_path="${script_path}/../"
swiftlint_path="$(which swiftlint)"

echo "linter path:"
echo $swiftlint_path

lint_command="${swiftlint_path} lint"
echo "linter command: ${lint_command}"

pushd "${source_path}"
# Run swiftlint but filter out "Linting ..." to clean up output
$lint_command 2>&1 | grep -v 'Linting '
popd
else
echo "Warning: SwiftLint not installed in ${HOMEBREW_BINARY_DESTINATION}, download from https://github.com/realm/SwiftLint"
fi

swift package plugin swiftlint lint --quiet
Copy link
Member Author

@JayShortway JayShortway Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Running this in a Run Script Build Phase in Xcode doesn't work. I suspect it has to do with environment variables because I can reproduce it in the terminal if I source the Xcode environment first. This is the error I get:

Invalid manifest (compiled with: [a-ton-of-options])
warning: using sysroot for 'iPhoneSimulator' but targeting 'MacOSX'
error: unable to load standard library for target 'arm64-apple-macosx13.0'

Happens on CI too, see here.

Copy link
Contributor

@jamesrb1 jamesrb1 Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This gets it going on my machine!

Suggested change
swift package plugin swiftlint lint --quiet
/usr/bin/xcrun --sdk macosx swift package plugin swiftlint lint --quiet
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome, thanks! Is there an advantage of using the absolute path to xcrun? Isn't /usr/bin usually part of the $PATH?

Copy link
Contributor

Choose a reason for hiding this comment

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

No advantage if it's already available via $PATH.