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

Pin SwiftLint #4258

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

Pin SwiftLint #4258

wants to merge 14 commits into from

Conversation

JayShortway
Copy link
Member

@JayShortway JayShortway commented Sep 10, 2024

Installs SwiftLint through SPM, and runs it as

This is a draft because of the problems linked above.

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

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.

Comment on lines +1094 to +1099
swift package plugin \
--allow-writing-to-directory fastlane/test_output/swiftlint \
swiftlint \
--strict \
--reporter junit \
--output fastlane/test_output/swiftlint/junit.xml
Copy link
Member Author

@JayShortway JayShortway Sep 10, 2024

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.

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
Contributor

@jamesrb1 jamesrb1 Sep 10, 2024

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.

@vegaro vegaro added pr:other and removed pr:ci labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants