* Request for change 610e2b9240 reversal @ 2020-11-08 11:15 Jean-Yves Avenard 2020-11-10 11:38 ` René Scharfe 0 siblings, 1 reply; 4+ messages in thread From: Jean-Yves Avenard @ 2020-11-08 11:15 UTC (permalink / raw) To: git Hi At Mozilla we use a mercurial repository, however many developers are using a git repository; we maintain two git mirrors, mozilla-unified accessed via the cinnabar plugin and a geck-dev native git mirror. Our repositories contain a .git-blame-ignore-revs that is used for both repositories. https://searchfox.org/mozilla-central/source/.git-blame-ignore-revs That git was ignoring invalid entries (for the currently in use repo) is a requirement for our use. Following the merge of 610e2b9240 jc/blame-ignore-fix later we have lost the ability to run git blame on any of our files. Could we get that change reversed? If it ain't broken, don't fix it as they say. Thank you Jean-Yves Avenard ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Request for change 610e2b9240 reversal 2020-11-08 11:15 Request for change 610e2b9240 reversal Jean-Yves Avenard @ 2020-11-10 11:38 ` René Scharfe 2020-11-10 13:33 ` Barret Rhoden 0 siblings, 1 reply; 4+ messages in thread From: René Scharfe @ 2020-11-10 11:38 UTC (permalink / raw) To: Jean-Yves Avenard, git; +Cc: Junio C Hamano, Barret Rhoden Am 08.11.20 um 12:15 schrieb Jean-Yves Avenard: > Hi > > At Mozilla we use a mercurial repository, however many developers are > using a git repository; we maintain two git mirrors, mozilla-unified > accessed via the cinnabar plugin and a geck-dev native git mirror. > > Our repositories contain a .git-blame-ignore-revs that is used for > both repositories. > https://searchfox.org/mozilla-central/source/.git-blame-ignore-revs > > That git was ignoring invalid entries (for the currently in use repo) > is a requirement for our use. > Following the merge of 610e2b9240 jc/blame-ignore-fix later we have > lost the ability to run git blame on any of our files. > > Could we get that change reversed? > If it ain't broken, don't fix it as they say. That looks like one tidily maintained ignore list! I saw that coming ([1], [2]), but didn't communicate my doubts clearly enough and undermined them when I wrote that I'd run into the same issue due to laziness. So I feel some responsibility for your trouble. Your actual use case is not lazy -- it uses the fundamental fact that Git object IDs are practically unique worldwide, without synchronization between repositories, i.e. they form a single flat namespace. 610e2b9240 (blame: validate and peel the object names on the ignore list, 2020-09-24) introduced two changes: Support for peeling tags and rejection of non-commits after peeling. Here's a patch for silently ignoring such non-commits read from a file. René [1] https://public-inbox.org/git/40488753-c179-4ce2-42d0-e57b5b1ec6cd@web.de/ [2] https://public-inbox.org/git/1fa730c4-eaef-2f32-e1b4-716a27ed4646@web.de/ -- >8 -- Subject: [PATCH] blame: silently ignore invalid ignore file objects Since 610e2b9240 (blame: validate and peel the object names on the ignore list, 2020-09-24) git blame reports checks if objects specified with --ignore-rev and in files loaded with --ignore-revs-file and config option blame.ignoreRevsFile are actual objects and dies if they aren't. The intent is to report typos to the user. This also breaks the ability to use a single ignore file for multiple repositories. Typos are presumably less likely in files than on the command line, so alerting is less useful here. Restore that feature by skipping non-commits without dying. Reported-by: Jean-Yves Avenard <jyavenard@mozilla.com> Signed-off-by: René Scharfe <l.s.r@web.de> --- oidset.c | 5 +++-- t/t8013-blame-ignore-revs.sh | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/oidset.c b/oidset.c index 2d0ab76fb5..5aac633c1f 100644 --- a/oidset.c +++ b/oidset.c @@ -72,9 +72,10 @@ void oidset_parse_file_carefully(struct oidset *set, const char *path, if (!sb.len) continue; - if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0' || - (fn && fn(&oid, cbdata))) + if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0') die("invalid object name: %s", sb.buf); + if (fn && fn(&oid, cbdata)) + continue; oidset_insert(set, &oid); } if (ferror(fp)) diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh index 24ae5018e8..b18633dee1 100755 --- a/t/t8013-blame-ignore-revs.sh +++ b/t/t8013-blame-ignore-revs.sh @@ -39,10 +39,10 @@ test_expect_success 'validate --ignore-rev' ' test_must_fail git blame --ignore-rev X^{tree} file ' -# Ensure bogus --ignore-revs-file requests are caught +# Ensure bogus --ignore-revs-file requests are silently accepted test_expect_success 'validate --ignore-revs-file' ' git rev-parse X^{tree} >ignore_x && - test_must_fail git blame --ignore-revs-file ignore_x file + git blame --ignore-revs-file ignore_x file ' for I in X XT -- 2.29.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: Request for change 610e2b9240 reversal 2020-11-10 11:38 ` René Scharfe @ 2020-11-10 13:33 ` Barret Rhoden 2020-11-10 17:05 ` Junio C Hamano 0 siblings, 1 reply; 4+ messages in thread From: Barret Rhoden @ 2020-11-10 13:33 UTC (permalink / raw) To: René Scharfe, Jean-Yves Avenard, git; +Cc: Junio C Hamano Hi - On 11/10/20 6:38 AM, René Scharfe wrote: [snip] > Reported-by: Jean-Yves Avenard <jyavenard@mozilla.com> > Signed-off-by: René Scharfe <l.s.r@web.de> patch looks good to me. Reviewed-by: Barret Rhoden <brho@google.com> Thanks, Barret > --- > oidset.c | 5 +++-- > t/t8013-blame-ignore-revs.sh | 4 ++-- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/oidset.c b/oidset.c > index 2d0ab76fb5..5aac633c1f 100644 > --- a/oidset.c > +++ b/oidset.c > @@ -72,9 +72,10 @@ void oidset_parse_file_carefully(struct oidset *set, const char *path, > if (!sb.len) > continue; > > - if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0' || > - (fn && fn(&oid, cbdata))) > + if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0') > die("invalid object name: %s", sb.buf); > + if (fn && fn(&oid, cbdata)) > + continue; > oidset_insert(set, &oid); > } > if (ferror(fp)) > diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh > index 24ae5018e8..b18633dee1 100755 > --- a/t/t8013-blame-ignore-revs.sh > +++ b/t/t8013-blame-ignore-revs.sh > @@ -39,10 +39,10 @@ test_expect_success 'validate --ignore-rev' ' > test_must_fail git blame --ignore-rev X^{tree} file > ' > > -# Ensure bogus --ignore-revs-file requests are caught > +# Ensure bogus --ignore-revs-file requests are silently accepted > test_expect_success 'validate --ignore-revs-file' ' > git rev-parse X^{tree} >ignore_x && > - test_must_fail git blame --ignore-revs-file ignore_x file > + git blame --ignore-revs-file ignore_x file > ' > > for I in X XT > -- > 2.29.2 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Request for change 610e2b9240 reversal 2020-11-10 13:33 ` Barret Rhoden @ 2020-11-10 17:05 ` Junio C Hamano 0 siblings, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2020-11-10 17:05 UTC (permalink / raw) To: Barret Rhoden; +Cc: René Scharfe, Jean-Yves Avenard, git Barret Rhoden <brho@google.com> writes: > On 11/10/20 6:38 AM, René Scharfe wrote: > [snip] >> Reported-by: Jean-Yves Avenard <jyavenard@mozilla.com> >> Signed-off-by: René Scharfe <l.s.r@web.de> > > patch looks good to me. > > Reviewed-by: Barret Rhoden <brho@google.com> > > Thanks, > > Barret Yes, I do recall that we discussed reverting half of that change and then I forgot to follow through. Thanks, all. Will queue. > > >> --- >> oidset.c | 5 +++-- >> t/t8013-blame-ignore-revs.sh | 4 ++-- >> 2 files changed, 5 insertions(+), 4 deletions(-) >> diff --git a/oidset.c b/oidset.c >> index 2d0ab76fb5..5aac633c1f 100644 >> --- a/oidset.c >> +++ b/oidset.c >> @@ -72,9 +72,10 @@ void oidset_parse_file_carefully(struct oidset *set, const char *path, >> if (!sb.len) >> continue; >> - if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0' || >> - (fn && fn(&oid, cbdata))) >> + if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0') >> die("invalid object name: %s", sb.buf); >> + if (fn && fn(&oid, cbdata)) >> + continue; >> oidset_insert(set, &oid); >> } >> if (ferror(fp)) >> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh >> index 24ae5018e8..b18633dee1 100755 >> --- a/t/t8013-blame-ignore-revs.sh >> +++ b/t/t8013-blame-ignore-revs.sh >> @@ -39,10 +39,10 @@ test_expect_success 'validate --ignore-rev' ' >> test_must_fail git blame --ignore-rev X^{tree} file >> ' >> -# Ensure bogus --ignore-revs-file requests are caught >> +# Ensure bogus --ignore-revs-file requests are silently accepted >> test_expect_success 'validate --ignore-revs-file' ' >> git rev-parse X^{tree} >ignore_x && >> - test_must_fail git blame --ignore-revs-file ignore_x file >> + git blame --ignore-revs-file ignore_x file >> ' >> for I in X XT >> -- >> 2.29.2 >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-10 17:05 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-08 11:15 Request for change 610e2b9240 reversal Jean-Yves Avenard 2020-11-10 11:38 ` René Scharfe 2020-11-10 13:33 ` Barret Rhoden 2020-11-10 17:05 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).