realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.55k stars 2.21k forks source link

Xcode 14 introduces new run script build phase warning for SwiftLint #4015

Open opfeffer opened 2 years ago

opfeffer commented 2 years ago

New Issue Checklist

Describe the bug

Possibly somewhat of a duple of #2783, but Xcode 14 appears to emit a new warning for run script build phases without specified outputs:

warning build: Run script build phase 'SwiftLint' will be run during every build because it does not specify any outputs. To address this warning, either add output dependencies to the script phase, or configure it to run in every build by unchecking "Based on dependency analysis" in the script phase.

image

I'm following the usage guidance as documented in the README.md. I'm curious what the proper configuration should be for to appease this warning as by default SwiftLint doesn't really modify any files. That said, it seems excessive to run in every build if no input files have changed....

Adding link to Apple's documentation of Run Script build phases that highly recommends specifying input and output files: https://developer.apple.com/documentation/xcode/running-custom-scripts-during-a-build?changes=_2_3

Complete output when running SwiftLint, including the stack trace and command used
export PATH="$PATH:/opt/homebrew/bin"

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

Environment

samitaha2 commented 2 years ago

Also following! Thank you for opening up this task, I was just about to do the same

kamaal111 commented 2 years ago

Under the script you can uncheck the checkbox with the label Based on dependency analysis and this warning will go away

opfeffer commented 2 years ago

Under the script you can uncheck the checkbox with the label Based on dependency analysis and this warning will go away

You're right that checking that box makes this warning disappear. But afaict that doesn't really solve the bigger issue that swiftlint runs on every file regardless of modification status during incremental builds. Depending on the size of your codebase this can have significant impact on compile time.

Would love to find a way to use Xcode's input/output files to only run swiftlint on the files that actually changed maybe? 🙏

cdhoffmann commented 2 years ago

Under the script you can uncheck the checkbox with the label Based on dependency analysis and this warning will go away

You're right that checking that box makes this warning disappear. But afaict that doesn't really solve the bigger issue that swiftlint runs on every file regardless of modification status during incremental builds. Depending on the size of your codebase this can have significant impact on compile time.

Would love to find a way to use Xcode's input/output files to only run swiftlint on the files that actually changed maybe? 🙏

100%

yosi199 commented 2 years ago

Under the script you can uncheck the checkbox with the label Based on dependency analysis and this warning will go away

You're right that checking that box makes this warning disappear. But afaict that doesn't really solve the bigger issue that swiftlint runs on every file regardless of modification status during incremental builds. Depending on the size of your codebase this can have significant impact on compile time.

Would love to find a way to use Xcode's input/output files to only run swiftlint on the files that actually changed maybe? 🙏

+1 on that, looking for a solution

fahad-frontend commented 2 years ago

Under the script you can uncheck the checkbox with the label Based on dependency analysis and this warning will go away

You're right that checking that box makes this warning disappear. But afaict that doesn't really solve the bigger issue that swiftlint runs on every file regardless of modification status during incremental builds. Depending on the size of your codebase this can have significant impact on compile time.

Would love to find a way to use Xcode's input/output files to only run swiftlint on the files that actually changed maybe? 🙏

100% agree

AndreaMiotto commented 1 year ago

Any news on this? I can see some PRs have been merged but I'm still having the warning. And im not sure if disabling the Xcode checkmark is a good idea.

jifang commented 1 year ago

Under the script you can uncheck the checkbox with the label Based on dependency analysis and this warning will go away

You're right that checking that box makes this warning disappear. But afaict that doesn't really solve the bigger issue that swiftlint runs on every file regardless of modification status during incremental builds. Depending on the size of your codebase this can have significant impact on compile time.

Would love to find a way to use Xcode's input/output files to only run swiftlint on the files that actually changed maybe? 🙏

This is what we do for running swiftlint only on modified files (based on git status)

git status --porcelain | grep -v '^ \?D' | cut -c 4- | sed 's/.* -> //' | tr -d '"' | while read file 
do
    if [[ "$file" == *.swift ]]; then
        swiftlint "$file" --fix
    fi
done
mywristbands commented 1 year ago

This is what we do for running swiftlint only on modified files (based on git status)

git status --porcelain | grep -v '^ \?D' | cut -c 4- | sed 's/.* -> //' | tr -d '"' | while read file 
do
    if [[ "$file" == *.swift ]]; then
        swiftlint "$file" --fix
    fi
done

Thanks for that, but I still want to make sure all files are passing the linter. What I'd like is for Xcode to only run swiftlint on files that have been changed since the last incremental build.

KrisLau commented 1 year ago

https://github.com/realm/realm-swift/issues/7891#issuecomment-1192447849

Gobans commented 1 year ago

If anyone who using Tuist, modify TargetAction option make sure basedOnDependencyAnalysis would be false.

import ProjectDescription

public extension TargetScript {
    static let SwiftLintString = TargetScript.pre(script: """
if test -d "/opt/homebrew/bin/"; then
    PATH="/opt/homebrew/bin/:${PATH}"
fi

export PATH="$PATH:/opt/homebrew/bin"

if which swiftlint > /dev/null; then
    swiftlint
else
    echo "warning: SwiftLint not installed, download from https://github.com/realm/SwiftLint"
fi
""", name: "SwiftLintString", basedOnDependencyAnalysis: false) // here change the option
}

Because of this warning, I was suffered a little 😂 Wish this comment could be helpful

kenji21 commented 1 year ago

This is what we do for running swiftlint only on modified files (based on git status)

git status --porcelain | grep -v '^ \?D' | cut -c 4- | sed 's/.* -> //' | tr -d '"' | while read file 
do
    if [[ "$file" == *.swift ]]; then
        swiftlint "$file" --fix
    fi
done

Thanks for that, but I still want to make sure all files are passing the linter. What I'd like is for Xcode to only run swiftlint on files that have been changed since the last incremental build.

we even do use the "Product -> Analyse" to lint all files (and disable for previews/IBDesignables ones) :

if [[ "$BUILD_DIR" == *"IBDesignables"* ]] || [[ "$BUILD_DIR" == *"Previews"* ]] ; then
    echo "not linting for IBDesignables/SwiftUI Previews builds";
    exit 0
fi

if [[ $RUN_CLANG_STATIC_ANALYZER == "YES" ]] ; then
    time swiftlint
else 
[.....]
    time swiftlint --use-script-input-files
fi
razor313 commented 1 year ago

None of the previous solutions worked for me. do you have any suggestion, preferably a simple one? :)))

alexanderkhitev commented 1 year ago

Hey guys! Do we have any news/updates? Thanks

kaiserabliz commented 1 year ago

Still watching this thread.

steven851007 commented 1 year ago

I've solved the issue by adding a pre-build action script that dynamically generates the input files before every build for SwitLint. This setup correctly solves the warning and also improves the incremental build times. You can find a sample project with this setup here, with the script included and with an explanation of how it works:

https://github.com/steven851007/SwiftLint_build_phase_example

kaiserabliz commented 1 year ago

I've solved the issue by adding a pre-build action script that dynamically generates the input files before every build for SwitLint. This setup correctly solves the warning and also improves the incremental build times. You can find a sample project with this setup here, with the script included and with an explanation of how it works:

https://github.com/steven851007/SwiftLint_build_phase_example

@steven851007 Thanks, this is really interesting, Looking forward to testing it out today.

cabyambo commented 1 year ago

Has there been any official fix for this?

jonnyijapan commented 1 year ago

Threads like this one on SO could be worth a follow.

Iikeli commented 1 year ago

Has there been any work on this? Or is there a good hack that works?

jrtibbetts commented 1 year ago

I'm seriously considering removing the SwiftLint build phase entirely until this is fixed. I can run it manually as needed, and keep it in the CI/CD flow.

mildm8nnered commented 1 year ago

I'm seriously considering removing the SwiftLint build phase entirely until this is fixed. I can run it manually as needed, and keep it in the CI/CD flow.

This is a feature of, and these warnings are coming from, Xcode, and there's not very much that SwiftLint can do about it.

For any run script build phase, you will either need to uncheck the "Based on dependency analysis" checkbox, in which case you will see a message, but not a warning per se, about how the script will be run on every build, or leave the box checked, but not specify any output and input files, in which case you will see the warning, or you can add input and output files, as per the referenced SO thread, or the other suggestions above.

jfahrenkrug commented 1 year ago

I'm not familiar with the SwiftLint codebase, but here's a question: Would adding a flag like --incremental or something along those lines be an option? It could write two files into - maybe - a .swiftlint directory. One file that contains info about the files that have been linted in the last run, (maybe with last modified values or a hash), and a file that contains info about the linting results for the last run. If those two files are used as the output params in Xcode's run script build phase, and all swift files in the project are used as the input.... would that solve this issue? Or is that a naive idea and I'm missing something?

mildm8nnered commented 1 year ago

I'm not familiar with the SwiftLint codebase, but here's a question: Would adding a flag like --incremental or something along those lines be an option? It could write two files into - maybe - a .swiftlint directory. One file that contains info about the files that have been linted in the last run, (maybe with last modified values or a hash), and a file that contains info about the linting results for the last run. If those two files are used as the output params in Xcode's run script build phase, and all swift files in the project are used as the input.... would that solve this issue? Or is that a naive idea and I'm missing something?

You could just touch somefile in your Run Script build phase, and make somefile your output file, and that would be pretty much exactly functionally equivalent to the above.

rajivd15 commented 1 year ago

@opfeffer Did you find a good solution for this issue? I do see similar warnings for Crashlytics, GraphQL build phases. One thing I tried was to just add an Output file to the build phase as I do not have any inputs to the script -$(DERIVED_FILE_DIR)/Pods-checkSwiftLintResult.txt and the warning didn't show up. I just wanted to see if someone can confirm - would all the SwiftLint errors will show up in that file? not sure how CI/CD pipeline will be affected by this change.

alexanderkhitev commented 11 months ago

Hey guys! Do we have any news/updates? Thanks

nguyenlam96 commented 11 months ago

hi, any updates to optimize build time? thanks

jonnyijapan commented 11 months ago

Just an idea... remove swiftlint from Xcode build phase(s), and only use it before GIT commits?

MakarUrbanov commented 11 months ago

Guys just add $(DERIVED_FILE_DIR)/swiftlint_output into your swiftlint build phrase to output files list and the warning is gone

My xcodegen:

postCompileScripts:
      - path: "Scripts/swiftLint.sh"
        name: SwiftLint
        outputFiles: ["$(DERIVED_FILE_DIR)/swiftlint_output"]
jonnyijapan commented 11 months ago

Guys just add $(DERIVED_FILE_DIR)/swiftlint_output into your swiftlint build phrase to output files list and the warning is gone

My xcodegen:


postCompileScripts:

      - path: "Scripts/swiftLint.sh"

        name: SwiftLint

        outputFiles: ["$(DERIVED_FILE_DIR)/swiftlint_output"]

I tried this but and yes it got rid of that message but actually it now fails to add comment on lines that failed lint testing. So I assume that the lint testing stops working with this change. Maybe it depends on what you have in that swiftLint.sh file?

MakarUrbanov commented 11 months ago

Guys just add $(DERIVED_FILE_DIR)/swiftlint_output into your swiftlint build phrase to output files list and the warning is gone My xcodegen:


postCompileScripts:

      - path: "Scripts/swiftLint.sh"

        name: SwiftLint

        outputFiles: ["$(DERIVED_FILE_DIR)/swiftlint_output"]

I tried this but and yes it got rid of that message but actually it now fails to add comment on lines that failed lint testing. So I assume that the lint testing stops working with this change. Maybe it depends on what you have in that swiftLint.sh file?

Yeah you're right. My solution doesn't work correctly 🙃 So now in this case, I don’t know how to get rid of this warning, but “Based on dependency analysis” solves the problem on small projects

NikKovIos commented 9 months ago

The task is not just remove a warning. Swiftlint takes a time to lint every time. So we need to note lint not changed files.

litoarias commented 7 months ago

Guys just add $(DERIVED_FILE_DIR)/swiftlint_output into your swiftlint build phrase to output files list and the warning is gone My xcodegen:


postCompileScripts:

      - path: "Scripts/swiftLint.sh"

        name: SwiftLint

        outputFiles: ["$(DERIVED_FILE_DIR)/swiftlint_output"]

I tried this but and yes it got rid of that message but actually it now fails to add comment on lines that failed lint testing. So I assume that the lint testing stops working with this change. Maybe it depends on what you have in that swiftLint.sh file?

Yeah you're right. My solution doesn't work correctly 🙃 So now in this case, I don’t know how to get rid of this warning, but “Based on dependency analysis” solves the problem on small projects

Maybe is so later to respond this for 'xcodegen' users, but for if anyme helps I use the following code to clean the warning:

....
postCompileScripts:
   - path: "Scripts/swiftLint.sh"
     name: SwiftLint
     basedOnDependencyAnalysis: false
....