Add blob find pickaxe option
Subsystems (2)
cover Add blob find pickaxe option
This is a resend for sb/diff-blobfind, but now the option --find-object is a native of the pickaxe family.
Stefan Beller (1):
diffcore: add a pickaxe option to find a specific blob
Documentation/diff-options.txt | 10 +++++++ diff.c | 21 ++++++++++++- diff.h | 3 ++ diffcore-pickaxe.c | 14 +++++++-- revision.c | 3 ++ t/t4064-diff-oidfind.sh | 68 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 115 insertions(+), 4 deletions(-) create mode 100755 t/t4064-diff-oidfind.sh
Signature · 1 line
2.15.1.620.gb9897f4670-goog
Documentation/diff-options.txt +9 -5
@@ -492,6 +492,15 @@ occurrences of that string did not change). See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more information. +--find-object=<object-id>:: + Look for differences that change the number of occurrences of + the specified object. Similar to `-S`, just the argument is different + in that it doesn't search for a specific string but for a specific + object id. ++ +The object can be a blob or a submodule commit. It implies the `-t` option in +`git-log` to also find trees. + --pickaxe-all:: When `-S` or `-G` finds a change, show all the changes in that changeset, not just the files that contain the change
@@ -501,11 +510,6 @@ information. Treat the <string> given to `-S` as an extended POSIX regular expression to match. ---find-object=<object-id>:: - Restrict the output such that one side of the diff - matches the given object id. The object can be a blob, - gitlink entry or tree (when `-t` is given). - endif::git-format-patch[] -O<orderfile>::
Makefile +0 -1
@@ -775,7 +775,6 @@ LIB_OBJS += date.o LIB_OBJS += decorate.o LIB_OBJS += diffcore-break.o LIB_OBJS += diffcore-delta.o -LIB_OBJS += diffcore-objfind.o LIB_OBJS += diffcore-order.o LIB_OBJS += diffcore-pickaxe.o LIB_OBJS += diffcore-rename.o
builtin/log.c +1 -1
@@ -181,7 +181,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,init_display_notes (&rev->notes_opt);if (rev->diffopt.pickaxe || rev->diffopt.filter || - rev->diffopt.flags.follow_renames || rev->diffopt.objfind) + rev->diffopt.flags.follow_renames) rev->always_show_header =0 ;if (source)
diff.c +4 -3
@@ -4497,6 +4497,10 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)if (!opt->objfind) opt->objfind =xcalloc (1 ,sizeof (*opt->objfind)); + + opt->pickaxe ="" ;/* trigger pickaxe */ + opt->flags.recursive =1 ; + opt->flags.tree_in_recursive =1 ;oidset_insert (opt->objfind, &oid);return 1 ; }
@@ -5785,9 +5789,6 @@ void diffcore_std(struct diff_options *options)diffcore_skip_stat_unmatch (options);if (!options->found_follow) {/* See try_to_follow_renames() in tree-diff.c */ - -if (options->objfind) -diffcore_objfind (options);if (options->break_opt !=-1 )diffcore_break (options->break_opt);if (options->detect_rename)
diffcore-objfind.c deleted +0 -42
42 lines not on this page · raw message
diffcore-pickaxe.c +11 -3
@@ -124,13 +124,21 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,mmfile_t mf1, mf2;int ret; -if (!o->pickaxe[0 ]) -return 0 ; -/* ignore unmerged */ if (!DIFF_FILE_VALID (p->one) && !DIFF_FILE_VALID (p->two))return 0 ; +if (o->objfind) { +if ((DIFF_FILE_VALID (p->one) && +oidset_contains (o->objfind, &p->one->oid)) || + (DIFF_FILE_VALID (p->two) && +oidset_contains (o->objfind, &p->two->oid))) +return 1 ; + } + +if (!o->pickaxe[0 ]) +return 0 ; +if (o->flags.allow_textconv) { textconv_one =get_textconv (p->one); textconv_two =get_textconv (p->two);
diffcore.h +0 -1
@@ -107,7 +107,6 @@ extern struct diff_filepair *diff_queue(struct diff_queue_struct *, struct diff_filespec *);extern void diff_q (struct diff_queue_struct *,struct diff_filepair *); -extern void diffcore_objfind (struct diff_options *);extern void diffcore_break (int );extern void diffcore_rename (struct diff_options *);extern void diffcore_merge_broken (void );
revision.c +4 -4
@@ -2409,10 +2409,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s/* Pickaxe, diff-filter and rename following need diffs */ if (revs->diffopt.pickaxe || revs->diffopt.filter || - revs->diffopt.flags.follow_renames || - revs->diffopt.objfind) + revs->diffopt.flags.follow_renames) revs->diff =1 ; +if (revs->diffopt.objfind) + revs->simplify_history =0 ; +if (revs->topo_order) revs->limited =1 ;
@@ -2884,8 +2886,6 @@ int prepare_revision_walk(struct rev_info *revs)simplify_merges (revs);if (revs->children.name)set_children (revs); -if (revs->diffopt.objfind) - revs->simplify_history =0 ;return 0 ; }
t/helper/test-lazy-init-name-hash.c +1 -1
@@ -112,7 +112,7 @@ static void analyze_run(void) {uint64_t t1s, t1m, t2s, t2m;int cache_nr_limit; -int nr_threads_used; +int nr_threads_used =0 ;int i;int nr;
PATCH 1/1 diffcore: add a pickaxe option to find a specific blob
Sometimes users are given a hash of an object and they want to identify it further (ex.: Use verify-pack to find the largest blobs, but what are these? or [1])
One might be tempted to extend git-describe to also work with blobs, such that git describe <blob-id> gives a description as '<commit-ish>:<path>'. This was implemented at [2]; as seen by the sheer number of responses (>110), it turns out this is tricky to get right. The hard part to get right is picking the correct 'commit-ish' as that could be the commit that (re-)introduced the blob or the blob that removed the blob; the blob could exist in different branches.
Junio hinted at a different approach of solving this problem, which this patch implements. Teach the diff machinery another flag for restricting the information to what is shown. For example:
$ ./git log --oneline --find-object=v2.0.0:Makefile b2feb64309 Revert the whole "ask curl-config" topic for now 47fbfded53 i18n: only extract comments marked with "TRANSLATORS:"
we observe that the Makefile as shipped with 2.0 was appeared in v1.9.2-471-g47fbfded53 and in v2.0.0-rc1-5-gb2feb6430b. The reason why these commits both occur prior to v2.0.0 are evil merges that are not found using this new mechanism.
[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob
[2] https://public-inbox.org/git/[email protected]/
--- Documentation/diff-options.txt | 10 +++++++ diff.c | 21 ++++++++++++- diff.h | 3 ++ diffcore-pickaxe.c | 14 +++++++-- revision.c | 3 ++ t/t4064-diff-oidfind.sh | 68 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 115 insertions(+), 4 deletions(-) create mode 100755 t/t4064-diff-oidfind.sh
Documentation/diff-options.txt +10 -0
@@ -492,6 +492,15 @@ occurrences of that string did not change). See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more information. +--find-object=<object-id>:: + Look for differences that change the number of occurrences of + the specified object. Similar to `-S`, just the argument is different + in that it doesn't search for a specific string but for a specific + object id. ++ +The object can be a blob or a submodule commit. It implies the `-t` option in +`git-log` to also find trees. + --pickaxe-all:: When `-S` or `-G` finds a change, show all the changes in that changeset, not just the files that contain the change
@@ -500,6 +509,7 @@ information. --pickaxe-regex:: Treat the <string> given to `-S` as an extended POSIX regular expression to match. + endif::git-format-patch[] -O<orderfile>::
diff.c +20 -1
@@ -4082,6 +4082,7 @@ void diff_setup(struct diff_options *options) options->interhunkcontext = diff_interhunk_context_default; options->ws_error_highlight = ws_error_highlight_default; options->flags.rename_empty =1 ; + options->objfind =NULL ;/* pathchange left =NULL by default */ options->change = diff_change;
@@ -4487,6 +4488,23 @@ static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *arreturn 1 ; } +static int parse_objfind_opt (struct diff_options *opt,const char *arg) +{ +struct object_id oid; + +if (get_oid (arg, &oid)) +return error ("unable to resolve '%s'" , arg); + +if (!opt->objfind) + opt->objfind =xcalloc (1 ,sizeof (*opt->objfind)); + + opt->pickaxe ="" ;/* trigger pickaxe */ + opt->flags.recursive =1 ; + opt->flags.tree_in_recursive =1 ; +oidset_insert (opt->objfind, &oid); +return 1 ; +} +int diff_opt_parse (struct diff_options *options,const char **av,int ac,const char *prefix) {
@@ -4736,7 +4754,8 @@ int diff_opt_parse(struct diff_options *options,else if ((argcount =short_opt ('O' , av, &optarg))) { options->orderfile =prefix_filename (prefix, optarg);return argcount; - } + }else if (skip_prefix (arg,"--find-object=" , &arg)) +return parse_objfind_opt (options, arg);else if ((argcount =parse_long_opt ("diff-filter" , av, &optarg))) {int offending =parse_diff_filter_opt (optarg, options);if (offending)
diff.h +3 -0
@@ -7,6 +7,7 @@#include "tree-walk.h" #include "pathspec.h" #include "object.h" +#include "oidset.h" struct rev_info ;struct diff_options ;
@@ -174,6 +175,8 @@ struct diff_options {enum diff_words_type word_diff;enum diff_submodule_format submodule_format; +struct oidset *objfind; +/* this is set by diffcore for DIFF_FORMAT_PATCH */ int found_changes;
diffcore-pickaxe.c +11 -3
@@ -124,13 +124,21 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,mmfile_t mf1, mf2;int ret; -if (!o->pickaxe[0 ]) -return 0 ; -/* ignore unmerged */ if (!DIFF_FILE_VALID (p->one) && !DIFF_FILE_VALID (p->two))return 0 ; +if (o->objfind) { +if ((DIFF_FILE_VALID (p->one) && +oidset_contains (o->objfind, &p->one->oid)) || + (DIFF_FILE_VALID (p->two) && +oidset_contains (o->objfind, &p->two->oid))) +return 1 ; + } + +if (!o->pickaxe[0 ]) +return 0 ; +if (o->flags.allow_textconv) { textconv_one =get_textconv (p->one); textconv_two =get_textconv (p->two);
revision.c +3 -0
@@ -2412,6 +2412,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s revs->diffopt.flags.follow_renames) revs->diff =1 ; +if (revs->diffopt.objfind) + revs->simplify_history =0 ; +if (revs->topo_order) revs->limited =1 ;
t/t4064-diff-oidfind.sh new file +68 -0
68 lines not on this page · raw message
Signature · 1 line
2.15.1.620.gb9897f4670-goog
Re: [PATCH 1/1] diffcore: add a pickaxe option to find a specific blob
diff.c static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *ar · line 4501
Quoted 11 lines · Stefan Beller
diff.c +11
-0+static int parse_objfind_opt (struct diff_options *opt,const char *arg) +{ +struct object_id oid; + +if (get_oid (arg, &oid)) +return error ("unable to resolve '%s'" , arg); + +if (!opt->objfind) + opt->objfind =xcalloc (1 ,sizeof (*opt->objfind)); + + opt->pickaxe ="" ;/* trigger pickaxe */
Hmmm. This feels like an ugly hack to me. Do we declare that "git log -S''" never matches anything right now (if that is the case the I do not think there is any compatibility issues)?
Quoted 16 lines
diffcore-pickaxe.c +1
-3@@ -124,13 +124,21 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,mmfile_t mf1, mf2;int ret; -if (!o->pickaxe[0 ]) -return 0 ; -/* ignore unmerged */ if (!DIFF_FILE_VALID (p->one) && !DIFF_FILE_VALID (p->two))return 0 ; +if (o->objfind) {
diffcore-pickaxe.c static int pickaxe_match(struct diff_filepair *p, struct diff_options *o, · line 141
Quoted 10 lines
diffcore-pickaxe.c +10
-0+if ((DIFF_FILE_VALID (p->one) && +oidset_contains (o->objfind, &p->one->oid)) || + (DIFF_FILE_VALID (p->two) && +oidset_contains (o->objfind, &p->two->oid))) +return 1 ; + } + +if (!o->pickaxe[0 ]) +return 0 ; +
Very nice. With just one place, the code covers both cases with and without pickaxe-all option in effect.
Re: [PATCH 1/1] diffcore: add a pickaxe option to find a specific blob
Quoted 15 lines · Junio C Hamano
diff.c static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *ar · line 4501
Quoted 11 lines · Stefan Beller
diff.c +11
-0+static int parse_objfind_opt (struct diff_options *opt,const char *arg) +{ +struct object_id oid; + +if (get_oid (arg, &oid)) +return error ("unable to resolve '%s'" , arg); + +if (!opt->objfind) + opt->objfind =xcalloc (1 ,sizeof (*opt->objfind)); + + opt->pickaxe ="" ;/* trigger pickaxe */ Hmmm. This feels like an ugly hack to me.
Because it is.
$ git grep pickaxe diff.h diff.h:94: unsigned pickaxe_ignore_case:1; diff.h:130: const char *pickaxe; diff.h:149: int pickaxe_opts; diff.h:361:" --pickaxe-all\n" \
We could promote pickaxe_opts to be the important part for pickaxing, currently we're using pickaxe in diff.c to make a decision:
if (options->pickaxe)
diffcore_pickaxe(options);So I think I'll cleanup the pickaxe_opts to be a real unsigned flags field (such that pickaxe_ignore_case could be part of it, too!) and put a new flag in there in addition to DIFF_PICKAXE_KIND_{G, S}
Quoted 3 lines
Do we declare that "git
log -S''" never matches anything right now (if that is the case the I do not think there is any compatibility issues)?
Let's not go this route.
Quoted 16 lines
Quoted 16 lines
diffcore-pickaxe.c +0
-3@@ -124,13 +124,21 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,mmfile_t mf1, mf2;int ret; -if (!o->pickaxe[0 ]) -return 0 ; -/* ignore unmerged */ if (!DIFF_FILE_VALID (p->one) && !DIFF_FILE_VALID (p->two))return 0 ;
Quoted 14 lines
diffcore-pickaxe.c static int pickaxe_match(struct diff_filepair *p, struct diff_options *o, · line 141
Quoted 11 lines
diffcore-pickaxe.c +11
-0+if (o->objfind) { +if ((DIFF_FILE_VALID (p->one) && +oidset_contains (o->objfind, &p->one->oid)) || + (DIFF_FILE_VALID (p->two) && +oidset_contains (o->objfind, &p->two->oid))) +return 1 ; + } + +if (!o->pickaxe[0 ]) +return 0 ; +Very nice. With just one place, the code covers both cases with and without pickaxe-all option in effect.
and this is already fixed regarding the ugly hack, too.
Thanks,
Stefan