* Re: [PATCH v3 05/10] bulk-checkin: extract abstract `bulk_checkin_source`
From: Junio C Hamano @ 2023-10-18 23:10 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <da52ec838025a59a3f4f4ffaf2e6f9098a37547e.1697648864.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> A future commit will want to implement a very similar routine as in
> `stream_blob_to_pack()` with two notable changes:
>
> - Instead of streaming just OBJ_BLOBs, this new function may want to
> stream objects of arbitrary type.
>
> - Instead of streaming the object's contents from an open
> file-descriptor, this new function may want to "stream" its contents
> from memory.
>
> To avoid duplicating a significant chunk of code between the existing
> `stream_blob_to_pack()`, extract an abstract `bulk_checkin_source`. This
> concept currently is a thin layer of `lseek()` and `read_in_full()`, but
> will grow to understand how to perform analogous operations when writing
> out an object's contents from memory.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> bulk-checkin.c | 61 +++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 53 insertions(+), 8 deletions(-)
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index f4914fb6d1..fc1d902018 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -140,8 +140,41 @@ static int already_written(struct bulk_checkin_packfile *state, struct object_id
> return 0;
> }
>
> +struct bulk_checkin_source {
> + enum { SOURCE_FILE } type;
> +
> + /* SOURCE_FILE fields */
> + int fd;
> +
> + /* common fields */
> + size_t size;
> + const char *path;
> +};
Looks OK, even though I expected to see a bit more involved object
orientation with something like
struct bulk_checkin_source {
off_t (*read)(struct bulk_checkin_source *, void *, size_t);
off_t (*seek)(struct bulk_checkin_source *, off_t);
union {
struct {
int fd;
size_t size;
const char *path;
} from_fd;
struct {
...
} incore;
} data;
};
As there will only be two subclasses of this thing, it may not
matter all that much right now, but it would be much nicer as your
methods do not have to care about "switch (enum) { case FILE: ... }".
^ permalink raw reply
* Re: [PATCH v3 08/10] bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
From: Junio C Hamano @ 2023-10-18 23:18 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <8667b763652ffa71b52b7bd78821e46a6e5fe5a9.1697648864.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> Now that we have factored out many of the common routines necessary to
> index a new object into a pack created by the bulk-checkin machinery, we
> can introduce a variant of `index_blob_bulk_checkin()` that acts on
> blobs whose contents we can fit in memory.
Hmph.
Doesn't the duplication between the main loop of the new
deflate_obj_contents_to_pack() with existing deflate_blob_to_pack()
bother you?
A similar duplication in the previous round resulted in a nice
refactoring of patches 5 and 6 in this round. Compared to that,
the differences in the set-up code between the two functions may be
much larger, but subtle and unnecessary differences between the code
that was copied and pasted (e.g., we do not check the errors from
the seek_to method here, but the original does) is a sign that over
time a fix to one will need to be carried over to the other, adding
unnecessary maintenance burden, isn't it?
> +static int deflate_obj_contents_to_pack_incore(struct bulk_checkin_packfile *state,
> + git_hash_ctx *ctx,
> + struct hashfile_checkpoint *checkpoint,
> + struct object_id *result_oid,
> + const void *buf, size_t size,
> + enum object_type type,
> + const char *path, unsigned flags)
> +{
> + struct pack_idx_entry *idx = NULL;
> + off_t already_hashed_to = 0;
> + struct bulk_checkin_source source = {
> + .type = SOURCE_INCORE,
> + .buf = buf,
> + .size = size,
> + .read = 0,
> + .path = path,
> + };
> +
> + /* Note: idx is non-NULL when we are writing */
> + if (flags & HASH_WRITE_OBJECT)
> + CALLOC_ARRAY(idx, 1);
> +
> + while (1) {
> + prepare_checkpoint(state, checkpoint, idx, flags);
> +
> + if (!stream_obj_to_pack(state, ctx, &already_hashed_to, &source,
> + type, flags))
> + break;
> + truncate_checkpoint(state, checkpoint, idx);
> + bulk_checkin_source_seek_to(&source, 0);
> + }
> +
> + finalize_checkpoint(state, ctx, checkpoint, idx, result_oid);
> +
> + return 0;
> +}
> +
> +static int deflate_blob_to_pack_incore(struct bulk_checkin_packfile *state,
> + struct object_id *result_oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags)
> +{
> + git_hash_ctx ctx;
> + struct hashfile_checkpoint checkpoint = {0};
> +
> + format_object_header_hash(the_hash_algo, &ctx, &checkpoint, OBJ_BLOB,
> + size);
> +
> + return deflate_obj_contents_to_pack_incore(state, &ctx, &checkpoint,
> + result_oid, buf, size,
> + OBJ_BLOB, path, flags);
> +}
> +
> static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> struct object_id *result_oid,
> int fd, size_t size,
> @@ -456,6 +509,17 @@ int index_blob_bulk_checkin(struct object_id *oid,
> return status;
> }
>
> +int index_blob_bulk_checkin_incore(struct object_id *oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags)
> +{
> + int status = deflate_blob_to_pack_incore(&bulk_checkin_packfile, oid,
> + buf, size, path, flags);
> + if (!odb_transaction_nesting)
> + flush_bulk_checkin_packfile(&bulk_checkin_packfile);
> + return status;
> +}
> +
> void begin_odb_transaction(void)
> {
> odb_transaction_nesting += 1;
> diff --git a/bulk-checkin.h b/bulk-checkin.h
> index aa7286a7b3..1b91daeaee 100644
> --- a/bulk-checkin.h
> +++ b/bulk-checkin.h
> @@ -13,6 +13,10 @@ int index_blob_bulk_checkin(struct object_id *oid,
> int fd, size_t size,
> const char *path, unsigned flags);
>
> +int index_blob_bulk_checkin_incore(struct object_id *oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags);
> +
> /*
> * Tell the object database to optimize for adding
> * multiple objects. end_odb_transaction must be called
^ permalink raw reply
* Re: [PATCH v4 00/17] bloom: changed-path Bloom filters v2 (& sundries)
From: Junio C Hamano @ 2023-10-18 23:26 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <cover.1697653929.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> (Rebased onto the tip of 'master', which is 3a06386e31 (The fifteenth
> batch, 2023-10-04), at the time of writing).
Judging from 17/17 that has a free_commit_graph() call in
close_commit_graph(), that was merged in the eighteenth batch,
the above is probably untrue. I'll apply to the current master and
see how it goes instead.
> Thanks to Jonathan, Peff, and SZEDER who have helped a great deal in
> assembling these patches. As usual, a range-diff is included below.
> Thanks in advance for your
> review!
Thanks.
^ permalink raw reply
* [PATCH] am: align placeholder for --whitespace option with apply
From: Junio C Hamano @ 2023-10-18 23:35 UTC (permalink / raw)
To: git
`git am` passes the value given to its `--whitespace` option through
to the underlying `git apply`, and the value is called <action> over
there. Fix the documentation for the command that calls the value
<option> to say <action> instead.
Note that the option help given by `git am -h` already calls the
value <action>, so there is no need to make a matching change there.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-am.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git i/Documentation/git-am.txt w/Documentation/git-am.txt
index 0c1dfb3c98..de3d5dde99 100644
--- i/Documentation/git-am.txt
+++ w/Documentation/git-am.txt
@@ -12,7 +12,7 @@ SYNOPSIS
'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8] [--no-verify]
[--[no-]3way] [--interactive] [--committer-date-is-author-date]
[--ignore-date] [--ignore-space-change | --ignore-whitespace]
- [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
+ [--whitespace=<action>] [-C<n>] [-p<n>] [--directory=<dir>]
[--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
[--quoted-cr=<action>]
@@ -116,7 +116,7 @@ include::rerere-options.txt[]
--ignore-space-change::
--ignore-whitespace::
---whitespace=<option>::
+--whitespace=<action>::
-C<n>::
-p<n>::
--directory=<dir>::
^ permalink raw reply related
* What's cooking in git.git (Oct 2023, #07; Wed, 18)
From: Junio C Hamano @ 2023-10-18 23:36 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
There are too many topics marked with "Needs review" label. I can
help reviewing some of them myself, but obviously it will not scale.
For now, I'll let unreviewed new topics accumulating in the mailing
list archive without picking them up in 'seen' to encourage folks
help reduce the review backlog.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* jc/merge-ort-attr-index-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at b139b87502)
+ merge-ort: initialize repo in index state
Fix "git merge-tree" to stop segfaulting when the --attr-source
option is used.
source: <pull.1583.v3.git.git.1696857660374.gitgitgadget@gmail.com>
* sn/cat-file-doc-update (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at 6719ba7bd4)
+ doc/cat-file: make synopsis and description less confusing
"git cat-file" documentation updates.
source: <20231009175604.2361700-1-stepnem@smrk.net>
* tb/repack-max-cruft-size (2023-10-09) 5 commits
(merged to 'next' on 2023-10-09 at 38f039e880)
+ repack: free existing_cruft array after use
(merged to 'next' on 2023-10-06 at b3ca6df3b9)
+ builtin/repack.c: avoid making cruft packs preferred
+ builtin/repack.c: implement support for `--max-cruft-size`
+ builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
+ t7700: split cruft-related tests to t7704
"git repack" learned "--max-cruft-size" to prevent cruft packs from
growing without bounds.
source: <cover.1696293862.git.me@ttaylorr.com>
source: <20231007172031.GA1524950@coredump.intra.peff.net>
source: <035393935108d02aaf8927189b05102f4f74f340.1696370003.git.me@ttaylorr.com>
* xz/commit-title-soft-limit-doc (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at 0bf3d9ed51)
+ doc: correct the 50 characters soft limit (+)
Doc update.
source: <pull.1585.v2.git.git.1696778367151.gitgitgadget@gmail.com>
--------------------------------------------------
[New Topics]
* jc/commit-new-underscore-index-fix (2023-10-17) 1 commit
- commit: do not use cryptic "new_index" in end-user facing messages
source: <xmqqo7gwvd8c.fsf_-_@gitster.g>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/pathspec-error-wo-repository-fix (2023-10-17) 1 commit
- grep: die gracefully when outside repository
source: <087c92e3904dd774f672373727c300bf7f5f6369.1697317276.git.code@khaugsbakk.name>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
source: <cover.1697319294.git.code@khaugsbakk.name>
* ni/die-message-fix-for-git-add (2023-10-17) 1 commit
- builtin/add.c: clean up die() messages
source: <20231017113946.747-1-naomi.ibeh69@gmail.com>
* ps/git-repack-doc-fixes (2023-10-16) 2 commits
- doc/git-repack: don't mention nonexistent "--unpacked" option
- doc/git-repack: fix syntax for `-g` shorthand option
source: <cover.1697440686.git.ps@pks.im>
* tb/merge-tree-write-pack (2023-10-17) 7 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: factor our `finalize_checkpoint()`
- bulk-checkin: factor out `truncate_checkpoint()`
- bulk-checkin: factor out `prepare_checkpoint()`
- bulk-checkin: factor out `format_object_header_hash()`
source: <cover.1697560266.git.me@ttaylorr.com>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* tb/format-pack-doc-update (2023-10-12) 2 commits
- Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
- Documentation/gitformat-pack.txt: fix typo
Doc update.
Expecting a reroll.
cf. <xmqq5y3b4id2.fsf@gitster.g>
source: <cover.1697144959.git.me@ttaylorr.com>
* ps/do-not-trust-commit-graph-blindly-for-existence (2023-10-13) 1 commit
- commit: detect commits that exist in commit-graph but not in the ODB
(this branch is used by kn/rev-list-missing-fix.)
The codepath to traverse the commit-graph learned to notice that a
commit is missing (e.g., corrupt repository lost an object), even
though it knows something about the commit (like its parents) from
what is in commit-graph.
Comments?
source: <b0bf576c51a706367a758b8e30eca37edb9c2734.1697200576.git.ps@pks.im>
* tb/pair-chunk-expect-size (2023-10-14) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- midx: read `OIDF` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDF` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
(this branch uses jk/chunk-bounds.)
Code clean-up for jk/chunk-bounds topic.
Comments?
source: <45cac29403e63483951f7766c6da3c022c68d9f0.1697225110.git.me@ttaylorr.com>
source: <cover.1697225110.git.me@ttaylorr.com>
* jc/fail-stash-to-store-non-stash (2023-10-11) 1 commit
(merged to 'next' on 2023-10-16 at e26db57315)
+ stash: be careful what we store
Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.
Will merge to 'master'.
source: <xmqqbkd4lwj0.fsf_-_@gitster.g>
* bc/racy-4gb-files (2023-10-13) 2 commits
(merged to 'next' on 2023-10-16 at c60962dfee)
+ Prevent git from rehashing 4GiB files
+ t: add a test helper to truncate files
The index file has room only for lower 32-bit of the file size in
the cached stat information, which means cached stat information
will have 0 in its sd_size member for a file whose size is multiple
of 4GiB. This is mistaken for a racily clean path. Avoid it by
storing a bogus sd_size value instead for such files.
Will merge to 'master'.
source: <20231012160930.330618-1-sandals@crustytoothpaste.net>
* jc/grep-f-relative-to-cwd (2023-10-12) 1 commit
- grep: -f <path> is relative to $cwd
"cd sub && git grep -f patterns" tried to read "patterns" file at
the top level of the working tree; it has been corrected to read
"sub/patterns" instead.
Needs review.
source: <xmqqedhzg37z.fsf@gitster.g>
* tb/path-filter-fix (2023-10-10) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1696969994.git.me@ttaylorr.com>
* ak/pretty-decorate-more-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-12 at 3cbb4c2268)
+ pretty: fix ref filtering for %(decorate) formats
Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
not auto-initialize the decoration subsystem, which has been
corrected.
Will merge to 'master'.
source: <20231008202307.1568477-1-andy.koppe@gmail.com>
* en/docfixes (2023-10-09) 25 commits
(merged to 'next' on 2023-10-17 at 1e3cdeb427)
+ documentation: add missing parenthesis
+ documentation: add missing quotes
+ documentation: add missing fullstops
+ documentation: add some commas where they are helpful
+ documentation: fix whitespace issues
+ documentation: fix capitalization
+ documentation: fix punctuation
+ documentation: use clearer prepositions
+ documentation: add missing hyphens
+ documentation: remove unnecessary hyphens
+ documentation: add missing article
+ documentation: fix choice of article
+ documentation: whitespace is already generally plural
+ documentation: fix singular vs. plural
+ documentation: fix verb vs. noun
+ documentation: fix adjective vs. noun
+ documentation: fix verb tense
+ documentation: employ consistent verb tense for a list
+ documentation: fix subject/verb agreement
+ documentation: remove extraneous words
+ documentation: add missing words
+ documentation: fix apostrophe usage
+ documentation: fix typos
+ documentation: fix small error
+ documentation: wording improvements
Documentation typo and grammo fixes.
Will merge to 'master'.
source: <pull.1595.git.1696747527.gitgitgadget@gmail.com>
* kn/rev-list-missing-fix (2023-10-16) 4 commits
. rev-list: add commit object support in `--missing` option
. rev-list: move `show_commit()` to the bottom
. revision: rename bit to `do_not_die_on_missing_objects`
. Merge branch 'ps/do-not-trust-commit-graph-blindly-for-existence' into kn/rev-list-missing-fix
(this branch uses ps/do-not-trust-commit-graph-blindly-for-existence.)
"git rev-list --missing" did not work for missing commit objects,
which has been corrected.
cf. <xmqqbkcyo7ro.fsf@gitster.g>
source: <20231016103830.56486-1-karthik.188@gmail.com>
* jk/chunk-bounds (2023-10-14) 21 commits
(merged to 'next' on 2023-10-16 at 68c9e37980)
+ t5319: make corrupted large-offset test more robust
(merged to 'next' on 2023-10-10 at 21139603ce)
+ chunk-format: drop pair_chunk_unsafe()
+ commit-graph: detect out-of-order BIDX offsets
+ commit-graph: check bounds when accessing BIDX chunk
+ commit-graph: check bounds when accessing BDAT chunk
+ commit-graph: bounds-check generation overflow chunk
+ commit-graph: check size of generations chunk
+ commit-graph: bounds-check base graphs chunk
+ commit-graph: detect out-of-bounds extra-edges pointers
+ commit-graph: check size of commit data chunk
+ midx: check size of revindex chunk
+ midx: bounds-check large offset chunk
+ midx: check size of object offset chunk
+ midx: enforce chunk alignment on reading
+ midx: check size of pack names chunk
+ commit-graph: check consistency of fanout table
+ midx: check size of oid lookup chunk
+ commit-graph: check size of oid fanout chunk
+ midx: stop ignoring malformed oid fanout chunk
+ t: add library for munging chunk-format files
+ chunk-format: note that pair_chunk() is unsafe
(this branch is used by tb/pair-chunk-expect-size.)
The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.
Will merge to 'master'.
source: <20231009205544.GA3281950@coredump.intra.peff.net>
* ps/rewritten-is-per-worktree-doc (2023-10-10) 1 commit
(merged to 'next' on 2023-10-12 at 501b960e8c)
+ doc/git-worktree: mention "refs/rewritten" as per-worktree refs
Doc update.
Will merge to 'master'.
source: <985ac850eb6e60ae76601acc8bfbcd56f99348b4.1696935657.git.ps@pks.im>
* sn/typo-grammo-phraso-fixes (2023-10-05) 5 commits
(merged to 'next' on 2023-10-18 at 575d767f9a)
+ t/README: fix multi-prerequisite example
+ doc/gitk: s/sticked/stuck/
+ git-jump: admit to passing merge mode args to ls-files
+ doc/diff-options: improve wording of the log.diffMerges mention
+ doc: fix some typos, grammar and wording issues
Many typos, ungrammatical sentences and wrong phrasing have been
fixed.
Will merge to 'master'.
source: <20231003082107.3002173-1-stepnem@smrk.net>
* so/diff-merges-dd (2023-10-09) 3 commits
(merged to 'next' on 2023-10-16 at 71b5e29625)
+ completion: complete '--dd'
+ diff-merges: introduce '--dd' option
+ diff-merges: improve --diff-merges documentation
"git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".
Will merge to 'master'.
source: <20231009160535.236523-1-sorganov@gmail.com>
* vd/loose-ref-iteration-optimization (2023-10-09) 4 commits
(merged to 'next' on 2023-10-12 at 99e2f83855)
+ files-backend.c: avoid stat in 'loose_fill_ref_dir'
+ dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
+ dir.[ch]: expose 'get_dtype'
+ ref-cache.c: fix prefix matching in ref iteration
The code to iterate over loose references have been optimized to
reduce the number of lstat() system calls.
Will merge to 'master'.
source: <pull.1594.v2.git.1696888736.gitgitgadget@gmail.com>
* jc/update-list-references-to-lore (2023-10-06) 1 commit
- doc: update list archive reference to use lore.kernel.org
Doc update.
Needs review.
source: <xmqq7cnz741s.fsf@gitster.g>
* cc/git-replay (2023-10-10) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
Needs (hopefully final and quick) review.
source: <20231010123847.2777056-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-03) 1 commit
- decorate: add color.decorate.symbols config option
A new config for coloring.
Needs review.
source: <20231003205442.22963-1-andy.koppe@gmail.com>
* jc/attr-tree-config (2023-10-13) 2 commits
- attr: add attr.tree for setting the treeish to read attributes from
- attr: read attributes from HEAD when bare repo
The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.
Will merge to 'next'?
source: <pull.1577.v5.git.git.1697218770.gitgitgadget@gmail.com>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-09-26) 4 commits
- trailer: only use trailer_block_* variables if trailers were found
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Needs review.
source: <pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* ty/merge-tree-strategy-options (2023-10-11) 2 commits
(merged to 'next' on 2023-10-12 at 9b873601df)
+ merge: introduce {copy|clear}_merge_options()
(merged to 'next' on 2023-09-29 at aa65b54416)
+ merge-tree: add -X strategy option
"git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.
Will merge to 'master'.
source: <pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* rs/parse-options-value-int (2023-09-18) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
Not ready yet.
cf. <4014e490-c6c1-453d-b0ed-645220e3e614@web.de>
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* js/doc-unit-tests (2023-10-16) 5 commits
- fixup! ci: run unit tests in CI
- fixup! unit tests: add TAP unit test framework
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Expecting a (hopefully final and minor) reroll.
cf. <20231016134421.21659-1-phillip.wood123@gmail.com>
source: <cover.1696889529.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-10-16) 7 commits
- cmake: handle also unit tests
- cmake: use test names instead of full paths
- cmake: fix typo in variable name
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Needs review.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/doc-unit-tests-fixup (2023-10-11) 1 commit
. SQUASH???
(this branch uses js/doc-unit-tests and js/doc-unit-tests-with-cmake.)
Quick fix for jc/doc-unit-tests topic to unbreak CI running on 'seen'.
Superseded by a fixup! in the base series.
source: <xmqqwmvskf8t.fsf@gitster.g>
^ permalink raw reply
* Re: [PATCH 00/11] t: reduce direct disk access to data structures
From: Junio C Hamano @ 2023-10-18 23:40 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <ZS9vp9dnoVVUsMIt@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> As usual, I forgot to mention that this applies on top of current
> master, which is at a9ecda2788 (The eighteenth batch, 2023-10-13) at the
> time of writing. I look forward to the day where I remember to include
> this information in the cover letter...
I heard rumors that we have a --base option that records such a
piece of information in the format-patch output (I haven't used it
myself). Would it have helped, or perhaps it still needs some
usability tweak?
^ permalink raw reply
* Re: [PATCH v3 05/17] t/helper/test-read-graph.c: extract `dump_graph_info()`
From: Junio C Hamano @ 2023-10-18 23:56 UTC (permalink / raw)
To: Taylor Blau
Cc: Patrick Steinhardt, git, Jonathan Tan, Jeff King,
SZEDER Gábor
In-Reply-To: <ZTAXyMWeQLRWcpuu@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
> On Tue, Oct 17, 2023 at 10:45:18AM +0200, Patrick Steinhardt wrote:
>> On Tue, Oct 10, 2023 at 04:33:33PM -0400, Taylor Blau wrote:
>> > Prepare for the 'read-graph' test helper to perform other tasks besides
>> > dumping high-level information about the commit-graph by extracting its
>> > main routine into a separate function.
>> >
>> > Signed-off-by: Taylor Blau <me@ttaylorr.com>
>> > Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
>> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> > Signed-off-by: Taylor Blau <me@ttaylorr.com>
>>
>> Nit: your signoff is duplicated here. This is also still the case for
>> some of the other commits.
>
> Yeah, this is an artifact of having tossed these patches back and forth
> (originally Jonathan sent some of these, then I sent another round, then
> Jonathan, now me again). It's a little verbose, but accurately tracks
> the DCO across multiple rounds.
Well, between you and Jonathan Tan, that might be true, but my
involvement in the chain is only that I happened to have looked at
an earlier round and without doing anything else. Even if you
refetched from my tree and based the new round on that version, it
would have been the same if you started from what you sent out
earlier without even looking at my tree. So I do not think that
chain of DCO adds very little value by recording my sign-off there.
^ permalink raw reply
* Re: [Outreachy] [PATCH v2] t/t7601: use "test_path_is_file"etc. instead of "test -f"
From: Junio C Hamano @ 2023-10-18 23:59 UTC (permalink / raw)
To: Dorcas AnonoLitunya; +Cc: christian.couder, git
In-Reply-To: <20231018124538.68549-2-anonolitunya@gmail.com>
Dorcas AnonoLitunya <anonolitunya@gmail.com> writes:
> Some tests in t7601 use "test -f" and "test ! -f" to see if a path
> exists or is missing.
>
> Use test_path_is_file and test_path_is_missing helper functions to
> clarify these tests a bit better. This especially matters for the
> "missing" case because "test ! -f F" will be happy if "F" exists as a
> directory, but the intent of the test is that "F" should not exist, even
> as a directory. The updated code expresses this better.
>
> Signed-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>
> ---
> t/t7601-merge-pull-config.sh | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
Nicely done; it seems that the title line got garbled whitespace for
some reason but I'll fix them up while queuing.
Thanks. Will queue.
> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> index bd238d89b0..e08767df66 100755
> --- a/t/t7601-merge-pull-config.sh
> +++ b/t/t7601-merge-pull-config.sh
> @@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '
>
> test_expect_success 'merge c1 with c2' '
> git reset --hard c1 &&
> - test -f c0.c &&
> - test -f c1.c &&
> - test ! -f c2.c &&
> - test ! -f c3.c &&
> + test_path_is_file c0.c &&
> + test_path_is_file c1.c &&
> + test_path_is_missing c2.c &&
> + test_path_is_missing c3.c &&
> git merge c2 &&
> - test -f c1.c &&
> - test -f c2.c
> + test_path_is_file c1.c &&
> + test_path_is_file c2.c
> '
>
> test_expect_success 'fast-forward pull succeeds with "true" in pull.ff' '
> @@ -411,8 +411,8 @@ test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
> git reset --hard c1 &&
> git config pull.twohead ours &&
> git merge c2 &&
> - test -f c1.c &&
> - ! test -f c2.c
> + test_path_is_file c1.c &&
> + test_path_is_missing c2.c
> '
>
> test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
> @@ -431,10 +431,10 @@ test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octo
> test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
> git diff --exit-code &&
> - test -f c0.c &&
> - test -f c1.c &&
> - test -f c2.c &&
> - test -f c3.c
> + test_path_is_file c0.c &&
> + test_path_is_file c1.c &&
> + test_path_is_file c2.c &&
> + test_path_is_file c3.c
> '
>
> conflict_count()
^ permalink raw reply
* Re: [PATCH] doc: update list archive reference to use lore.kernel.org
From: Dragan Simic @ 2023-10-19 0:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7cnz741s.fsf@gitster.g>
On 2023-10-07 00:57, Junio C Hamano wrote:
> No disrespect to other mailing list archives, but the local part of
> their URLs will become pretty much meaningless once the archives go
> out of service, and we learned the lesson hard way when $gmane
> stopped serving.
>
> Let's point into https://lore.kernel.org/ for an article that can be
> found there, because the local part of the URL has the Message-Id:
> that can be used to find the same message in other archives, even if
> lore goes down.
You asked for this patch to be reviewed, so I did it. It looks
perfectly fine to me.
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> Documentation/CodingGuidelines | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/CodingGuidelines
> b/Documentation/CodingGuidelines
> index 65af8d82ce..71afc5b259 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -24,7 +24,7 @@ code. For Git in general, a few rough rules are:
>
> "Once it _is_ in the tree, it's not really worth the patch noise to
> go and fix it up."
> - Cf. http://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html
> + Cf.
> https://lore.kernel.org/all/20100126160632.3bdbe172.akpm@linux-foundation.org/
>
> - Log messages to explain your changes are as important as the
> changes themselves. Clearly written code and in-code comments
^ permalink raw reply
* [PATCH v2] git-p4 shouldn't attempt to store symlinks in LFS
From: Matthew McClain @ 2023-10-19 0:25 UTC (permalink / raw)
To: git; +Cc: sandals, gitster, Matthew McClain
In-Reply-To: <ZTBbqobbqQpxHPI2@tapette.crustytoothpaste.net>
git-p4.py would attempt to put a symlink in LFS if its file extension
matched git-p4.largeFileExtensions.
Git LFS doesn't store symlinks because smudge/clean filters don't handle
symlinks. They never get passed to the filter process nor the
smudge/clean filters, nor could that occur without a change to the
protocol or command-line interface. Unless Git learned how to send them
to the filters, Git LFS would have a hard time using them in any useful
way.
Git LFS's goal is to move large files out of the repository history, and
symlinks are functionally limited to 4 KiB or a similar size on most
systems.
Signed-off-by: Matthew McClain <mmcclain@noprivs.com>
---
git-p4.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index d26a980e5a..0eb3bb4c47 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1522,6 +1522,10 @@ def processContent(self, git_mode, relPath, contents):
file is stored in the large file system and handles all necessary
steps.
"""
+ # symlinks aren't processed by smudge/clean filters
+ if git_mode == "120000":
+ return (git_mode, contents)
+
if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath):
contentTempFile = self.generateTempFile(contents)
pointer_git_mode, contents, localLargeFile = self.generatePointer(contentTempFile)
--
2.39.3
^ permalink raw reply related
* Re: Is there any interest in localizing term delimiters in git messages?
From: Jiang Xin @ 2023-10-19 5:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexander Shopov, Git List, jmas, alexhenrie24, ralf.thielow,
matthias.ruester, phillip.szelat, vyruss, christopher.diaz.riv,
jn.avila, flashcode, bagasdotme,
Ævar Arnfjörð Bjarmason, alessandro.menti,
elongbug, cwryu, uneedsihyeon, arek_koz, dacs.git,
insolor@gmail.com, peter, bitigchi, ark, kate,
vnwildman@gmail.com, pclouds, dyroneteng@gmail.com,
oldsharp@gmail.com, lilydjwg@gmail.com, me, pan93412@gmail.com,
franklin@goodhorse.idv.tw
In-Reply-To: <xmqqwmvkve83.fsf@gitster.g>
On Wed, Oct 18, 2023 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > Starting with the release of git 2.34.0 two years ago, we had a new
> > l10n pipeline and the git-po-helper tool as part of our l10n workflow.
> > The first version of git-po-helper introduced a validator to protect
> > git command parameters and variable names in megid.
>
> Ahh, that is the piece I was missing. I didn't know you guys are
> doing extra checks that could trigger false positives.
>
> > E.g. In pull
> > request 541 (https://github.com/git-l10n/git-po/pull/541), a
> > mismatched variable name "new_index" was reported in bg.po as below:
> >
> > level=warning msg="mismatch variable names in msgstr: new_index"
> > level=warning msg=">> msgid: unable to write new_index file"
> > level=warning msg=">> msgstr: новият индекс не може да бъде записан"
> >
> > And po/bg.po changed as below:
> >
> > msgid "unable to write new_index file"
> > msgstr "новият индекс (new_index) не може да бъде записан"
>
> Wait. Is this supposed to be a good example of validator working
> well? We use this exact message three times in builtin/commit.c; is
> the validator insisting on the translated message to have verbatim
> string "new_index" in it so that the end-users will see it?
>
> I may still be confused, but if that is what is going on, I think it
> is a wrong validation in this particular case. I can understand if
> we were creating say .git/new_index file and it helps the end users
> to diagnose a troubled repository by running "ls .git" to see if a
> file called "new_index" exists and getting in the way, but I do not
> think it is the case. A new file ".git/index.lock" is created via
> repo_hold_locked_index() and I do not think it helps the end-user to
> know that we may be calling it "new_index" internally among the
> developers' circle. If the message were about "index.lock", it
> might be a different story, but such an error would probably have
> been issued long before write_locked_index() gets called.
>
> I'd suggest doing s/new_index/new index/ to msgid string for these
> anyway.
I tried to find similar patterns in `po/bg.po` using:
$ git grep -h -B5 '([a-zA-Z_\.]*_[a-zA-Z_\.]\+)' po/bg.po
And find other translated variable names in Bulgarian as follows:
* cookie_result in builtin/fsmonitor--daemon.c:
error(_("fsmonitor: cookie_result '%d' != SEEN"),
* run_command in builtin/submodule--helper.c:
die(_("run_command returned non-zero status for %s\n."),
die(_("run_command returned non-zero status while "
* crlf_action in convert.c:
warning(_("illegal crlf_action %d"), (int)crlf_action);
* lazy_dir in name-hash.c:
die(_("unable to create lazy_dir thread: %s"),
* lazy_name in name-hash.c:
die(_("unable to create lazy_name thread: %s"),
die(_("unable to join lazy_name thread: %s"),
* load_cache_entries in read-cache.c:
die(_("unable to create load_cache_entries thread: %s"),
die(_("unable to join load_cache_entries thread: %s"),
* load_index_extensions in read-cache.c:
die(_("unable to create load_index_extensions thread: %s"),
die(_("unable to join load_index_extensions thread: %s"),
Apart from "new_index", it seems that none of the above sentences can
be rewritten simply by removing the underscores in variable names
without breaking the grammar, and I suppose it would be better to keep
those variable names unchanged.
^ permalink raw reply
* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-19 6:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Karthik Nayak, Taylor Blau
In-Reply-To: <xmqqjzrlhzci.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]
On Tue, Oct 17, 2023 at 11:34:53AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Fair point indeed. The following is a worst-case scenario benchmark of
> > of the change where we do a full topological walk of all reachable
> > commits in the graph, executed in linux.git. We parse commit parents via
> > `repo_parse_commit_gently()`, so the new code path now basically has to
> > check for object existence of every reachable commit:
> > ...
> > The added check does lead to a performance regression indeed, which is
> > not all that unexpected. That being said, the commit-graph still results
> > in a significant speedup compared to the case where we don't have it.
>
> Yeah, I agree that both points are expected. An extra check that is
> much cheaper than the full parsing is paying a small price to be a
> bit more careful than before. The question is if the price is small
> enough. I am still not sure if the extra carefulness is warranted
> for all normal cases to spend 30% extra cycles.
>
> Thanks.
Well, seen in contexts like the thread that spawned this discussion I
think it's preferable to take a relatively smaller price compared to
disabling the commit graph entirely in some situations. With that in
mind, personally I'd be happy with either of two outcomes here:
- We take the patch I proposed as a hardening measure, which allows
us to use the commit graph in basically any situation where we
could parse objects from the ODB without any downside except for a
performance hit.
- We say that corrupt repositories do not need to be accounted for
when using metadata caches like the commit-graph. That would mean
in consequence that for the `--missing` flag we would not have to
disable commit graphs.
The test failure that was exposed in Karthik's test only stems from the
fact that we manually corrupt the repository and is not specific to the
`--missing` flag. This is further stressed by the new test aded in my
own patch, as we can trigger a similar bug when not using `--missing`.
For end users, object pruning would happen either via git-gc(1) or via
git-maintenance(1), and both of them do know to update commit graphs
already. This should significantly decrease the chance that such stale
commit graphs would be found out there in the wild, but it's still not
impossible of course. Especially in cases where you use lower-level
commands like git-repack(1) with cruft packs or git-prune(1), which is
often the case for Git hosters, the risk is higher though.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [Outreachy][PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken June Ibizugbe @ 2023-10-19 7:41 UTC (permalink / raw)
To: git; +Cc: rjusto, christian.couder, gitster, Isoken June Ibizugbe
Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
---
builtin/branch.c | 66 ++++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ec190b14a..938c40bfaa 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
(head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
if (merged)
warning(_("deleting branch '%s' that has been merged to\n"
- " '%s', but not yet merged to HEAD."),
+ " '%s', but not yet merged to HEAD"),
name, reference_name);
else
warning(_("not deleting branch '%s' that is not yet merged to\n"
- " '%s', even though it is merged to HEAD."),
+ " '%s', even though it is merged to HEAD"),
name, reference_name);
}
free(reference_name_to_free);
@@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
if (!force && !rev) {
- error(_("Couldn't look up commit object for '%s'"), refname);
+ error(_("couldn't look up commit object for '%s'"), refname);
return -1;
}
if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("The branch '%s' is not fully merged.\n"
+ error(_("the branch '%s' is not fully merged.\n"
"If you are sure you want to delete it, "
- "run 'git branch -D %s'."), branchname, branchname);
+ "run 'git branch -D %s'"), branchname, branchname);
return -1;
}
return 0;
@@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "branch.%s", branchname);
if (git_config_rename_section(buf.buf, NULL) < 0)
- warning(_("Update of config-file failed"));
+ warning(_("update of config-file failed"));
strbuf_release(&buf);
}
@@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
if (kinds == FILTER_REFS_BRANCHES) {
const char *path;
if ((path = branch_checked_out(name))) {
- error(_("Cannot delete branch '%s' "
+ error(_("cannot delete branch '%s' "
"used by worktree at '%s'"),
bname.buf, path);
ret = 1;
@@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
&oid, &flags);
if (!target) {
if (remote_branch) {
- error(_("remote-tracking branch '%s' not found."), bname.buf);
+ error(_("remote-tracking branch '%s' not found"), bname.buf);
} else {
char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
char *virtual_target = resolve_refdup(virtual_name,
@@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
"Did you forget --remote?"),
bname.buf);
else
- error(_("branch '%s' not found."), bname.buf);
+ error(_("branch '%s' not found"), bname.buf);
FREE_AND_NULL(virtual_target);
}
ret = 1;
@@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
continue;
if (is_worktree_being_rebased(wt, target))
- die(_("Branch %s is being rebased at %s"),
+ die(_("branch %s is being rebased at %s"),
target, wt->path);
if (is_worktree_being_bisected(wt, target))
- die(_("Branch %s is being bisected at %s"),
+ die(_("branch %s is being bisected at %s"),
target, wt->path);
}
}
@@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (ref_exists(oldref.buf))
recovery = 1;
else
- die(_("Invalid branch name: '%s'"), oldname);
+ die(_("invalid branch name: '%s'"), oldname);
}
for (int i = 0; worktrees[i]; i++) {
@@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
if (oldref_usage & IS_HEAD)
- die(_("No commit on branch '%s' yet."), oldname);
+ die(_("no commit on branch '%s' yet"), oldname);
else
- die(_("No branch named '%s'."), oldname);
+ die(_("no branch named '%s'"), oldname);
}
/*
@@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (!copy && !(oldref_usage & IS_ORPHAN) &&
rename_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch rename failed"));
+ die(_("branch rename failed"));
if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch copy failed"));
+ die(_("branch copy failed"));
if (recovery) {
if (copy)
- warning(_("Created a copy of a misnamed branch '%s'"),
+ warning(_("created a copy of a misnamed branch '%s'"),
interpreted_oldname);
else
- warning(_("Renamed a misnamed branch '%s' away"),
+ warning(_("renamed a misnamed branch '%s' away"),
interpreted_oldname);
}
if (!copy && (oldref_usage & IS_HEAD) &&
replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
logmsg.buf))
- die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+ die(_("branch renamed to %s, but HEAD is not updated"), newname);
strbuf_release(&logmsg);
strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
strbuf_addf(&newsection, "branch.%s", interpreted_newname);
if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is renamed, but update of config-file failed"));
+ die(_("branch is renamed, but update of config-file failed"));
if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is copied, but update of config-file failed"));
+ die(_("branch is copied, but update of config-file failed"));
strbuf_release(&oldref);
strbuf_release(&newref);
strbuf_release(&oldsection);
@@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
head = resolve_refdup("HEAD", 0, &head_oid, NULL);
if (!head)
- die(_("Failed to resolve HEAD as a valid ref."));
+ die(_("failed to resolve HEAD as a valid ref"));
if (!strcmp(head, "HEAD"))
filter.detached = 1;
else if (!skip_prefix(head, "refs/heads/", &head))
@@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc) {
if (filter.detached)
- die(_("Cannot give description to detached HEAD"));
+ die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
@@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf))
error((!argc || branch_checked_out(branch_ref.buf))
- ? _("No commit on branch '%s' yet.")
- : _("No branch named '%s'."),
+ ? _("no commit on branch '%s' yet")
+ : _("no branch named '%s'"),
branch_name);
else if (!edit_branch_description(branch_name))
ret = 0; /* happy */
@@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc)
die(_("branch name required"));
else if ((argc == 1) && filter.detached)
- die(copy? _("cannot copy the current branch while not on any.")
- : _("cannot rename the current branch while not on any."));
+ die(copy? _("cannot copy the current branch while not on any")
+ : _("cannot rename the current branch while not on any"));
else if (argc == 1)
copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
else if (argc == 2)
@@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not set upstream of HEAD to %s when "
- "it does not point to any branch."),
+ "it does not point to any branch"),
new_upstream);
die(_("no such branch '%s'"), argv[0]);
}
if (!ref_exists(branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
- die(_("No commit on branch '%s' yet."), branch->name);
+ die(_("no commit on branch '%s' yet"), branch->name);
die(_("branch '%s' does not exist"), branch->name);
}
@@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not unset upstream of HEAD when "
- "it does not point to any branch."));
+ "it does not point to any branch"));
die(_("no such branch '%s'"), argv[0]);
}
if (!branch_has_merge_config(branch))
- die(_("Branch '%s' has no upstream information"), branch->name);
+ die(_("branch '%s' has no upstream information"), branch->name);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", branch->name);
@@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *start_name = argc == 2 ? argv[1] : head;
if (filter.kind != FILTER_REFS_BRANCHES)
- die(_("The -a, and -r, options to 'git branch' do not take a branch name\n"
+ die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
"Did you mean to use: -a|-r --list <pattern>?"));
if (track == BRANCH_TRACK_OVERRIDE)
- die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
+ die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
if (recurse_submodules) {
create_branches_recursively(the_repository, branch_name,
--
2.42.0.346.g24618a8a3e.dirty
^ permalink raw reply related
* Re: [Outreachy][PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken Ibizugbe @ 2023-10-19 7:48 UTC (permalink / raw)
To: git; +Cc: rjusto, christian.couder, gitster
In-Reply-To: <20231019074126.567638-1-isokenjune@gmail.com>
On Thu, Oct 19, 2023 at 8:41 AM Isoken June Ibizugbe
<isokenjune@gmail.com> wrote:
>
> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
> ---
> builtin/branch.c | 66 ++++++++++++++++++++++++------------------------
> 1 file changed, 33 insertions(+), 33 deletions(-)
please ignore this patch. It didn't include the changes for the test files.
thanks
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 2ec190b14a..938c40bfaa 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
> (head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
> if (merged)
> warning(_("deleting branch '%s' that has been merged to\n"
> - " '%s', but not yet merged to HEAD."),
> + " '%s', but not yet merged to HEAD"),
> name, reference_name);
> else
> warning(_("not deleting branch '%s' that is not yet merged to\n"
> - " '%s', even though it is merged to HEAD."),
> + " '%s', even though it is merged to HEAD"),
> name, reference_name);
> }
> free(reference_name_to_free);
> @@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
> {
> struct commit *rev = lookup_commit_reference(the_repository, oid);
> if (!force && !rev) {
> - error(_("Couldn't look up commit object for '%s'"), refname);
> + error(_("couldn't look up commit object for '%s'"), refname);
> return -1;
> }
> if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
> - error(_("The branch '%s' is not fully merged.\n"
> + error(_("the branch '%s' is not fully merged.\n"
> "If you are sure you want to delete it, "
> - "run 'git branch -D %s'."), branchname, branchname);
> + "run 'git branch -D %s'"), branchname, branchname);
> return -1;
> }
> return 0;
> @@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
> struct strbuf buf = STRBUF_INIT;
> strbuf_addf(&buf, "branch.%s", branchname);
> if (git_config_rename_section(buf.buf, NULL) < 0)
> - warning(_("Update of config-file failed"));
> + warning(_("update of config-file failed"));
> strbuf_release(&buf);
> }
>
> @@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
> if (kinds == FILTER_REFS_BRANCHES) {
> const char *path;
> if ((path = branch_checked_out(name))) {
> - error(_("Cannot delete branch '%s' "
> + error(_("cannot delete branch '%s' "
> "used by worktree at '%s'"),
> bname.buf, path);
> ret = 1;
> @@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
> &oid, &flags);
> if (!target) {
> if (remote_branch) {
> - error(_("remote-tracking branch '%s' not found."), bname.buf);
> + error(_("remote-tracking branch '%s' not found"), bname.buf);
> } else {
> char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
> char *virtual_target = resolve_refdup(virtual_name,
> @@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
> "Did you forget --remote?"),
> bname.buf);
> else
> - error(_("branch '%s' not found."), bname.buf);
> + error(_("branch '%s' not found"), bname.buf);
> FREE_AND_NULL(virtual_target);
> }
> ret = 1;
> @@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
> continue;
>
> if (is_worktree_being_rebased(wt, target))
> - die(_("Branch %s is being rebased at %s"),
> + die(_("branch %s is being rebased at %s"),
> target, wt->path);
>
> if (is_worktree_being_bisected(wt, target))
> - die(_("Branch %s is being bisected at %s"),
> + die(_("branch %s is being bisected at %s"),
> target, wt->path);
> }
> }
> @@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
> if (ref_exists(oldref.buf))
> recovery = 1;
> else
> - die(_("Invalid branch name: '%s'"), oldname);
> + die(_("invalid branch name: '%s'"), oldname);
> }
>
> for (int i = 0; worktrees[i]; i++) {
> @@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>
> if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
> if (oldref_usage & IS_HEAD)
> - die(_("No commit on branch '%s' yet."), oldname);
> + die(_("no commit on branch '%s' yet"), oldname);
> else
> - die(_("No branch named '%s'."), oldname);
> + die(_("no branch named '%s'"), oldname);
> }
>
> /*
> @@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>
> if (!copy && !(oldref_usage & IS_ORPHAN) &&
> rename_ref(oldref.buf, newref.buf, logmsg.buf))
> - die(_("Branch rename failed"));
> + die(_("branch rename failed"));
> if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
> - die(_("Branch copy failed"));
> + die(_("branch copy failed"));
>
> if (recovery) {
> if (copy)
> - warning(_("Created a copy of a misnamed branch '%s'"),
> + warning(_("created a copy of a misnamed branch '%s'"),
> interpreted_oldname);
> else
> - warning(_("Renamed a misnamed branch '%s' away"),
> + warning(_("renamed a misnamed branch '%s' away"),
> interpreted_oldname);
> }
>
> if (!copy && (oldref_usage & IS_HEAD) &&
> replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
> logmsg.buf))
> - die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
> + die(_("branch renamed to %s, but HEAD is not updated"), newname);
>
> strbuf_release(&logmsg);
>
> strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
> strbuf_addf(&newsection, "branch.%s", interpreted_newname);
> if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
> - die(_("Branch is renamed, but update of config-file failed"));
> + die(_("branch is renamed, but update of config-file failed"));
> if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
> - die(_("Branch is copied, but update of config-file failed"));
> + die(_("branch is copied, but update of config-file failed"));
> strbuf_release(&oldref);
> strbuf_release(&newref);
> strbuf_release(&oldsection);
> @@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
> head = resolve_refdup("HEAD", 0, &head_oid, NULL);
> if (!head)
> - die(_("Failed to resolve HEAD as a valid ref."));
> + die(_("failed to resolve HEAD as a valid ref"));
> if (!strcmp(head, "HEAD"))
> filter.detached = 1;
> else if (!skip_prefix(head, "refs/heads/", &head))
> @@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
> if (!argc) {
> if (filter.detached)
> - die(_("Cannot give description to detached HEAD"));
> + die(_("cannot give description to detached HEAD"));
> branch_name = head;
> } else if (argc == 1) {
> strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
> @@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
> if (!ref_exists(branch_ref.buf))
> error((!argc || branch_checked_out(branch_ref.buf))
> - ? _("No commit on branch '%s' yet.")
> - : _("No branch named '%s'."),
> + ? _("no commit on branch '%s' yet")
> + : _("no branch named '%s'"),
> branch_name);
> else if (!edit_branch_description(branch_name))
> ret = 0; /* happy */
> @@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if (!argc)
> die(_("branch name required"));
> else if ((argc == 1) && filter.detached)
> - die(copy? _("cannot copy the current branch while not on any.")
> - : _("cannot rename the current branch while not on any."));
> + die(copy? _("cannot copy the current branch while not on any")
> + : _("cannot rename the current branch while not on any"));
> else if (argc == 1)
> copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
> else if (argc == 2)
> @@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if (!branch) {
> if (!argc || !strcmp(argv[0], "HEAD"))
> die(_("could not set upstream of HEAD to %s when "
> - "it does not point to any branch."),
> + "it does not point to any branch"),
> new_upstream);
> die(_("no such branch '%s'"), argv[0]);
> }
>
> if (!ref_exists(branch->refname)) {
> if (!argc || branch_checked_out(branch->refname))
> - die(_("No commit on branch '%s' yet."), branch->name);
> + die(_("no commit on branch '%s' yet"), branch->name);
> die(_("branch '%s' does not exist"), branch->name);
> }
>
> @@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> if (!branch) {
> if (!argc || !strcmp(argv[0], "HEAD"))
> die(_("could not unset upstream of HEAD when "
> - "it does not point to any branch."));
> + "it does not point to any branch"));
> die(_("no such branch '%s'"), argv[0]);
> }
>
> if (!branch_has_merge_config(branch))
> - die(_("Branch '%s' has no upstream information"), branch->name);
> + die(_("branch '%s' has no upstream information"), branch->name);
>
> strbuf_reset(&buf);
> strbuf_addf(&buf, "branch.%s.remote", branch->name);
> @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> const char *start_name = argc == 2 ? argv[1] : head;
>
> if (filter.kind != FILTER_REFS_BRANCHES)
> - die(_("The -a, and -r, options to 'git branch' do not take a branch name\n"
> + die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
> "Did you mean to use: -a|-r --list <pattern>?"));
>
> if (track == BRANCH_TRACK_OVERRIDE)
> - die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
> + die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
>
> if (recurse_submodules) {
> create_branches_recursively(the_repository, branch_name,
> --
> 2.42.0.346.g24618a8a3e.dirty
>
^ permalink raw reply
* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-19 8:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Karthik Nayak, Taylor Blau
In-Reply-To: <ZTDQjangLsQ1cSJl@tanuki>
[-- Attachment #1: Type: text/plain, Size: 2654 bytes --]
On Thu, Oct 19, 2023 at 08:45:34AM +0200, Patrick Steinhardt wrote:
> On Tue, Oct 17, 2023 at 11:34:53AM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > Fair point indeed. The following is a worst-case scenario benchmark of
> > > of the change where we do a full topological walk of all reachable
> > > commits in the graph, executed in linux.git. We parse commit parents via
> > > `repo_parse_commit_gently()`, so the new code path now basically has to
> > > check for object existence of every reachable commit:
> > > ...
> > > The added check does lead to a performance regression indeed, which is
> > > not all that unexpected. That being said, the commit-graph still results
> > > in a significant speedup compared to the case where we don't have it.
> >
> > Yeah, I agree that both points are expected. An extra check that is
> > much cheaper than the full parsing is paying a small price to be a
> > bit more careful than before. The question is if the price is small
> > enough. I am still not sure if the extra carefulness is warranted
> > for all normal cases to spend 30% extra cycles.
> >
> > Thanks.
>
> Well, seen in contexts like the thread that spawned this discussion I
> think it's preferable to take a relatively smaller price compared to
> disabling the commit graph entirely in some situations. With that in
> mind, personally I'd be happy with either of two outcomes here:
>
> - We take the patch I proposed as a hardening measure, which allows
> us to use the commit graph in basically any situation where we
> could parse objects from the ODB without any downside except for a
> performance hit.
>
> - We say that corrupt repositories do not need to be accounted for
> when using metadata caches like the commit-graph. That would mean
> in consequence that for the `--missing` flag we would not have to
> disable commit graphs.
>
> The test failure that was exposed in Karthik's test only stems from the
> fact that we manually corrupt the repository and is not specific to the
> `--missing` flag. This is further stressed by the new test aded in my
> own patch, as we can trigger a similar bug when not using `--missing`.
There's another way to handle this, which is to conditionally enable the
object existence check. This would be less of a performance hit compared
to disabling commit graphs altogether with `--missing`, but still ensure
that repository corruption was detected. Second, it would not regress
performance for all preexisting users of `repo_parse_commit_gently()`.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken June Ibizugbe @ 2023-10-19 8:40 UTC (permalink / raw)
To: git; +Cc: Isoken June Ibizugbe, rjusto, christian.couder, gitster
In-Reply-To: <e08b2ec4-786a-4c18-b7af-0a6a250ae0f0@gmail.com>
As per the CodingGuidelines document, it is recommended that a single-line
message provided to error messages such as die(), error() and warning(),
should start with a lowercase letter and should not end with a period.
Also this patch fixes the tests broken by the changes.
Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
---
builtin/branch.c | 66 +++++++++++++++++++--------------------
t/t2407-worktree-heads.sh | 2 +-
t/t3200-branch.sh | 16 +++++-----
t/t3202-show-branch.sh | 10 +++---
4 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ec190b14a..e7ee9bd0f1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
(head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
if (merged)
warning(_("deleting branch '%s' that has been merged to\n"
- " '%s', but not yet merged to HEAD."),
+ " '%s', but not yet merged to HEAD"),
name, reference_name);
else
warning(_("not deleting branch '%s' that is not yet merged to\n"
- " '%s', even though it is merged to HEAD."),
+ " '%s', even though it is merged to HEAD"),
name, reference_name);
}
free(reference_name_to_free);
@@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
if (!force && !rev) {
- error(_("Couldn't look up commit object for '%s'"), refname);
+ error(_("couldn't look up commit object for '%s'"), refname);
return -1;
}
if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("The branch '%s' is not fully merged.\n"
+ error(_("the branch '%s' is not fully merged.\n"
"If you are sure you want to delete it, "
- "run 'git branch -D %s'."), branchname, branchname);
+ "run 'git branch -D %s'"), branchname, branchname);
return -1;
}
return 0;
@@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "branch.%s", branchname);
if (git_config_rename_section(buf.buf, NULL) < 0)
- warning(_("Update of config-file failed"));
+ warning(_("update of config-file failed"));
strbuf_release(&buf);
}
@@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
if (kinds == FILTER_REFS_BRANCHES) {
const char *path;
if ((path = branch_checked_out(name))) {
- error(_("Cannot delete branch '%s' "
+ error(_("cannot delete branch '%s' "
"used by worktree at '%s'"),
bname.buf, path);
ret = 1;
@@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
&oid, &flags);
if (!target) {
if (remote_branch) {
- error(_("remote-tracking branch '%s' not found."), bname.buf);
+ error(_("remote-tracking branch '%s' not found"), bname.buf);
} else {
char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
char *virtual_target = resolve_refdup(virtual_name,
@@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
"Did you forget --remote?"),
bname.buf);
else
- error(_("branch '%s' not found."), bname.buf);
+ error(_("branch '%s' not found"), bname.buf);
FREE_AND_NULL(virtual_target);
}
ret = 1;
@@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
continue;
if (is_worktree_being_rebased(wt, target))
- die(_("Branch %s is being rebased at %s"),
+ die(_("branch %s is being rebased at %s"),
target, wt->path);
if (is_worktree_being_bisected(wt, target))
- die(_("Branch %s is being bisected at %s"),
+ die(_("branch %s is being bisected at %s"),
target, wt->path);
}
}
@@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (ref_exists(oldref.buf))
recovery = 1;
else
- die(_("Invalid branch name: '%s'"), oldname);
+ die(_("invalid branch name: '%s'"), oldname);
}
for (int i = 0; worktrees[i]; i++) {
@@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
if (oldref_usage & IS_HEAD)
- die(_("No commit on branch '%s' yet."), oldname);
+ die(_("no commit on branch '%s' yet"), oldname);
else
- die(_("No branch named '%s'."), oldname);
+ die(_("no branch named '%s'"), oldname);
}
/*
@@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
if (!copy && !(oldref_usage & IS_ORPHAN) &&
rename_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch rename failed"));
+ die(_("branch rename failed"));
if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
- die(_("Branch copy failed"));
+ die(_("branch copy failed"));
if (recovery) {
if (copy)
- warning(_("Created a copy of a misnamed branch '%s'"),
+ warning(_("created a copy of a misnamed branch '%s'"),
interpreted_oldname);
else
- warning(_("Renamed a misnamed branch '%s' away"),
+ warning(_("renamed a misnamed branch '%s' away"),
interpreted_oldname);
}
if (!copy && (oldref_usage & IS_HEAD) &&
replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
logmsg.buf))
- die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+ die(_("branch renamed to %s, but HEAD is not updated"), newname);
strbuf_release(&logmsg);
strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
strbuf_addf(&newsection, "branch.%s", interpreted_newname);
if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is renamed, but update of config-file failed"));
+ die(_("branch is renamed, but update of config-file failed"));
if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
- die(_("Branch is copied, but update of config-file failed"));
+ die(_("branch is copied, but update of config-file failed"));
strbuf_release(&oldref);
strbuf_release(&newref);
strbuf_release(&oldsection);
@@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
head = resolve_refdup("HEAD", 0, &head_oid, NULL);
if (!head)
- die(_("Failed to resolve HEAD as a valid ref."));
+ die(_("failed to resolve HEAD as a valid ref"));
if (!strcmp(head, "HEAD"))
filter.detached = 1;
else if (!skip_prefix(head, "refs/heads/", &head))
@@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc) {
if (filter.detached)
- die(_("Cannot give description to detached HEAD"));
+ die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
@@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf))
error((!argc || branch_checked_out(branch_ref.buf))
- ? _("No commit on branch '%s' yet.")
- : _("No branch named '%s'."),
+ ? _("no commit on branch '%s' yet")
+ : _("no branch named '%s'"),
branch_name);
else if (!edit_branch_description(branch_name))
ret = 0; /* happy */
@@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!argc)
die(_("branch name required"));
else if ((argc == 1) && filter.detached)
- die(copy? _("cannot copy the current branch while not on any.")
- : _("cannot rename the current branch while not on any."));
+ die(copy? _("cannot copy the current branch while not on any")
+ : _("cannot rename the current branch while not on any"));
else if (argc == 1)
copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
else if (argc == 2)
@@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not set upstream of HEAD to %s when "
- "it does not point to any branch."),
+ "it does not point to any branch"),
new_upstream);
die(_("no such branch '%s'"), argv[0]);
}
if (!ref_exists(branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
- die(_("No commit on branch '%s' yet."), branch->name);
+ die(_("no commit on branch '%s' yet"), branch->name);
die(_("branch '%s' does not exist"), branch->name);
}
@@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!branch) {
if (!argc || !strcmp(argv[0], "HEAD"))
die(_("could not unset upstream of HEAD when "
- "it does not point to any branch."));
+ "it does not point to any branch"));
die(_("no such branch '%s'"), argv[0]);
}
if (!branch_has_merge_config(branch))
- die(_("Branch '%s' has no upstream information"), branch->name);
+ die(_("branch '%s' has no upstream information"), branch->name);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", branch->name);
@@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
const char *start_name = argc == 2 ? argv[1] : head;
if (filter.kind != FILTER_REFS_BRANCHES)
- die(_("The -a, and -r, options to 'git branch' do not take a branch name.\n"
+ die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
"Did you mean to use: -a|-r --list <pattern>?"));
if (track == BRANCH_TRACK_OVERRIDE)
- die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
+ die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
if (recurse_submodules) {
create_branches_recursively(the_repository, branch_name,
diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
index 469443d8ae..f6835c91dc 100755
--- a/t/t2407-worktree-heads.sh
+++ b/t/t2407-worktree-heads.sh
@@ -45,7 +45,7 @@ test_expect_success 'refuse to overwrite: checked out in worktree' '
grep "cannot force update the branch" err &&
test_must_fail git branch -D wt-$i 2>err &&
- grep "Cannot delete branch" err || return 1
+ grep "cannot delete branch" err || return 1
done
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 080e4f24a6..3182abde27 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -291,10 +291,10 @@ test_expect_success 'git branch -M topic topic should work when main is checked
test_expect_success 'git branch -M and -C fail on detached HEAD' '
git checkout HEAD^{} &&
test_when_finished git checkout - &&
- echo "fatal: cannot rename the current branch while not on any." >expect &&
+ echo "fatal: cannot rename the current branch while not on any" >expect &&
test_must_fail git branch -M must-fail 2>err &&
test_cmp expect err &&
- echo "fatal: cannot copy the current branch while not on any." >expect &&
+ echo "fatal: cannot copy the current branch while not on any" >expect &&
test_must_fail git branch -C must-fail 2>err &&
test_cmp expect err
'
@@ -943,7 +943,7 @@ test_expect_success 'deleting currently checked out branch fails' '
git worktree add -b my7 my7 &&
test_must_fail git -C my7 branch -d my7 &&
test_must_fail git branch -d my7 2>actual &&
- grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
+ grep "^error: cannot delete branch .my7. used by worktree at " actual &&
rm -r my7 &&
git worktree prune
'
@@ -954,7 +954,7 @@ test_expect_success 'deleting in-use branch fails' '
git -C my7 bisect start HEAD HEAD~2 &&
test_must_fail git -C my7 branch -d my7 &&
test_must_fail git branch -d my7 2>actual &&
- grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
+ grep "^error: cannot delete branch .my7. used by worktree at " actual &&
rm -r my7 &&
git worktree prune
'
@@ -1024,7 +1024,7 @@ test_expect_success '--set-upstream-to fails on multiple branches' '
test_expect_success '--set-upstream-to fails on detached HEAD' '
git checkout HEAD^{} &&
test_when_finished git checkout - &&
- echo "fatal: could not set upstream of HEAD to main when it does not point to any branch." >expect &&
+ echo "fatal: could not set upstream of HEAD to main when it does not point to any branch" >expect &&
test_must_fail git branch --set-upstream-to main 2>err &&
test_cmp expect err
'
@@ -1072,7 +1072,7 @@ test_expect_success 'use --set-upstream-to modify a particular branch' '
'
test_expect_success '--unset-upstream should fail if given a non-existent branch' '
- echo "fatal: Branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
+ echo "fatal: branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
test_must_fail git branch --unset-upstream i-dont-exist 2>err &&
test_cmp expect err
'
@@ -1094,7 +1094,7 @@ test_expect_success 'test --unset-upstream on HEAD' '
test_must_fail git config branch.main.remote &&
test_must_fail git config branch.main.merge &&
# fail for a branch without upstream set
- echo "fatal: Branch '"'"'main'"'"' has no upstream information" >expect &&
+ echo "fatal: branch '"'"'main'"'"' has no upstream information" >expect &&
test_must_fail git branch --unset-upstream 2>err &&
test_cmp expect err
'
@@ -1108,7 +1108,7 @@ test_expect_success '--unset-upstream should fail on multiple branches' '
test_expect_success '--unset-upstream should fail on detached HEAD' '
git checkout HEAD^{} &&
test_when_finished git checkout - &&
- echo "fatal: could not unset upstream of HEAD when it does not point to any branch." >expect &&
+ echo "fatal: could not unset upstream of HEAD when it does not point to any branch" >expect &&
test_must_fail git branch --unset-upstream 2>err &&
test_cmp expect err
'
diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index b17f388f56..2cdb834b37 100755
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -10,7 +10,7 @@ GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
test_expect_success 'error descriptions on empty repository' '
current=$(git branch --show-current) &&
cat >expect <<-EOF &&
- error: No commit on branch '\''$current'\'' yet.
+ error: no commit on branch '\''$current'\'' yet
EOF
test_must_fail git branch --edit-description 2>actual &&
test_cmp expect actual &&
@@ -21,7 +21,7 @@ test_expect_success 'error descriptions on empty repository' '
test_expect_success 'fatal descriptions on empty repository' '
current=$(git branch --show-current) &&
cat >expect <<-EOF &&
- fatal: No commit on branch '\''$current'\'' yet.
+ fatal: no commit on branch '\''$current'\'' yet
EOF
test_must_fail git branch --set-upstream-to=non-existent 2>actual &&
test_cmp expect actual &&
@@ -224,7 +224,7 @@ done
test_expect_success 'error descriptions on non-existent branch' '
cat >expect <<-EOF &&
- error: No branch named '\''non-existent'\'.'
+ error: no branch named '\''non-existent'\''
EOF
test_must_fail git branch --edit-description non-existent 2>actual &&
test_cmp expect actual
@@ -238,7 +238,7 @@ test_expect_success 'fatal descriptions on non-existent branch' '
test_cmp expect actual &&
cat >expect <<-EOF &&
- fatal: No branch named '\''non-existent'\''.
+ fatal: no branch named '\''non-existent'\''
EOF
test_must_fail git branch -c non-existent new-branch 2>actual &&
test_cmp expect actual &&
@@ -253,7 +253,7 @@ test_expect_success 'error descriptions on orphan branch' '
test_branch_op_in_wt() {
test_orphan_error() {
test_must_fail git $* 2>actual &&
- test_i18ngrep "No commit on branch .orphan-branch. yet.$" actual
+ test_i18ngrep "no commit on branch .orphan-branch. yet$" actual
} &&
test_orphan_error -C wt branch $1 $2 && # implicit branch
test_orphan_error -C wt branch $1 orphan-branch $2 && # explicit branch
--
2.42.0.346.g24618a8a3e.dirty
^ permalink raw reply related
* [RFC][Outreachy] Seeking Git Community Feedback on My Application
From: Isoken Ibizugbe @ 2023-10-19 9:25 UTC (permalink / raw)
To: git, Christian Couder
Dear Git Community and Mentors,
I hope you're doing well. I'm excited to share my application draft
for the Outreachy program with the Git project. Your feedback is
invaluable, and I'm eager to align the project with the community's
needs. Please review the attached draft and share your insights.
Thank you for your support.
Project Application
----
About Me:
My name is Isoken June Ibizugbe, my language is primarily English, and
I am a resident of Nigeria. I am a student at an online coding school
called African Leadership Xcelerator (ALX), participating in the
software engineering program.
What project am I applying for?
Moving Existing Tests to a Unit Testing Framework
Why am I interested in working with the Git chosen project?
Git has been a cornerstone for software development, enabling
developers worldwide to collaborate, innovate, and create exceptional
software. I would say without Git, my journey to pursuing my software
engineering career would be impossible, as I use it almost every day.
Yet, in this constantly evolving landscape, there is always room for
improvement, even in a well-established project. The Git project
currently relies on end-to-end tests, and this is where I see an
opportunity to make a profound impact. Being able to test libraries in
isolation via unit tests or mocks speeds up determining the root cause
of bugs. I am deeply passionate about contributing to this project and
firmly believe in the power of open-source software and the collective
intelligence of the community. A successful completion of this project
will significantly improve Git's testing capabilities and bring the
benefits of fewer errors, faster work and better testing for all
parts.
My motivation for joining the Git community stemmed from my desire to
immerse myself in the world of open-source software and, ultimately,
to become a part of the Outreachy program. My time spent contributing
to Git has been nothing short of transformative. It has been a
remarkable learning experience that has introduced me to a new form of
collaboration using a mailing list and contributing through patches
rather than the typical Pull Request (PR). This collaborative
atmosphere has been pivotal in my growth as a developer, and I am
eager to continue this journey, making meaningful contributions to
this remarkable open-source project.
Contributions to Git
I have actively participated in Git's mailing list discussions and
contributed to a micro-project;
- builtin/branch.c: Adjust error messages such as die(), error(), and
warning() messages used in branch, to conform to coding guidelines
(https://lore.kernel.org/git/20231019084052.567922-1-isokenjune@gmail.com/)
- Implemented changes to fix broken tests based on reviews from the
community (https://lore.kernel.org/git/20231019084052.567922-1-isokenjune@gmail.com/)
- In review.
Project Goals:
- Improve Testing Efficiency: Transitioning from end-to-end tests to
unit tests will enable more efficient testing of error conditions.
- Codebase Stability: Unit tests enhance code stability and facilitate
easier debugging through isolation.
- Simplify Testing: Writing unit tests in pure C simplifies test
setup, data passing, and reduces testing runtime by eliminating
separate processes for each test.
Project Milestones:
- Add useful tests of library-like code
- Integrate with stdlib work
- Run alongside regular make test target
Project Timeline:
1. Oct 2 - Nov 20: Community Bonding
- Understanding the structure of Git
- Getting familiar with the code
2. Dec 4 - Jan 15: Add useful tests of library-like code
- Identify and document the current state of the tests in the Git
t/helper directory.
- Confirm the licensing and compatibility requirements for the chosen
unit testing framework.
- Develop unit tests for these library-like components.
- Execute the tests and ensure they cover various scenarios, including
error conditions.
- Run the tests and address any initial issues or bugs to ensure they
work as intended.
- Document the new tests and their coverage.
- Seek feedback and support from mentors and the Git community
3. Jan 15 - Feb 15: Integrate with Stdlib Work
- Collaborate with the team working on standard library integration.
- Ensure that the tests for library-like code align with stdlib work.
- Verify that the tests effectively check the compatibility and
interaction of the code with standard libraries.
- Gather feedback and insights from the Git community on the
integrated tests, addressing any concerns or suggestions.
4. Feb 15 - March 1: Run Alongside Regular 'make test' Target and finalize
- Configure the testing framework to run alongside the regular 'make
test' target.
- Ensure that the new tests are included in the standard testing suite.
- Execute 'make test' with the new tests and verify that they pass successfully.
- Document the integration process and how the new tests are included
in the standard testing procedure.
- Perform comprehensive testing of the entire unit testing framework.
- Ensure all migrated tests are working correctly within the new framework.
- Document the entire process of migrating Git's tests
- Prepare a final project report
Technical Requirements
According to the documentation on the unit test project
(https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc),
the suggested best framework for the Git project is the "Custom TAP
framework" (Phillip Wood's TAP implementation), as it aligns with
Git's licensing requirements, is vendorable, and can be customized by
Git's developers as needed, but it may require some additional
development work for features like parallel execution and mock
support, but it offers a strong foundation for unit testing within the
Git project.
Relevant Projects
Simple shell - A project based on emulating a shell. It was a
collaborative project which we managed using Git.
(https://github.com/Junie06/simple_shell).
This project was written in C, which allowed me to apply my C language
knowledge, essential for Git projects.
I'm proficient in using Shell for scripting, redirections, and
permissions, as shown in my work
(https://github.com/Junie06/alx-system_engineering-devops).
Creating the simple shell project deepened my understanding of how
shells work, and I even attempted to replicate a shell environment.
Collaborating on the Simple Shell project reinforced my Git skills.
^ permalink raw reply
* pygit2 claims repository does not exist on GIT_DIR_INVALID_OWNERSHIP
From: Michal Suchánek @ 2023-10-19 9:33 UTC (permalink / raw)
To: git
Hello,
after upgrade from git 2.26 to git 2.35 pygit would claim that my
repository does not exist:
:~> git ls-remote /srv/git/kernel-source.git | head -n3
41037b9c54949ab7df9d32e8bc753c059b27c66c HEAD
7a68c4c0c640ac07b89722271f866287b9047459 refs/heads/ALP-current
4993d1b0a96a0fa7fb0e87d3b1725bc775162283 refs/heads/ALP-current-RT
:~> python3
Python 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pygit2
>>> pygit2.Repository("/srv/git/kernel-source.git")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib64/python3.6/site-packages/pygit2/repository.py", line 1498, in __init__
path_backend = init_file_backend(path, flags)
_pygit2.GitError: Repository not found at /srv/git/kernel-source.git
>>>
Could a reasonable diagnostic be provided?
Thanks
Michal
^ permalink raw reply
* Re: [PATCH 00/11] t: reduce direct disk access to data structures
From: Han-Wen Nienhuys @ 2023-10-19 10:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, git
In-Reply-To: <xmqqbkcwuetq.fsf@gitster.g>
On Wed, Oct 18, 2023 at 5:32 PM Junio C Hamano <gitster@pobox.com> wrote:
> > this patch series refactors a bunch of our tests to perform less direct
> > disk access to on-disk data structures. Instead, the tests are converted
> > to use Git tools or our test-tool to access data to the best extent
> > possible. This serves two benefits:
>
> Laudable goal.
>
> > - We increase test coverage of our own code base.
>
> Meaning the new code added to test-tool for this series will also
> get tested and bugs spotted?
>
> > - We become less dependent on the actual on-disk format.
>
> Yes, this is very desirable. Without looking at the implementation,
> I see some issues aiming for this goal may involve. [a] using the
> production code for validation would mean our expectation to be
> compared to the reality to be validated can be affected by the same
> bug, making two wrongs to appear right; [b] using a separate
> implementation used only for validation would at least mean we will
> have to make the same mistake in unique part of both implementations
> that is less likely to miss bugs compared to [a], but bugs in shared
> part of the production code and validation code will be hidden the
> same way as [a].
I think it would be really great if there were separate unittests for
the ref backend API. Some of the reftable work was needlessly
difficult because the contract of the API was underspecified. The API
is well compartmentalized in refs-internal.h, and a lot of the API
behavior can be tested as a black box, eg.
* setup symref HEAD pointing to R1
* setup transaction updating ref R1 from C1 to C2
* commit transaction, check that it succeeds
* read ref R1, check if it is C2
* read reflog for R1, see that it has a C1 => C2 update
* read reflog for HEAD, see that it has a C1 => C2 update
Tests for the loose/packed backend could directly mess with the
on-disk files to test failure scenarios.
With unittests like that, the tests can zoom in on the functionality
of the ref backend, and provide more convenient coverage for
dynamic/static analysis.
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Liana Sebastian
^ permalink raw reply
* Re: pygit2 claims repository does not exist on GIT_DIR_INVALID_OWNERSHIP
From: Konstantin Khomoutov @ 2023-10-19 10:10 UTC (permalink / raw)
To: git
In-Reply-To: <20231019093344.GS6241@kitsune.suse.cz>
On Thu, Oct 19, 2023 at 11:33:44AM +0200, Michal Suchánek wrote:
> after upgrade from git 2.26 to git 2.35 pygit would claim that my
> repository does not exist:
>
> :~> git ls-remote /srv/git/kernel-source.git | head -n3
> 41037b9c54949ab7df9d32e8bc753c059b27c66c HEAD
> 7a68c4c0c640ac07b89722271f866287b9047459 refs/heads/ALP-current
> 4993d1b0a96a0fa7fb0e87d3b1725bc775162283 refs/heads/ALP-current-RT
> :~> python3
> Python 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import pygit2
> >>> pygit2.Repository("/srv/git/kernel-source.git")
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> File "/usr/lib64/python3.6/site-packages/pygit2/repository.py", line 1498, in __init__
> path_backend = init_file_backend(path, flags)
> _pygit2.GitError: Repository not found at /srv/git/kernel-source.git
> >>>
>
> Could a reasonable diagnostic be provided?
It's a bit hard to ask what exactly you are after.
If you meant more diagnistics from Git, then I think they are in place, but
pygit2 does not display them (that is, it reads and possibly parses them
itself).
You can possibly run your Python code while having at least GIT_TRACE=1 in the
environment (see the output of `git help git` for the various environment
variables having the word "TRACE" in their names for ways to get more copious
outputs) and see whether you'll be able to get that trace printed - Git prints
which command it calls and with which arguments; you will then be able to call
them by hand and see exactly how they fail.
If you ask about better diagnistics in pygit2, then this question is
misdirected, I'm afraid, as pygit2 is not being developed using this mailing
list - ask in Discussions or issues tracker at [1] instead.
1. https://github.com/libgit2/pygit2
^ permalink raw reply
* Re: pygit2 claims repository does not exist on GIT_DIR_INVALID_OWNERSHIP
From: Michal Suchánek @ 2023-10-19 12:07 UTC (permalink / raw)
To: git
In-Reply-To: <20231019093344.GS6241@kitsune.suse.cz>
On Thu, Oct 19, 2023 at 11:33:44AM +0200, Michal Suchánek wrote:
> Hello,
>
> after upgrade from git 2.26 to git 2.35 pygit would claim that my
> repository does not exist:
>
> :~> git ls-remote /srv/git/kernel-source.git | head -n3
> 41037b9c54949ab7df9d32e8bc753c059b27c66c HEAD
> 7a68c4c0c640ac07b89722271f866287b9047459 refs/heads/ALP-current
> 4993d1b0a96a0fa7fb0e87d3b1725bc775162283 refs/heads/ALP-current-RT
> :~> python3
> Python 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import pygit2
> >>> pygit2.Repository("/srv/git/kernel-source.git")
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> File "/usr/lib64/python3.6/site-packages/pygit2/repository.py", line 1498, in __init__
> path_backend = init_file_backend(path, flags)
> _pygit2.GitError: Repository not found at /srv/git/kernel-source.git
> >>>
>
> Could a reasonable diagnostic be provided?
It turns out that I have relatively recent python3-pygit2 but it's
linked against ancient libgit2 causing this error.
Rebuilding python3-pygit2 fixes the problem.
Sorry about the noise.
Thanks
Michal
^ permalink raw reply
* [PATCH v3 0/3] rev-list: add support for commits in `--missing`
From: Karthik Nayak @ 2023-10-19 12:10 UTC (permalink / raw)
To: karthik.188; +Cc: git, gitster, ps
In-Reply-To: <20231016103830.56486-1-karthik.188@gmail.com>
The `--missing` option in git-rev-list(1) was introduced intitally
to deal with missing blobs in the context of promissory notes.
Eventually the option was extended to also support tree objects in
7c0fe330d5 (rev-list: handle missing tree objects properly,2018-10-05).
This patch series extends the `--missing` option to also add support for
commit objects. We do this by introducing a new flag `MISSING` which is
added whenever we encounter a missing commit during traversal. Then in
`builtin/rev-list` we check for this flag and take the appropriate
action based on the `--missing=*` option used.
This series is an alternate to the patch series I had posted earlier:
https://lore.kernel.org/git/20230908174208.249184-1-karthik.188@gmail.com/.
In that patch, we introduced an option `--ignore-missing-links` which
was added to expose the `ignore_missing_links` bit to the user. The
issue in that patch was that, the option `--ignore-missing-links` didn't
play well the pre-existing `--missing` option. This series avoids that
route and just extends the `--missing` option for commits to solve the
same problem.
V2 of the series can be found here: http://public-inbox.org/git/ZSkCGS3JPEQ71dOF@tanuki/T/#m10b562ec02834d0b222835a24a18e6ca725f99d1
Changes from v2:
* Change the flag MISSING's bit number to avoid collision with NEEDS_BITMAP
which was causing t5310 and t532 to fail.
Range diff from v2:
1: a7018a319a = 1: fa35cd21e6 revision: rename bit to `do_not_die_on_missing_objects`
2: b186a679d4 = 2: 31074df376 rev-list: move `show_commit()` to the bottom
3: 0c8c140c27 ! 3: d9b0d6da0c rev-list: add commit object support in `--missing` option
@@ object.h: void object_array_init(struct object_array *array);
/*
* object flag allocation:
- * revision.h: 0---------10 15 23------27
-+ * revision.h: 0---------10 15 22------27
++ * revision.h: 0---------10 15 22------28
* fetch-pack.c: 01 67
* negotiator/default.c: 2--5
* walker.c: 0-2
+@@ object.h: void object_array_init(struct object_array *array);
+ * builtin/show-branch.c: 0-------------------------------------------26
+ * builtin/unpack-objects.c: 2021
+ */
+-#define FLAG_BITS 28
++#define FLAG_BITS 29
+
+ #define TYPE_BITS 3
+
## revision.c ##
@@ revision.c: static int process_parents(struct rev_info *revs, struct commit *commit,
@@ revision.c: static int process_parents(struct rev_info *revs, struct commit *com
unsigned pass_flags;
- if (commit->object.flags & ADDED)
-+ if (commit->object.flags & ADDED || commit->object.flags & MISSING)
++ if (commit->object.flags & (ADDED | MISSING))
return 0;
commit->object.flags |= ADDED;
@@ revision.c: static int process_parents(struct rev_info *revs, struct commit *com
## revision.h ##
@@
- /* WARNING: This is also used as REACHABLE in commit-graph.c. */
- #define PULL_MERGE (1u<<15)
+ #define ANCESTRY_PATH (1u<<27)
+ #define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | PULL_MERGE)
-+/* WARNING: This is also used as NEEDS_BITMAP in pack-bitmap.c. */
-+#define MISSING (1u<<22)
++#define MISSING (1u<<28)
+
- #define TOPO_WALK_EXPLORED (1u<<23)
- #define TOPO_WALK_INDEGREE (1u<<24)
+ #define DECORATE_SHORT_REFS 1
+ #define DECORATE_FULL_REFS 2
## t/t6022-rev-list-missing.sh (new) ##
Karthik Nayak (3):
revision: rename bit to `do_not_die_on_missing_objects`
rev-list: move `show_commit()` to the bottom
rev-list: add commit object support in `--missing` option
builtin/reflog.c | 2 +-
builtin/rev-list.c | 93 +++++++++++++++++++------------------
list-objects.c | 4 +-
object.h | 4 +-
revision.c | 11 +++--
revision.h | 19 ++++----
t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++
7 files changed, 147 insertions(+), 60 deletions(-)
create mode 100755 t/t6022-rev-list-missing.sh
--
2.42.0
^ permalink raw reply
* [PATCH v3 1/3] revision: rename bit to `do_not_die_on_missing_objects`
From: Karthik Nayak @ 2023-10-19 12:10 UTC (permalink / raw)
To: karthik.188; +Cc: git, gitster, ps
In-Reply-To: <20231019121024.194317-1-karthik.188@gmail.com>
The bit `do_not_die_on_missing_tree` is used in revision.h to ensure the
revision walker does not die when encountering a missing tree. This is
currently exclusively set within `builtin/rev-list.c` to ensure the
`--missing` option works with missing trees.
In the upcoming commits, we will extend `--missing` to also support
missing commits. So let's rename the bit to
`do_not_die_on_missing_objects`, which is object type agnostic and can
be used for both trees/commits.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/reflog.c | 2 +-
builtin/rev-list.c | 2 +-
list-objects.c | 2 +-
revision.h | 17 +++++++++--------
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index df63a5892e..9e369a5977 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -298,7 +298,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
struct rev_info revs;
repo_init_revisions(the_repository, &revs, prefix);
- revs.do_not_die_on_missing_tree = 1;
+ revs.do_not_die_on_missing_objects = 1;
revs.ignore_missing = 1;
revs.ignore_missing_links = 1;
if (verbose)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ff715d6918..ea77489c38 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -561,7 +561,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
}
if (arg_missing_action)
- revs.do_not_die_on_missing_tree = 1;
+ revs.do_not_die_on_missing_objects = 1;
argc = setup_revisions(argc, argv, &revs, &s_r_opt);
diff --git a/list-objects.c b/list-objects.c
index c25c72b32c..47296dff2f 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -177,7 +177,7 @@ static void process_tree(struct traversal_context *ctx,
is_promisor_object(&obj->oid))
return;
- if (!revs->do_not_die_on_missing_tree)
+ if (!revs->do_not_die_on_missing_objects)
die("bad tree object %s", oid_to_hex(&obj->oid));
}
diff --git a/revision.h b/revision.h
index 50091bbd13..c73c92ef40 100644
--- a/revision.h
+++ b/revision.h
@@ -212,18 +212,19 @@ struct rev_info {
/*
* Blobs are shown without regard for their existence.
- * But not so for trees: unless exclude_promisor_objects
+ * But not so for trees/commits: unless exclude_promisor_objects
* is set and the tree in question is a promisor object;
* OR ignore_missing_links is set, the revision walker
- * dies with a "bad tree object HASH" message when
- * encountering a missing tree. For callers that can
- * handle missing trees and want them to be filterable
+ * dies with a "bad <type> object HASH" message when
+ * encountering a missing object. For callers that can
+ * handle missing trees/commits and want them to be filterable
* and showable, set this to true. The revision walker
- * will filter and show such a missing tree as usual,
- * but will not attempt to recurse into this tree
- * object.
+ * will filter and show such a missing object as usual,
+ * but will not attempt to recurse into this tree/commit
+ * object. The revision walker will also set the MISSING
+ * flag for such objects.
*/
- do_not_die_on_missing_tree:1,
+ do_not_die_on_missing_objects:1,
/* for internal use only */
exclude_promisor_objects:1;
--
2.42.0
^ permalink raw reply related
* [PATCH v3 2/3] rev-list: move `show_commit()` to the bottom
From: Karthik Nayak @ 2023-10-19 12:10 UTC (permalink / raw)
To: karthik.188; +Cc: git, gitster, ps
In-Reply-To: <20231019121024.194317-1-karthik.188@gmail.com>
The `show_commit()` function already depends on `finish_commit()`, and
in the upcoming commit, we'll also add a dependency on
`finish_object__ma()`. Since in C symbols must be declared before
they're used, let's move `show_commit()` below both `finish_commit()`
and `finish_object__ma()`, so the code is cleaner as a whole without the
need for declarations.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/rev-list.c | 85 +++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 43 deletions(-)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ea77489c38..98542e8b3c 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -100,7 +100,48 @@ static off_t get_object_disk_usage(struct object *obj)
return size;
}
-static void finish_commit(struct commit *commit);
+static inline void finish_object__ma(struct object *obj)
+{
+ /*
+ * Whether or not we try to dynamically fetch missing objects
+ * from the server, we currently DO NOT have the object. We
+ * can either print, allow (ignore), or conditionally allow
+ * (ignore) them.
+ */
+ switch (arg_missing_action) {
+ case MA_ERROR:
+ die("missing %s object '%s'",
+ type_name(obj->type), oid_to_hex(&obj->oid));
+ return;
+
+ case MA_ALLOW_ANY:
+ return;
+
+ case MA_PRINT:
+ oidset_insert(&missing_objects, &obj->oid);
+ return;
+
+ case MA_ALLOW_PROMISOR:
+ if (is_promisor_object(&obj->oid))
+ return;
+ die("unexpected missing %s object '%s'",
+ type_name(obj->type), oid_to_hex(&obj->oid));
+ return;
+
+ default:
+ BUG("unhandled missing_action");
+ return;
+ }
+}
+
+static void finish_commit(struct commit *commit)
+{
+ free_commit_list(commit->parents);
+ commit->parents = NULL;
+ free_commit_buffer(the_repository->parsed_objects,
+ commit);
+}
+
static void show_commit(struct commit *commit, void *data)
{
struct rev_list_info *info = data;
@@ -219,48 +260,6 @@ static void show_commit(struct commit *commit, void *data)
finish_commit(commit);
}
-static void finish_commit(struct commit *commit)
-{
- free_commit_list(commit->parents);
- commit->parents = NULL;
- free_commit_buffer(the_repository->parsed_objects,
- commit);
-}
-
-static inline void finish_object__ma(struct object *obj)
-{
- /*
- * Whether or not we try to dynamically fetch missing objects
- * from the server, we currently DO NOT have the object. We
- * can either print, allow (ignore), or conditionally allow
- * (ignore) them.
- */
- switch (arg_missing_action) {
- case MA_ERROR:
- die("missing %s object '%s'",
- type_name(obj->type), oid_to_hex(&obj->oid));
- return;
-
- case MA_ALLOW_ANY:
- return;
-
- case MA_PRINT:
- oidset_insert(&missing_objects, &obj->oid);
- return;
-
- case MA_ALLOW_PROMISOR:
- if (is_promisor_object(&obj->oid))
- return;
- die("unexpected missing %s object '%s'",
- type_name(obj->type), oid_to_hex(&obj->oid));
- return;
-
- default:
- BUG("unhandled missing_action");
- return;
- }
-}
-
static int finish_object(struct object *obj, const char *name UNUSED,
void *cb_data)
{
--
2.42.0
^ permalink raw reply related
* [PATCH v3 3/3] rev-list: add commit object support in `--missing` option
From: Karthik Nayak @ 2023-10-19 12:10 UTC (permalink / raw)
To: karthik.188; +Cc: git, gitster, ps
In-Reply-To: <20231019121024.194317-1-karthik.188@gmail.com>
The `--missing` object option in rev-list currently works only with
missing blobs/trees. For missing commits the revision walker fails with
a fatal error.
Let's extend the functionality of `--missing` option to also support
commit objects. This is done by adding a new `MISSING` flag that the
revision walker sets whenever it encounters a missing tree/commit. The
revision walker will now continue the traversal and call `show_commit()`
even for missing commits. In rev-list we can then check for this flag
and call the existing code for parsing `--missing` objects.
A scenario where this option would be used is to find the boundary
objects between different object directories. Consider a repository with
a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate
object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a
repository, using the `--missing=print` option while disabling the
alternate object directory allows us to find the boundary objects
between the main and alternate object directory.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/rev-list.c | 6 +++
list-objects.c | 2 +-
object.h | 4 +-
revision.c | 11 ++++--
revision.h | 2 +
t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++++++++++
6 files changed, 93 insertions(+), 6 deletions(-)
create mode 100755 t/t6022-rev-list-missing.sh
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 98542e8b3c..604ae01d0c 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -149,6 +149,12 @@ static void show_commit(struct commit *commit, void *data)
display_progress(progress, ++progress_counter);
+ if (revs->do_not_die_on_missing_objects &&
+ commit->object.flags & MISSING) {
+ finish_object__ma(&commit->object);
+ return;
+ }
+
if (show_disk_usage)
total_disk_usage += get_object_disk_usage(&commit->object);
diff --git a/list-objects.c b/list-objects.c
index 47296dff2f..60c5533721 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -387,7 +387,7 @@ static void do_traverse(struct traversal_context *ctx)
* an uninteresting boundary commit may not have its tree
* parsed yet, but we are not going to show them anyway
*/
- if (!ctx->revs->tree_objects)
+ if (!ctx->revs->tree_objects || commit->object.flags & MISSING)
; /* do not bother loading tree */
else if (repo_get_commit_tree(the_repository, commit)) {
struct tree *tree = repo_get_commit_tree(the_repository,
diff --git a/object.h b/object.h
index 114d45954d..b76830fce1 100644
--- a/object.h
+++ b/object.h
@@ -62,7 +62,7 @@ void object_array_init(struct object_array *array);
/*
* object flag allocation:
- * revision.h: 0---------10 15 23------27
+ * revision.h: 0---------10 15 22------28
* fetch-pack.c: 01 67
* negotiator/default.c: 2--5
* walker.c: 0-2
@@ -82,7 +82,7 @@ void object_array_init(struct object_array *array);
* builtin/show-branch.c: 0-------------------------------------------26
* builtin/unpack-objects.c: 2021
*/
-#define FLAG_BITS 28
+#define FLAG_BITS 29
#define TYPE_BITS 3
diff --git a/revision.c b/revision.c
index 219dc76716..8100806068 100644
--- a/revision.c
+++ b/revision.c
@@ -1110,7 +1110,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
struct commit_list *parent = commit->parents;
unsigned pass_flags;
- if (commit->object.flags & ADDED)
+ if (commit->object.flags & (ADDED | MISSING))
return 0;
commit->object.flags |= ADDED;
@@ -1168,7 +1168,8 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
for (parent = commit->parents; parent; parent = parent->next) {
struct commit *p = parent->item;
int gently = revs->ignore_missing_links ||
- revs->exclude_promisor_objects;
+ revs->exclude_promisor_objects ||
+ revs->do_not_die_on_missing_objects;
if (repo_parse_commit_gently(revs->repo, p, gently) < 0) {
if (revs->exclude_promisor_objects &&
is_promisor_object(&p->object.oid)) {
@@ -1176,7 +1177,11 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
break;
continue;
}
- return -1;
+
+ if (!revs->do_not_die_on_missing_objects)
+ return -1;
+ else
+ p->object.flags |= MISSING;
}
if (revs->sources) {
char **slot = revision_sources_at(revs->sources, p);
diff --git a/revision.h b/revision.h
index c73c92ef40..d3c1ca0f4e 100644
--- a/revision.h
+++ b/revision.h
@@ -53,6 +53,8 @@
#define ANCESTRY_PATH (1u<<27)
#define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | PULL_MERGE)
+#define MISSING (1u<<28)
+
#define DECORATE_SHORT_REFS 1
#define DECORATE_FULL_REFS 2
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
new file mode 100755
index 0000000000..40265a4f66
--- /dev/null
+++ b/t/t6022-rev-list-missing.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+test_description='handling of missing objects in rev-list'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# We setup the repository with two commits, this way HEAD is always
+# available and we can hide commit 1.
+test_expect_success 'create repository and alternate directory' '
+ test_commit 1 &&
+ test_commit 2 &&
+ test_commit 3
+'
+
+for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
+do
+ test_expect_success "rev-list --missing=error fails with missing object $obj" '
+ oid="$(git rev-parse $obj)" &&
+ path=".git/objects/$(test_oid_to_path $oid)" &&
+
+ mv "$path" "$path.hidden" &&
+ test_when_finished "mv $path.hidden $path" &&
+
+ test_must_fail git rev-list --missing=error --objects \
+ --no-object-names HEAD
+ '
+done
+
+for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
+do
+ for action in "allow-any" "print"
+ do
+ test_expect_success "rev-list --missing=$action with missing $obj" '
+ oid="$(git rev-parse $obj)" &&
+ path=".git/objects/$(test_oid_to_path $oid)" &&
+
+ # Before the object is made missing, we use rev-list to
+ # get the expected oids.
+ git rev-list --objects --no-object-names \
+ HEAD ^$obj >expect.raw &&
+
+ # Blobs are shared by all commits, so evethough a commit/tree
+ # might be skipped, its blob must be accounted for.
+ if [ $obj != "HEAD:1.t" ]; then
+ echo $(git rev-parse HEAD:1.t) >>expect.raw &&
+ echo $(git rev-parse HEAD:2.t) >>expect.raw
+ fi &&
+
+ mv "$path" "$path.hidden" &&
+ test_when_finished "mv $path.hidden $path" &&
+
+ git rev-list --missing=$action --objects --no-object-names \
+ HEAD >actual.raw &&
+
+ # When the action is to print, we should also add the missing
+ # oid to the expect list.
+ case $action in
+ allow-any)
+ ;;
+ print)
+ grep ?$oid actual.raw &&
+ echo ?$oid >>expect.raw
+ ;;
+ esac &&
+
+ sort actual.raw >actual &&
+ sort expect.raw >expect &&
+ test_cmp expect actual
+ '
+ done
+done
+
+test_done
--
2.42.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox