From: Junio C Hamano <hidden> Date: 2016-06-15 22:45:07
Karl Hasselström [off-list ref] writes:
quoted
The feature of --stdin to take a commit and its parents on one line was
broken before that to support the common
rev-list --parents $commits... -- $paths... |
diff-tree --stdin -v -p
usage pattern by Porcelains. For diff-tree to talk sensibly about
commits, it needs to see commits, not just trees.
But is there any fundamental reason why it couldn't accept tree-ishes
as well?
The -v option given to diff-tree is the key. Without it, it could take
trees.
From: Karl Hasselström <hidden> Date: 2016-06-15 22:45:07
Into a first half that determines what operation to do, and a second
half that does it.
Currently the only operation is diffing one or more commits, but a
later patch will add diffing of trees, at which point this refactoring
will pay off.
Signed-off-by: Karl Hasselström <redacted>
---
builtin-diff-tree.c | 31 +++++++++++++++++++------------
1 files changed, 19 insertions(+), 12 deletions(-)
@@ -14,20 +14,10 @@ static int diff_tree_commit_sha1(const unsigned char *sha1)returnlog_tree_commit(&log_tree_opt,commit);}-staticintdiff_tree_stdin(char*line)+/* Diff one or more commits. */+staticintstdin_diff_commit(structcommit*commit,char*line,intlen){-intlen=strlen(line);unsignedcharsha1[20];-structcommit*commit;--if(!len||line[len-1]!='\n')-return-1;-line[len-1]=0;-if(get_sha1_hex(line,sha1))-return-1;-commit=lookup_commit(sha1);-if(!commit||parse_commit(commit))-return-1;if(isspace(line[40])&&!get_sha1_hex(line+41,sha1)){/* Graft the fake parents locally to the commit */intpos=41;
From: Karl Hasselström <hidden> Date: 2016-06-15 22:45:07
Here's two patches that implements diffing of trees: first a
refactoring, then the actual functionality.
I'm not familiar with git's API, so I might have made mistakes in
choosing or using functions. Extra eyeballs appreciated. But the test
suite passes, it does what I want, and it does make StGit faster, so
_I'm_ happy ...
---
Karl Hasselström (3):
Add test for diff-tree --stdin with two trees
Teach git diff-tree --stdin to diff trees
Refactoring: Split up diff_tree_stdin
Documentation/git-diff-tree.txt | 14 ++++++---
builtin-diff-tree.c | 58 +++++++++++++++++++++++++++++++--------
t/t4002-diff-basic.sh | 14 +++++++++
3 files changed, 69 insertions(+), 17 deletions(-)
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
@@ -169,6 +169,20 @@ test_expect_success \cmp-s.test-a.test-recursive-AB' test_expect_success\+'diff-tree --stdin of known trees.'\+'echo$tree_A$tree_B|gitdiff-tree--stdin>.test-a&&+echo$tree_A$tree_B>.test-plain-ABx&&+cat.test-plain-AB>>.test-plain-ABx&&+cmp-s.test-a.test-plain-ABx'++test_expect_success\+'diff-tree --stdin of known trees.'\+'echo$tree_A$tree_B|gitdiff-tree-r--stdin>.test-a&&+echo$tree_A$tree_B>.test-recursive-ABx&&+cat.test-recursive-AB>>.test-recursive-ABx&&+cmp-s.test-a.test-recursive-ABx'++test_expect_success\'diff-cache O with A in cache'\'gitread-tree$tree_A&&gitdiff-index--cached$tree_O>.test-a&&
From: Karl Hasselström <hidden> Date: 2016-06-15 22:45:07
In addition to accepting lines with one or more commits, it now
accepts lines with precisely two trees.
When diffing trees, the -m, -s, -v, --pretty, --abbrev-commit,
--encoding, --no-commit-id, -c, --cc, and --always options are
ignored, since they do not apply to trees. This is the same behavior
you get when specifying two trees on the command line instead of with
--stdin.
Signed-off-by: Karl Hasselström <redacted>
---
Documentation/git-diff-tree.txt | 14 +++++++++-----
builtin-diff-tree.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 40 insertions(+), 9 deletions(-)
@@ -49,13 +49,17 @@ include::diff-options.txt[] --stdin:: When '--stdin' is specified, the command does not take <tree-ish> arguments from the command line. Instead, it- reads either one <commit> or a list of <commit>- separated with a single space from its standard input.+ reads lines containing either two <tree>, one <commit>, or a+ list of <commit> from its standard input. (Use a single space+ as separator.) +-When a single commit is given on one line of such input, it compares-the commit with its parents. The following flags further affects its-behavior. The remaining commits, when given, are used as if they are+When two trees are given, it compares the first tree with the second.+When a single commit is given, it compares the commit with its+parents. The remaining commits, when given, are used as if they are parents of the first commit.+++The following flags further affects the behavior when comparing+commits (but not trees). -m:: By default, 'git-diff-tree --stdin' does not show
@@ -42,21 +42,48 @@ static int stdin_diff_commit(struct commit *commit, char *line, int len)returnlog_tree_commit(&log_tree_opt,commit);}+/* Diff two trees. */+staticintstdin_diff_trees(structtree*tree1,char*line,intlen)+{+unsignedcharsha1[20];+structtree*tree2;+if(len!=82||!isspace(line[40])||get_sha1_hex(line+41,sha1)){+error("Need precisely two trees, separated by one space");+return-1;+}+tree2=lookup_tree(sha1);+if(!tree2||parse_tree(tree2))+return-1;+printf("%s %s\n",sha1_to_hex(tree1->object.sha1),+sha1_to_hex(tree2->object.sha1));+diff_tree_sha1(tree1->object.sha1,tree2->object.sha1,+"",&log_tree_opt.diffopt);+log_tree_diff_flush(&log_tree_opt);+return0;+}+staticintdiff_tree_stdin(char*line){intlen=strlen(line);unsignedcharsha1[20];-structcommit*commit;+structobject*obj;if(!len||line[len-1]!='\n')return-1;line[len-1]=0;if(get_sha1_hex(line,sha1))return-1;-commit=lookup_commit(sha1);-if(!commit||parse_commit(commit))+obj=lookup_object(sha1);+obj=obj?obj:parse_object(sha1);+if(!obj)return-1;-returnstdin_diff_commit(commit,line,len);+if(obj->type==OBJ_COMMIT)+returnstdin_diff_commit((structcommit*)obj,line,len);+if(obj->type==OBJ_TREE)+returnstdin_diff_trees((structtree*)obj,line,len);+error("Object %s is a %s, not a commit or tree",+sha1_to_hex(sha1),typename(obj->type));+return-1;}staticconstchardiff_tree_usage[]=
From: Jeff King <hidden> Date: 2016-06-15 22:45:07
On Fri, Aug 08, 2008 at 10:48:29PM +0200, Karl Hasselström wrote:
--stdin::
When '--stdin' is specified, the command does not take
<tree-ish> arguments from the command line. Instead, it
- reads either one <commit> or a list of <commit>
- separated with a single space from its standard input.
+ reads lines containing either two <tree>, one <commit>, or a
+ list of <commit> from its standard input. (Use a single space
+ as separator.)
Hmm. Just looking at this as a git user, I would have expected it to
take one or more hashes, separated by spaces. If only one, then it must
be a commit, and it is diffed against its parents. If more than one,
then each must be a tree-ish. So you could diff a commit against a tree
(or a tag against a commit, or...).
And I think it might even be easier to code. ;)
-Peff
From: Karl Hasselström <hidden> Date: 2016-06-15 22:45:07
On 2008-08-08 17:45:23 -0400, Jeff King wrote:
Hmm. Just looking at this as a git user, I would have expected it to
take one or more hashes, separated by spaces. If only one, then it
must be a commit, and it is diffed against its parents. If more than
one, then each must be a tree-ish. So you could diff a commit
against a tree (or a tag against a commit, or...).
I agree.
And I think it might even be easier to code. ;)
Not for someone who's almost entirely unfamiliar with the git API.
Finding the right functions to call takes a lot of time ... which is
why I decided to chicken out and implement only the subset I actually
needed. But it can be added later -- perhaps by me.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
From: Jeff King <hidden> Date: 2016-06-15 22:45:08
On Sat, Aug 09, 2008 at 12:00:49PM +0200, Karl Hasselström wrote:
quoted
And I think it might even be easier to code. ;)
Not for someone who's almost entirely unfamiliar with the git API.
Finding the right functions to call takes a lot of time ... which is
why I decided to chicken out and implement only the subset I actually
needed. But it can be added later -- perhaps by me.
:) I took a quick look, and I don't think it would be too hard to reuse
the logic from the command-line codepath. However, your patches are
already in 'next', and I don't see much point doing it the other way
unless there is actually some demand for mixed commit/tree input.
-Peff