{"thread":{"id":"16455","subject":"[EGIT PATCH] Avoid breaking the walk prematurely","startedAt":"2008-11-24T23:45:48Z","lastAt":"2008-11-25T00:12:01Z","messageCount":2,"participants":["Robin Rosenberg","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"96444","messageId":"1227570348-20092-1-git-send-email-robin.rosenberg@dewire.com","threadId":"16455","inReplyTo":null,"subject":"[EGIT PATCH] Avoid breaking the walk prematurely","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-11-24T23:45:48Z","receivedAt":"2008-11-24T23:45:48Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"The optimization broke the walk when all changes we adds, assuming\nthe parent was an empty tree. This assumption was wrong. The tree does\nnot have to bee empty, it can contain other trees and blobs.\n\nhttp://code.google.com/p/egit/issues/detail?id=46\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../spearce/jgit/revwalk/RewriteTreeFilter.java    |    7 -------\n 1 files changed, 0 insertions(+), 7 deletions(-)\n\nWe could introduce a fixed version later on if we supply an option to select\nwhether to optimize the walk this way. The problem with doing it without an\noption is that imports from legacy SCMs do not necessarily record file moves\nas add/removes in direct sucession, but other events may happen in beteens\nfor various reasons.\n\n-- robin \n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java\nindex 587d6d2..a5edbf0 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RewriteTreeFilter.java\n@@ -118,13 +118,6 @@ public boolean include(final RevWalk walker, final RevCommit c)\n \t\t\t\t//\n \t\t\t\tc.flags |= REWRITE;\n \t\t\t\treturn false;\n-\t\t\t} else if (chgs == adds) {\n-\t\t\t\t// We added everything, so the parent may as well just\n-\t\t\t\t// be an empty tree. Kill our parent, we can assume\n-\t\t\t\t// it did not supply interesting changes.\n-\t\t\t\t//\n-\t\t\t\tc.parents = RevCommit.NO_PARENTS;\n-\t\t\t\treturn true;\n \t\t\t} else {\n \t\t\t\t// We have interesting items, but neither of the special\n \t\t\t\t// cases denoted above.\n-- \n1.6.0.3.640.g6331a\n"},{"id":"96447","messageId":"20081125001201.GH2932@spearce.org","threadId":"16455","inReplyTo":"1227570348-20092-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH] Avoid breaking the walk prematurely","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-25T00:12:01Z","receivedAt":"2008-11-25T00:12:01Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> The optimization broke the walk when all changes we adds, assuming\n> the parent was an empty tree. This assumption was wrong. The tree does\n> not have to bee empty, it can contain other trees and blobs.\n> \n> http://code.google.com/p/egit/issues/detail?id=46\n> \n> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n> ---\n>  .../spearce/jgit/revwalk/RewriteTreeFilter.java    |    7 -------\n>  1 files changed, 0 insertions(+), 7 deletions(-)\n> \n> We could introduce a fixed version later on if we supply an option to select\n> whether to optimize the walk this way. The problem with doing it without an\n> option is that imports from legacy SCMs do not necessarily record file moves\n> as add/removes in direct sucession, but other events may happen in beteens\n> for various reasons.\n\nThis is a fairly useful optimization in git.  Most people use it,\nbecause most people don't pass \"--full-history\" when they run\n\"git log -- path\".  So I think we really want to do it by default\nin jgit.\n\nEnabling this optimization again probably requires adding a new\nmethod to TreeFilter which allows the filter graph to return only\nnodes which select paths by name, and then do something like:\n\n  else if (chgs == adds && isPathSetEmpty(c.getTree())) {\n    c.parents = RevCommit.NO_PARENTS;\n    return true;\n  }\n\nwhere isPathSetEmpty runs a second TreeWalk iteration but uses the\nnew (smaller) filter graph that only matches on paths.\n\nI'm not going to write that out today.  But I thought I'd at least\nwrite an email while the idea was fresh in my head.\n \n-- \nShawn.\n"}]}