Work around .swiftlint.yml failure when using as a dependency

[mokagio]

For some reason, when swift or xcodebuild try to resolve WordPressShared as a dependency, they don't have access to .swiftlint.yml and the resolution fails.

This PR updates the version setting logic to fallback to an hardcoded version if .swiftlint.yml cannot be read.

---

Update As I was looking at this PR and writing the description line above, I realized that having an hardcoded fallback sort of defeats the purpose of reading the version dynamically? I mean, it solves the crash issue right now, but leaves the door open for the version going out of sync etc.

Maybe it would be best to do the following:

1. Only use hardcoded version. I.e. remove the custom logic to read the version.
2. Have a check to ensure the Package.swift (maybe Package.resolved) and .swiftlint.yml configs are aligned.

cc @iangmaia

---

Before

After

---

• I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

[dangermattic]

	1 Warning
⚠️	Package.swift was changed without updating its corresponding Package.resolved. Please resolve the Swift packages in Xcode.

Generated by 🚫 Danger

[crazytonyli]

IMO, I don't think we should use swift build for this library, because swift build runs against macOS, but this library is an iOS library.

[mokagio]

@crazytonyli

I don't think we should use swift build for this library

Just in case, the library experiencing the issue is WordPressKit when resolving the packages. However, the point about not using swift build is 100% correct for both WordPressKit and this library.

As a matter of fact, while hacking around I used Fastlane to run_test via xcodebuild on the scheme generated by the package, code.

The result is the same, though:

[crazytonyli]

SPM plugin is a feature in 5.6. Have you tried to bump the swift version in Package.swift to a more recent version like 5.10: // swift-tools-version:5.5?

[mokagio]

SPM plugin is a feature in 5.6. Have you tried to bump the swift version in Package.swift to a more recent version like 5.10: // swift-tools-version:5.5?

@crazytonyli I did not. Good catch.

I'll try it later. But I don't expect it to work. My theory is that swift/xcodebuild does the package resolution without checking out the whole repo, or maybe it loads the Package.swift in a different folder. Regardless of how that happens, the result is that when it parses this file it doesn't have access to the .swiftconfig.yml file.

We'll see if I'm on the right track.

It might still be the case that later toolchain versions have a resolution system that doesn't run into that issue. But then... how does the swift-tools version of the dependency affect the behavior? Shouldn't it be a problem depending on the swift-tools version of the app or package that does the resolution? In our case, that's WordPressKit at 5.9.

Ugh... lots of moving parts. 🤔

[iangmaia]

Maybe it would be best to do the following:

1. Only use hardcoded version. I.e. remove the custom logic to read the version.
2. Have a check to ensure the Package.swift (maybe Package.resolved) and .swiftlint.yml configs are aligned.

cc @iangmaia

+1 to having a single source of truth. I think with the lack of a better option, this isn't that bad -- in fact, it can be a viable pattern for libraries 🤔

Not sure if this would work, but it came to my mind the feature of bundling resources in SPM. Perhaps something like this would work for .swiftlint.yml? 🤔

[crazytonyli]

the check will fail when 'swift build' attempts to resolve the package as a dependency.

Have you tried using #filePath here?

[mokagio]

I'll give it a shot. But, I don't expect it to make much difference.

According to the docs:

Currently, #file has the same value as #filePath.

[mokagio]

Tried via mokagio/swiftlint-file-path branch. Same result 😞

[mokagio]

I added this to the package to "debug" step by step

 func loadSwiftLintVersion() -> Version {
+    // Testing stuff out...
+    let packagePath = #filePath
+    let packageURL = URL(fileURLWithPath: #filePath)
+    let packageFolder = packageURL.deletingLastPathComponent
+
+    fatalError(
+        """
+        packagePath: (#filePath) = \(packagePath)
+        packageURL: (file URL with ... #filePath) = \(packageURL)
+        packageFolder: (packageURL deletingLastPathComponent) = \(packageFolder)
+        """
+    )
+
     let swiftLintConfigURL = URL(fileURLWithPath: #filePath)
         .deletingLastPathComponent()
         .appendingPathComponent(".swiftlint.yml")

The result during package resolution in swift build (I used that because it's faster to run on the repo without moving Xcode files out of the way)

^main/Package.swift:71:
Fatal error: 
packagePath: (#filePath) = /Package.swift
packageURL: (file URL with ... #filePath) = file:///Package.swift
packageFolder: (packageURL deletingLastPathComponent) = file:///
in https://github.com/wordpress-mobile/WordPress-iOS-Shared.git

This confirms that the problem is not with .swiftlint.yml but with how swift/xcodebuild reads Package.swift during resolution.

[kean]

There still seems to be an issue when installing the package:

x-xcode-log://F5477906-731E-45AF-89D2-B1A6FEF32FF6 product 'SwiftLintPlugin' required by package 'wordpress-ios-shared' target 'WordPressShared' not found in package 'SwiftLint'. Did you mean 'SwiftLintBinary'?

I'd consider not using SPM for installing plugins as they end up being downloaded by everyone who consumes this API and doesn't care about how its CI is setup.

[mokagio]

I'd consider not using SPM for installing plugins as they end up being downloaded by everyone who consumes this API and doesn't care about how it's CI setup.

That's fair. Particularly for SwiftLint which only lints the sources instead of performing something as a build plugin that is required for the build to succeed. If it was required for the build to succeed, then I'd argue it's appropriate for consumers to download it (and I'd say that's the intention behind Apple's design) but that's not the case here...

Side note: This makes me wish for support for development dependencies like Node packages and Ruby gems do.

If we weren't using the default package structure to get SwiftLint, what would you suggest using?

Also, looks like upstream SwiftLint changed the location of their plugin, which may help with this error in the short term: https://github.com/realm/SwiftLint/blob/5c195a4bdbeefdfde2a486c35a4caffc198bb626/README.md#swift-package-manager

[kean]

If we weren't using the default package structure to get SwiftLint, what would you suggest using?

I don't know what the state-of-the-art approach is. As a package developer, having Xcode automatically install SwiftLint for you seems optimal. As a consumer, there are multiple reasons why it's not great. For instance, Xcode doesn't make any distinction between "actual" dependencies and plugins when showing the list of your dependencies in Xcode, and SwiftLint has many transient dependencies. But it doesn't mean that the approach itself is flawed.

I'm not sure it's formalized yet, but WP-iOS is going with a monorepo to cut the development costs. Tony has made massive strides by moving WordPressKit and WordPressAuthentificator to WP-iOS. I've also done a lot of work just today to reduce the number of dependencies: wordpress-mobile/WordPress-iOS#23392. I'm so close to removing CocoaPods. There is still some work left, but maybe it'll open opportunities for organizing it differently.

If it was required for the build to succeed

I haven't yet worked with SPM plugins myself, but I've been following the work of folks who've been maintaining CreateAPI. This seems like a good use case where you have to use a plugin to generate the code for the package, as it won't work without it. SwiftLint, on the other hand, by definition shouldn't be run on the consumers' machines because you are not supposed to edit the sources of the dependencies you install, so there is that.

[mokagio]

If we weren't using the default package structure to get SwiftLint, what would you suggest using?

I don't know what the state-of-the-art approach is.

Yeah, neither do I.

If I recall correctly, the main reason for using Package.swift to get SwiftLint was to ensure devs have it loaded locally with the same version CI uses. The idea being that if one gets the SwiftLint feedback locally, it's cheaper to address.

But, I think we could make a different tradeoff in favor of not having the kind valid of issues you described above (which I all blame to SPM not having support for development dependencies) and shifting the responsibility of setting up SwiftLint locally to each dev in the way they please.

@jkmassel and I bounced ideas about this offline at some point, I think. CI runs SwiftLint, so we know we'll get the feedback at some point. Devs could decide whether they want to integrate SwiftLint in their local workflow or not. If they do, there could be a recommended path to install and run (Homebrew + Git Hook? Script to vendor in the project? An rbenv like tool that downloads different versions and shims them? etc.)

---

For what is worth, please feel free to make any change that will make the development on your in WordPress end faster.