* [PATCH v3 07/10] bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
From: Taylor Blau @ 2023-10-18 17:08 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1697648864.git.me@ttaylorr.com>
The existing `stream_blob_to_pack()` function is named based on the fact
that it knows only how to stream blobs into a bulk-checkin pack.
But there is no longer anything in this function which prevents us from
writing objects of arbitrary types to the bulk-checkin pack. Prepare to
write OBJ_TREEs by removing this assumption, adding an `enum
object_type` parameter to this function's argument list, and renaming it
to `stream_obj_to_pack()` accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
bulk-checkin.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 133e02ce36..f0115efb2e 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -204,10 +204,10 @@ static ssize_t bulk_checkin_source_read(struct bulk_checkin_source *source,
* status before calling us just in case we ask it to call us again
* with a new pack.
*/
-static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
- git_hash_ctx *ctx, off_t *already_hashed_to,
- struct bulk_checkin_source *source,
- unsigned flags)
+static int stream_obj_to_pack(struct bulk_checkin_packfile *state,
+ git_hash_ctx *ctx, off_t *already_hashed_to,
+ struct bulk_checkin_source *source,
+ enum object_type type, unsigned flags)
{
git_zstream s;
unsigned char ibuf[16384];
@@ -220,8 +220,7 @@ static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
git_deflate_init(&s, pack_compression_level);
- hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB,
- size);
+ hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), type, size);
s.next_out = obuf + hdrlen;
s.avail_out = sizeof(obuf) - hdrlen;
@@ -402,8 +401,8 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
while (1) {
prepare_checkpoint(state, &checkpoint, idx, flags);
- if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
- &source, flags))
+ if (!stream_obj_to_pack(state, &ctx, &already_hashed_to,
+ &source, OBJ_BLOB, flags))
break;
truncate_checkpoint(state, &checkpoint, idx);
if (bulk_checkin_source_seek_to(&source, seekback) == (off_t)-1)
--
2.42.0.408.g97fac66ae4
^ permalink raw reply related
* [PATCH v3 10/10] builtin/merge-tree.c: implement support for `--write-pack`
From: Taylor Blau @ 2023-10-18 17:08 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1697648864.git.me@ttaylorr.com>
When using merge-tree often within a repository[^1], it is possible to
generate a relatively large number of loose objects, which can result in
degraded performance, and inode exhaustion in extreme cases.
Building on the functionality introduced in previous commits, the
bulk-checkin machinery now has support to write arbitrary blob and tree
objects which are small enough to be held in-core. We can use this to
write any blob/tree objects generated by ORT into a separate pack
instead of writing them out individually as loose.
This functionality is gated behind a new `--write-pack` option to
`merge-tree` that works with the (non-deprecated) `--write-tree` mode.
The implementation is relatively straightforward. There are two spots
within the ORT mechanism where we call `write_object_file()`, one for
content differences within blobs, and another to assemble any new trees
necessary to construct the merge. In each of those locations,
conditionally replace calls to `write_object_file()` with
`index_blob_bulk_checkin_incore()` or `index_tree_bulk_checkin_incore()`
depending on which kind of object we are writing.
The only remaining task is to begin and end the transaction necessary to
initialize the bulk-checkin machinery, and move any new pack(s) it
created into the main object store.
[^1]: Such is the case at GitHub, where we run presumptive "test merges"
on open pull requests to see whether or not we can light up the merge
button green depending on whether or not the presumptive merge was
conflicted.
This is done in response to a number of user-initiated events,
including viewing an open pull request whose last test merge is stale
with respect to the current base and tip of the pull request. As a
result, merge-tree can be run very frequently on large, active
repositories.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-merge-tree.txt | 4 ++
builtin/merge-tree.c | 5 ++
merge-ort.c | 42 +++++++++++----
merge-recursive.h | 1 +
t/t4301-merge-tree-write-tree.sh | 93 ++++++++++++++++++++++++++++++++
5 files changed, 136 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index ffc4fbf7e8..9d37609ef1 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -69,6 +69,10 @@ OPTIONS
specify a merge-base for the merge, and specifying multiple bases is
currently not supported. This option is incompatible with `--stdin`.
+--write-pack::
+ Write any new objects into a separate packfile instead of as
+ individual loose objects.
+
[[OUTPUT]]
OUTPUT
------
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 0de42aecf4..672ebd4c54 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -18,6 +18,7 @@
#include "quote.h"
#include "tree.h"
#include "config.h"
+#include "bulk-checkin.h"
static int line_termination = '\n';
@@ -414,6 +415,7 @@ struct merge_tree_options {
int show_messages;
int name_only;
int use_stdin;
+ int write_pack;
};
static int real_merge(struct merge_tree_options *o,
@@ -440,6 +442,7 @@ static int real_merge(struct merge_tree_options *o,
init_merge_options(&opt, the_repository);
opt.show_rename_progress = 0;
+ opt.write_pack = o->write_pack;
opt.branch1 = branch1;
opt.branch2 = branch2;
@@ -548,6 +551,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
&merge_base,
N_("commit"),
N_("specify a merge-base for the merge")),
+ OPT_BOOL(0, "write-pack", &o.write_pack,
+ N_("write new objects to a pack instead of as loose")),
OPT_END()
};
diff --git a/merge-ort.c b/merge-ort.c
index 7857ce9fbd..e198d2bc2b 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -48,6 +48,7 @@
#include "tree.h"
#include "unpack-trees.h"
#include "xdiff-interface.h"
+#include "bulk-checkin.h"
/*
* We have many arrays of size 3. Whenever we have such an array, the
@@ -2107,10 +2108,19 @@ static int handle_content_merge(struct merge_options *opt,
if ((merge_status < 0) || !result_buf.ptr)
ret = error(_("failed to execute internal merge"));
- if (!ret &&
- write_object_file(result_buf.ptr, result_buf.size,
- OBJ_BLOB, &result->oid))
- ret = error(_("unable to add %s to database"), path);
+ if (!ret) {
+ ret = opt->write_pack
+ ? index_blob_bulk_checkin_incore(&result->oid,
+ result_buf.ptr,
+ result_buf.size,
+ path, 1)
+ : write_object_file(result_buf.ptr,
+ result_buf.size,
+ OBJ_BLOB, &result->oid);
+ if (ret)
+ ret = error(_("unable to add %s to database"),
+ path);
+ }
free(result_buf.ptr);
if (ret)
@@ -3596,7 +3606,8 @@ static int tree_entry_order(const void *a_, const void *b_)
b->string, strlen(b->string), bmi->result.mode);
}
-static int write_tree(struct object_id *result_oid,
+static int write_tree(struct merge_options *opt,
+ struct object_id *result_oid,
struct string_list *versions,
unsigned int offset,
size_t hash_size)
@@ -3630,8 +3641,14 @@ static int write_tree(struct object_id *result_oid,
}
/* Write this object file out, and record in result_oid */
- if (write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid))
+ ret = opt->write_pack
+ ? index_tree_bulk_checkin_incore(result_oid,
+ buf.buf, buf.len, "", 1)
+ : write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid);
+
+ if (ret)
ret = -1;
+
strbuf_release(&buf);
return ret;
}
@@ -3796,8 +3813,8 @@ static int write_completed_directory(struct merge_options *opt,
*/
dir_info->is_null = 0;
dir_info->result.mode = S_IFDIR;
- if (write_tree(&dir_info->result.oid, &info->versions, offset,
- opt->repo->hash_algo->rawsz) < 0)
+ if (write_tree(opt, &dir_info->result.oid, &info->versions,
+ offset, opt->repo->hash_algo->rawsz) < 0)
ret = -1;
}
@@ -4331,9 +4348,13 @@ static int process_entries(struct merge_options *opt,
fflush(stdout);
BUG("dir_metadata accounting completely off; shouldn't happen");
}
- if (write_tree(result_oid, &dir_metadata.versions, 0,
+ if (write_tree(opt, result_oid, &dir_metadata.versions, 0,
opt->repo->hash_algo->rawsz) < 0)
ret = -1;
+
+ if (opt->write_pack)
+ end_odb_transaction();
+
cleanup:
string_list_clear(&plist, 0);
string_list_clear(&dir_metadata.versions, 0);
@@ -4877,6 +4898,9 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
*/
strmap_init(&opt->priv->conflicts);
+ if (opt->write_pack)
+ begin_odb_transaction();
+
trace2_region_leave("merge", "allocate/init", opt->repo);
}
diff --git a/merge-recursive.h b/merge-recursive.h
index b88000e3c2..156e160876 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -48,6 +48,7 @@ struct merge_options {
unsigned renormalize : 1;
unsigned record_conflict_msgs_as_headers : 1;
const char *msg_header_prefix;
+ unsigned write_pack : 1;
/* internal fields used by the implementation */
struct merge_options_internal *priv;
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 250f721795..2d81ff4de5 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -922,4 +922,97 @@ test_expect_success 'check the input format when --stdin is passed' '
test_cmp expect actual
'
+packdir=".git/objects/pack"
+
+test_expect_success 'merge-tree can pack its result with --write-pack' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+
+ # base has lines [3, 4, 5]
+ # - side adds to the beginning, resulting in [1, 2, 3, 4, 5]
+ # - other adds to the end, resulting in [3, 4, 5, 6, 7]
+ #
+ # merging the two should result in a new blob object containing
+ # [1, 2, 3, 4, 5, 6, 7], along with a new tree.
+ test_commit -C repo base file "$(test_seq 3 5)" &&
+ git -C repo branch -M main &&
+ git -C repo checkout -b side main &&
+ test_commit -C repo side file "$(test_seq 1 5)" &&
+ git -C repo checkout -b other main &&
+ test_commit -C repo other file "$(test_seq 3 7)" &&
+
+ find repo/$packdir -type f -name "pack-*.idx" >packs.before &&
+ tree="$(git -C repo merge-tree --write-pack \
+ refs/tags/side refs/tags/other)" &&
+ blob="$(git -C repo rev-parse $tree:file)" &&
+ find repo/$packdir -type f -name "pack-*.idx" >packs.after &&
+
+ test_must_be_empty packs.before &&
+ test_line_count = 1 packs.after &&
+
+ git show-index <$(cat packs.after) >objects &&
+ test_line_count = 2 objects &&
+ grep "^[1-9][0-9]* $tree" objects &&
+ grep "^[1-9][0-9]* $blob" objects
+'
+
+test_expect_success 'merge-tree can write multiple packs with --write-pack' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ git config pack.packSizeLimit 512 &&
+
+ test_seq 512 >f &&
+
+ # "f" contains roughly ~2,000 bytes.
+ #
+ # Each side ("foo" and "bar") adds a small amount of data at the
+ # beginning and end of "base", respectively.
+ git add f &&
+ test_tick &&
+ git commit -m base &&
+ git branch -M main &&
+
+ git checkout -b foo main &&
+ {
+ echo foo && cat f
+ } >f.tmp &&
+ mv f.tmp f &&
+ git add f &&
+ test_tick &&
+ git commit -m foo &&
+
+ git checkout -b bar main &&
+ echo bar >>f &&
+ git add f &&
+ test_tick &&
+ git commit -m bar &&
+
+ find $packdir -type f -name "pack-*.idx" >packs.before &&
+ # Merging either side should result in a new object which is
+ # larger than 1M, thus the result should be split into two
+ # separate packs.
+ tree="$(git merge-tree --write-pack \
+ refs/heads/foo refs/heads/bar)" &&
+ blob="$(git rev-parse $tree:f)" &&
+ find $packdir -type f -name "pack-*.idx" >packs.after &&
+
+ test_must_be_empty packs.before &&
+ test_line_count = 2 packs.after &&
+ for idx in $(cat packs.after)
+ do
+ git show-index <$idx || return 1
+ done >objects &&
+
+ # The resulting set of packs should contain one copy of both
+ # objects, each in a separate pack.
+ test_line_count = 2 objects &&
+ grep "^[1-9][0-9]* $tree" objects &&
+ grep "^[1-9][0-9]* $blob" objects
+
+ )
+'
+
test_done
--
2.42.0.408.g97fac66ae4
^ permalink raw reply related
* [PATCH v3 03/10] bulk-checkin: factor out `truncate_checkpoint()`
From: Taylor Blau @ 2023-10-18 17:07 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1697648864.git.me@ttaylorr.com>
In a similar spirit as previous commits, factor our the routine to
truncate a bulk-checkin packfile when writing past the pack size limit.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
bulk-checkin.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index c1f5450583..b92d7a6f5a 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -276,6 +276,22 @@ static void prepare_checkpoint(struct bulk_checkin_packfile *state,
}
}
+static void truncate_checkpoint(struct bulk_checkin_packfile *state,
+ struct hashfile_checkpoint *checkpoint,
+ struct pack_idx_entry *idx)
+{
+ /*
+ * Writing this object to the current pack will make
+ * it too big; we need to truncate it, start a new
+ * pack, and write into it.
+ */
+ if (!idx)
+ BUG("should not happen");
+ hashfile_truncate(state->f, checkpoint);
+ state->offset = checkpoint->offset;
+ flush_bulk_checkin_packfile(state);
+}
+
static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
struct object_id *result_oid,
int fd, size_t size,
@@ -304,16 +320,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
fd, size, path, flags))
break;
- /*
- * Writing this object to the current pack will make
- * it too big; we need to truncate it, start a new
- * pack, and write into it.
- */
- if (!idx)
- BUG("should not happen");
- hashfile_truncate(state->f, &checkpoint);
- state->offset = checkpoint.offset;
- flush_bulk_checkin_packfile(state);
+ truncate_checkpoint(state, &checkpoint, idx);
if (lseek(fd, seekback, SEEK_SET) == (off_t) -1)
return error("cannot seek back");
}
--
2.42.0.408.g97fac66ae4
^ permalink raw reply related
* Re: [PATCH v2 5/7] bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
From: Taylor Blau @ 2023-10-18 16:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <xmqq5y34wu5f.fsf@gitster.g>
On Tue, Oct 17, 2023 at 07:18:04PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > bulk-checkin.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
> > bulk-checkin.h | 4 ++
> > 2 files changed, 122 insertions(+)
>
> Unlike the previous four, which were very clear refactoring to
> create reusable helper functions, this step leaves a bad aftertaste
> after reading twice, and I think what is disturbing is that many new
> lines are pretty much literally copied from stream_blob_to_pack().
>
> I wonder if we can introduce an "input" source abstraction, that
> replaces "fd" and "size" (and "path" for error reporting) parameters
> to the stream_blob_to_pack(), so that the bulk of the implementation
> of stream_blob_to_pack() can call its .read() method to read bytes
> up to "size" from such an abstracted interface? That would be a
> good sized first half of this change. Then in the second half, you
> can add another "input" source that works with in-core "buf" and
> "size", whose .read() method will merely be a memcpy().
Thanks, I like this idea. I had initially avoided it in the first couple
of rounds, because the abstraction felt clunky and involved an
unnecessary extra memcpy().
But having applied your suggestion here, I think that the price is well
worth the result, which is that `stream_blob_to_pack()` does not have to
be implemented twice with very subtle differences.
Thanks again for the suggestion, I'm really pleased with how it came
out. Reroll coming shortly...
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 03/11] t: convert tests to use helpers for reference existence
From: Junio C Hamano @ 2023-10-18 16:28 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <ac6a49c7c84ad2b836f099557dd6989703ebda8f.1697607222.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Convert tests that use `test_path_is_file` and `test_path_is_missing` to
> instead use our new helpers `test_ref_exists` and `test_ref_missing`.
> These hook into the reference database directly and thus work indepently
> of the actual reference backend that is being used.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> t/t1430-bad-ref-name.sh | 27 ++++++++++++++++++---------
> t/t3200-branch.sh | 33 ++++++++++++++++++---------------
> t/t5521-pull-options.sh | 4 ++--
> t/t5605-clone-local.sh | 2 +-
> 4 files changed, 39 insertions(+), 27 deletions(-)
I scanned through the changes, and all of them looked sensible. I
also very much liked the added "now we have done something to create
'badname' branch, let's make sure it exists, before trying to see if
the machinery to delete it works correctly" tests.
Looking good.
Thanks.
> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> index ff1c967d550..7b7d6953c62 100755
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -205,8 +205,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git update-ref --no-deref -d refs/heads/badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -216,8 +217,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git branch -d badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
> test_must_be_empty error
> '
> @@ -225,8 +227,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
> test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git update-ref --no-deref -d refs/heads/badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -234,8 +237,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok
> test_expect_success 'branch -d can delete dangling symref to broken name' '
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git branch -d badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
> test_must_be_empty error
> '
> @@ -245,8 +249,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/broken...ref &&
> git update-ref -d refs/heads/badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...ref &&
> + test_ref_missing refs/heads/broken...ref &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -254,8 +259,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
> test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
> printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -263,8 +269,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name
> test_expect_success 'branch -d can delete symref with broken name' '
> printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git branch -d broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
> test_must_be_empty error
> '
> @@ -272,8 +279,9 @@ test_expect_success 'branch -d can delete symref with broken name' '
> test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
> printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -281,8 +289,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
> test_expect_success 'branch -d can delete dangling symref with broken name' '
> printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git branch -d broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
> test_must_be_empty error
> '
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 080e4f24a6e..bde4f1485b7 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -25,7 +25,7 @@ test_expect_success 'prepare a trivial repository' '
>
> test_expect_success 'git branch --help should not have created a bogus branch' '
> test_might_fail git branch --man --help </dev/null >/dev/null 2>&1 &&
> - test_path_is_missing .git/refs/heads/--help
> + test_ref_missing refs/heads/--help
> '
>
> test_expect_success 'branch -h in broken repository' '
> @@ -40,7 +40,8 @@ test_expect_success 'branch -h in broken repository' '
> '
>
> test_expect_success 'git branch abc should create a branch' '
> - git branch abc && test_path_is_file .git/refs/heads/abc
> + git branch abc &&
> + test_ref_exists refs/heads/abc
> '
>
> test_expect_success 'git branch abc should fail when abc exists' '
> @@ -61,11 +62,13 @@ test_expect_success 'git branch --force abc should succeed when abc exists' '
> '
>
> test_expect_success 'git branch a/b/c should create a branch' '
> - git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c
> + git branch a/b/c &&
> + test_ref_exists refs/heads/a/b/c
> '
>
> test_expect_success 'git branch mb main... should create a branch' '
> - git branch mb main... && test_path_is_file .git/refs/heads/mb
> + git branch mb main... &&
> + test_ref_exists refs/heads/mb
> '
>
> test_expect_success 'git branch HEAD should fail' '
> @@ -78,14 +81,14 @@ EOF
> test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
> GIT_COMMITTER_DATE="2005-05-26 23:30" \
> git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
> - test_path_is_file .git/refs/heads/d/e/f &&
> + test_ref_exists refs/heads/d/e/f &&
> test_path_is_file .git/logs/refs/heads/d/e/f &&
> test_cmp expect .git/logs/refs/heads/d/e/f
> '
>
> test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
> git branch -d d/e/f &&
> - test_path_is_missing .git/refs/heads/d/e/f &&
> + test_ref_missing refs/heads/d/e/f &&
> test_must_fail git reflog exists refs/heads/d/e/f
> '
>
> @@ -213,7 +216,7 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
> test_commit initial &&
> git checkout --orphan lonely &&
> grep lonely .git/HEAD &&
> - test_path_is_missing .git/refs/head/lonely &&
> + test_ref_missing refs/head/lonely &&
> git branch -M main mistress &&
> grep lonely .git/HEAD
> )
> @@ -799,8 +802,8 @@ test_expect_success 'deleting a symref' '
> git symbolic-ref refs/heads/symref refs/heads/target &&
> echo "Deleted branch symref (was refs/heads/target)." >expect &&
> git branch -d symref >actual &&
> - test_path_is_file .git/refs/heads/target &&
> - test_path_is_missing .git/refs/heads/symref &&
> + test_ref_exists refs/heads/target &&
> + test_ref_missing refs/heads/symref &&
> test_cmp expect actual
> '
>
> @@ -809,16 +812,16 @@ test_expect_success 'deleting a dangling symref' '
> test_path_is_file .git/refs/heads/dangling-symref &&
> echo "Deleted branch dangling-symref (was nowhere)." >expect &&
> git branch -d dangling-symref >actual &&
> - test_path_is_missing .git/refs/heads/dangling-symref &&
> + test_ref_missing refs/heads/dangling-symref &&
> test_cmp expect actual
> '
>
> test_expect_success 'deleting a self-referential symref' '
> git symbolic-ref refs/heads/self-reference refs/heads/self-reference &&
> - test_path_is_file .git/refs/heads/self-reference &&
> + test_ref_exists refs/heads/self-reference &&
> echo "Deleted branch self-reference (was refs/heads/self-reference)." >expect &&
> git branch -d self-reference >actual &&
> - test_path_is_missing .git/refs/heads/self-reference &&
> + test_ref_missing refs/heads/self-reference &&
> test_cmp expect actual
> '
>
> @@ -826,8 +829,8 @@ test_expect_success 'renaming a symref is not allowed' '
> git symbolic-ref refs/heads/topic refs/heads/main &&
> test_must_fail git branch -m topic new-topic &&
> git symbolic-ref refs/heads/topic &&
> - test_path_is_file .git/refs/heads/main &&
> - test_path_is_missing .git/refs/heads/new-topic
> + test_ref_exists refs/heads/main &&
> + test_ref_missing refs/heads/new-topic
> '
>
> test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
> @@ -1142,7 +1145,7 @@ EOF
> test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
> GIT_COMMITTER_DATE="2005-05-26 23:30" \
> git checkout -b g/h/i -l main &&
> - test_path_is_file .git/refs/heads/g/h/i &&
> + test_ref_exists refs/heads/g/h/i &&
> test_path_is_file .git/logs/refs/heads/g/h/i &&
> test_cmp expect .git/logs/refs/heads/g/h/i
> '
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index 079b2f2536e..3681859f983 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -143,7 +143,7 @@ test_expect_success 'git pull --dry-run' '
> cd clonedry &&
> git pull --dry-run ../parent &&
> test_path_is_missing .git/FETCH_HEAD &&
> - test_path_is_missing .git/refs/heads/main &&
> + test_ref_missing refs/heads/main &&
> test_path_is_missing .git/index &&
> test_path_is_missing file
> )
> @@ -157,7 +157,7 @@ test_expect_success 'git pull --all --dry-run' '
> git remote add origin ../parent &&
> git pull --all --dry-run &&
> test_path_is_missing .git/FETCH_HEAD &&
> - test_path_is_missing .git/refs/remotes/origin/main &&
> + test_ref_missing refs/remotes/origin/main &&
> test_path_is_missing .git/index &&
> test_path_is_missing file
> )
> diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
> index 1d7b1abda1a..946c5751885 100755
> --- a/t/t5605-clone-local.sh
> +++ b/t/t5605-clone-local.sh
> @@ -69,7 +69,7 @@ test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
> git clone a d &&
> (cd d &&
> git fetch &&
> - test ! -e .git/refs/remotes/origin/HEAD)
> + test_ref_missing refs/remotes/origin/HEAD)
> '
>
> test_expect_success 'bundle clone without .bundle suffix' '
^ permalink raw reply
* Re: [PATCH 02/11] t: allow skipping expected object ID in `ref-store update-ref`
From: Junio C Hamano @ 2023-10-18 16:08 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <1f615d62f99e9ab47d37500f05b29615bafffba2.1697607222.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> We require the caller to pass both the old and new expected object ID to
> our `test-tool ref-store update-ref` helper. When trying to update a
> symbolic reference though it's impossible to specify the expected object
> ID, which means that the test would instead have to force-update the
> reference. This is currently impossible though.
>
> Update the helper to optionally skip verification of the old object ID
> in case the test passes in an empty old object ID as input.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> t/helper/test-ref-store.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
Good.
Even better would be to make the old one optional, though.
> diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> index 7400f560ab6..7dc83137584 100644
> --- a/t/helper/test-ref-store.c
> +++ b/t/helper/test-ref-store.c
> @@ -322,16 +322,19 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)
> const char *new_sha1_buf = notnull(*argv++, "new-sha1");
> const char *old_sha1_buf = notnull(*argv++, "old-sha1");
> unsigned int flags = arg_flags(*argv++, "flags", transaction_flags);
> - struct object_id old_oid;
> + struct object_id old_oid, *old_oid_ptr = NULL;
> struct object_id new_oid;
>
> - if (get_oid_hex(old_sha1_buf, &old_oid))
> - die("cannot parse %s as %s", old_sha1_buf, the_hash_algo->name);
> + if (*old_sha1_buf) {
> + if (get_oid_hex(old_sha1_buf, &old_oid))
> + die("cannot parse %s as %s", old_sha1_buf, the_hash_algo->name);
> + old_oid_ptr = &old_oid;
> + }
> if (get_oid_hex(new_sha1_buf, &new_oid))
> die("cannot parse %s as %s", new_sha1_buf, the_hash_algo->name);
>
> return refs_update_ref(refs, msg, refname,
> - &new_oid, &old_oid,
> + &new_oid, old_oid_ptr,
> flags, UPDATE_REFS_DIE_ON_ERR);
> }
^ permalink raw reply
* Re: [PATCH 01/11] t: add helpers to test for reference existence
From: Junio C Hamano @ 2023-10-18 16:06 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <e947feb1c77f7e9f3c7f983bbe47137fbce42367.1697607222.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> There are two major ways to check for the existence of a reference in
> our tests:
>
> - `git rev-parse --verify` can be used to check for existence of a
> reference. This only works in the case where the reference is well
> formed though and resolves to an actual object ID. This does not
> work with malformed reference names or invalid contents.
>
> - `test_path_is_file` can be used to check for existence of a loose
> reference if it is known to not resolve to an actual object ID. It
> by necessity reaches into implementation details of the reference
> backend though.
True. It would be ideal if we can limit the use of latter when we
_care_ how the ref is stored (e.g., "we expect it to be stored as a
loose ref, not packed"). "The ref R at must be pointing at the
commit X" is better asserted by using the former (or "git show-ref")
as we not just only want to see the .git/refs/R file holding the
object name X, but also want to see the production git tools observe
the same---if what rev-parse or show-ref observes is different from
the expected state and they say ref R does not point at commit X, we
should complain (rev-parse or show-ref may be broken in the version
of Git being tested, but we can assume that their breakage will be
caught elsewhere in the test suite as well, so as long as we trust
them, using them as the validator is better than going into the
implementation detail and assuming things like "new refs always
appear as a loose ref" that we might want to change in the future).
> Similarly, there are two equivalent ways to check for the absence of a
> reference:
>
> - `test_must_fail git rev-parse` can be used to check for the
> absence of a reference. It could fail due to a number of reasons
> though, and all of these reasons will be thrown into the same bag
> as an absent reference.
>
> - `test_path_is_missing` can be used to check explicitly for the
> absence of a loose reference, but again reaches into internal
> implementation details of the reference backend.
>
> So both our tooling to check for the presence and for the absence of
> references in tests is lacking as either failure cases are thrown into
> the same bag or we need to reach into internal implementation details of
> the respective reference backend.
> Introduce a new subcommand for our ref-store test helper that explicitly
> checks only for the presence or absence of a reference. This addresses
> these limitations:
>
> - We can check for the presence of references with malformed names.
But for the purpose of tests, we can control the input. When we
perform an operation that we expect a ref R to be created, we would
know R is well formed and we can validate using a tool that we know
would be broken when fed a malformed name. So I do not see this as
a huge "limitation".
> - We can check for the presence of references that don't resolve.
Do you mean a dangling symbolic ref? We are using a wrong tool if
you are using rev-parse for that, aren't we? Isn't symbolic-ref
there for us for this exact use case? That is
> - We can explicitly handle the case where a reference is missing by
> special-casing ENOENT errors.
You probably know the error conditions refs_read_raw_ref() can be
broken better than I do, but this feels a bit too intimate with how
the method for the files backend happens to be implemented, which at
the same time, can risk that [a] other backends can implement their
"ref does not resolve to an object name---is it because it is
missing?" report incorrectly and [b] we would eventually want to
know error conditions other than "the ref requested is missing" and
at that point we would need more "special casing", which does not
smell easy to scale.
> - We don't need to reach into implementation details of the backend,
> which would allow us to use this helper for the future reftable
> backend.
This is exactly what we want to aim for.
> Next to this subcommand we also provide two wrappers `test_ref_exists`
> and `test_ref_missing` that make the helper easier to use.
Hmmmm. This may introduce "who watches the watchers" problem, no?
I briefly wondered if a better approach is to teach the production
code, e.g., rev-parse, to optionally give more detailed diag. It
essentially may be the same (making the code in test-ref-store.c
added by this patch available from rev-parse, we would easily get
there), so I do not think the distinction matters.
> diff --git a/t/README b/t/README
> index 61080859899..779f7e7dd86 100644
> --- a/t/README
> +++ b/t/README
> @@ -928,6 +928,15 @@ see test-lib-functions.sh for the full list and their options.
> committer times to defined state. Subsequent calls will
> advance the times by a fixed amount.
>
> + - test_ref_exists <ref>, test_ref_missing <ref>
> +
> + Check whether a reference exists or is missing. In contrast to
> + git-rev-parse(1), these helpers also work with invalid reference
> + names and references whose contents are unresolvable. The latter
> + function also distinguishes generic errors from the case where a
> + reference explicitly doesn't exist and is thus safer to use than
> + `test_must_fail git rev-parse`.
> +
> - test_commit <message> [<filename> [<contents>]]
>
> Creates a commit with the given message, committing the given
> diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> index 48552e6a9e0..7400f560ab6 100644
> --- a/t/helper/test-ref-store.c
> +++ b/t/helper/test-ref-store.c
> @@ -1,6 +1,6 @@
> #include "test-tool.h"
> #include "hex.h"
> -#include "refs.h"
> +#include "refs/refs-internal.h"
> #include "setup.h"
> #include "worktree.h"
> #include "object-store-ll.h"
> @@ -221,6 +221,30 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv)
> return ret;
> }
>
> +static int cmd_ref_exists(struct ref_store *refs, const char **argv)
> +{
> + const char *refname = notnull(*argv++, "refname");
> + struct strbuf unused_referent = STRBUF_INIT;
> + struct object_id unused_oid;
> + unsigned int unused_type;
> + int failure_errno;
> +
> + if (refs_read_raw_ref(refs, refname, &unused_oid, &unused_referent,
> + &unused_type, &failure_errno)) {
> + /*
> + * We handle ENOENT separately here such that it is possible to
> + * distinguish actually-missing references from any kind of
> + * generic error.
> + */
> + if (failure_errno == ENOENT)
> + return 17;
Can we tell between the cases where the ref itself is missing, and
the requested ref is symbolic and points at a missing ref? This
particular case might be OK, but there may other cases where this
"special case" may not be narrow enough.
As long we are going to spend cycles to refine the classification of
error conditions, which is a very good thing to aim for the reason
described in the proposed log message, namely "rev-parse can fail
for reasons other than the ref being absent", I have to wonder again
that the fruit of such an effort should become available in the
production code, instead of being kept only in test-tool.
^ permalink raw reply
* Re: [PATCH 00/11] t: reduce direct disk access to data structures
From: Junio C Hamano @ 2023-10-18 15:32 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> 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].
But you have thought about this series a lot deeper and longer than
I have, so let me read on and find out what your solution is.
> The main motivation for this patch series was the second bullet point as
> it is preparatory work to get the reftable backend upstreamed.
Yay.
> My intent
> is to get rid of many or even most of the current blockers in the Git
> project before trying to send the reftable implementation upstream.
> While this will be a lot of up-front work that is going to span over a
> long time period, I think this approach will make everyones live easier
> by doing comparatively small and incremental improvements to the Git
> project.
Thank you very much for being considerate. I assume this approach
is what those in the Contributors' Summit agreed to be the best way
forward to do anything sizeable.
> Ultimately, the final patch series should in the best case only
> contain the new backend as well as testing infrastructure, but not much
> else.
;-)
^ permalink raw reply
* Re: [PATCH 08/11] t4207: delete replace references via git-update-ref(1)
From: Han-Wen Nienhuys @ 2023-10-18 13:27 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <c4d09e3e5dbd11221cc4d229b815434d441cdb4d.1697607222.git.ps@pks.im>
On Wed, Oct 18, 2023 at 7:35 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> In t4207 we set up a set of replace objects via git-replace(1). Because
> these references should not be impacting subsequent tests we also set up
> some cleanup logic that deletes the replacement references via a call to
> `rm -rf`. This reaches into the internal implementation details of the
> reference backend and will thus break when we grow an alternative refdb
> implementation.
>
> Refactor the tests to delete the replacement refs via Git commands so
> that we become independent of the actual refdb that's in use. As we
> don't have a nice way to delete all replacements or all references in a
> certain namespace, we opt for a combination of git-for-each-ref(1) and
> git-update-ref(1)'s `--stdin` mode.
There is a test helper that can directly access the ref database,
t/helper/test-ref-store.c.
If you use that manipulate refs for testing purposes, you make the
test independent of behavior
git-for-each-ref/git-update-ref, which is what you want for testing
replace-objects?
--
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: How to combine multiple commit diffs?
From: ZheNing Hu @ 2023-10-18 13:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqpm1jg4dk.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> 于2023年10月13日周五 01:03写道:
>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> > This may seem similar to cherry-picking a few commits from a pile of
> > commits, but in fact, we do not expect to actually perform
> > cherry-picking.
>
> If you said "We do not want to", I may be sympathetic, but it is
> curious that you said "We do not expect to", which hints that you
> already have some implementation in mind.
>
Our code platform has the ability to cherry-pick (by libgit2),
but using cherry-pick to merge and display the diff of the same feature
might be challenging for users (as it can easily meet conflicts).
> Whether you use a checkout of the history to a working tree and
> perform a series of "git cherry-pick" to implement it, or use the
> "git replay" machinery to perform them in-core, at the conceptual
> level, you would need to pick a base commit and apply the effect of
> those commits you would want to look at on top of that base in
> sequence, before you can see the combined change, no?
>
Yes, "cherry pick" and "rebase -i" are indeed the most natural
approaches to merging and displaying diffs. It's unfortunate
that we didn't choose to use them in our technical selection.
> Puzzled.
I can only roughly estimate how Jetbrains Space solves this problem:
1. The user selects a few non-consecutive commits in the commit list.
2. The backend service will calculate the set of modified files for the selected
commits in the CodeReview.
3. The backend service it calculates the latest commit and oldest parent commit
of each file that introduced the diff in the selected commits.
4. it calculates the file diff using these two commits.
5. Afterwards, there is some way to filter out the intermediate, non-selected
commits that modified the files. I don't quite understand this
step, it feels rather hacker-like.
^ permalink raw reply
* Re: [PATCH] t/t7601: Modernize test scripts using functions
From: Dorcas Litunya @ 2023-10-18 12:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: christian.couder, git
In-Reply-To: <xmqqjzrlgftp.fsf@gitster.g>
On Tue, Oct 17, 2023 at 01:21:54PM -0700, Junio C Hamano wrote:
> Dorcas Litunya <anonolitunya@gmail.com> writes:
>
> > Bcc:
> > Subject: Re: [PATCH] t/t7601: Modernize test scripts using functions
> > Reply-To:
> > In-Reply-To: <xmqq1qdumrto.fsf@gitster.g>
>
> What are these lines doing here?
>
Sorry, I formatted the email wrongly.
> > So should I replace this in the next version or leave this as is?
>
> Perhaps I was not clear enough, but I found the commit title and
> description need to be updated to clearly record the intent of the
> change with a handful of points, so I will not be accepting the
> patch as-is.
>
> These two sections may be of help.
>
> Documentation/MyFirstContribution.txt::now-what
> Documentation/MyFirstContribution.txt::reviewing
>
Thanks for the resources and feedback. Ihave edited the patch based on
it and sent v2.
> Thanks.
^ permalink raw reply
* Re: How to combine multiple commit diffs?
From: ZheNing Hu @ 2023-10-18 12:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git List, Junio C Hamano
In-Reply-To: <22688989-1b51-4989-b92d-5a5891ec9265@kdbg.org>
Johannes Sixt <j6t@kdbg.org> 于2023年10月13日周五 00:41写道:
>
> [For general support questions, please address only the mailing list.
> It's not necessary to bother the maintainer personally. Though, I'll not
> remove him from Cc, yet, as to comply with this ML's etiquette.]
>
OK... I get it.
> Am 12.10.23 um 14:00 schrieb ZheNing Hu:
> > Hi everyone,
> >
> > Our company wants to design a "small-batch" code review feature.
> > Simply put, this "small-batch" means being able to treat multiple
> > related commits within a MergeRequest as an independent "small" code
> > review.
> >
> > Let me give you an example: We have five commits: A1, B, A2, C, A3.
> > Among them, A1, A2, and A3 are multiple commits for the same feature.
> > So when the user selects these commits, the page will return a
> > "combine diff" that combines them together.
> >
> > A1 B A2 A3 C
> > *--------*----*-----*-------* (branch)
> > \ A1' \ A2' \ A3'
> > *------------*------*------- (small branch code review)
> >
> > This may seem similar to cherry-picking a few commits from a pile of
> > commits, but in fact, we do not expect to actually perform
> > cherry-picking.
> >
> > Do you have any suggestions on how we can merge a few commits together
> > and display the diff? The only reference we have is the non-open
> > source platform, JetBrains Space CodeReview, they support selecting
> > multiple commits for CodeReview. [1], .
>
>
> Take a step back. Then ask: What are the consequences of the review?
> What if the result is: the feature is perfect, we want it merged,
> however, we cannot, because we do not want commit B. What if the result
> is the opposite? You need B, but you can't merge it because the feature
> is not ready, yet?
>
The CodeReview here is only expected to review the Diff changes
(just like jetbrains Space). If it is truly blocked by B, users should
understand to remove B or cherrypick A1, A2, A3 into a new branch.
> You are looking for a technical workaround for a non-optimal workflow.
> If A1,A2,A3 are a feature on their own, they, and only they, should be
> in their own feature branch.
>
I have to admit that this is addressing the issue for users who are not
very familiar with the git workflow, as they might add 70 commits in a
CodeReview.
My goal is to enable these users to display the diff of multiple commits
that they consider to be related together.
> So, I would say, the best solution is to reorder the commits in a better
> manageable order. You do know about git rebase --interactive, don't you?
>
It would be great if users knew how to do that, and I wouldn't have to
explore such unconventional technical solutions :(
> -- Hannes
>
^ permalink raw reply
* [Outreachy] [PATCH v2] t/t7601: use "test_path_is_file"etc. instead of "test -f"
From: Dorcas AnonoLitunya @ 2023-10-18 12:45 UTC (permalink / raw)
To: christian.couder; +Cc: anonolitunya, git, gitster
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(-)
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()
--
2.42.0.345.gaab89be2eb
^ permalink raw reply related
* Re: Git does not work on my machine!!!
From: Matthias Aßhauer @ 2023-10-18 9:44 UTC (permalink / raw)
To: John Trites; +Cc: git
In-Reply-To: <002301da006e$2e2d3c50$8a87b4f0$@tritesengserv.com>
On Mon, 16 Oct 2023, jtrites@tritesengserv.com wrote:
> I would appreciate some help resolving this issue!!!
>
> JohnTrites@MSI MINGW64 /c/Users/JohnTrites
> $ git config --global user.name "JohnTrites"
> error: could not lock config file C:/Users/John
> Trites/AppData/Roaming/SPB_16.6/.gitconfig: No such file or directory
Hi John,
it looks like some of your environment variables are misconfigured.
This could be $HOME or $HOMEDRIVE and $HOMEPATH or $USERPROFILE, that
points to a seemingly non-existent Directory:
"C:/Users/John Trites/AppData/Roaming/SPB_16.6/"
hope that helps.
Best regards
Matthias
^ permalink raw reply
* [PATCH] git-gui: add support for filenames starting with tilde
From: Matthias Aßhauer via GitGitGadget @ 2023-10-18 8:50 UTC (permalink / raw)
To: git; +Cc: Pratyush Yadav, Matthias Aßhauer, Matthias Aßhauer
From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
When git-gui encounters a file name starting with a tilde character (~),
TCL "helpfully" expands that tilde into a (probably non-existing) users
home directory. But in git-gui we're often not dealing with user supplied
paths, where such an expansion might be expected, but actual names of files.
Prevent TCL from doing tilde expansion on these literal filenames.
This fixes https://github.com/git-for-windows/git/issues/4349
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
---
git-gui: add support for filenames starting with tilde
I've originally submitted this patch as a pull request to Pratyush's
git-gui repository [1] on March 23rd, but that hasn't received any
feedback in almost seven months, so I'm resubmitting it on the mailing
list, with an improved patch description.
[1] https://github.com/prati0100/git-gui/pull/96
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1599%2Frimrul%2Fgit-gui-tilde-filenames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1599/rimrul/git-gui-tilde-filenames-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1599
git-gui/git-gui.sh | 1 +
git-gui/lib/diff.tcl | 1 +
git-gui/lib/index.tcl | 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 3e5907a4609..e8d60cf3324 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2273,6 +2273,7 @@ proc do_explore {} {
# Open file relative to the working tree by the default associated app.
proc do_file_open {file} {
global _gitworktree
+ if {[string index $file 0] eq {~}} {set file ./$file}
set explorer [get_explorer]
set full_file_path [file join $_gitworktree $file]
exec $explorer [file nativename $full_file_path] &
diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index 871ad488c2a..b7686caa962 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -190,6 +190,7 @@ proc show_other_diff {path w m cont_info} {
set max_sz 100000
set type unknown
if {[catch {
+ if {[string index $path 0] eq {~}} {set path ./$path}
set type [file type $path]
switch -- $type {
directory {
diff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl
index d2ec24bd80e..2fbe1447975 100644
--- a/git-gui/lib/index.tcl
+++ b/git-gui/lib/index.tcl
@@ -617,7 +617,7 @@ proc delete_helper {path_list path_index deletion_errors batch_size \
set path [lindex $path_list $path_index]
- set deletion_failed [catch {file delete -- $path} deletion_error]
+ set deletion_failed [catch {file delete -- ./$path} deletion_error]
if {$deletion_failed} {
lappend deletion_errors [list "$deletion_error"]
base-commit: a9ecda2788e229afc9b611acaa26d0d9d4da53ed
--
gitgitgadget
^ permalink raw reply related
* Новости ЕРЗ.РФ
From: ЕРЗ.РФ @ 2023-10-18 6:24 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 14602 bytes --]
/* ///////// CLIENT-SPECIFIC STYLES ///////// */#outlook a{padding:0;} /* Force Outlook to provide a 'view in browser' message */.ReadMsgBody{width:100%;} .ExternalClass{width:100%;} /* Force Hotmail to display emails at full width */.ExternalClass, .ExternalClass p, .ExternalClass span, .ExternalClass font, .ExternalClass td, .ExternalClass div {line-height: 100%;} /* Force Hotmail to display normal line spacing */body, table, td, p, a, li, blockquote{-webkit-text-size-adjust:100%; -ms-text-size-adjust:100%;} /* Prevent WebKit and Windows mobile changing default text sizes */table, td{mso-table-lspace:0pt; mso-table-rspace:0pt;} /* Remove spacing between tables in Outlook 2007 and up */img{-ms-interpolation-mode:bicubic;} /* Allow smoother rendering of resized image in Internet Explorer *//* ///////// RESET STYLES ///////// */body{margin:0; padding:0;}img{border:0; height:auto; line-height:100%; outline:none; text-decoration:none;}table{border-collapse:collapse !important;}table td { border-collapse: collapse !important;}body, #bodyTable, #bodyCell{height:100% !important; margin:0; padding:0; width:100% !important;}#mailBody.mailBody .uni-block.button-block { display:block; } /* Specific ukr.net style*/body {margin: 0;text-align: left;}pre {white-space: pre;white-space: pre-wrap;word-wrap: break-word;}table.mhLetterPreview { width:100%; }table {mso-table-lspace:0pt;mso-table-rspace:0pt;}img {-ms-interpolation-mode:bicubic;}@media all and (max-width: 480px), only screen and (max-device-width : 480px) { body{width:100% !important; min-width:100% !important;} /* Prevent iOS Mail from adding padding to the body */ table[class='container-table'] { width:100% !important; } td.image-wrapper { padding: 0 !important; } td.image-wrapper, td.text-wrapper { display:block !important; width:100% !important; max-width:initial !important; } table[class='wrapper1'] { table-layout: fixed !important; padding: 0 !important; max-width: 600px !important; } td[class='wrapper-row'] { table-layout: fixed !important; box-sizing: border-box !important; width:100% !important; min-width:100% !important; } table[class='wrapper2'] { table-layout: fixed !important; border: none !important; width: 100% !important; max-width: 600px !important; min-height: 520px !important; } div[class='column-wrapper']{ max-width:300px !important; } table.uni-block { max-width:100% !important; height:auto !important; min-height: auto !important; } table[class='block-wrapper-inner-table'] { height:auto !important; min-height: auto !important; } td[class='block-wrapper'] { height:auto !important; min-height: auto !important; } .submit-button-block .button-wrapper { height: auto !important; width: auto !important; min-height: initial !important; max-height: initial !important; min-width: initial !important; max-width: initial !important; } img[class='image-element'] { height:auto !important; box-sizing: border-box !important; }}@media all and (max-width: 615px), only screen and (max-device-width : 615px) { td[class='wrapper-row'] { padding: 0 !important; margin: 0 !important; } .column { width:100% !important; max-width:100% !important; }}
(https://erzrf.ru)
НОВОСТИ
21 мая 2021 18:27
По программе поддержки региональных застройщиков Банк ДОМ.РФ выделил полмиллиарда рублей владимирскому девелоперу СЗ Ренова (https://erzrf.ru/news/po-programme-podderzhki-regionalnykh-zastroyshchikov-bank-domrf-vydelil-polmilliarda-rubley--vladimirskomu-developeru-sz-renova)
Средства кредитной линии в размере 499 млн руб. будут направлены в рамках госпрограммы поддержки низкомаржинальных застройщиков на строительство ЖК «Наследие» во Владимире, сообщает пресс-служба кредитной организации.
21 мая 2021 16:30
Новые изменения в положениях о дачной амнистии (https://erzrf.ru/news/novyye-izmeneniya-v-polozheniyakh-o-dachnoy-amnistii)
В Госдуму внесен проект федерального закона «О внесении изменения в Федеральный закон «О государственной регистрации недвижимости».
21 мая 2021 15:20
В Москве решили стимулировать застройщиков строить объекты образования наряду с жильем (https://erzrf.ru/news/v-moskve-reshili-stimulirovat-zastroyshchikov-stroit-obyekty-obrazovaniya-naryadu-s-zhilyem)
На последнем заседании Мосгордумы (МГД) большинство ее депутатов проголосовали за соответствующий законопроект «О внесении изменения в статью 7 Закона города Москвы от 19 декабря 2007 года №48 «О землепользовании в городе Москве».
21 мая 2021 11:24
ЕНиР разрешат применять на добровольной основе (https://erzrf.ru/news/-yenir-razreshat-primenyat-na-dobrovolnoy-osnove)
На федеральном портале проектов нормативных правовых актов опубликован проект Постановления Правительства РФ «О внесении изменений в постановление Правительства РФ от 13.06.2020 №857».
21 мая 2021 10:44
Эксперты: пока российским банкам выгодно снижать ставки по льготной ипотеке, но скоро это закончится (https://erzrf.ru/news/eksperty-poka-rossiyskim-bankam-vygodno-snizhat-stavki-po-lgotnoy-ipoteke-no-skoro-eto-zakonchitsya)
По мнению представителей рейтинговых агентств, опрошенных РИА Недвижимость, банки активно делают это, стремясь извлечь дополнительный процентный доход, привлечь качественных клиентов и нарастить выдачи до 1 июля — предполагаемого окончания госпрограммы льготной ипотеки.
21 мая 2021 10:00
Банк России: эскроу — самый быстрорастущий и доступный для застройщиков сегмент банковского корпоративного кредитования (https://erzrf.ru/news/bank-rossii-eskrou--samyy-bystrorastushchiy-i-dostupnyy-dlya-zastroyshchikov-segment-bankovskogo-korporativnogo-kreditovaniya)
Аналитики ЦБ подготовили обзор, посвященный проектному финансированию жилья по итогам I квартала 2021 года.
21 мая 2021 08:54
С января по апрель 2021 года положительное заключение экспертизы получили на треть больше больше МКД, чем годом ранее (графики) (https://erzrf.ru/news/s-yanvarya-po-aprel-2021-goda-polozhitelnoye-zaklyucheniye-ekspertizy-poluchili-na-tret-bolshe-bolshe-mkd-chem-godom-raneye-grafiki)
В апреле 2021 года положительное заключение экспертизы проектной документации получили 212 объектов от 149 застройщиков.
Еще больше новостей в Telegram-канале ЕРЗ.РФ t.me/erzrf (https://t.me/erzrf)
(https://talks.gmk.ru/maximum?utm_source=10/06_erzemail&utm_medium=10/06_erzemail&utm_campaign=10/06_erzemail)
ТОП НОВОСТЕЙ
19 мая 2021 19:34
«Деловая Россия» направила в Правительство предложения по снижению сроков строительства и его себестоимости (https://erzrf.ru/news/delovaya-rossiya-napravila-v-pravitelstvo-predlozheniya-po-snizheniyu-srokov-stroitelstva-i-yego-sebestoimosti)
Соответствующее письмо за подписью председателя организации Алексея Репика составлено на основе поступивших обращений от застройщиков и адресовано профильному вице-премьеру Марату Хуснуллину, сообщают «Известия», в распоряжении которых оказался текст послания.
19 мая 2021 18:31
ТОП застройщиков по потребительским качествам ЖК: в мае 2021 года в тройку лидеров вошла ИСГ Мармакс (https://erzrf.ru/news/top-zastroyshchikov-po-potrebitelskim-kachestvam-zhk-v-maye-2021-goda-v-troyku-liderov-voshla-isg-marmaks)
Рязанская компания ИСГ Мармакс вышла на третье место с оценкой 94,64 балла.
19 мая 2021 12:51
Пилотный проект ГК Самолет по продаже меблированного жилья: подробности (https://erzrf.ru/news/pilotnyy-proyekt-gk-samolet-po-prodazhe-meblirovannogo-zhilya-podrobnosti)
Первый пул квартир с мебелью доступен покупателям в двух жилых комплексах — в Новой Москве и подмосковных Люберцах, сообщили в пресс-службе застройщика.
19 мая 2021 13:42
Порядок повторного межевания ЗУ для КРТ упростят (https://erzrf.ru/news/poryadok-povtornogo-mezhevaniya-zu-dlya-krt-uprostyat)
В Госдуму внесен проект федерального закона «О внесении изменений в Земельный кодекс РФ и отдельные законодательные акты РФ».
11 февраля 2021 18:08
Средняя рыночная стоимость 1 кв. м жилья на II квартал 2021 года: смена лидеров в регионах с самым дешевым жильем (https://erzrf.ru/news/srednyaya-rynochnaya-stoimost-1-kv-m-zhilya-na-ii-kvartal-2021-goda-smena-liderov-v-regionakh-s-samym-deshevym-zhilyem?search=%D1%81%D1%80%D0%B5%D0%B4%D0%BD%D1%8F%D1%8F)
На портале проектов нормативных правовых актов опубликован проект приказа Минстроя России «О показателях средней рыночной стоимости одного квадратного метра общей площади жилого помещения по субъектам Российской Федерации на II квартал 2021 года».
(https://talks.gmk.ru/maximum?utm_source=10/06_erzemail&utm_medium=10/06_erzemail&utm_campaign=10/06_erzemail)
ПУБЛИКАЦИИ
18 мая 2021 13:06
Группа «Эталон» выстроит бесшовный процесс цифровой сделки с помощью решений Profitbase (https://erzrf.ru/publikacii/gruppa-etalon-vystroit-besshovnyy-protsess-tsifrovoy-sdelki-s-pomoshchyu-resheniy-profitbase)
14 мая 2021 13:31
Новый алгоритм привлечения трудовых мигрантов: все этапы (https://erzrf.ru/publikacii/novyy-algoritm-privlecheniya-trudovykh-migrantov-vse-etapy)
13 мая 2021 11:02
Нюансы изъятия недвижимости для целей КРТ (https://erzrf.ru/publikacii/nyuansy-izyatiya-nedvizhimosti-dlya-tseley-krt)
12 мая 2021 17:10
Институт экономики города: главный недостаток системы строительных сберкасс — ее полная зависимость от бюджетных субсидий (https://erzrf.ru/publikacii/institut-ekonomiki-goroda-glavnyy-nedostatok-sistemy-stroitelnykh-sberkass--yeye-polnaya-zavisimost-ot-byudzhetnykh-subsidiy)
26 апреля 2021 08:45
Особенности использования земельных участков для дошкольных образовательных учреждений встроенно-пристроенного типа (https://erzrf.ru/publikacii/osobennosti-ispolzovaniya-zemelnykh-uchastkov-dlya-doshkolnykh-obrazovatelnykh-uchrezhdeniy-vstroyenno-pristroyennogo-tipa)
26 апреля 2021 08:40
Profitbase — в едином реестре российского ПО (https://erzrf.ru/publikacii/profitbase--v-yedinom-reyestre-rossiyskogo-po)
________________________________________________________________________ (https://erzrf.ru/publikacii/aktualnyye-voprosy-stroitelstva-mnogokvartirnykh-domov-na-zemlyakh-prednaznachennykh-dlya-izhs)
(https://www.facebook.com/1821125691540760/) (https://twitter.com/erzrf_ru) (https://www.instagram.com/erzrf.ru/) (https://vk.com/erzrf_ru) (https://t.me/erzrf)
Редакция портала «Единый ресурс застройщиков» с благодарностью примет для размещения любые интересные для читателей материалы в сфере жилищного строительства: решения судов; разъяснения законодательства; проекты нормативных актов; аналитические обзоры; высказывания профессионалов; ссылки на интересные публикации и т.д. Чтобы отправить нам материалы, перейдите по ссылке (https://docs.google.com/forms/d/1Y4y2vhvo0W7syVLflIjEXJxzyCAiwy1FWOy6l6lwMK0/viewform?edit_requested=true).
Единый ресурс застройщиков (ЕРЗ.РФ) https://erzrf.ru (https://erzrf.ru/?region=moskva®ionKey=143443001&costType=1)
Чтобы отписаться от этой рассылки, перейдите по ссылке (https://emlstart.com/ru/unsubscribe?hash=64nkiebxrhmtz9cs998ee5suekpatocribkobtoqz8ajjtc34eic3nxwtuw8adqi18km8h3rjf9tkdis59efmo7wi4h#no_tracking)
[-- Attachment #1.2: Type: text/html, Size: 193831 bytes --]
^ permalink raw reply
* Re: [PATCH 00/11] t: reduce direct disk access to data structures
From: Patrick Steinhardt @ 2023-10-18 5:39 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3303 bytes --]
On Wed, Oct 18, 2023 at 07:35:03AM +0200, Patrick Steinhardt wrote:
> Hi,
>
> 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:
>
> - We increase test coverage of our own code base.
>
> - We become less dependent on the actual on-disk format.
>
> The main motivation for this patch series was the second bullet point as
> it is preparatory work to get the reftable backend upstreamed. My intent
> is to get rid of many or even most of the current blockers in the Git
> project before trying to send the reftable implementation upstream.
> While this will be a lot of up-front work that is going to span over a
> long time period, I think this approach will make everyones live easier
> by doing comparatively small and incremental improvements to the Git
> project. Ultimately, the final patch series should in the best case only
> contain the new backend as well as testing infrastructure, but not much
> else.
>
> Patrick
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...
Patrick
> Patrick Steinhardt (11):
> t: add helpers to test for reference existence
> t: allow skipping expected object ID in `ref-store update-ref`
> t: convert tests to use helpers for reference existence
> t: convert tests to not write references via the filesystem
> t: convert tests to not access symrefs via the filesystem
> t: convert tests to not access reflog via the filesystem
> t1450: convert tests to remove worktrees via git-worktree(1)
> t4207: delete replace references via git-update-ref(1)
> t7300: assert exact states of repo
> t7900: assert the absence of refs via git-for-each-ref(1)
> t: mark several tests that assume the files backend with REFFILES
>
> t/README | 9 ++++
> t/helper/test-ref-store.c | 38 +++++++++++++--
> t/t1400-update-ref.sh | 49 ++++++++++----------
> t/t1430-bad-ref-name.sh | 39 ++++++++++------
> t/t1450-fsck.sh | 46 ++++++++++---------
> t/t2011-checkout-invalid-head.sh | 16 +++----
> t/t3200-branch.sh | 74 ++++++++++++++++--------------
> t/t3400-rebase.sh | 2 +-
> t/t3404-rebase-interactive.sh | 2 +-
> t/t4013-diff-various.sh | 2 +-
> t/t4202-log.sh | 2 +-
> t/t4207-log-decoration-colors.sh | 4 +-
> t/t5521-pull-options.sh | 4 +-
> t/t5526-fetch-submodules.sh | 2 +-
> t/t5605-clone-local.sh | 6 +--
> t/t5702-protocol-v2.sh | 24 +++++++---
> t/t7300-clean.sh | 23 ++++++----
> t/t7900-maintenance.sh | 3 +-
> t/t9133-git-svn-nested-git-repo.sh | 2 +-
> t/test-lib-functions.sh | 66 ++++++++++++++++++++++++++
> 20 files changed, 277 insertions(+), 136 deletions(-)
>
> --
> 2.42.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 11/11] t: mark several tests that assume the files backend with REFFILES
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 8915 bytes --]
Add the REFFILES prerequisite to several tests that assume we're using
the files backend. There are various reasons why we cannot easily
convert those tests to be backend-independent, where the most common
one is that we have no way to write corrupt references into the refdb
via our tooling. We may at a later point in time grow the tooling to
make this possible, but for now we just mark these tests as requiring
the files backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 8 ++++----
t/t1450-fsck.sh | 6 +++---
t/t2011-checkout-invalid-head.sh | 16 ++++++++--------
t/t3200-branch.sh | 8 ++++----
t/t3400-rebase.sh | 2 +-
t/t5605-clone-local.sh | 2 +-
6 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b1d2c014132..feeba6ed7cd 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -236,7 +236,7 @@ test_expect_success 'update-ref --no-deref -d can delete self-reference' '
test_must_fail git show-ref --verify -q refs/heads/self
'
-test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '
+test_expect_success REFFILES 'update-ref --no-deref -d can delete reference to bad ref' '
>.git/refs/heads/bad &&
test_when_finished "rm -f .git/refs/heads/bad" &&
git symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&
@@ -288,7 +288,7 @@ test_expect_success "set $m (logged by touch)" '
test $A = $(git show-ref -s --verify $m)
'
-test_expect_success 'empty directory removal' '
+test_expect_success REFFILES 'empty directory removal' '
git branch d1/d2/r1 HEAD &&
git branch d1/r2 HEAD &&
test_path_is_file .git/refs/heads/d1/d2/r1 &&
@@ -300,7 +300,7 @@ test_expect_success 'empty directory removal' '
test_path_is_file .git/logs/refs/heads/d1/r2
'
-test_expect_success 'symref empty directory removal' '
+test_expect_success REFFILES 'symref empty directory removal' '
git branch e1/e2/r1 HEAD &&
git branch e1/r2 HEAD &&
git checkout e1/e2/r1 &&
@@ -1638,7 +1638,7 @@ test_expect_success PIPE 'transaction flushes status updates' '
test_cmp expected actual
'
-test_expect_success 'directory not created deleting packed ref' '
+test_expect_success REFFILES 'directory not created deleting packed ref' '
git branch d1/d2/r1 HEAD &&
git pack-refs --all &&
test_path_is_missing .git/refs/heads/d1/d2 &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index d356605d132..ca5a2719a19 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -122,7 +122,7 @@ test_expect_success 'branch pointing to non-commit' '
test_i18ngrep "not a commit" out
'
-test_expect_success 'HEAD link pointing at a funny object' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object' '
saved_head=$(git rev-parse --verify HEAD) &&
test_when_finished "git update-ref HEAD ${saved_head}" &&
echo $ZERO_OID >.git/HEAD &&
@@ -140,7 +140,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
test_i18ngrep "HEAD points to something strange" out
'
-test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' '
saved_head=$(git rev-parse --verify HEAD) &&
test_when_finished "git update-ref HEAD $saved_head" &&
test_when_finished "git worktree remove -f wt" &&
@@ -151,7 +151,7 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
test_i18ngrep "main-worktree/HEAD: detached HEAD points" out
'
-test_expect_success 'other worktree HEAD link pointing at a funny object' '
+test_expect_success REFFILES 'other worktree HEAD link pointing at a funny object' '
test_when_finished "git worktree remove -f other" &&
git worktree add other &&
echo $ZERO_OID >.git/worktrees/other/HEAD &&
diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
index d9997e7b6b4..3c8135831b8 100755
--- a/t/t2011-checkout-invalid-head.sh
+++ b/t/t2011-checkout-invalid-head.sh
@@ -18,18 +18,18 @@ test_expect_success 'checkout should not start branch from a tree' '
test_must_fail git checkout -b newbranch main^{tree}
'
-test_expect_success 'checkout main from invalid HEAD' '
+test_expect_success REFFILES 'checkout main from invalid HEAD' '
echo $ZERO_OID >.git/HEAD &&
git checkout main --
'
-test_expect_success 'checkout notices failure to lock HEAD' '
+test_expect_success REFFILES 'checkout notices failure to lock HEAD' '
test_when_finished "rm -f .git/HEAD.lock" &&
>.git/HEAD.lock &&
test_must_fail git checkout -b other
'
-test_expect_success 'create ref directory/file conflict scenario' '
+test_expect_success REFFILES 'create ref directory/file conflict scenario' '
git update-ref refs/heads/outer/inner main &&
# do not rely on symbolic-ref to get a known state,
@@ -39,26 +39,26 @@ test_expect_success 'create ref directory/file conflict scenario' '
}
'
-test_expect_success 'checkout away from d/f HEAD (unpacked, to branch)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to branch)' '
reset_to_df &&
git checkout main
'
-test_expect_success 'checkout away from d/f HEAD (unpacked, to detached)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to detached)' '
reset_to_df &&
git checkout --detach main
'
-test_expect_success 'pack refs' '
+test_expect_success REFFILES 'pack refs' '
git pack-refs --all --prune
'
-test_expect_success 'checkout away from d/f HEAD (packed, to branch)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to branch)' '
reset_to_df &&
git checkout main
'
-test_expect_success 'checkout away from d/f HEAD (packed, to detached)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to detached)' '
reset_to_df &&
git checkout --detach main
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 933aa9eebbd..606c50fe66c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -28,7 +28,7 @@ test_expect_success 'git branch --help should not have created a bogus branch' '
test_ref_missing refs/heads/--help
'
-test_expect_success 'branch -h in broken repository' '
+test_expect_success REFFILES 'branch -h in broken repository' '
mkdir broken &&
(
cd broken &&
@@ -245,7 +245,7 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
git worktree prune
'
-test_expect_success 'git branch -M fails if updating any linked working tree fails' '
+test_expect_success REFFILES 'git branch -M fails if updating any linked working tree fails' '
git worktree add -b baz bazdir1 &&
git worktree add -f bazdir2 baz &&
touch .git/worktrees/bazdir1/HEAD.lock &&
@@ -836,14 +836,14 @@ test_expect_success 'renaming a symref is not allowed' '
test_ref_missing refs/heads/new-topic
'
-test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
+test_expect_success SYMLINKS,REFFILES 'git branch -m u v should fail when the reflog for u is a symlink' '
git branch --create-reflog u &&
mv .git/logs/refs/heads/u real-u &&
ln -s real-u .git/logs/refs/heads/u &&
test_must_fail git branch -m u v
'
-test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
+test_expect_success SYMLINKS,REFFILES 'git branch -m with symlinked .git/refs' '
test_when_finished "rm -rf subdir" &&
git init --bare subdir &&
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index d3df19a51f8..435943a0891 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -424,7 +424,7 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '
test_i18ngrep "already used by worktree at" err
'
-test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
+test_expect_success REFFILES,MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
git checkout main &&
mv .git/logs actual_logs &&
cmd //c "mklink /D .git\logs ..\actual_logs" &&
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index bedd29d0550..a3055869bc7 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -157,7 +157,7 @@ test_expect_success 'cloning locally respects "-u" for fetching refs' '
test_must_fail git clone --bare -u false a should_not_work.git
'
-test_expect_success 'local clone from repo with corrupt refs fails gracefully' '
+test_expect_success REFFILES 'local clone from repo with corrupt refs fails gracefully' '
git init corrupt &&
test_commit -C corrupt one &&
echo a >corrupt/.git/refs/heads/topic &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 10/11] t7900: assert the absence of refs via git-for-each-ref(1)
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]
We're asserting that a prefetch of remotes via git-maintenance(1)
doesn't write any references in refs/remotes by validating that the
directory ".git/refs/remotes" is missing. This is quite roundabout: we
don't care about the directory existing, we care about the references
not existing, and the way these are stored is on the behest of the
reference database.
Convert the test to instead check via git-for-each-ref(1) whether any
remote reference exist.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t7900-maintenance.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index e56f5980dc4..cefecee732f 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -157,7 +157,8 @@ test_expect_success 'prefetch multiple remotes' '
fetchargs="--prefetch --prune --no-tags --no-write-fetch-head --recurse-submodules=no --quiet" &&
test_subcommand git fetch remote1 $fetchargs <run-prefetch.txt &&
test_subcommand git fetch remote2 $fetchargs <run-prefetch.txt &&
- test_path_is_missing .git/refs/remotes &&
+ git for-each-ref refs/remotes >actual &&
+ test_must_be_empty actual &&
git log prefetch/remotes/remote1/one &&
git log prefetch/remotes/remote2/two &&
git fetch --all &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 09/11] t7300: assert exact states of repo
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]
Some of the tests in t7300 verify that git-clean(1) doesn't touch
repositories that are embedded into the main repository. This is done by
asserting a small set of substructures that are assumed to always exist,
like the "refs/", "objects/" or "HEAD". This has the downside that we
need to assume a specific repository structure that may be subject to
change when new backends for the refdb land. At the same time, we don't
thoroughly assert that git-clean(1) really didn't end up cleaning any
files in the repository either.
Convert the tests to instead assert that all files continue to exist
after git-clean(1) by comparing a file listing via find(1) before and
after executing clean. This makes our actual assertions stricter while
having to care less about the repository's actual on-disk format.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t7300-clean.sh | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 0ef7b784573..d7d9202f37f 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -517,8 +517,12 @@ test_expect_success 'nested (empty) git should be kept' '
git init empty_repo &&
mkdir to_clean &&
>to_clean/should_clean.this &&
+ # Note that we put the expect file in the .git directory so that it
+ # does not get cleaned.
+ find empty_repo | sort >.git/expect &&
git clean -f -d &&
- test_path_is_file empty_repo/.git/HEAD &&
+ find empty_repo | sort >actual &&
+ test_cmp .git/expect actual &&
test_path_is_missing to_clean
'
@@ -559,10 +563,10 @@ test_expect_success 'giving path in nested git work tree will NOT remove it' '
mkdir -p bar/baz &&
test_commit msg bar/baz/hello.world
) &&
+ find repo | sort >expect &&
git clean -f -d repo/bar/baz &&
- test_path_is_file repo/.git/HEAD &&
- test_path_is_dir repo/bar/ &&
- test_path_is_file repo/bar/baz/hello.world
+ find repo | sort >actual &&
+ test_cmp expect actual
'
test_expect_success 'giving path to nested .git will not remove it' '
@@ -573,10 +577,10 @@ test_expect_success 'giving path to nested .git will not remove it' '
git init &&
test_commit msg hello.world
) &&
+ find repo | sort >expect &&
git clean -f -d repo/.git &&
- test_path_is_file repo/.git/HEAD &&
- test_path_is_dir repo/.git/refs &&
- test_path_is_dir repo/.git/objects &&
+ find repo | sort >actual &&
+ test_cmp expect actual &&
test_path_is_dir untracked/
'
@@ -588,9 +592,10 @@ test_expect_success 'giving path to nested .git/ will NOT remove contents' '
git init &&
test_commit msg hello.world
) &&
+ find repo | sort >expect &&
git clean -f -d repo/.git/ &&
- test_path_is_dir repo/.git &&
- test_path_is_file repo/.git/HEAD &&
+ find repo | sort >actual &&
+ test_cmp expect actual &&
test_path_is_dir untracked/
'
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 08/11] t4207: delete replace references via git-update-ref(1)
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]
In t4207 we set up a set of replace objects via git-replace(1). Because
these references should not be impacting subsequent tests we also set up
some cleanup logic that deletes the replacement references via a call to
`rm -rf`. This reaches into the internal implementation details of the
reference backend and will thus break when we grow an alternative refdb
implementation.
Refactor the tests to delete the replacement refs via Git commands so
that we become independent of the actual refdb that's in use. As we
don't have a nice way to delete all replacements or all references in a
certain namespace, we opt for a combination of git-for-each-ref(1) and
git-update-ref(1)'s `--stdin` mode.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t4207-log-decoration-colors.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 21986a866df..d138e513a04 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -71,7 +71,7 @@ ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
'
test_expect_success 'test coloring with replace-objects' '
- test_when_finished rm -rf .git/refs/replace* &&
+ test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
test_commit C &&
test_commit D &&
@@ -99,7 +99,7 @@ EOF
'
test_expect_success 'test coloring with grafted commit' '
- test_when_finished rm -rf .git/refs/replace* &&
+ test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
git replace --graft HEAD HEAD~2 &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 07/11] t1450: convert tests to remove worktrees via git-worktree(1)
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2562 bytes --]
Some of our tests in t1450 create worktrees and then corrupt them.
As it is impossible to delete such worktrees via a normal call to `git
worktree remove`, we instead opt to remove them manually by calling
rm(1) instead.
This is ultimately unnecessary though as we can use the `-f` switch to
remove the worktree. Let's convert the tests to do so such that we don't
have to reach into internal implementation details of worktrees.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1450-fsck.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 804a5594ddd..d356605d132 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -143,7 +143,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
saved_head=$(git rev-parse --verify HEAD) &&
test_when_finished "git update-ref HEAD $saved_head" &&
- test_when_finished "rm -rf .git/worktrees wt" &&
+ test_when_finished "git worktree remove -f wt" &&
git worktree add wt &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
@@ -152,7 +152,7 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
'
test_expect_success 'other worktree HEAD link pointing at a funny object' '
- test_when_finished "rm -rf .git/worktrees other" &&
+ test_when_finished "git worktree remove -f other" &&
git worktree add other &&
echo $ZERO_OID >.git/worktrees/other/HEAD &&
test_must_fail git fsck 2>out &&
@@ -160,7 +160,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny object' '
'
test_expect_success 'other worktree HEAD link pointing at missing object' '
- test_when_finished "rm -rf .git/worktrees other" &&
+ test_when_finished "git worktree remove -f other" &&
git worktree add other &&
object_id=$(echo "Contents missing from repo" | git hash-object --stdin) &&
test-tool -C other ref-store main update-ref msg HEAD $object_id "" REF_NO_DEREF,REF_SKIP_OID_VERIFICATION &&
@@ -169,7 +169,7 @@ test_expect_success 'other worktree HEAD link pointing at missing object' '
'
test_expect_success 'other worktree HEAD link pointing at a funny place' '
- test_when_finished "rm -rf .git/worktrees other" &&
+ test_when_finished "git worktree remove -f other" &&
git worktree add other &&
git -C other symbolic-ref HEAD refs/funny/place &&
test_must_fail git fsck 2>out &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 06/11] t: convert tests to not access reflog via the filesystem
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7125 bytes --]
Some of our tests reach directly into the filesystem in order to both
read or modify the reflog, which will break once we have a second
reference backend in our codebase that stores reflogs differently.
Refactor these tests to either use git-reflog(1) or the ref-store test
helper. Note that the refactoring to use git-reflog(1) also requires us
to adapt our expectations in some cases where we previously verified the
exact on-disk log entries. This seems like an acceptable tradeoff though
to ensure that different backends have the same user-visible behaviour
as any user would typically use git-reflog(1) anyway to access the logs.
Any backend-specific verification of the written on-disk format should
be implemented in a separate, backend-specific test.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 17 +++++++++++------
t/t3200-branch.sh | 24 ++++++++++++------------
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 5f505e2f353..b1d2c014132 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -90,7 +90,8 @@ test_expect_success "deleting current branch adds message to HEAD's log" '
git symbolic-ref HEAD $m &&
git update-ref -m delete-$m -d $m &&
test_must_fail git show-ref --verify -q $m &&
- grep "delete-$m$" .git/logs/HEAD
+ test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+ grep "delete-$m$" actual
'
test_expect_success "deleting by HEAD adds message to HEAD's log" '
@@ -99,7 +100,8 @@ test_expect_success "deleting by HEAD adds message to HEAD's log" '
git symbolic-ref HEAD $m &&
git update-ref -m delete-by-head -d HEAD &&
test_must_fail git show-ref --verify -q $m &&
- grep "delete-by-head$" .git/logs/HEAD
+ test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+ grep "delete-by-head$" actual
'
test_expect_success 'update-ref does not create reflogs by default' '
@@ -130,7 +132,7 @@ test_expect_success 'creates no reflog in bare repository' '
test_expect_success 'core.logAllRefUpdates=true creates reflog in bare repository' '
test_when_finished "git -C $bare config --unset core.logAllRefUpdates && \
- rm $bare/logs/$m" &&
+ test-tool ref-store main delete-reflog $m" &&
git -C $bare config core.logAllRefUpdates true &&
git -C $bare update-ref $m $bareB &&
git -C $bare rev-parse $bareB >expect &&
@@ -263,7 +265,10 @@ test_expect_success "(not) changed .git/$m" '
! test $B = $(git show-ref -s --verify $m)
'
-rm -f .git/logs/refs/heads/main
+test_expect_success "clean up reflog" '
+ test-tool ref-store main delete-reflog $m
+'
+
test_expect_success "create $m (logged by touch)" '
test_config core.logAllRefUpdates false &&
GIT_COMMITTER_DATE="2005-05-26 23:30" \
@@ -316,7 +321,7 @@ $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000 Switch
$B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150860 +0000
EOF
test_expect_success "verifying $m's log (logged by touch)" '
- test_when_finished "git update-ref -d $m && rm -rf .git/logs actual expect" &&
+ test_when_finished "git update-ref -d $m && git reflog expire --expire=all --all && rm -rf actual expect" &&
test-tool ref-store main for-each-reflog-ent $m >actual &&
test_cmp actual expect
'
@@ -346,7 +351,7 @@ $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 +0000 Switch
$B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 +0000
EOF
test_expect_success "verifying $m's log (logged by config)" '
- test_when_finished "git update-ref -d $m && rm -rf .git/logs actual expect" &&
+ test_when_finished "git update-ref -d $m && git reflog expire --expire=all --all && rm -rf actual expect" &&
test-tool ref-store main for-each-reflog-ent $m >actual &&
test_cmp actual expect
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 874520e3f10..933aa9eebbd 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -76,14 +76,14 @@ test_expect_success 'git branch HEAD should fail' '
'
cat >expect <<EOF
-$ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from main
+$HEAD refs/heads/d/e/f@{0}: branch: Created from main
EOF
test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
test_ref_exists refs/heads/d/e/f &&
- test_path_is_file .git/logs/refs/heads/d/e/f &&
- test_cmp expect .git/logs/refs/heads/d/e/f
+ git reflog show --no-abbrev-commit refs/heads/d/e/f >actual &&
+ test_cmp expect actual
'
test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
@@ -203,10 +203,9 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
test $(git rev-parse --abbrev-ref HEAD) = bam
'
-test_expect_success 'git branch -M baz bam should add entries to .git/logs/HEAD' '
- msg="Branch: renamed refs/heads/baz to refs/heads/bam" &&
- grep " $ZERO_OID.*$msg$" .git/logs/HEAD &&
- grep "^$ZERO_OID.*$msg$" .git/logs/HEAD
+test_expect_success 'git branch -M baz bam should add entries to HEAD reflog' '
+ git reflog show HEAD >actual &&
+ grep "HEAD@{0}: Branch: renamed refs/heads/baz to refs/heads/bam" actual
'
test_expect_success 'git branch -M should leave orphaned HEAD alone' '
@@ -228,7 +227,7 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
test_expect_success 'resulting reflog can be shown by log -g' '
oid=$(git rev-parse HEAD) &&
cat >expect <<-EOF &&
- HEAD@{0} $oid $msg
+ HEAD@{0} $oid Branch: renamed refs/heads/baz to refs/heads/bam
HEAD@{2} $oid checkout: moving from foo to baz
EOF
git log -g --format="%gd %H %gs" -2 HEAD >actual &&
@@ -702,7 +701,8 @@ test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out'
test_expect_success 'git branch -C c1 c2 should never touch HEAD' '
msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
- ! grep "$msg$" .git/logs/HEAD
+ git reflog HEAD >actual &&
+ ! grep "$msg$" actual
'
test_expect_success 'git branch -C main should work when main is checked out' '
@@ -1143,14 +1143,14 @@ test_expect_success '--set-upstream-to notices an error to set branch as own ups
# Keep this test last, as it changes the current branch
cat >expect <<EOF
-$ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from main
+$HEAD refs/heads/g/h/i@{0}: branch: Created from main
EOF
test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git checkout -b g/h/i -l main &&
test_ref_exists refs/heads/g/h/i &&
- test_path_is_file .git/logs/refs/heads/g/h/i &&
- test_cmp expect .git/logs/refs/heads/g/h/i
+ git reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&
+ test_cmp expect actual
'
test_expect_success 'checkout -b makes reflog by default' '
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 05/11] t: convert tests to not access symrefs via the filesystem
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 13718 bytes --]
Some of our tests access symbolic references via the filesystem
directly. While this works with the current files reference backend, it
this will break once we have a second reference backend in our codebase.
Refactor these tests to instead use git-symbolic-ref(1) or our
`ref-store` test tool. The latter is required in some cases where safety
checks of git-symbolic-ref(1) would otherwise reject writing a symbolic
reference.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 8 ++++----
t/t1430-bad-ref-name.sh | 12 ++++++------
t/t1450-fsck.sh | 4 ++--
t/t3200-branch.sh | 9 ++++++---
t/t4013-diff-various.sh | 2 +-
t/t4202-log.sh | 2 +-
t/t5605-clone-local.sh | 2 +-
t/t5702-protocol-v2.sh | 24 ++++++++++++++++++------
t/t9133-git-svn-nested-git-repo.sh | 2 +-
9 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index cd24018ce99..5f505e2f353 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -221,15 +221,15 @@ test_expect_success 'delete symref without dereference when the referred ref is
test_expect_success 'update-ref -d is not confused by self-reference' '
git symbolic-ref refs/heads/self refs/heads/self &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
- test_path_is_file .git/refs/heads/self
+ git symbolic-ref --no-recurse refs/heads/self
'
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
git symbolic-ref refs/heads/self refs/heads/self &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
test_must_fail git show-ref --verify -q refs/heads/self
'
@@ -239,7 +239,7 @@ test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '
test_when_finished "rm -f .git/refs/heads/bad" &&
git symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&
test_when_finished "git update-ref -d refs/heads/ref-to-bad" &&
- test_path_is_file .git/refs/heads/ref-to-bad &&
+ git symbolic-ref --no-recurse refs/heads/ref-to-bad &&
git update-ref --no-deref -d refs/heads/ref-to-bad &&
test_must_fail git show-ref --verify -q refs/heads/ref-to-bad
'
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 7b7d6953c62..5debb91f7b7 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -164,9 +164,9 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
test_expect_success 'for-each-ref emits warnings for broken names' '
test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git for-each-ref >output 2>error &&
! grep -e "broken\.\.\.ref" output &&
@@ -257,7 +257,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
'
test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
@@ -267,7 +267,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name
'
test_expect_success 'branch -d can delete symref with broken name' '
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
@@ -277,7 +277,7 @@ test_expect_success 'branch -d can delete symref with broken name' '
'
test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
@@ -287,7 +287,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
'
test_expect_success 'branch -d can delete dangling symref with broken name' '
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 5cce24f1006..804a5594ddd 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -134,7 +134,7 @@ test_expect_success 'HEAD link pointing at a funny object' '
test_expect_success 'HEAD link pointing at a funny place' '
saved_head=$(git rev-parse --verify HEAD) &&
test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
- echo "ref: refs/funny/place" >.git/HEAD &&
+ test-tool ref-store main create-symref HEAD refs/funny/place &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
test_i18ngrep "HEAD points to something strange" out
@@ -171,7 +171,7 @@ test_expect_success 'other worktree HEAD link pointing at missing object' '
test_expect_success 'other worktree HEAD link pointing at a funny place' '
test_when_finished "rm -rf .git/worktrees other" &&
git worktree add other &&
- echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
+ git -C other symbolic-ref HEAD refs/funny/place &&
test_must_fail git fsck 2>out &&
test_i18ngrep "worktrees/other/HEAD points to something strange" out
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index bde4f1485b7..874520e3f10 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -215,10 +215,13 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
cd orphan &&
test_commit initial &&
git checkout --orphan lonely &&
- grep lonely .git/HEAD &&
+ git symbolic-ref HEAD >expect &&
+ echo refs/heads/lonely >actual &&
+ test_cmp expect actual &&
test_ref_missing refs/head/lonely &&
git branch -M main mistress &&
- grep lonely .git/HEAD
+ git symbolic-ref HEAD >expect &&
+ test_cmp expect actual
)
'
@@ -809,7 +812,7 @@ test_expect_success 'deleting a symref' '
test_expect_success 'deleting a dangling symref' '
git symbolic-ref refs/heads/dangling-symref nowhere &&
- test_path_is_file .git/refs/heads/dangling-symref &&
+ git symbolic-ref --no-recurse refs/heads/dangling-symref &&
echo "Deleted branch dangling-symref (was nowhere)." >expect &&
git branch -d dangling-symref >actual &&
test_ref_missing refs/heads/dangling-symref &&
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759..5abbea36b39 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -514,7 +514,7 @@ test_expect_success 'log -S requires an argument' '
'
test_expect_success 'diff --cached on unborn branch' '
- echo ref: refs/heads/unborn >.git/HEAD &&
+ git symbolic-ref HEAD refs/heads/unborn &&
git diff --cached >result &&
process_diffs result >actual &&
process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached" >expected &&
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index af4a123cd22..57b298a4e22 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2265,7 +2265,7 @@ test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
test_expect_success REFFILES 'log diagnoses bogus HEAD symref' '
git init empty &&
- echo "ref: refs/heads/invalid.lock" > empty/.git/HEAD &&
+ test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep broken stderr &&
test_must_fail git -C empty log --default totally-bogus 2>stderr &&
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 946c5751885..bedd29d0550 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -65,7 +65,7 @@ test_expect_success 'Even without -l, local will make a hardlink' '
'
test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
- echo "ref: refs/heads/nonexistent" > a.git/HEAD &&
+ git -C a.git symbolic-ref HEAD refs/heads/nonexistent &&
git clone a d &&
(cd d &&
git fetch &&
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 6af5c2062fd..dcc4cd95fe7 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -221,7 +221,9 @@ test_expect_success 'clone of empty repo propagates name of default branch' '
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
git -c init.defaultBranch=main -c protocol.version=2 \
clone "file://$(pwd)/file_empty_parent" file_empty_child &&
- grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+ echo refs/heads/mydefaultbranch >expect &&
+ git -C file_empty_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success '...but not if explicitly forbidden by config' '
@@ -234,7 +236,9 @@ test_expect_success '...but not if explicitly forbidden by config' '
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
git -c init.defaultBranch=main -c protocol.version=2 \
clone "file://$(pwd)/file_empty_parent" file_empty_child &&
- ! grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+ echo refs/heads/main >expect &&
+ git -C file_empty_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success 'bare clone propagates empty default branch' '
@@ -247,7 +251,9 @@ test_expect_success 'bare clone propagates empty default branch' '
git -c init.defaultBranch=main -c protocol.version=2 \
clone --bare \
"file://$(pwd)/file_empty_parent" file_empty_child.git &&
- grep "refs/heads/mydefaultbranch" file_empty_child.git/HEAD
+ echo "refs/heads/mydefaultbranch" >expect &&
+ git -C file_empty_child.git symbolic-ref HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success 'clone propagates unborn HEAD from non-empty repo' '
@@ -265,7 +271,9 @@ test_expect_success 'clone propagates unborn HEAD from non-empty repo' '
git -c init.defaultBranch=main -c protocol.version=2 \
clone "file://$(pwd)/file_unborn_parent" \
file_unborn_child 2>stderr &&
- grep "refs/heads/mydefaultbranch" file_unborn_child/.git/HEAD &&
+ echo "refs/heads/mydefaultbranch" >expect &&
+ git -C file_unborn_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
grep "warning: remote HEAD refers to nonexistent ref" stderr
'
@@ -295,7 +303,9 @@ test_expect_success 'bare clone propagates unborn HEAD from non-empty repo' '
git -c init.defaultBranch=main -c protocol.version=2 \
clone --bare "file://$(pwd)/file_unborn_parent" \
file_unborn_child.git 2>stderr &&
- grep "refs/heads/mydefaultbranch" file_unborn_child.git/HEAD &&
+ echo "refs/heads/mydefaultbranch" >expect &&
+ git -C file_unborn_child.git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
! grep "warning:" stderr
'
@@ -315,7 +325,9 @@ test_expect_success 'defaulted HEAD uses remote branch if available' '
git -c init.defaultBranch=branchwithstuff -c protocol.version=2 \
clone "file://$(pwd)/file_unborn_parent" \
file_unborn_child 2>stderr &&
- grep "refs/heads/branchwithstuff" file_unborn_child/.git/HEAD &&
+ echo "refs/heads/branchwithstuff" >expect &&
+ git -C file_unborn_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
test_path_is_file file_unborn_child/stuff.t &&
! grep "warning:" stderr
'
diff --git a/t/t9133-git-svn-nested-git-repo.sh b/t/t9133-git-svn-nested-git-repo.sh
index d8d536269cf..8ca24670acb 100755
--- a/t/t9133-git-svn-nested-git-repo.sh
+++ b/t/t9133-git-svn-nested-git-repo.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup repo with a git repo inside it' '
(
cd s &&
git init &&
- test -f .git/HEAD &&
+ git symbolic-ref HEAD &&
> .git/a &&
echo a > a &&
svn_cmd add .git a &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 04/11] t: convert tests to not write references via the filesystem
From: Patrick Steinhardt @ 2023-10-18 5:35 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9212 bytes --]
Some of our tests manually create, update or delete references by
writing the respective new values into the filesystem directly. While
this works with the current files reference backend, this will break
once we have a second reference backend implementation in our codebase.
Refactor these tests to instead use git-update-ref(1) or our `ref-store`
test tool. The latter is required in some cases where safety checks of
git-update-ref(1) would otherwise reject a reference update.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 16 +++++++---------
t/t1450-fsck.sh | 28 +++++++++++++++-------------
t/t3404-rebase-interactive.sh | 2 +-
t/t5526-fetch-submodules.sh | 2 +-
4 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 4d66cd7f4a1..cd24018ce99 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -9,8 +9,6 @@ test_description='Test git update-ref and basic ref logging'
Z=$ZERO_OID
m=refs/heads/main
-n_dir=refs/heads/gu
-n=$n_dir/fixes
outside=refs/foo
bare=bare-repo
@@ -62,10 +60,10 @@ test_expect_success "delete $m without oldvalue verification" '
test_must_fail git show-ref --verify -q $m
'
-test_expect_success "fail to create $n" '
- test_when_finished "rm -f .git/$n_dir" &&
- touch .git/$n_dir &&
- test_must_fail git update-ref $n $A
+test_expect_success "fail to create $n due to file/directory conflict" '
+ test_when_finished "git update-ref -d refs/heads/gu" &&
+ git update-ref refs/heads/gu $A &&
+ test_must_fail git update-ref refs/heads/gu/fixes $A
'
test_expect_success "create $m (by HEAD)" '
@@ -222,7 +220,7 @@ test_expect_success 'delete symref without dereference when the referred ref is
test_expect_success 'update-ref -d is not confused by self-reference' '
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
test_path_is_file .git/refs/heads/self
@@ -230,7 +228,7 @@ test_expect_success 'update-ref -d is not confused by self-reference' '
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
test_must_fail git show-ref --verify -q refs/heads/self
@@ -434,7 +432,7 @@ test_expect_success 'Query "main@{2005-05-28}" (past end of history)' '
test_i18ngrep -F "warning: log for ref $m unexpectedly ended on $ld" e
'
-rm -f .git/$m .git/logs/$m expect
+git update-ref -d $m
test_expect_success 'creating initial files' '
test_when_finished rm -f M &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 10a539158c4..5cce24f1006 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -115,15 +115,16 @@ test_expect_success 'zlib corrupt loose object output ' '
'
test_expect_success 'branch pointing to non-commit' '
- git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
+ tree_oid=$(git rev-parse --verify HEAD^{tree}) &&
+ test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
test_when_finished "git update-ref -d refs/heads/invalid" &&
test_must_fail git fsck 2>out &&
test_i18ngrep "not a commit" out
'
test_expect_success 'HEAD link pointing at a funny object' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
+ saved_head=$(git rev-parse --verify HEAD) &&
+ test_when_finished "git update-ref HEAD ${saved_head}" &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ -131,8 +132,8 @@ test_expect_success 'HEAD link pointing at a funny object' '
'
test_expect_success 'HEAD link pointing at a funny place' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
+ saved_head=$(git rev-parse --verify HEAD) &&
+ test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
echo "ref: refs/funny/place" >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ -140,10 +141,10 @@ test_expect_success 'HEAD link pointing at a funny place' '
'
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
+ saved_head=$(git rev-parse --verify HEAD) &&
+ test_when_finished "git update-ref HEAD $saved_head" &&
test_when_finished "rm -rf .git/worktrees wt" &&
git worktree add wt &&
- mv .git/HEAD .git/SAVED_HEAD &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail git -C wt fsck 2>out &&
@@ -161,7 +162,8 @@ test_expect_success 'other worktree HEAD link pointing at a funny object' '
test_expect_success 'other worktree HEAD link pointing at missing object' '
test_when_finished "rm -rf .git/worktrees other" &&
git worktree add other &&
- echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
+ object_id=$(echo "Contents missing from repo" | git hash-object --stdin) &&
+ test-tool -C other ref-store main update-ref msg HEAD $object_id "" REF_NO_DEREF,REF_SKIP_OID_VERIFICATION &&
test_must_fail git fsck 2>out &&
test_i18ngrep "worktrees/other/HEAD: invalid sha1 pointer" out
'
@@ -391,7 +393,7 @@ test_expect_success 'tag pointing to nonexistent' '
tag=$(git hash-object -t tag -w --stdin <invalid-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/invalid &&
+ git update-ref refs/tags/invalid $tag &&
test_when_finished "git update-ref -d refs/tags/invalid" &&
test_must_fail git fsck --tags >out &&
test_i18ngrep "broken link" out
@@ -411,7 +413,7 @@ test_expect_success 'tag pointing to something else than its type' '
tag=$(git hash-object -t tag -w --stdin <wrong-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
test_must_fail git fsck --tags
'
@@ -428,7 +430,7 @@ test_expect_success 'tag with incorrect tag name & missing tagger' '
tag=$(git hash-object --literally -t tag -w --stdin <wrong-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
git fsck --tags 2>out &&
@@ -452,7 +454,7 @@ test_expect_success 'tag with bad tagger' '
tag=$(git hash-object --literally -t tag -w --stdin <wrong-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
test_must_fail git fsck --tags 2>out &&
test_i18ngrep "error in tag .*: invalid author/committer" out
@@ -471,7 +473,7 @@ test_expect_success 'tag with NUL in header' '
tag=$(git hash-object --literally -t tag -w --stdin <tag-NUL-header) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
test_must_fail git fsck --tags 2>out &&
test_i18ngrep "error in tag $tag.*unterminated header: NUL at offset" out
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8ea2bf13026..d2a7a91f170 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -2160,7 +2160,7 @@ test_expect_success '--update-refs: check failed ref update' '
# recorded in the update-refs file. We will force-update the
# "second" ref, but "git branch -f" will not work because of
# the lock in the update-refs file.
- git rev-parse third >.git/refs/heads/second &&
+ git update-ref refs/heads/second third &&
test_must_fail git rebase --continue 2>err &&
grep "update_ref failed for ref '\''refs/heads/second'\''" err &&
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 26e933f93ae..7ab220fa313 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -771,7 +771,7 @@ test_expect_success 'fetching submodule into a broken repository' '
git -C dst fetch --recurse-submodules &&
# Break the receiving submodule
- rm -f dst/sub/.git/HEAD &&
+ test-tool -C dst/sub ref-store main delete-refs REF_NO_DEREF msg HEAD &&
# NOTE: without the fix the following tests will recurse forever!
# They should terminate with an error.
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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