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

[Fastlane.Swift] Allow overriding LaneFileProtocol lifecycles when subclassing LaneFile #20563

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SvenTiigi
Copy link
Contributor

@SvenTiigi SvenTiigi commented Aug 16, 2022

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Resolves #21849

When using Fastlane.Swift in the context of a Swift Package the LaneFileProtocol lifecycles such as beforeAll, onError and afterAll can not be overridden by a subclass of LaneFile.

Package.swift
// swift-tools-version: 5.6

import PackageDescription

let package = Package(
    name: "FastlaneSwiftPackage",
    products: [
        .executable(
            name: "FastlaneSwiftPackage",
            targets: [
                "FastlaneSwiftPackage"
            ]
        )
    ],
    dependencies: [
        .package(
            url: "https://github.com/fastlane/fastlane",
            .exact("2.209.0")
        )
    ],
    targets: [
        .executableTarget(
            name: "FastlaneSwiftPackage",
            dependencies: [
                .product(
                    name: "Fastlane",
                    package: "fastlane"
                )
            ]
        )
    ]
)
import Foundation
import Fastlane

@main
class Fastfile: LaneFile {
    
    static func main() {
        Main().run(with: Fastfile())
    }
    
    // ❌ Method does not override any method from its superclass
    override func beforeAll(
        with lane: String
    ) {
        super.beforeAll(with: lane)
    }
    
    // ❌ Overriding non-open instance method outside of its defining module
    override func onError(
        currentLane: String,
        errorInfo: String,
        errorClass: String?,
        errorMessage: String?
    ) {
        super.onError(
            currentLane: currentLane,
            errorInfo: errorInfo,
            errorClass: errorClass,
            errorMessage: errorMessage
        )
    }
    
    // ❌ Method does not override any method from its superclass
    override func afterAll(
        with lane: String
    ) {
        super.afterAll(with: lane)
    }
    
}

Description

This PR changes the modifier of the onError function of the LaneFile from public to open to fix the Overriding non-open instance method outside of its defining module error (b9dba0a).

Additionally, the beforeAll and afterAll lifecycles have been added as open functions to the LaneFile to fix the Method does not override any method from its superclass error (f4f5380).

Allowing subclasses to override the onError function
Allowing subclasses to override the lifecycles
@karthikAdaptavant
Copy link

karthikAdaptavant commented Dec 5, 2022

Hi SvenTiigi, I have a quick doubt, I have made a new executable package by adding Fastlane spm as dependency.
I couldn't find any documentation on integrating the newly created executable package in my Xcode project to run the lanes.

Mostly I could find fastlane init swift which asked me to configure fastlane in my root folder with the new Fastfile.swift.
It would be really help if you share any example project or documentation to overriding
FastlaneRunner -project ./fastlane/swift/FastlaneSwiftRunner/FastlaneSwiftRunner.xcodeprojto my new executable package runner.

New exec Package.swift with my custom lanes.

let package = Package(
    name: "fastlaneRunner",
    products: [
        .executable(name: "fastlaneRunner", targets: ["fastlaneRunner"])
    ],
    dependencies: [
        .package(url: "https://github.com/fastlane/fastlane", .upToNextMinor(from: .init(2, 179, 0)))
    ],
    targets: [
        .executableTarget(
            name: "fastlaneRunner",
            dependencies: [
                .product(name: "Fastlane", package: "fastlane") 
            ],
            path: "Sources"
        )
    ]
)

@pchmelar
Copy link

pchmelar commented Feb 7, 2023

Just bumped into this as I'm experimenting with Fastlane.swift via SPM.
Would be helpful to merge this, so we can define custom beforeAll / afterAll etc
@joshdholtz What do you think? 🙂

@karthikAdaptavant Not sure if I get your question, but you can either integrate the fastlaneRunner package with other packages in your project, or you can simply run the executable via swift run fastlaneRunner lane [yourLane] - see docs here https://docs.fastlane.tools/getting-started/ios/fastlane-swift/

@pchmelar
Copy link

I just discovered that when swift run fastlaneRunner lane [yourLane] fails with some error the exit code is 0. This present a serious issue when running on CI, as the job fails, but a pipeline doesn't stop. The solution would be to handle this in onError, but that's not possible when using Fastlane.swift via SPM. 😥

So.. can we please merge this PR? It's just a simple change. cc @mollyIV @janpio @joshdholtz

@pchmelar
Copy link

Bump, can we please merge this? It's the only major issue preventing us from using Fastlane.swift in production 😿
cc @mollyIV @janpio @joshdholtz

Copy link
Collaborator

@lacostej lacostej left a comment

Choose a reason for hiding this comment

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

Thanks for the nagging :)

Looks good to me, but I am not a Swift user, so I'll ask others in the team to review as well.

Thanks for your patience!

@SvenTiigi
Copy link
Contributor Author

Just a small hint regarding the attached issue #21849:
This PR does not directly resolve the issue that when running Fastlane.swift via SPM e.g. swift run MyFastlaneSwiftExecutable lane myLane the execution always exits with a zero success code even when an error occurred.

So in addition to this PR it would be great to add an appropriate invocation of exit(EXIT_FAILURE) or fatalError around the following lines of code wrapped in a #if SWIFT_PACKAGE since this problem does not occur when running Fastlane.Swift via the "traditional" Fastlane Swift Runner started via the fastlane CLI.

if !LaneFile.onErrorCalled.contains(lane) {
fastfileInstance.afterAll(with: lane)
}
log(message: "Done running lane: \(lane) 🚀")
return true

There is a similar open PR #20552 to this problem.

@rogerluan
Copy link
Member

@SvenTiigi thanks for all this valuable info 🙌 I like your approach to restrict the changes in the PR #20552 to Swift Packages only, do you think they work, though? I found this: https://forums.swift.org/t/unable-to-compile-swift-package-when-used-in-xcode/36081

@SvenTiigi
Copy link
Contributor Author

@rogerluan since the #if SWIFT_PACKAGE preprocessor macro is already used in the LaneFileProtocol.swift file I think it should work properly 🤔

@mwhrtin
Copy link

mwhrtin commented Feb 21, 2024

This PR is not getting the attention it needs considering the issues it fixes, so I just want to chime in that we're also waiting on this. 🙂

We've also been experimenting with Fastlane.swift through SPM and the issues this PR is taking care of are the ones that are currently blocking us from trying it out in a live environment.

@SvenTiigi
Copy link
Contributor Author

@rogerluan Do you have any updates on this PR's status?

@pchmelar
Copy link

@rogerluan @lacostej Can we please merge this? 🙏 Is there anything that needs to be done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fastlane.Swift] When FastlaneRunner fails, the exit code is 0 instead of 1
7 participants