Skip to content

Commit 957cb7a

Browse files
lazer-devlazer-dev
committed
uninstall fix
1 parent 259ec38 commit 957cb7a

File tree

9 files changed

+227
-19
lines changed

9 files changed

+227
-19
lines changed

‎lib/src/main/java/com/diffplug/spotless/GitPrePushHookInstaller.java

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
*/
3232
public abstract class GitPrePushHookInstaller {
3333

34-
private static final String HOOK_HEADLINE = "##### SPOTLESS HOOK START #####";
34+
private static final String HOOK_HEADER = "##### SPOTLESS HOOK START #####";
3535
private static final String HOOK_FOOTER = "##### SPOTLESS HOOK END #####";
3636

3737
/**
@@ -128,10 +128,48 @@ public void install() throws Exception {
128128
*/
129129
private void uninstall(File gitHookFile) throws Exception {
130130
final var hook = Files.readString(gitHookFile.toPath(), UTF_8);
131-
final var hookStart = hook.indexOf(HOOK_HEADLINE);
132-
final var hookEnd = hook.indexOf(HOOK_FOOTER) + HOOK_FOOTER.length();
131+
final int hookStart = hook.indexOf(HOOK_HEADER);
132+
final int hookEnd = hook.indexOf(HOOK_FOOTER) + HOOK_FOOTER.length(); // hookEnd exclusive, so must be last symbol \n
133+
134+
/* Detailed explanation:
135+
* 1. hook.indexOf(HOOK_FOOTER) - finds the starting position of footer "##### SPOTLESS HOOK END #####"
136+
* 2. + HOOK_FOOTER.length() is needed because String.substring(startIndex, endIndex) treats endIndex as exclusive
137+
*
138+
* For example, if file content is:
139+
* #!/bin/sh
140+
* ##### SPOTLESS HOOK START #####
141+
* ... hook code ...
142+
* ##### SPOTLESS HOOK END #####
143+
* other content
144+
*
145+
* When we later use this in: hook.substring(hookStart, hookEnd)
146+
* - Since substring's endIndex is exclusive (it stops BEFORE that index)
147+
* - We need hookEnd to point to the position AFTER the last '#'
148+
* - This ensures the entire footer "##### SPOTLESS HOOK END #####" is included in the substring
149+
*
150+
* This exclusive behavior is why in the subsequent code:
151+
* if (hookEnd < hook.length() && hook.charAt(hookEnd) == '\n') {
152+
* hookScript += "\n";
153+
* }
154+
*
155+
* We can directly use hookEnd to check the next character after the footer
156+
* - Since hookEnd is already pointing to the position AFTER the footer
157+
* - No need for hookEnd + 1 in charAt()
158+
* - This makes the code more consistent with the substring's exclusive nature
159+
*/
160+
161+
var hookScript = hook.substring(hookStart, hookEnd);
162+
if (hookStart >= 1 && hook.charAt(hookStart - 1) == '\n') {
163+
hookScript = "\n" + hookScript;
164+
}
165+
166+
if (hookStart >= 2 && hook.charAt(hookStart - 2) == '\n') {
167+
hookScript = "\n" + hookScript;
168+
}
133169

134-
final var hookScript = hook.substring(hookStart, hookEnd);
170+
if (hookEnd < hook.length() && hook.charAt(hookEnd) == '\n') {
171+
hookScript += "\n";
172+
}
135173

136174
final var uninstalledHook = hook.replace(hookScript, "");
137175

@@ -155,8 +193,10 @@ private void uninstall(File gitHookFile) throws Exception {
155193
* @return A string template representing the Spotless Git pre-push hook content.
156194
*/
157195
protected String preHookTemplate(String executor, String commandCheck, String commandApply) {
158-
var spotlessHook = "\n";
159-
spotlessHook += "\n" + HOOK_HEADLINE;
196+
var spotlessHook = "";
197+
198+
spotlessHook += "\n";
199+
spotlessHook += "\n" + HOOK_HEADER;
160200
spotlessHook += "\nSPOTLESS_EXECUTOR=" + executor;
161201
spotlessHook += "\nif ! $SPOTLESS_EXECUTOR " + commandCheck + " ; then";
162202
spotlessHook += "\n echo 1>&2 \"spotless found problems, running " + commandApply + "; commit the result and re-push\"";
@@ -165,6 +205,7 @@ protected String preHookTemplate(String executor, String commandCheck, String co
165205
spotlessHook += "\nfi";
166206
spotlessHook += "\n" + HOOK_FOOTER;
167207
spotlessHook += "\n";
208+
168209
return spotlessHook;
169210
}
170211

@@ -186,7 +227,7 @@ private boolean isGitInstalled() {
186227
*/
187228
private boolean isGitHookInstalled(File gitHookFile) throws Exception {
188229
final var hook = Files.readString(gitHookFile.toPath(), UTF_8);
189-
return hook.contains(HOOK_HEADLINE);
230+
return hook.contains(HOOK_HEADER) && hook.contains(HOOK_FOOTER);
190231
}
191232

192233
/**

‎lib/src/main/java/com/diffplug/spotless/GitPrePushHookInstallerGradle.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ public GitPrePushHookInstallerGradle(GitPreHookLogger logger, File root) {
3838
*/
3939
@Override
4040
protected String preHookContent() {
41-
return preHookTemplate(executor(), "spotlessCheck", "spotlessApply");
41+
return preHookTemplate(executorPath(), "spotlessCheck", "spotlessApply");
4242
}
4343

44-
private String executor() {
44+
private String executorPath() {
4545
if (gradlew.exists()) {
4646
return gradlew.getAbsolutePath();
4747
}

‎lib/src/main/java/com/diffplug/spotless/GitPrePushHookInstallerMaven.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ public GitPrePushHookInstallerMaven(GitPreHookLogger logger, File root) {
3535
*/
3636
@Override
3737
protected String preHookContent() {
38-
return preHookTemplate(executor(), "spotless:check", "spotless:apply");
38+
return preHookTemplate(executorPath(), "spotless:check", "spotless:apply");
3939
}
4040

41-
private String executor() {
41+
private String executorPath() {
4242
if (mvnw.exists()) {
4343
return mvnw.getAbsolutePath();
4444
}

‎plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessInstallPrePushHookTaskTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public void should_append_to_existing_pre_hook_file_when_hook_file_exists() thro
7373
assertThat(output).contains("Installing git pre-push hook");
7474
assertThat(output).contains("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push"));
7575

76-
final var content = getTestResource("git_pre_hook/pre-push.existing-added-tpl")
76+
final var content = getTestResource("git_pre_hook/pre-push.existing-installed-end-tpl")
7777
.replace("${executor}", "gradle")
7878
.replace("${checkCommand}", "spotlessCheck")
7979
.replace("${applyCommand}", "spotlessApply");

‎plugin-maven/src/test/java/com/diffplug/spotless/maven/SpotlessInstallPrePushHookMojoTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public void should_append_to_existing_pre_hook_file_when_hook_file_exists() thro
6767
assertThat(output).contains("Installing git pre-push hook");
6868
assertThat(output).contains("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push"));
6969

70-
final var content = getTestResource("git_pre_hook/pre-push.existing-added-tpl")
70+
final var content = getTestResource("git_pre_hook/pre-push.existing-installed-end-tpl")
7171
.replace("${executor}", newFile("mvnw").getAbsolutePath())
7272
.replace("${checkCommand}", "spotless:check")
7373
.replace("${applyCommand}", "spotless:apply");

‎testlib/src/main/resources/git_pre_hook/pre-push.existing-added-tpl renamed to ‎testlib/src/main/resources/git_pre_hook/pre-push.existing-installed-end-tpl

File renamed without changes.

‎testlib/src/main/resources/git_pre_hook/pre-push.reinstalled-tpl renamed to ‎testlib/src/main/resources/git_pre_hook/pre-push.existing-installed-middle-tpl

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ do
5151
done
5252

5353

54-
55-
56-
5754
##### SPOTLESS HOOK START #####
5855
SPOTLESS_EXECUTOR=${executor}
5956
if ! $SPOTLESS_EXECUTOR ${checkCommand} ; then
@@ -62,3 +59,36 @@ if ! $SPOTLESS_EXECUTOR ${checkCommand} ; then
6259
exit 1
6360
fi
6461
##### SPOTLESS HOOK END #####
62+
63+
64+
# some additional pre-push code
65+
remote="$1"
66+
url="$2"
67+
68+
zero=$(git hash-object --stdin </dev/null | tr '[0-9a-f]' '0')
69+
70+
while read local_ref local_oid remote_ref remote_oid
71+
do
72+
if test "$local_oid" = "$zero"
73+
then
74+
# Handle delete
75+
:
76+
else
77+
if test "$remote_oid" = "$zero"
78+
then
79+
# New branch, examine all commits
80+
range="$local_oid"
81+
else
82+
# Update to existing branch, examine new commits
83+
range="$remote_oid..$local_oid"
84+
fi
85+
86+
# Check for WIP commit
87+
commit=$(git rev-list -n 1 --grep '^WIP' "$range")
88+
if test -n "$commit"
89+
then
90+
echo >&2 "Found WIP commit in $local_ref, not pushing"
91+
exit 1
92+
fi
93+
fi
94+
done

‎testlib/src/main/resources/git_pre_hook/pre-push.existing-reinstalled-middle-tpl

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
#!/bin/sh
2+
3+
# An example hook script to verify what is about to be pushed. Called by "git
4+
# push" after it has checked the remote status, but before anything has been
5+
# pushed. If this script exits with a non-zero status nothing will be pushed.
6+
#
7+
# This hook is called with the following parameters:
8+
#
9+
# $1 -- Name of the remote to which the push is being done
10+
# $2 -- URL to which the push is being done
11+
#
12+
# If pushing without using a named remote those arguments will be equal.
13+
#
14+
# Information about the commits which are being pushed is supplied as lines to
15+
# the standard input in the form:
16+
#
17+
# <local ref> <local oid> <remote ref> <remote oid>
18+
#
19+
# This sample shows how to prevent push of commits where the log message starts
20+
# with "WIP" (work in progress).
21+
22+
remote="$1"
23+
url="$2"
24+
25+
zero=$(git hash-object --stdin </dev/null | tr '[0-9a-f]' '0')
26+
27+
while read local_ref local_oid remote_ref remote_oid
28+
do
29+
if test "$local_oid" = "$zero"
30+
then
31+
# Handle delete
32+
:
33+
else
34+
if test "$remote_oid" = "$zero"
35+
then
36+
# New branch, examine all commits
37+
range="$local_oid"
38+
else
39+
# Update to existing branch, examine new commits
40+
range="$remote_oid..$local_oid"
41+
fi
42+
43+
# Check for WIP commit
44+
commit=$(git rev-list -n 1 --grep '^WIP' "$range")
45+
if test -n "$commit"
46+
then
47+
echo >&2 "Found WIP commit in $local_ref, not pushing"
48+
exit 1
49+
fi
50+
fi
51+
done
52+
53+
54+
# some additional pre-push code
55+
remote="$1"
56+
url="$2"
57+
58+
zero=$(git hash-object --stdin </dev/null | tr '[0-9a-f]' '0')
59+
60+
while read local_ref local_oid remote_ref remote_oid
61+
do
62+
if test "$local_oid" = "$zero"
63+
then
64+
# Handle delete
65+
:
66+
else
67+
if test "$remote_oid" = "$zero"
68+
then
69+
# New branch, examine all commits
70+
range="$local_oid"
71+
else
72+
# Update to existing branch, examine new commits
73+
range="$remote_oid..$local_oid"
74+
fi
75+
76+
# Check for WIP commit
77+
commit=$(git rev-list -n 1 --grep '^WIP' "$range")
78+
if test -n "$commit"
79+
then
80+
echo >&2 "Found WIP commit in $local_ref, not pushing"
81+
exit 1
82+
fi
83+
fi
84+
done
85+
86+
87+
##### SPOTLESS HOOK START #####
88+
SPOTLESS_EXECUTOR=${executor}
89+
if ! $SPOTLESS_EXECUTOR ${checkCommand} ; then
90+
echo 1>&2 "spotless found problems, running ${applyCommand}; commit the result and re-push"
91+
$SPOTLESS_EXECUTOR ${applyCommand}
92+
exit 1
93+
fi
94+
##### SPOTLESS HOOK END #####

‎testlib/src/test/java/com/diffplug/spotless/GitPrePushHookInstallerTest.java

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public void should_use_global_gradle_when_gradlew_is_not_installed() throws Exce
8282
public void should_reinstall_pre_hook_file_when_hook_already_installed() throws Exception {
8383
// given
8484
final var gradle = new GitPrePushHookInstallerGradle(logger, rootFolder());
85-
final var installedGlobally = gradleHookContent("git_pre_hook/pre-push.existing-added-tpl", ExecutorType.GLOBAL);
85+
final var installedGlobally = gradleHookContent("git_pre_hook/pre-push.existing-installed-end-tpl", ExecutorType.GLOBAL);
8686
final var hookFile = setFile(".git/hooks/pre-push").toContent(installedGlobally);
8787

8888
setFile("gradlew").toContent("");
@@ -97,7 +97,50 @@ public void should_reinstall_pre_hook_file_when_hook_already_installed() throws
9797
assertThat(logs).element(1).isEqualTo("Git pre-push hook already installed, reinstalling it");
9898
assertThat(logs).element(2).isEqualTo("Git pre-push hook installed successfully to the file " + hookFile.getAbsolutePath());
9999

100-
final var content = gradleHookContent("git_pre_hook/pre-push.reinstalled-tpl", ExecutorType.WRAPPER);
100+
final var content = gradleHookContent("git_pre_hook/pre-push.existing-installed-end-tpl", ExecutorType.WRAPPER);
101+
assertFile(".git/hooks/pre-push").hasContent(content);
102+
}
103+
104+
@Test
105+
public void should_reinstall_pre_hook_file_when_hook_already_installed_in_the_middle_of_file() throws Exception {
106+
// given
107+
final var gradle = new GitPrePushHookInstallerGradle(logger, rootFolder());
108+
final var installedGlobally = gradleHookContent("git_pre_hook/pre-push.existing-installed-middle-tpl", ExecutorType.GLOBAL);
109+
final var hookFile = setFile(".git/hooks/pre-push").toContent(installedGlobally);
110+
111+
setFile("gradlew").toContent("");
112+
setFile(".git/config").toContent("");
113+
114+
// when
115+
gradle.install();
116+
117+
// then
118+
assertThat(logs).hasSize(3);
119+
assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
120+
assertThat(logs).element(1).isEqualTo("Git pre-push hook already installed, reinstalling it");
121+
assertThat(logs).element(2).isEqualTo("Git pre-push hook installed successfully to the file " + hookFile.getAbsolutePath());
122+
123+
final var content = gradleHookContent("git_pre_hook/pre-push.existing-reinstalled-middle-tpl", ExecutorType.WRAPPER);
124+
assertFile(".git/hooks/pre-push").hasContent(content);
125+
}
126+
127+
@Test
128+
public void should_reinstall_a_few_times_pre_hook_file_when_hook_already_installed_in_the_middle_of_file() throws Exception {
129+
// given
130+
final var gradle = new GitPrePushHookInstallerGradle(logger, rootFolder());
131+
final var installedGlobally = gradleHookContent("git_pre_hook/pre-push.existing-installed-middle-tpl", ExecutorType.GLOBAL);
132+
setFile(".git/hooks/pre-push").toContent(installedGlobally);
133+
134+
setFile("gradlew").toContent("");
135+
setFile(".git/config").toContent("");
136+
137+
// when
138+
gradle.install();
139+
gradle.install();
140+
gradle.install();
141+
142+
// then
143+
final var content = gradleHookContent("git_pre_hook/pre-push.existing-reinstalled-middle-tpl", ExecutorType.WRAPPER);
101144
assertFile(".git/hooks/pre-push").hasContent(content);
102145
}
103146

@@ -137,7 +180,7 @@ public void should_append_to_existing_pre_hook_file_when_hook_file_exists() thro
137180
assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
138181
assertThat(logs).element(1).isEqualTo("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
139182

140-
final var content = gradleHookContent("git_pre_hook/pre-push.existing-added-tpl", ExecutorType.WRAPPER);
183+
final var content = gradleHookContent("git_pre_hook/pre-push.existing-installed-end-tpl", ExecutorType.WRAPPER);
141184
assertFile(".git/hooks/pre-push").hasContent(content);
142185
}
143186

0 commit comments

Comments
 (0)