From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Patrick Steinhardt <ps@pks.im>, John Cai <johncai86@gmail.com>,
Linus Arver <linusa@google.com>,
Eric Sunshine <sunshine@sunshineco.com>,
Christian Couder <christian.couder@gmail.com>
Subject: [PATCH v3 0/5] rev-list: allow missing tips with --missing
Date: Wed, 14 Feb 2024 15:25:08 +0100 [thread overview]
Message-ID: <20240214142513.4002639-1-christian.couder@gmail.com> (raw)
In-Reply-To: <20240208135055.2705260-1-christian.couder@gmail.com>
# Intro
A recent patch series, kn/rev-list-missing-fix [1] extended the
`--missing` option in git-rev-list(1) to support commit objects.
Unfortunately, git-rev-list(1) with `--missing` set to something other
than 'error' still fails, usually with a "fatal: bad object <oid>"
error message, when a missing object is passed as an argument.
This patch series removes this limitation and when using
`--missing=print` allows all missing objects to be printed including
those that are passed as arguments.
[1] https://lore.kernel.org/git/20231026101109.43110-1-karthik.188@gmail.com/
# Patch overview
Patch 1/5 (t9210: do not rely on lazy fetching to fail) is a test fix
suggested by Junio, so that a mostly unrelated test will not wrongly
fail when this series is merged.
Patches 2/5 (revision: clarify a 'return NULL' in get_reference()),
3/5 (oidset: refactor oidset_insert_from_set()) and
4/5 (t6022: fix 'test' style and 'even though' typo) are very small
preparatory cleanups.
Patch 5/5 (rev-list: allow missing tips with --missing=[print|allow*])
allows git-rev-list(1) with `--missing=<arg>` when <arg> is not
'error' to not fail if some tips it is passed are missing.
# Changes since V2
Thanks to Linus Arver, and Junio who commented on V2!
The changes since V2 are the following:
- Patch 1/5 (t9210: do not rely on lazy fetching to fail) was added
to fix a test that wrongly failed when this series was applied,
thanks to Junio who authored it.
- In patch 5/5 (rev-list: allow missing tips with
--missing=[print|allow*]), some grammos and typos were fixed in the
commit message, thanks to Junio and Linus.
- In patch 5/5 (rev-list: allow missing tips with
--missing=[print|allow*]), the NEEDSWORK comment was improved, thanks
to Junio. In particular, it doesn't use "ugly" and "dumb" anymore
and gives an example of what's broken.
- In patch 5/5 (rev-list: allow missing tips with
--missing=[print|allow*]), a code comment as been clarified by removing
"already" from it.
# Range-diff since V2
-: ---------- > 1: 6e6f2cc26b t9210: do not rely on lazy fetching to fail
1: 5233a83181 = 2: 733c78144e revision: clarify a 'return NULL' in get_reference()
2: cfa72c8cf1 = 3: 4c9e032456 oidset: refactor oidset_insert_from_set()
3: 5668340516 = 4: 35ca6e7c3d t6022: fix 'test' style and 'even though' typo
4: 55792110ca ! 5: da36843b44 rev-list: allow missing tips with --missing=[print|allow*]
@@ Commit message
We could introduce a new option to make it work like this, but most
users are likely to prefer the command to have this behavior as the
default one. Introducing a new option would require another dumb loop
- to look for that options early, which isn't nice.
+ to look for that option early, which isn't nice.
Also we made `git rev-list` work with missing commits very recently
and the command is most often passed commits as arguments. So let's
- consider this as a bug fix related to these previous change.
+ consider this as a bug fix related to these recent changes.
While at it let's add a NEEDSWORK comment to say that we should get
rid of the existing ugly dumb loops that parse the
@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
* Let "--missing" to conditionally set fetch_if_missing.
*/
+ /*
-+ * NEEDSWORK: These dump loops to look for some options early
-+ * are ugly. We really need setup_revisions() to have a
-+ * mechanism to allow and disallow some sets of options for
-+ * different commands (like rev-list, replay, etc). Such
-+ * mechanism should do an early parsing of option and be able
-+ * to manage the `--exclude-promisor-objects` and `--missing=...`
-+ * options below.
++ * NEEDSWORK: These loops that attempt to find presence of
++ * options without understanding that the options they are
++ * skipping are broken (e.g., it would not know "--grep
++ * --exclude-promisor-objects" is not triggering
++ * "--exclude-promisor-objects" option). We really need
++ * setup_revisions() to have a mechanism to allow and disallow
++ * some sets of options for different commands (like rev-list,
++ * replay, etc). Such a mechanism should do an early parsing
++ * of options and be able to manage the `--missing=...` and
++ * `--exclude-promisor-objects` options below.
+ */
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
- if (arg_missing_action == MA_PRINT)
+ if (arg_missing_action == MA_PRINT) {
oidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);
-+ /* Already add missing tips */
++ /* Add missing tips */
+ oidset_insert_from_set(&missing_objects, &revs.missing_commits);
+ oidset_clear(&revs.missing_commits);
+ }
Christian Couder (4):
revision: clarify a 'return NULL' in get_reference()
oidset: refactor oidset_insert_from_set()
t6022: fix 'test' style and 'even though' typo
rev-list: allow missing tips with --missing=[print|allow*]
Junio C Hamano (1):
t9210: do not rely on lazy fetching to fail
Documentation/rev-list-options.txt | 4 ++
builtin/rev-list.c | 18 ++++++++-
list-objects-filter.c | 11 +-----
oidset.c | 10 +++++
oidset.h | 6 +++
revision.c | 16 ++++++--
t/t6022-rev-list-missing.sh | 61 +++++++++++++++++++++++++++++-
t/t9210-scalar.sh | 9 ++++-
8 files changed, 117 insertions(+), 18 deletions(-)
--
2.44.0.rc0.51.gda36843b44
next prev parent reply other threads:[~2024-02-14 14:26 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-08 13:50 [PATCH v2 0/4] rev-list: allow missing tips with --missing Christian Couder
2024-02-08 13:50 ` [PATCH v2 1/4] revision: clarify a 'return NULL' in get_reference() Christian Couder
2024-02-08 13:50 ` [PATCH v2 2/4] oidset: refactor oidset_insert_from_set() Christian Couder
2024-02-08 17:33 ` Junio C Hamano
2024-02-13 21:02 ` Linus Arver
2024-02-14 14:33 ` Christian Couder
2024-02-16 1:10 ` Linus Arver
2024-02-16 10:38 ` Christian Couder
2024-02-16 20:27 ` Linus Arver
2024-02-08 13:50 ` [PATCH v2 3/4] t6022: fix 'test' style and 'even though' typo Christian Couder
2024-02-08 13:50 ` [PATCH v2 4/4] rev-list: allow missing tips with --missing=[print|allow*] Christian Couder
2024-02-08 17:44 ` Junio C Hamano
2024-02-13 22:38 ` Linus Arver
2024-02-14 14:34 ` Christian Couder
2024-02-14 16:49 ` Junio C Hamano
2024-02-14 14:39 ` Christian Couder
2024-02-13 22:33 ` Linus Arver
2024-02-14 14:38 ` Christian Couder
2024-02-16 1:24 ` Linus Arver
2024-02-08 23:15 ` [PATCH v2 0/4] rev-list: allow missing tips with --missing Junio C Hamano
2024-02-14 14:26 ` Christian Couder
2024-02-14 14:25 ` Christian Couder [this message]
2024-02-14 14:25 ` [PATCH v3 1/5] t9210: do not rely on lazy fetching to fail Christian Couder
2024-02-14 14:25 ` [PATCH v3 2/5] revision: clarify a 'return NULL' in get_reference() Christian Couder
2024-02-14 14:25 ` [PATCH v3 3/5] oidset: refactor oidset_insert_from_set() Christian Couder
2024-02-14 14:25 ` [PATCH v3 4/5] t6022: fix 'test' style and 'even though' typo Christian Couder
2024-02-14 14:25 ` [PATCH v3 5/5] rev-list: allow missing tips with --missing=[print|allow*] Christian Couder
2024-02-16 21:56 ` [PATCH v3 0/5] rev-list: allow missing tips with --missing Linus Arver
2024-02-28 9:10 ` [PATCH] revision: fix --missing=[print|allow*] for annotated tags Christian Couder
2024-02-28 17:46 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240214142513.4002639-1-christian.couder@gmail.com \
--to=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johncai86@gmail.com \
--cc=linusa@google.com \
--cc=ps@pks.im \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).