Skip to content

chore: merge and merge status #38495

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.

Sign up for GitHub

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

Merged
sondermanish merged 11 commits into from
Jan 8, 2025
Merged

chore: merge and merge status #38495

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
927fc5d
added git resource map
sondermanish Jan 3, 2025
41f5faa
added reviewed comments
sondermanish Jan 3, 2025
f464a95
added changes
sondermanish Jan 3, 2025
7a3406e
review comments
sondermanish Jan 3, 2025
70ad2a1
modified mocking for test failures
sondermanish Jan 3, 2025
8877a5b
more mocking
sondermanish Jan 3, 2025
1283574
added merge
sondermanish Jan 4, 2025
2976a4c
merge with origin/release
sondermanish Jan 6, 2025
3a72c16
Merge remote-tracking branch 'origin/release' into chore/status
sondermanish Jan 7, 2025
9500e83
review comments
sondermanish Jan 8, 2025
ab5e336
further modification
sondermanish Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 27 additions & 32 deletions app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.eclipse.jgit.api.ResetCommand;
import org.eclipse.jgit.api.Status;
import org.eclipse.jgit.api.TransportConfigCallback;
import org.eclipse.jgit.api.errors.CheckoutConflictException;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.BranchTrackingStatus;
import org.eclipse.jgit.lib.PersonIdent;
Expand Down Expand Up @@ -846,15 +845,14 @@ public Mono<String> mergeBranch(Path repoSuffix, String sourceBranch, String des
git -> Mono.fromCallable(() -> {
Stopwatch processStopwatch = StopwatchHelpers.startStopwatch(
repoSuffix, AnalyticsEvents.GIT_MERGE.getEventName());
log.debug(Thread.currentThread().getName() + ": Merge branch " + sourceBranch
+ " on " + destinationBranch);
try {
// checkout the branch on which the merge command is run
git.checkout()
.setName(destinationBranch)
.setCreateBranch(false)
.call();

log.info(
"{}: Merge branch {} on {}",
Thread.currentThread().getName(),
sourceBranch,
destinationBranch);

try {
MergeResult mergeResult = git.merge()
.include(git.getRepository().findRef(sourceBranch))
.setStrategy(MergeStrategy.RECURSIVE)
Expand Down Expand Up @@ -934,7 +932,7 @@ public Mono<String> fetchRemote(

@Override
public Mono<String> fetchRemote(
Path repoSuffix, String publicKey, String privateKey, boolean isRepoPath, String... branchNames) {
Path repoSuffix, String publicKey, String privateKey, boolean isRepoPath, String... refNames) {
Stopwatch processStopwatch =
StopwatchHelpers.startStopwatch(repoSuffix, AnalyticsEvents.GIT_FETCH.getEventName());
Path repoPath = TRUE.equals(isRepoPath) ? repoSuffix : createRepoPath(repoSuffix);
Expand All @@ -946,9 +944,9 @@ public Mono<String> fetchRemote(
String fetchMessages;

List<RefSpec> refSpecs = new ArrayList<>();
for (String branchName : branchNames) {
for (String refName : refNames) {
RefSpec ref = new RefSpec(
"refs/heads/" + branchName + ":refs/remotes/origin/" + branchName);
"refs/heads/" + refName + ":refs/remotes/origin/" + refName);

@nidhi-nair nidhi-nair Jan 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This format is only applicable to branches, not tags. We'll need a separate path for tags

refSpecs.add(ref);
}

Expand Down Expand Up @@ -980,30 +978,13 @@ public Mono<MergeStatusDTO> isMergeBranch(Path repoSuffix, String sourceBranch,
return Mono.using(
() -> Git.open(createRepoPath(repoSuffix).toFile()),
git -> Mono.fromCallable(() -> {
log.debug(
Thread.currentThread().getName()
+ ": Check mergeability for repo {} with src: {}, dest: {}",
log.info(
"{}: Check mergeability for repo {} with src: {}, dest: {}",
Thread.currentThread().getName(),
repoSuffix,
sourceBranch,
destinationBranch);

// checkout the branch on which the merge command is run
try {
git.checkout()
.setName(destinationBranch)
.setCreateBranch(false)
.call();
} catch (GitAPIException e) {
if (e instanceof CheckoutConflictException) {
MergeStatusDTO mergeStatus = new MergeStatusDTO();
mergeStatus.setMergeAble(false);
mergeStatus.setConflictingFiles(
((CheckoutConflictException) e).getConflictingPaths());
processStopwatch.stopAndLogTimeInMillis();
return mergeStatus;
}
}

MergeResult mergeResult = git.merge()
.include(git.getRepository().findRef(sourceBranch))
.setFastForward(MergeCommand.FastForwardMode.NO_FF)
Expand Down Expand Up @@ -1054,6 +1035,20 @@ public Mono<MergeStatusDTO> isMergeBranch(Path repoSuffix, String sourceBranch,
return Mono.error(e);
}
})
.onErrorResume(error -> {
try {

@nidhi-nair nidhi-nair Jan 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a try catch here?

@sondermanish sondermanish Jan 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of errors we would need to reset it to last commit, and the method reset to last commit is throwing checked exceptions which needs to be handled.

MergeStatusDTO mergeStatusDTO = new MergeStatusDTO();
mergeStatusDTO.setMergeAble(false);
mergeStatusDTO.setMessage(error.getMessage());
mergeStatusDTO.setReferenceDoc(
ErrorReferenceDocUrl.GIT_MERGE_CONFLICT.getDocUrl());
return resetToLastCommit(repoSuffix, destinationBranch)
.thenReturn(mergeStatusDTO);
} catch (GitAPIException | IOException e) {
log.error("Error while hard resetting to latest commit {0}", e);
return Mono.error(e);
}
})
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS)),
Git::close)
.subscribeOn(scheduler);
Expand Down
4 changes: 3 additions & 1 deletion ...ver/appsmith-interfaces/src/main/java/com/appsmith/external/git/handler/FSGitHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ Mono<MergeStatusDTO> pullApplication(
Mono<GitStatusDTO> getStatus(Path repoPath, String branchName);

/**
* This method merges source branch into destination branch for a git repository which is present on the partial
* path provided. <B> This assumes that the branch on which the merge will happen is already checked out </B>
* @param repoSuffix suffixedPath used to generate the base repo path this includes orgId, defaultAppId, repoName
* @param sourceBranch name of the branch whose commits will be referred amd merged to destinationBranch
* @param destinationBranch Merge operation is performed on this branch
Expand All @@ -158,7 +160,7 @@ Mono<String> fetchRemote(
boolean isFetchAll);

Mono<String> fetchRemote(
Path repoSuffix, String publicKey, String privateKey, boolean isRepoPath, String... branchNames);
Path repoSuffix, String publicKey, String privateKey, boolean isRepoPath, String... refNames);

/**
*
Expand Down
8 changes: 8 additions & 0 deletions ...er/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.appsmith.external.dtos.GitBranchDTO;
import com.appsmith.external.dtos.GitRefDTO;
import com.appsmith.external.dtos.GitStatusDTO;
import com.appsmith.external.dtos.MergeStatusDTO;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.git.dto.CommitDTO;
import com.appsmith.server.constants.ArtifactType;
Expand All @@ -13,6 +14,7 @@
import com.appsmith.server.dtos.AutoCommitResponseDTO;
import com.appsmith.server.dtos.GitConnectDTO;
import com.appsmith.server.dtos.GitDocsDTO;
import com.appsmith.server.dtos.GitMergeDTO;
import com.appsmith.server.dtos.GitPullDTO;
import reactor.core.publisher.Mono;

Expand Down Expand Up @@ -45,6 +47,12 @@ Mono<String> fetchRemoteChanges(
GitType gitType,
RefType refType);

Mono<MergeStatusDTO> mergeBranch(
String branchedArtifactId, ArtifactType artifactType, GitMergeDTO gitMergeDTO, GitType gitType);

Mono<MergeStatusDTO> isBranchMergable(
String branchedArtifactId, ArtifactType artifactType, GitMergeDTO gitMergeDTO, GitType gitType);

Mono<? extends Artifact> discardChanges(String branchedArtifactId, ArtifactType artifactType, GitType gitType);

Mono<GitStatusDTO> getStatus(
Expand Down
Loading