Skip to content

Commit 4a142af

Browse files
authored
chore: cherry pick git executor revert (#39954)
## Description - Git executor CE Impl revert PR #39949 Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > 🔴 🔴 🔴 Some tests have failed. > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14106311517> > Commit: cef9ab7 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14106311517&attempt=2&selectiontype=test&testsstatus=failed&specsstatus=fail" target="_blank">Cypress dashboard</a>. > Tags: @tag.Git > Spec: > The following are new failures, please fix them before merging the PR: <ol> > <li>cypress/e2e/Regression/Apps/ImportExportForkApplication_spec.js > <li>cypress/e2e/Regression/ClientSide/BugTests/CatchBlock_Spec.ts > <li>cypress/e2e/Regression/ClientSide/BugTests/DS_Bug28985_spec.ts > <li>cypress/e2e/Regression/ClientSide/BugTests/GraphQL_Binding_Bug16702_Spec.ts > <li>cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts > <li>cypress/e2e/Regression/ClientSide/Git/GitDiscardChange/DiscardChanges_spec.js > <li>cypress/e2e/Regression/ClientSide/Git/GitImport/GitImport_spec.js > <li>cypress/e2e/Regression/ClientSide/Git/GitSync/GitSyncGitBugs_spec.js > <li>cypress/e2e/Regression/ClientSide/Templates/ForkTemplateToGitConnectedApp.js > <li>cypress/e2e/Regression/ClientSide/Widgets/Migration_Spec.js > <li>cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts > <li>cypress/e2e/Regression/ServerSide/ApiTests/API_TestExecuteWithDynamicBindingInUrl_spec.ts > <li>cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad2_Spec.ts > <li>cypress/e2e/Regression/ServerSide/Params/PassingParams_Spec.ts</ol> > <a href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master" target="_blank">List of identified flaky tests</a>. > <hr>Thu, 27 Mar 2025 15:43:27 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved recovery for repository operations by automatically reverting to a stable state when errors occur, reducing conflict risks and ensuring a consistent environment. - **Refactor** - Optimized the overall error handling and state management logic to better maintain system stability during update and merge operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 43d4a7e + cef9ab7 commit 4a142af

File tree

1 file changed

+33
-5
lines changed

1 file changed

+33
-5
lines changed

app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java

+33-5
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ public Mono<MergeStatusDTO> pullApplication(
499499
}
500500
}
501501
})
502-
.onErrorResume(error -> Mono.error(error))
502+
.onErrorResume(error -> resetToLastCommit(git).flatMap(ignore -> Mono.error(error)))
503503
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
504504
.name(GitSpan.FS_PULL)
505505
.tap(Micrometer.observation(observationRegistry)),
@@ -626,6 +626,15 @@ public Mono<GitStatusDTO> getStatus(Path repoPath, String branchName) {
626626
response.setRemoteBranch("untracked");
627627
}
628628

629+
// Remove modified changes from current branch so that checkout to other branches
630+
// will be possible
631+
if (!status.isClean()) {
632+
return resetToLastCommit(git).map(ref -> {
633+
processStopwatch.stopAndLogTimeInMillis();
634+
jgitStatusSpan.end();
635+
return response;
636+
});
637+
}
629638
processStopwatch.stopAndLogTimeInMillis();
630639
jgitStatusSpan.end();
631640
return Mono.just(response);
@@ -844,7 +853,13 @@ public Mono<String> mergeBranch(Path repoSuffix, String sourceBranch, String des
844853
}
845854
})
846855
.onErrorResume(error -> {
847-
return Mono.error(error);
856+
try {
857+
return resetToLastCommit(repoSuffix, destinationBranch)
858+
.thenReturn(error.getMessage());
859+
} catch (GitAPIException | IOException e) {
860+
log.error("Error while hard resetting to latest commit {0}", e);
861+
return Mono.error(e);
862+
}
848863
})
849864
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
850865
.name(GitSpan.FS_MERGE)
@@ -1013,6 +1028,19 @@ public Mono<MergeStatusDTO> isMergeBranch(Path repoSuffix, String sourceBranch,
10131028
mergeResult.getMergeStatus().name());
10141029
return mergeStatus;
10151030
})
1031+
.flatMap(status -> {
1032+
try {
1033+
// Revert uncommitted changes if any
1034+
return resetToLastCommit(repoSuffix, destinationBranch)
1035+
.map(ignore -> {
1036+
processStopwatch.stopAndLogTimeInMillis();
1037+
return status;
1038+
});
1039+
} catch (GitAPIException | IOException e) {
1040+
log.error("Error for hard resetting to latest commit {0}", e);
1041+
return Mono.error(e);
1042+
}
1043+
})
10161044
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS)),
10171045
Git::close)
10181046
.subscribeOn(scheduler);
@@ -1093,12 +1121,12 @@ private Mono<Ref> resetToLastCommit(Git git) {
10931121
.subscribeOn(scheduler);
10941122
}
10951123

1096-
public Mono<Boolean> resetToLastCommit(Path repoSuffix, String branchName) {
1124+
public Mono<Boolean> resetToLastCommit(Path repoSuffix, String branchName) throws GitAPIException, IOException {
10971125
return Mono.using(
10981126
() -> Git.open(createRepoPath(repoSuffix).toFile()),
10991127
git -> this.resetToLastCommit(git)
11001128
.flatMap(ref -> checkoutToBranch(repoSuffix, branchName))
1101-
.thenReturn(true),
1129+
.flatMap(checkedOut -> resetToLastCommit(git).thenReturn(true)),
11021130
Git::close);
11031131
}
11041132

@@ -1128,7 +1156,7 @@ public Mono<Boolean> resetHard(Path repoSuffix, String branchName) {
11281156
}
11291157

11301158
public Mono<Boolean> rebaseBranch(Path repoSuffix, String branchName) {
1131-
return this.resetToLastCommit(repoSuffix, branchName).flatMap(isCheckedOut -> Mono.using(
1159+
return this.checkoutToBranch(repoSuffix, branchName).flatMap(isCheckedOut -> Mono.using(
11321160
() -> Git.open(createRepoPath(repoSuffix).toFile()),
11331161
git -> Mono.fromCallable(() -> {
11341162
Span jgitRebaseSpan = observationHelper.createSpan(GitSpan.JGIT_REBASE);

0 commit comments

Comments
 (0)