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 | 221 ++++++++++++++---- .../org/eclipse/jgit/internal/JGitText.java | 2 + 3 files changed, 175 insertions(+), 50 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 6def50b17..fcdce8302 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 f1b5d6234..a5c535f77 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.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.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 @@ * >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 List call() throws GitAPIException, 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 List call() throws GitAPIException, 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 List call() throws GitAPIException, } } 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; - } - - 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(); + monitor.start(2); + monitor.beginTask(JGitText.get().deletingBranches, + branchNames.size()); + try { + for (String branchName : branchNames) { + if (branchName == null) { + monitor.update(1); + continue; } - } else - throw new JGitInternalException(MessageFormat.format( - JGitText.get().deleteBranchUnexpectedResult, - deleteResult.name())); + 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(); + + 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; + } + } + } 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 DeleteBranchCommand setBranchNames(String... branchnames) { 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 DeleteBranchCommand setForce(boolean force) { 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 6c2c850d7..b86a18a61 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 static JGitText get() { /***/ 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 static JGitText get() { /***/ public String unsupportedRepositoryDescription; /***/ public String unsupportedSizesObjSizeIndex; /***/ public String updateRequiresOldIdAndNewId; + /***/ public String updatingConfig; /***/ public String updatingHeadFailed; /***/ public String updatingReferences; /***/ public String updatingRefFailed;