Skip to content

Commit 5aa9d1d

Browse files
committed
Fix things being committed even when nothing was written
Committing nothing is fine and not the problem here. The issue is that we only have exclusive access to git if we have a writing scope. Otherwise, we get issues with multiple threads trying to commit at the same time, this errors even though they're committing nothing.
1 parent 74c9467 commit 5aa9d1d

File tree

1 file changed

+30
-13
lines changed

1 file changed

+30
-13
lines changed

platform_api/src/main/java/net/modfest/platform/git/GitScopeManager.java

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ public class GitScopeManager {
1111
private final Git git;
1212
private final GitConfig config;
1313
private final ThreadLocal<GitScope> gitScopes = new ThreadLocal<>();
14+
/**
15+
* Helps manage which scope is the current "write" scope. Many threads can set
16+
* a scope, but once the scope is actually used¹ the thread that owns the scope will
17+
* retrieve this lock. The thread that owns the locks owns the writing scope.
18+
*
19+
* ¹ Often a scope will be set but nothing will actually be written and no locking is needed.
20+
* For example, each request sets a scope.
21+
*/
1422
private final ReentrantLock scopeLock = new ReentrantLock();
1523

1624
public GitScopeManager(Git git, GitConfig config) {
@@ -27,12 +35,20 @@ public void closeScope() {
2735
this.gitScopes.remove();
2836
}
2937

38+
/**
39+
* Sets the git commit message for any writes that may or may not be
40+
* done during the provided runnable. All writes will be batched into a
41+
* single commit.
42+
*/
3043
public void runWithScope(GitScope scope, Runnable r) {
3144
setScope(scope);
3245
r.run();
3346
closeScope();
3447
}
3548

49+
/**
50+
* Activates the current scope to be used for writing
51+
*/
3652
void runWithScopedGit(GitFunction r) {
3753
var threadScope = gitScopes.get();
3854
try {
@@ -60,19 +76,20 @@ Git getGitUnscoped() {
6076
}
6177

6278
private void finalizeScope(GitScope scope) {
63-
try {
64-
this.git.commit()
65-
.setAuthor(config.getUser(), config.getEmail())
66-
.setMessage(scope.commitMessage())
67-
.setAllowEmpty(false)
68-
.setSign(false)
69-
.call();
70-
} catch (EmptyCommitException ignored) {
71-
// If nothing changed we simply do not commit anything
72-
} catch (GitAPIException e) {
73-
throw new RuntimeException(e);
74-
} finally {
75-
if (this.scopeLock.isHeldByCurrentThread()) {
79+
// We only need to commit if the scope was locked for use in writing
80+
if (this.scopeLock.isHeldByCurrentThread()) {
81+
try {
82+
this.git.commit()
83+
.setAuthor(config.getUser(), config.getEmail())
84+
.setMessage(scope.commitMessage())
85+
.setAllowEmpty(false)
86+
.setSign(false)
87+
.call();
88+
} catch (EmptyCommitException ignored) {
89+
// If nothing changed we simply do not commit anything
90+
} catch (GitAPIException e) {
91+
throw new RuntimeException(e);
92+
} finally {
7693
this.scopeLock.unlock();
7794
}
7895
}

0 commit comments

Comments
 (0)