From b255eb0fb6ab9c7487423083e8df9b1e373bc37e Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 14 Aug 2022 16:34:50 +0200 Subject: [PATCH] DirCacheCheckout: load WorkingTreeOptions only once Previous code loaded the WorkingTreeOptions afresh for every single file being checked out. This checked the git config (all three files, repo, user and system config) for having been modified every time. These checks can be costly, for instance on Windows, or if one of the three config files is not on a local disk, or on an otherwise slow storage. Improve this by loading the options and thus checking the git config only once before the checkout. Bug: 579715 Change-Id: I21cd5a808f9d90b5ca2d022f91f0eeb8ca26091c Signed-off-by: Thomas Wolf --- .../jgit/treewalk/FileTreeIteratorTest.java | 10 ++- .../org/eclipse/jgit/api/CheckoutCommand.java | 28 +++--- .../eclipse/jgit/api/StashApplyCommand.java | 9 +- .../jgit/dircache/DirCacheCheckout.java | 86 ++++++++++++++----- .../eclipse/jgit/util/WorkTreeUpdater.java | 11 ++- 5 files changed, 102 insertions(+), 42 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java index 0d49cd3396..e463e9070a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java @@ -303,12 +303,16 @@ public class FileTreeIteratorTest extends RepositoryTestCase { DirCacheEntry dce = db.readDirCache().getEntry("symlink"); dce.setFileMode(FileMode.SYMLINK); try (ObjectReader objectReader = db.newObjectReader()) { - DirCacheCheckout.checkoutEntry(db, dce, objectReader, false, null); + WorkingTreeOptions options = db.getConfig() + .get(WorkingTreeOptions.KEY); + DirCacheCheckout.checkoutEntry(db, dce, objectReader, false, null, + options); FileTreeIterator fti = new FileTreeIterator(trash, db.getFS(), - db.getConfig().get(WorkingTreeOptions.KEY)); - while (!fti.getEntryPathString().equals("symlink")) + options); + while (!fti.getEntryPathString().equals("symlink")) { fti.next(1); + } assertFalse(fti.isModified(dce, false, objectReader)); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java index 847ab0a9a8..7319ff4b2f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java @@ -55,6 +55,7 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.treewalk.WorkingTreeOptions; import org.eclipse.jgit.treewalk.filter.PathFilterGroup; /** @@ -411,6 +412,8 @@ public class CheckoutCommand extends GitCommand { protected CheckoutCommand checkoutPaths() throws IOException, RefNotFoundException { actuallyModifiedPaths = new HashSet<>(); + WorkingTreeOptions options = repo.getConfig() + .get(WorkingTreeOptions.KEY); DirCache dc = repo.lockDirCache(); try (RevWalk revWalk = new RevWalk(repo); TreeWalk treeWalk = new TreeWalk(repo, @@ -419,10 +422,10 @@ public class CheckoutCommand extends GitCommand { if (!checkoutAllPaths) treeWalk.setFilter(PathFilterGroup.createFromStrings(paths)); if (isCheckoutIndex()) - checkoutPathsFromIndex(treeWalk, dc); + checkoutPathsFromIndex(treeWalk, dc, options); else { RevCommit commit = revWalk.parseCommit(getStartPointObjectId()); - checkoutPathsFromCommit(treeWalk, dc, commit); + checkoutPathsFromCommit(treeWalk, dc, commit, options); } } finally { try { @@ -439,7 +442,8 @@ public class CheckoutCommand extends GitCommand { return this; } - private void checkoutPathsFromIndex(TreeWalk treeWalk, DirCache dc) + private void checkoutPathsFromIndex(TreeWalk treeWalk, DirCache dc, + WorkingTreeOptions options) throws IOException { DirCacheIterator dci = new DirCacheIterator(dc); treeWalk.addTree(dci); @@ -465,8 +469,9 @@ public class CheckoutCommand extends GitCommand { if (stage > DirCacheEntry.STAGE_0) { if (checkoutStage != null) { if (stage == checkoutStage.number) { - checkoutPath(ent, r, new CheckoutMetadata( - eolStreamType, filterCommand)); + checkoutPath(ent, r, options, + new CheckoutMetadata(eolStreamType, + filterCommand)); actuallyModifiedPaths.add(path); } } else { @@ -475,8 +480,9 @@ public class CheckoutCommand extends GitCommand { throw new JGitInternalException(e.getMessage(), e); } } else { - checkoutPath(ent, r, new CheckoutMetadata(eolStreamType, - filterCommand)); + checkoutPath(ent, r, options, + new CheckoutMetadata(eolStreamType, + filterCommand)); actuallyModifiedPaths.add(path); } } @@ -488,7 +494,7 @@ public class CheckoutCommand extends GitCommand { } private void checkoutPathsFromCommit(TreeWalk treeWalk, DirCache dc, - RevCommit commit) throws IOException { + RevCommit commit, WorkingTreeOptions options) throws IOException { treeWalk.addTree(commit.getTree()); final ObjectReader r = treeWalk.getObjectReader(); DirCacheEditor editor = dc.editor(); @@ -510,7 +516,7 @@ public class CheckoutCommand extends GitCommand { } ent.setObjectId(blobId); ent.setFileMode(mode); - checkoutPath(ent, r, + checkoutPath(ent, r, options, new CheckoutMetadata(eolStreamType, filterCommand)); actuallyModifiedPaths.add(path); } @@ -520,10 +526,10 @@ public class CheckoutCommand extends GitCommand { } private void checkoutPath(DirCacheEntry entry, ObjectReader reader, - CheckoutMetadata checkoutMetadata) { + WorkingTreeOptions options, CheckoutMetadata checkoutMetadata) { try { DirCacheCheckout.checkoutEntry(repo, entry, reader, true, - checkoutMetadata); + checkoutMetadata, options); } catch (IOException e) { throw new JGitInternalException(MessageFormat.format( JGitText.get().checkoutConflictWithFile, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java index 1004d3e50f..17036a9cd3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java @@ -48,6 +48,7 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.treewalk.WorkingTreeOptions; /** * Command class to apply a stashed commit. @@ -382,6 +383,8 @@ public class StashApplyCommand extends GitCommand { private void resetUntracked(RevTree tree) throws CheckoutConflictException, IOException { Set actuallyModifiedPaths = new HashSet<>(); + WorkingTreeOptions options = repo.getConfig() + .get(WorkingTreeOptions.KEY); // TODO maybe NameConflictTreeWalk ? try (TreeWalk walk = new TreeWalk(repo)) { walk.addTree(tree); @@ -413,7 +416,7 @@ public class StashApplyCommand extends GitCommand { } } - checkoutPath(entry, reader, + checkoutPath(entry, reader, options, new CheckoutMetadata(eolStreamType, null)); actuallyModifiedPaths.add(entry.getPathString()); } @@ -426,10 +429,10 @@ public class StashApplyCommand extends GitCommand { } private void checkoutPath(DirCacheEntry entry, ObjectReader reader, - CheckoutMetadata checkoutMetadata) { + WorkingTreeOptions options, CheckoutMetadata checkoutMetadata) { try { DirCacheCheckout.checkoutEntry(repo, entry, reader, true, - checkoutMetadata); + checkoutMetadata, options); } catch (IOException e) { throw new JGitInternalException(MessageFormat.format( JGitText.get().checkoutConflictWithFile, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index f6fc393c45..2365c90d0d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -143,6 +143,8 @@ public class DirCacheCheckout { private boolean performingCheckout; + private WorkingTreeOptions options; + private ProgressMonitor monitor = NullProgressMonitor.INSTANCE; /** @@ -362,9 +364,12 @@ public class DirCacheCheckout { * Processing an entry in the context of {@link #prescanOneTree()} when only * one tree is given * - * @param m the tree to merge - * @param i the index - * @param f the working tree + * @param m + * the tree to merge + * @param i + * the index + * @param f + * the working tree * @throws IOException */ void processEntry(CanonicalTreeParser m, DirCacheBuildIterator i, @@ -489,6 +494,8 @@ public class DirCacheCheckout { MissingObjectException, IncorrectObjectTypeException, CheckoutConflictException, IndexWriteException, CanceledException { toBeDeleted.clear(); + options = repo.getConfig() + .get(WorkingTreeOptions.KEY); try (ObjectReader objectReader = repo.getObjectDatabase().newReader()) { if (headCommitTree != null) preScanTwoTrees(); @@ -558,7 +565,8 @@ public class DirCacheCheckout { if (FileMode.GITLINK.equals(entry.getRawMode())) { checkoutGitlink(path, entry); } else { - checkoutEntry(repo, entry, objectReader, false, meta); + checkoutEntry(repo, entry, objectReader, false, meta, + options); } e = null; @@ -594,7 +602,7 @@ public class DirCacheCheckout { } if (entry.getStage() == DirCacheEntry.STAGE_3) { checkoutEntry(repo, entry, objectReader, false, - null); + null, options); break; } ++entryIdx; @@ -1226,7 +1234,7 @@ public class DirCacheCheckout { checkoutEntry(repo, e, walk.getObjectReader(), false, new CheckoutMetadata(walk.getEolStreamType(CHECKOUT_OP), walk.getFilterCommand( - Constants.ATTR_FILTER_TYPE_SMUDGE))); + Constants.ATTR_FILTER_TYPE_SMUDGE)), options); } } } @@ -1392,12 +1400,6 @@ public class DirCacheCheckout { * cannot be renamed to file or link without deleting it recursively. *

* - *

- * TODO: this method works directly on File IO, we may need another - * abstraction (like WorkingTreeIterator). This way we could tell e.g. - * Eclipse that Files in the workspace got changed - *

- * * @param repo * repository managing the destination work tree. * @param entry @@ -1407,15 +1409,16 @@ public class DirCacheCheckout { * @throws java.io.IOException * @since 3.6 * @deprecated since 5.1, use - * {@link #checkoutEntry(Repository, DirCacheEntry, ObjectReader, boolean, CheckoutMetadata)} + * {@link #checkoutEntry(Repository, DirCacheEntry, ObjectReader, boolean, CheckoutMetadata, WorkingTreeOptions)} * instead */ @Deprecated public static void checkoutEntry(Repository repo, DirCacheEntry entry, ObjectReader or) throws IOException { - checkoutEntry(repo, entry, or, false, null); + checkoutEntry(repo, entry, or, false, null, null); } + /** * Updates the file in the working tree with content and mode from an entry * in the index. The new content is first written to a new temporary file in @@ -1428,10 +1431,45 @@ public class DirCacheCheckout { * recursively, independently if has any content. *

* + * @param repo + * repository managing the destination work tree. + * @param entry + * the entry containing new mode and content + * @param or + * object reader to use for checkout + * @param deleteRecursive + * true to recursively delete final path if it exists on the file + * system + * @param checkoutMetadata + * containing + *
    + *
  • smudgeFilterCommand to be run for smudging the entry to be + * checked out
    • + *
  • eolStreamType used for stream conversion
    • + *
+ * @throws java.io.IOException + * @since 4.2 + * @deprecated since 6.3, use + * {@link #checkoutEntry(Repository, DirCacheEntry, ObjectReader, boolean, CheckoutMetadata, WorkingTreeOptions)} + * instead + */ + @Deprecated + public static void checkoutEntry(Repository repo, DirCacheEntry entry, + ObjectReader or, boolean deleteRecursive, + CheckoutMetadata checkoutMetadata) throws IOException { + checkoutEntry(repo, entry, or, deleteRecursive, checkoutMetadata, null); + } + + /** + * Updates the file in the working tree with content and mode from an entry + * in the index. The new content is first written to a new temporary file in + * the same directory as the real file. Then that new file is renamed to the + * final filename. + * *

- * TODO: this method works directly on File IO, we may need another - * abstraction (like WorkingTreeIterator). This way we could tell e.g. - * Eclipse that Files in the workspace got changed + * Note: if the entry path on local file system exists as a file, it + * will be deleted and if it exists as a directory, it will be deleted + * recursively, independently if has any content. *

* * @param repo @@ -1450,14 +1488,19 @@ public class DirCacheCheckout { * checked out *
  • eolStreamType used for stream conversion
    • * + * @param options + * {@link WorkingTreeOptions} that are effective; if {@code null} + * they are loaded from the repository config * @throws java.io.IOException - * @since 4.2 + * @since 6.3 */ public static void checkoutEntry(Repository repo, DirCacheEntry entry, ObjectReader or, boolean deleteRecursive, - CheckoutMetadata checkoutMetadata) throws IOException { - if (checkoutMetadata == null) + CheckoutMetadata checkoutMetadata, WorkingTreeOptions options) + throws IOException { + if (checkoutMetadata == null) { checkoutMetadata = CheckoutMetadata.EMPTY; + } ObjectLoader ol = or.open(entry.getObjectId()); File f = new File(repo.getWorkTree(), entry.getPathString()); File parentDir = f.getParentFile(); @@ -1466,7 +1509,8 @@ public class DirCacheCheckout { } FileUtils.mkdirs(parentDir, true); FS fs = repo.getFS(); - WorkingTreeOptions opt = repo.getConfig().get(WorkingTreeOptions.KEY); + WorkingTreeOptions opt = options != null ? options + : repo.getConfig().get(WorkingTreeOptions.KEY); if (entry.getFileMode() == FileMode.SYMLINK && opt.getSymLinks() == SymLinks.TRUE) { byte[] bytes = ol.getBytes(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/WorkTreeUpdater.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/WorkTreeUpdater.java index f872f9fba5..88529ba091 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/WorkTreeUpdater.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/WorkTreeUpdater.java @@ -495,7 +495,9 @@ public class WorkTreeUpdater implements Closeable { new File(nonNullNonBareRepo().getWorkTree(), entry.getKey()).mkdirs(); } else { DirCacheCheckout.checkoutEntry( - repo, dirCacheEntry, reader, false, checkoutMetadata.get(entry.getKey())); + repo, dirCacheEntry, reader, false, + checkoutMetadata.get(entry.getKey()), + workingTreeOptions); result.modifiedFiles.add(entry.getKey()); } } @@ -520,7 +522,8 @@ public class WorkTreeUpdater implements Closeable { DirCacheEntry entry = dirCache.getEntry(path); if (entry != null) { DirCacheCheckout.checkoutEntry( - repo, entry, reader, false, cleanupMetadata.get(path)); + repo, entry, reader, false, cleanupMetadata.get(path), + workingTreeOptions); } } } @@ -563,7 +566,7 @@ public class WorkTreeUpdater implements Closeable { try { try (TemporaryBuffer buf = buffer) { DirCacheCheckout.getContent(repo, path, metadata, - resultStreamLoader, null, buf); + resultStreamLoader, workingTreeOptions, buf); } try (InputStream bufIn = buffer.openInputStream()) { Files.copy(bufIn, file.toPath(), @@ -576,7 +579,7 @@ public class WorkTreeUpdater implements Closeable { } try (OutputStream outputStream = new FileOutputStream(file)) { DirCacheCheckout.getContent(repo, path, metadata, - resultStreamLoader, null, outputStream); + resultStreamLoader, workingTreeOptions, outputStream); } } -- 2.39.5