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

Possible build failure on Carthage due to Unneeded Build Dependency #74

Closed
toshi0383 opened this issue Sep 30, 2016 · 5 comments
Closed
Labels

Comments

@toshi0383
Copy link
Contributor

Instruction has swiftlint Run Script Phase inside the xcodeproj.

source ~/.profile
if which swiftlint > /dev/null; then
    swiftlint
else
    echo "warning: SwiftLint not installed, download from https://github.com/realm/SwiftLint"
fi

This potentially causes build error on Carthage.
For example, ~/.profile is missing in my environment, so Run Script Build Phase failed.

IMO In general, if you are writing a library(not an app), SwiftLint check should be managed outside of xcodeproj.

@ephread
Copy link
Owner

ephread commented Sep 30, 2016

Hey @toshi0383! Thanks a lot for catching this lack of foresight on my part.

The main point of running SwiftLint as a script phase is to get the warning/errors from inside the IDE. The issue here is not the script phase, but rather the fact that it was poorly written (apologies for this, I made wrong assumptions). I'm fairly confident that SwiftLint checks can be managed from inside the .xcodeproj without causing build issues.

I will rewrite the script to make it more robust and transparent (i. e. can't make the build fail if ~/.profile or swiftlint aren't found).

@ephread ephread added the bug label Sep 30, 2016
@aloco
Copy link

aloco commented Sep 30, 2016

Hi, I ran into the same issue, I had no ~/.profile, fixed that by creating an empty .profile

@toshi0383
Copy link
Contributor Author

I will rewrite the script to make it more robust and transparent (i. e. can't make the build fail if ~/.profile or swiftlint aren't found).

No it should be this way. - Don't run swiftlint when building by Carthage.
Because... even the user has SwiftLint installed, how could you tell that SwiftLint future versions won't ever contain a bug ???

I looked over Carthage repo and I didn't know that they have been injecting env variables into xcodebuild command.
I will create a PR 👍

@ephread
Copy link
Owner

ephread commented Oct 3, 2016

No it should be this way. - Don't run swiftlint when building by Carthage.
Because... even the user has SwiftLint installed, how could you tell that SwiftLint future versions won't ever contain a bug ???

We both agree. I certainly failed to express myself properly, as I only meant that the script was not going away and that I would need to rethink the naive version. That included trying to circumvent the use of swiftlint while building externally. 😉

I wasn't aware of $CARTHAGE, though.

Thanks a lot for the PR!

@toshi0383
Copy link
Contributor Author

🚀🚢😆

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

No branches or pull requests

3 participants