-
Notifications
You must be signed in to change notification settings - Fork 463
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
How to setup spotless for a git pre commit hook that only check changed files #178
Comments
The spotless gradle plugin has fast up-to-date checking built-in, so performance should be very good even without limiting to changed files. If your objective is to enforce the check only on changed files, that is a good idea, but it is not supported at this time. EDIT: now supported via (bad advice from me deleted here) |
Understood. Following is my workaround now. Hope it helps someone else who want to achieve this too. #!/bin/sh
# From gist at https://gist.github.com/chadmaughan/5889802
echo '[git hook] executing gradle spotlessCheck before commit'
# stash any unstaged changes
git stash -q --keep-index
# run the spotlessCheck with the gradle wrapper
./gradlew spotlessCheck --daemon
# store the last exit code in a variable
RESULT=$?
# unstash the unstashed changes
git stash pop -q
# return the './gradlew spotlessCheck' exit code
exit $RESULT |
Our latest versions ( |
(Also, if there is a way to make |
I've got a slanted view here, since I use a git client that ignores the staging concept. I know that some users get a lot of power out of the staging area, and I'd love for Spotless to be helpful to them. Perhaps what you'd really like is If |
(Also, since it is possible to have both staged and non-staged changes in the same file, |
Looks like it can.
The staging area is not set of changes. For each file, git has three binary blobs. The working copy, the head, and the index (aka staging area). Each of those is a binary blob, and the "changes" are backed-out by diffing the blobs, not the other way around. In the simple way to use git, a file is either unstaged (index == head) or staged (index == working copy). In the "power" way, where only some "changes" are staged, all it means is that (index != head != working copy).
|
I created a pre-commit hook that formats the code and adds the formatted files that were initially in the staging area back to the staging area in order to commit the actual formatted code. It leaves the other files as is. you can install it by running this in your repository root:
#!/bin/bash
set -e
filesToAddAfterFormatting=()
containsJavaOrKotlin=0
# Collect all files currently in staging area, and check if there are any java or kotlin files
for entry in $(git status --porcelain | sed -r 's/[ \t]+/-/g')
do
# entry can be for example:
# MM-src/main/java/net/project/MyController.java
# -M-src/main/java/net/project/AnotherController.java
if [[ $entry == M* ]] ; then
filesToAddAfterFormatting+=(${entry:2}) # strips the prefix
fi
if [[ $entry == *.java ]] || [[ $entry == *.kt ]] ; then
containsJavaOrKotlin=1
fi
done;
# If any java or kotlin files are found, run spotlessApply
if [ "$containsJavaOrKotlin" == "1" ] ; then
echo "Kotlin and/or Java found in staging, running: ./gradlew -PdisableSpotlessCheck spotlessApply"
./gradlew -PdisableSpotlessCheck spotlessApply
else
echo "Not running spotlessApply"
fi
# Add the files that were in the staging area
for fileToAdd in $filesToAddAfterFormatting
do
echo "re-adding $fileToAdd after formatting"
git add "$fileToAdd"
done; |
Thanks for sharing @toefel18. |
@nedtwigg you are welcome :D, is there an option to make spotlessApply format one single file? |
Yes, the IDE hook. |
nice one - but that one seem to fail when using
this is failing on OSX since sed here does not support I fixed it by replacing the |
Not sure if it is helpful nowadays for someone but I created a pre-commit hook that runs spotless maven plugin goals in maven projects. |
I fell into a trap which I spent lots of time to be awared of, I left comment here to help some guy like me : The script above works well with reformat staged code when commit is not as simple as what at first glance. lint-staged introduces its algorithm |
Hi, just checking, Is there any solution here? I end up manually running |
Hey, I have created a pre-commit script that takes the approach described in lint-staged. It applies spotless on the staged changes and supports the changes that were partially stashed. You can find it here. The only issue that still stands is that if there are a bunch of changed files then this can be a bit slow as we can only pass one file to |
@bhaskarmelkani thanks for sharing. Speed improvements related to this would land in #623 |
Hey @nedtwigg, I would directly use But the problem I see with the ratchet is that it scans all the files in the index to decide if there are any changes in one of those files. Maybe I am missing something in the configuration or this is expected? |
I don't think that's true. It only scans the files which Gradle has identified as changed, and it scans them with git one file at a time. spotless/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java Lines 74 to 87 in 2ade60a
The first invocation might be a little slow, but after that it should be the fastest possible way thanks to the Gradle daemon's filesystem monitoring. It also takes advantage of the Gradle buildcache, which can speedup even the very first invocation if you are using it. My guess is that you might have some |
Hey, thanks for getting back :) By a large number of files, I say for instance I have like 16k java files in my project that come under the target. If I understood correctly the So maybe calling the method I will check and reply here if I find something more :) |
For the very first run ever, it will call the method 16k times, and that will indeed be very slow. But after that, Gradle will only call the method for files which have changed on disk, and it will use its daemon filesystem monitor to prune that number to single digits. If you are using the gradle build cache, then even on a fresh clone it should not need to call the method 16k times because it can use the cached value. |
This is a great thread, nice to see this has been discussed in such depth already. Is there currently a specific recommendation for folks who want to run |
So from how I understand this thread, simply adding an option to pass multiple files to spotless instead of just a single file would already be a great improvement? As that allows for custom git hooks that don't have to start a new spotless process for each file. And a Edit: It seems there was a |
I've been using lint-staged to run spotless on staged files. It's been working beautifully, but lint-staged is an npm package. It only makes sense to use lint-staged if your project is using node. |
Sorta... For the long-tail of users who are not happy with our current integration with git workflows, I think I don't understand how you see the problem. The way I see the problem is that spotless is designed to enforce a formatting invariant, always. That formatting invariant is either
I can imagine that you might have some arbitrary list of files, so maybe bringing back the |
tl;dr If Thanks very much for the detailed explanation. I guess I should have been a little clearer on my thought process and what our use case is. Our project currently doesn't use any automatic, but we'd like to add it. I'm currently checking out various options what we can use and how we can integrate it with git hooks. This plugin looks pretty promisingin that it isn't tied to any specific formatter and provides a standardized way to integrate with different formatters. Our goal is pretty standard: ensure all files are formatted and that people don't commit unformatted code. In that sense, the invariant enforced by standard mode would be all we need. But since it's a large project with many files -- definitely on the order of magnitude of 16k files, for which somebody in this thread mentioned performance issues when the plugin needs to check all files. So yes, the proposed A while ago I had experimented a little bit with git-code-format-maven-plugin on another smaller project. It does exactly the same as the proposed We probably won't be using that formatting plugin as it only formats Java files, but it illustrates a nice approach to the problem. Formatting staged files is also what prettier suggests, using other such as lint-staged and calling the prettier command with a list of staged files. To summarize, I'll need to experiment a little bit more with this plugin, and maybe current caching strategy is already fast enough so that no optimizations are needed. Being able to format only staged files would be a nice option to have if or when the hook turns out to need too much time (and some developers have slower machines than I do). |
With the simplest "spotlessApply 16k files" option, you have Gradle using build cache so that it's only ever slow once, and you also have Gradle file-monitor based up-to-date checking, so it's extra super fast on a given machine after the first run. Setting up git hooks automatically is a great idea, I added that to #623. Implementing |
Well, we are using Maven, not Gradle, but we'll see how performance turns out to be. I'm glad your open for PRs and I might take a look at it depending on how it turns out. |
Hi, finally we understood how the spotlessFiles are working :) and we put this in pre-commit hook
|
The solution described below should work for most situations. The pre-commit script described below eliminates the need to use Maven#!/bin/sh
echo '[git hook] executing spotless:apply to format code before commit'
# Stash any unstaged changes
git stash -q --keep-index
# Run spotless:apply
# We pass in ratchetFrom here to ensure that we only format the files that have changed since the last commit
if command -v mvn > /dev/null 2>&1 # Check if mvn command exists to support GitHub Desktop on Windows
then
mvn spotless:apply -DratchetFrom=HEAD # Requires Maven to be installed
else
./mvnw spotless:apply -DratchetFrom=HEAD # Otherwise call maven wrapper for Mac-OS / Unix / Git for Windows
fi
# Store the last exit code in a variable
RESULT=$?
# Stage formatting changes
git add -u
# Un-stash the stashed changes
git stash pop -q
# Return the 'spotless:apply' exit code
exit $RESULT
GradleThe Gradle equivalent would be: #!/bin/sh
echo '[git hook] executing spotlessApply to format code before commit'
# Stash any unstaged changes
git stash -q --keep-index
# Run spotlessApply
# We pass in ratchetFrom here to ensure that we only format the files that have changed since the last commit
if command -v gradle > /dev/null 2>&1 # Check if gradle command exists to support GitHub Desktop on Windows
then
gradle spotlessApply -PratchetFrom=HEAD # Requires Gradle to be installed
else
./gradlew spotlessApply -PratchetFrom=HEAD # Otherwise call gradle wrapper for Mac-OS / Unix / Git for Windows
fi
# Store the last exit code in a variable
RESULT=$?
# Stage formatting changes
git add -u
# Un-stash the stashed changes
git stash pop -q
# Return the 'spotlessApply' exit code
exit $RESULT But the above Gradle solution assumes a few things:
tasks.register('installLocalGitHook', Copy) {
from new File(rootProject.rootDir, '.hooks/pre-commit')
into { new File(rootProject.rootDir, '.git/hooks') }
filePermissions {
user {
read true
write true
execute true
}
group {
read true
write true
execute true
}
other {
read true
write false
execute true
}
}
}
spotless {
String ref = project.properties["ratchetFrom"]
if (ref != null) {
ref = ref.trim()
if (ref.length() > 0) {
ratchetFrom ref
}
}
...
} |
@mrlonis this can fail if there are staged and unstaged changes in the same file and applying formatting causes conflicts with unstaged section. A variant of @bhaskarmelkani script that calls spotlessApply once (with |
I didn't bother doing this for Gradle, but for Maven I think I can resolve the concerns of conflicts with unstaged changes with the following pre-commit #!/bin/sh
handleMergeCommit() {
if [ -f .git/MERGE_HEAD ]
then
echo "[git pre-commit hook] - Merge in progress. Exiting pre-commit hook."
exit 0
fi
}
handleEmptyCommitAtStart() {
git diff --cached --quiet
GIT_DIFF=$?
if [ "$GIT_DIFF" -eq 0 ]; then
echo "[git pre-commit hook] - No changes to commit! This is likely a rebase or merge commit. Exiting pre-commit hook."
exit 0
fi
}
stageUnStagedChangesInStagedFiles() {
STAGED_FILES=$(git diff --name-only --cached --diff-filter=ad)
if [ -n "$STAGED_FILES" ]; then
echo "$STAGED_FILES" | xargs git add
fi
}
spotlessApply() {
# We pass in ratchetFrom here to ensure that we only format the files that have changed since the last commit
if command -v mvn >/dev/null 2>&1; then # Check if mvn command exists to support GitHub Desktop on Windows
mvn spotless:apply -DratchetFrom=HEAD -q # Requires Maven to be installed
else
./mvnw spotless:apply -DratchetFrom=HEAD -q # Otherwise call maven wrapper for Mac-OS / Unix / Git for Windows
fi
}
handleMergeConflicts() {
conflictedFiles="$(git diff --name-only --diff-filter=U)"
if [ -n "$conflictedFiles" ]; then
for conflictedFile in $conflictedFiles; do
echo "[git pre-commit hook] - Resolving conflict for $conflictedFile"
git checkout --theirs "$(pwd)/$conflictedFile"
git restore --staged "$(pwd)/$conflictedFile"
if command -v mvn >/dev/null 2>&1; then
mvn spotless:apply -DspotlessFiles=".*$conflictedFile" -q
else
./mvnw spotless:apply -DspotlessFiles=".*$conflictedFile" -q
fi
git add "$conflictedFile"
done
fi
}
handleEmptyCommitAtEnd() {
git diff --cached --quiet
GIT_DIFF=$?
if [ "$GIT_DIFF" -eq 0 ]; then
echo "[git pre-commit hook] - No changes to commit! Aborting commit!"
# We end up with no changes to commit here due to the stashing and un-stashing of changes
# where we stash a bad formatting change, apply the formatting, and then un-stash the bad formatting change.
#
# Example:
# - We accidentally indented 1 line in a file by an extra space. This causes us to stash the extra space, apply the
# formatting which will remove the extra space, and then un-stash the extra space back into the file that the
# formatter just removed. By re-running spotless:apply, we remove the extra space again and remove the file from
# the staging area.
#
# This results in no changes to commit, so we exit with 1 to prevent committing an empty commit.
# We run spotless:apply again here to ensure that the files are formatted correctly and to remove the file from the staging area.
spotlessApply
exit 1
fi
}
handleMergeCommit
handleEmptyCommitAtStart
stageUnStagedChangesInStagedFiles
git stash clear
git stash -q --keep-index
echo "[git pre-commit hook] - Running spotless:apply"
spotlessApply
SPOTLESS_APPLY_RESULT=$?
git add -u
git stash pop -q
handleMergeConflicts
handleEmptyCommitAtEnd
exit $SPOTLESS_APPLY_RESULT post-commit #!/bin/sh
# We pass in ratchetFrom here to ensure that we only format the files that have changed since the last commit
if command -v mvn >/dev/null 2>&1; then # Check if mvn command exists to support GitHub Desktop on Windows
mvn spotless:apply -DratchetFrom=HEAD -q # Requires Maven to be installed
else
./mvnw spotless:apply -DratchetFrom=HEAD -q # Otherwise call maven wrapper for Mac-OS / Unix / Git for Windows
fi This seems to handle most scenarios I throw at it |
Super necro, but for gradle I've come up with this:
spotless {
String ref = project.properties["ratchetFrom"]
if (ref != null) {
ref = ref.trim()
if (ref.length() > 0) {
ratchetFrom ref
}
}
java {
target "**/*.java"
googleJavaFormat("1.18.0").aosp()
importOrder "", "javax", "java", "\\#"
}
groovyGradle {
target "**/*.gradle"
greclipse().configFile("tools/spotless-gradle.properties")
}
format 'styling', {
target '**/*.md', '**/*.yml', '**/*.yaml', '**/*.json'
prettier()
}
}
#!/usr/bin/env bash
set -euo pipefail
if [ "${PRE_COMMIT:-0}" -ne 1 ]; then
echo "This script should only be run by pre-commit! Exiting." >&2
exit 1
fi
REQUIREMENTS=("git" "java")
for i in "${REQUIREMENTS[@]}"; do
if ! command -v "${i}" &> /dev/null; then
echo "'${i}' is required to run this script. Please install it." >&2
exit 1
fi
done
RATCHET_REF="${PRE_COMMIT_FROM_REF:-HEAD^}"
echo "Running spotlessApply on files changed since ${RATCHET_REF}"
# shellcheck source=../gradlew
./gradlew --no-daemon --console=plain :spotlessApply -PratchetFrom="${RATCHET_REF}"
repos:
- repo: local
hooks:
- id: gradle-spotless-apply
name: Run Gradle Spotless Apply
language: script
entry: ./scripts/pre-commit__spotless-apply.sh
types_or:
- java
- groovy
- yaml
- markdown
- json
pass_filenames: false
require_serial: true In normal use (ie. as a git hook) it will ratchetFrom Obviously this is pretty tightly coupled with pre-commit, so YMMV with the needed context stuff. |
I'm using spotless with an android kotlin project now. And I have spotless set up like this
What I want to do is set up a git pre commit hook that apply
gradlew spotlessCheck
before the commit but only on the commited files. It's something like this eslint-pre-commit-checkI have search around the repo but can't figure out how to achieve this. Is there any way to do this? Thanks.
The text was updated successfully, but these errors were encountered: