From 621685d3ca5fbaba37a8fc79b347644b3e4425c0 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 8 Oct 2023 21:50:34 +0200 Subject: [PATCH] DeleteBranchCommand: update config only at the end When multiple branches were to be removed, the git config was updated after each and every branch. Newly do so only once at the end, after all branches have been deleted. Because there may be an exception after some branches have already been deleted, take care to update the config even if an exception is thrown. Bug: 451508 Change-Id: I645be8a1a59a1476d421e46933c3f7cbd0639fec Signed-off-by: Thomas Wolf --- .../eclipse/jgit/internal/JGitText.properties | 2 + .../eclipse/jgit/api/DeleteBranchCommand.java | 219 ++++++++++++++---- .../org/eclipse/jgit/internal/JGitText.java | 2 + 3 files changed, 174 insertions(+), 49 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 6def50b17a..fcdce83026 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -259,6 +259,7 @@ deleteFileFailed=Could not delete file {0} deletedOrphanInPackDir=Deleted orphaned file {} deleteRequiresZeroNewId=Delete requires new ID to be zero deleteTagUnexpectedResult=Delete tag returned unexpected result {0} +deletingBranches=Deleting branches... deletingNotSupported=Deleting {0} not supported. depthMustBeAt1=Depth must be >= 1 depthWithUnshallow=Depth and unshallow can\'t be used together @@ -854,6 +855,7 @@ unsupportedReftableVersion=Unsupported reftable version {0}. unsupportedRepositoryDescription=Repository description not supported unsupportedSizesObjSizeIndex=Unsupported sizes in object-size-index updateRequiresOldIdAndNewId=Update requires both old ID and new ID to be nonzero +updatingConfig=Updating git config updatingHeadFailed=Updating HEAD failed updatingReferences=Updating references updatingRefFailed=Updating the ref {0} to {1} failed. ReturnCode from RefUpdate.update() was {2} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java index f1b5d62349..a5c535f772 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java @@ -1,6 +1,6 @@ /* * Copyright (C) 2010, Mathias Kinzler - * Copyright (C) 2010, Chris Aniszczyk and others + * Copyright (C) 2010, 2023 Chris Aniszczyk and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -14,6 +14,7 @@ import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -25,6 +26,8 @@ import org.eclipse.jgit.api.errors.NotMergedException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate.Result; @@ -47,8 +50,11 @@ import org.eclipse.jgit.revwalk.RevWalk; * >Git documentation about Branch */ public class DeleteBranchCommand extends GitCommand> { + private final Set branchNames = new HashSet<>(); + private ProgressMonitor monitor = NullProgressMonitor.INSTANCE; + private boolean force; /** @@ -66,8 +72,29 @@ public class DeleteBranchCommand extends GitCommand> { NotMergedException, CannotDeleteCurrentBranchException { checkCallable(); List result = new ArrayList<>(); - if (branchNames.isEmpty()) + Set shortNames = new HashSet<>(); + if (branchNames.isEmpty()) { return result; + } + Exception error = null; + try { + deleteBranches(result, shortNames); + } catch (Exception e) { + error = e; + } + monitor.beginTask(JGitText.get().updatingConfig, 1); + try { + updateConfig(shortNames, error); + } finally { + monitor.update(1); + monitor.endTask(); + } + return result; + } + + private void deleteBranches(List result, + Set shortNames) throws GitAPIException, NotMergedException, + CannotDeleteCurrentBranchException { try { String currentBranch = repo.getFullBranch(); if (!force) { @@ -77,12 +104,13 @@ public class DeleteBranchCommand extends GitCommand> { RevCommit tip = walk .parseCommit(repo.resolve(Constants.HEAD)); for (String branchName : branchNames) { - if (branchName == null) + if (branchName == null) { continue; + } Ref currentRef = repo.findRef(branchName); - if (currentRef == null) + if (currentRef == null) { continue; - + } RevCommit base = walk .parseCommit(repo.resolve(branchName)); if (!walk.isMergedInto(base, tip)) { @@ -92,58 +120,105 @@ public class DeleteBranchCommand extends GitCommand> { } } setCallable(false); - for (String branchName : branchNames) { - if (branchName == null) - continue; - Ref currentRef = repo.findRef(branchName); - if (currentRef == null) - continue; - String fullName = currentRef.getName(); - if (fullName.equals(currentBranch)) - throw new CannotDeleteCurrentBranchException( - MessageFormat - .format( - JGitText.get().cannotDeleteCheckedOutBranch, - branchName)); - RefUpdate update = repo.updateRef(fullName); - update.setRefLogMessage("branch deleted", false); //$NON-NLS-1$ - update.setForceUpdate(true); - Result deleteResult = update.delete(); - - boolean ok = true; - switch (deleteResult) { - case IO_FAILURE: - case LOCK_FAILURE: - case REJECTED: - ok = false; - break; - default: - break; - } + monitor.start(2); + monitor.beginTask(JGitText.get().deletingBranches, + branchNames.size()); + try { + for (String branchName : branchNames) { + if (branchName == null) { + monitor.update(1); + continue; + } + Ref currentRef = repo.findRef(branchName); + if (currentRef == null) { + monitor.update(1); + continue; + } + String fullName = currentRef.getName(); + if (fullName.equals(currentBranch)) { + throw new CannotDeleteCurrentBranchException( + MessageFormat.format(JGitText + .get().cannotDeleteCheckedOutBranch, + branchName)); + } + RefUpdate update = repo.updateRef(fullName); + update.setRefLogMessage("branch deleted", false); //$NON-NLS-1$ + update.setForceUpdate(true); + Result deleteResult = update.delete(); - if (ok) { - result.add(fullName); - if (fullName.startsWith(Constants.R_HEADS)) { - String shortenedName = fullName - .substring(Constants.R_HEADS.length()); - // remove upstream configuration if any - final StoredConfig cfg = repo.getConfig(); - cfg.unsetSection( - ConfigConstants.CONFIG_BRANCH_SECTION, - shortenedName); - cfg.save(); + switch (deleteResult) { + case IO_FAILURE: + case LOCK_FAILURE: + case REJECTED: + throw new JGitInternalException(MessageFormat.format( + JGitText.get().deleteBranchUnexpectedResult, + deleteResult.name())); + default: + result.add(fullName); + if (fullName.startsWith(Constants.R_HEADS)) { + shortNames.add(fullName + .substring(Constants.R_HEADS.length())); + } + break; + } + monitor.update(1); + if (monitor.isCancelled()) { + break; } - } else - throw new JGitInternalException(MessageFormat.format( - JGitText.get().deleteBranchUnexpectedResult, - deleteResult.name())); + } + } finally { + monitor.endTask(); } - return result; } catch (IOException ioe) { throw new JGitInternalException(ioe.getMessage(), ioe); } } + private void updateConfig(Set shortNames, Exception error) + throws GitAPIException { + IOException configError = null; + if (!shortNames.isEmpty()) { + try { + // Remove upstream configurations if any + StoredConfig cfg = repo.getConfig(); + boolean changed = false; + for (String branchName : shortNames) { + changed |= cfg.removeSection( + ConfigConstants.CONFIG_BRANCH_SECTION, + branchName); + } + if (changed) { + cfg.save(); + } + } catch (IOException e) { + configError = e; + } + } + if (error == null) { + if (configError != null) { + throw new JGitInternalException(configError.getMessage(), + configError); + } + } else if (error instanceof GitAPIException) { + if (configError != null) { + error.addSuppressed(configError); + } + throw (GitAPIException) error; + } else if (error instanceof RuntimeException) { + if (configError != null) { + error.addSuppressed(configError); + } + throw (RuntimeException) error; + } else { + JGitInternalException internal = new JGitInternalException( + error.getMessage(), error); + if (configError != null) { + internal.addSuppressed(configError); + } + throw internal; + } + } + /** * Set the names of the branches to delete * @@ -159,6 +234,22 @@ public class DeleteBranchCommand extends GitCommand> { return this; } + /** + * Sets the names of the branches to delete + * + * @param branchNames + * the names of the branches to delete; if not set, this will do + * nothing; invalid branch names will simply be ignored + * @return {@code this} + * @since 6.8 + */ + public DeleteBranchCommand setBranchNames(Collection branchNames) { + checkCallable(); + this.branchNames.clear(); + this.branchNames.addAll(branchNames); + return this; + } + /** * Set whether to forcefully delete branches * @@ -175,4 +266,34 @@ public class DeleteBranchCommand extends GitCommand> { this.force = force; return this; } + + /** + * Retrieves the progress monitor. + * + * @return the {@link ProgressMonitor} for the delete operation + * @since 6.8 + */ + public ProgressMonitor getProgressMonitor() { + return monitor; + } + + /** + * Sets the progress monitor associated with the delete operation. By + * default, this is set to NullProgressMonitor + * + * @see NullProgressMonitor + * @param monitor + * a {@link ProgressMonitor} + * @return {@code this} + * @since 6.8 + */ + public DeleteBranchCommand setProgressMonitor(ProgressMonitor monitor) { + checkCallable(); + if (monitor == null) { + monitor = NullProgressMonitor.INSTANCE; + } + this.monitor = monitor; + return this; + } + } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 6c2c850d7a..b86a18a614 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -289,6 +289,7 @@ public class JGitText extends TranslationBundle { /***/ public String deletedOrphanInPackDir; /***/ public String deleteRequiresZeroNewId; /***/ public String deleteTagUnexpectedResult; + /***/ public String deletingBranches; /***/ public String deletingNotSupported; /***/ public String depthMustBeAt1; /***/ public String depthWithUnshallow; @@ -884,6 +885,7 @@ public class JGitText extends TranslationBundle { /***/ public String unsupportedRepositoryDescription; /***/ public String unsupportedSizesObjSizeIndex; /***/ public String updateRequiresOldIdAndNewId; + /***/ public String updatingConfig; /***/ public String updatingHeadFailed; /***/ public String updatingReferences; /***/ public String updatingRefFailed; -- 2.39.5