Re: Request for change 610e2b9240 reversal
- From
René Scharfe <l.s.r@web.de>
- Date
- Nov 10, 2020, 11:38 UTC
- Message-ID
- <171b9a99-d48a-babe-168b-a7cd410e4635@web.de>
- In-Reply-To
- <CA+phgpFS_MOKOib5+yAE7U2QPMqMD_t+yXhR29UG9aSO4pgk+A@mail.gmail.com>
Am 08.11.20 um 12:15 schrieb Jean-Yves Avenard:
Show 17 quoted lines
> 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