* [PATCH 00/14] Stop using `the_repository` in some trivial cases
@ 2024-12-17 6:43 Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 01/14] progress: stop using `the_repository` Patrick Steinhardt
` (15 more replies)
0 siblings, 16 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Hi,
this small patch series performs some refactorings to stop using
`the_repository` in several subsystems. There wasn't really any
criterium for which subsystems I picked, except that all of them have
been trivial to convert.
In this patch series I'm merely bubbling up `the_repository` one more
layer even though some calling contexts already have a repository
available. For the sake of triviality I decided not to handle these
cases though and instead let a future patch series worry about them.
This series is built on v2.48.0-rc0 with ps/build-sign-compare at
e03d2a9ccb (t/helper: don't depend on implicit wraparound, 2024-12-06)
merged into it. There's a single merge conflict with 'seen' that can be
solved like this:
diff --cc pack-bitmap.c
index 48e3b99efb,7d8f910588..0000000000
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@@ -2572,16 -2761,8 +2761,10 @@@ void test_bitmap_walk(struct rev_info *
if (prepare_revision_walk(revs))
die(_("revision walk setup failed"));
- tdata.bitmap_git = bitmap_git;
- tdata.base = bitmap_new();
- tdata.commits = ewah_to_bitmap(bitmap_git->commits);
- tdata.trees = ewah_to_bitmap(bitmap_git->trees);
- tdata.blobs = ewah_to_bitmap(bitmap_git->blobs);
- tdata.tags = ewah_to_bitmap(bitmap_git->tags);
+ prepare_bitmap_test_data(&tdata, bitmap_git);
- tdata.prg = start_progress("Verifying bitmap entries", result_popcnt);
+ tdata.prg = start_progress(revs->repo,
+ "Verifying bitmap entries",
+ result_popcnt);
- tdata.seen = 0;
traverse_commit_list(revs, &test_show_commit, &test_show_object, &tdata);
Thanks!
Patrick
---
Patrick Steinhardt (14):
progress: stop using `the_repository`
pager: stop using `the_repository`
trace: stop using `the_repository`
serve: stop using `the_repository`
send-pack: stop using `the_repository`
server-info: stop using `the_repository`
diagnose: stop using `the_repository`
mailinfo: stop using `the_repository`
credential: stop using `the_repository`
resolve-undo: stop using `the_repository`
tmp-objdir: stop using `the_repository`
add-interactive: stop using `the_repository`
graph: stop using `the_repository`
match-trees: stop using `the_repository`
add-interactive.c | 19 ++++++-----
add-patch.c | 2 +-
builtin/am.c | 6 ++--
builtin/blame.c | 6 ++--
builtin/bugreport.c | 2 +-
builtin/commit-graph.c | 1 +
builtin/credential.c | 6 ++--
builtin/diagnose.c | 4 ++-
builtin/fsck.c | 12 ++++---
builtin/grep.c | 4 +--
builtin/help.c | 4 +--
builtin/index-pack.c | 7 ++--
builtin/log.c | 7 ++--
builtin/mailinfo.c | 2 +-
builtin/pack-objects.c | 21 ++++++++----
builtin/prune.c | 3 +-
builtin/receive-pack.c | 4 +--
builtin/remote.c | 3 +-
builtin/repack.c | 2 +-
builtin/rev-list.c | 3 +-
builtin/send-pack.c | 2 +-
builtin/unpack-objects.c | 3 +-
builtin/update-server-info.c | 2 +-
builtin/upload-pack.c | 6 ++--
builtin/var.c | 2 +-
bulk-checkin.c | 2 +-
commit-graph.c | 20 ++++++++++--
credential.c | 34 +++++++++----------
credential.h | 11 ++++---
delta-islands.c | 3 +-
diagnose.c | 15 +++++----
diagnose.h | 5 ++-
diff.c | 4 +--
diffcore-rename.c | 1 +
entry.c | 4 ++-
git.c | 10 +++---
graph.c | 3 +-
http.c | 24 +++++++-------
imap-send.c | 10 +++---
log-tree.c | 2 +-
mailinfo.c | 5 ++-
mailinfo.h | 4 ++-
match-trees.c | 50 +++++++++++++++-------------
midx-write.c | 11 +++++--
midx.c | 13 +++++---
pack-bitmap-write.c | 6 ++--
pack-bitmap.c | 4 ++-
pager.c | 14 ++++----
pager.h | 7 ++--
preload-index.c | 4 ++-
progress.c | 34 +++++++++++--------
progress.h | 13 +++++---
prune-packed.c | 3 +-
pseudo-merge.c | 3 +-
read-cache.c | 7 ++--
remote-curl.c | 4 +--
resolve-undo.c | 14 ++++----
resolve-undo.h | 6 ++--
send-pack.c | 77 +++++++++++++++++++++++---------------------
send-pack.h | 3 +-
serve.c | 36 ++++++++++-----------
serve.h | 6 ++--
server-info.c | 40 ++++++++++++-----------
server-info.h | 4 ++-
t/helper/test-progress.c | 6 +++-
t/helper/test-serve-v2.c | 7 ++--
tmp-objdir.c | 15 +++++----
tmp-objdir.h | 5 +--
trace.c | 9 +++---
trace.h | 4 ++-
transport.c | 2 +-
unpack-trees.c | 4 ++-
walker.c | 3 +-
73 files changed, 406 insertions(+), 298 deletions(-)
---
base-commit: 063bcebf0c917140ca0e705cbe0fdea127e90086
change-id: 20241128-pks-use-the-repository-conversion-c346af3388ee
prerequisite-message-id: <20241206-pks-meson-v11-0-525ed4792b88@pks.im>
prerequisite-patch-id: 39559d044bc4db5a61d8d428ba227596b6f9a572
prerequisite-patch-id: 76040d8ec266b78f698065cdc891c7a44a7deb59
prerequisite-patch-id: 6a99a5acb7d2a9d331bd2fa364c5f048c4807630
prerequisite-patch-id: f9bf8d3c7bd9ac8f1beeafc0d7bfa99809624ff5
prerequisite-patch-id: b3b09e0cc0b35176a63e768e3dae4f39ed2bbd96
prerequisite-patch-id: b2f0b893f4093ac7ac4466efe23472c381c207bf
prerequisite-patch-id: 942b24fe8f6b5a1d2fd892e4dd52b83238b2bf6a
prerequisite-patch-id: 95f28ecdf77807e7e6968c0bc75e29ea8fa5d687
prerequisite-patch-id: 58e4f31b532d41d4394654ea1de6a3d2a3c54bff
prerequisite-patch-id: 331073f51ccb93b8b02f14b6e52f8fb70651afc5
prerequisite-patch-id: c357c6e7040737739a8d5d76424348bcc9444a79
prerequisite-patch-id: d643b636c49ef0bca14fd198290ffbda331d2823
prerequisite-patch-id: aeee0e7f421acd0c5e6d8a9cd45eb22b8f52a322
prerequisite-patch-id: 88175b9418f0575a627ed6b592c61406bed0972c
prerequisite-patch-id: 95241bfd7e6798b39187c3dfc03d47ab37ca50c4
prerequisite-patch-id: 73dd2f88f2629f3a54ab01c2882962b9effd6055
prerequisite-patch-id: 291c5ec532ed3738a59ad7001ef3a84e2c43aa78
prerequisite-patch-id: 3b29840c001ec89f137b8e37796d710f60436a6a
prerequisite-patch-id: 78bd944cca8c7404feb1f54d9adfcbfea481fa96
prerequisite-patch-id: 9f34d87c5455cada6a68350daa86cb1069e7accc
prerequisite-patch-id: 798827dcb73a002d4d604472e1b1a3b64f9b159d
prerequisite-patch-id: a110e45c7f97287acbd8c69640d6f03d2e7d9bef
prerequisite-patch-id: 3ce7cd65e984c138658841e3a4802178cc9129b2
prerequisite-patch-id: 9cb10256bf2b131cf434ec078807f27be3a4d6cf
prerequisite-patch-id: a97085715ee32eaaad3bcde53e7d41582424f0f9
prerequisite-patch-id: 7a5e50bc7d49ec891a9679b1ea71575f41483187
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 01/14] progress: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-31 6:42 ` Karthik Nayak
2025-01-06 20:57 ` Toon Claes
2024-12-17 6:43 ` [PATCH 02/14] pager: " Patrick Steinhardt
` (14 subsequent siblings)
15 siblings, 2 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "progress" subsystem by passing in a
repository when initializing `struct progress`. Furthermore, store a
pointer to the repository in that struct so that we can pass it to the
trace2 API when logging information.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/blame.c | 4 +++-
builtin/commit-graph.c | 1 +
builtin/fsck.c | 12 ++++++++----
builtin/index-pack.c | 7 +++++--
builtin/log.c | 3 ++-
builtin/pack-objects.c | 21 ++++++++++++++-------
builtin/prune.c | 3 ++-
builtin/remote.c | 3 ++-
builtin/rev-list.c | 3 ++-
builtin/unpack-objects.c | 3 ++-
commit-graph.c | 20 +++++++++++++++++---
delta-islands.c | 3 ++-
diffcore-rename.c | 1 +
entry.c | 4 +++-
midx-write.c | 11 ++++++++---
midx.c | 13 +++++++++----
pack-bitmap-write.c | 6 ++++--
pack-bitmap.c | 4 +++-
preload-index.c | 4 +++-
progress.c | 34 ++++++++++++++++++++--------------
progress.h | 13 +++++++++----
prune-packed.c | 3 ++-
pseudo-merge.c | 3 ++-
read-cache.c | 3 ++-
t/helper/test-progress.c | 6 +++++-
unpack-trees.c | 4 +++-
walker.c | 3 ++-
27 files changed, 136 insertions(+), 59 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 867032e4c16878ffd56df8a73162b89ca4bd2694..dd78288530f06efa99ec7a1ca767aa388805fd97 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1193,7 +1193,9 @@ int cmd_blame(int argc,
sb.found_guilty_entry = &found_guilty_entry;
sb.found_guilty_entry_data = π
if (show_progress)
- pi.progress = start_delayed_progress(_("Blaming lines"), num_lines);
+ pi.progress = start_delayed_progress(the_repository,
+ _("Blaming lines"),
+ num_lines);
assign_blame(&sb, opt);
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index bd70d052e706b6e34b5aaaceef158c63ea4863d5..8ca75262c59c480a33b53ce9010a882970066957 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -305,6 +305,7 @@ static int graph_write(int argc, const char **argv, const char *prefix,
oidset_init(&commits, 0);
if (opts.progress)
progress = start_delayed_progress(
+ the_repository,
_("Collecting commits from input"), 0);
while (strbuf_getline(&buf, stdin) != EOF) {
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0196c54eb68ee54c22de72d64b3f31602594e50b..7a4dcb0716052ff1b9236ea66b8901960fe1c55d 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -197,7 +197,8 @@ static int traverse_reachable(void)
unsigned int nr = 0;
int result = 0;
if (show_progress)
- progress = start_delayed_progress(_("Checking connectivity"), 0);
+ progress = start_delayed_progress(the_repository,
+ _("Checking connectivity"), 0);
while (pending.nr) {
result |= traverse_one_object(object_array_pop(&pending));
display_progress(progress, ++nr);
@@ -703,7 +704,8 @@ static void fsck_object_dir(const char *path)
fprintf_ln(stderr, _("Checking object directory"));
if (show_progress)
- progress = start_progress(_("Checking object directories"), 256);
+ progress = start_progress(the_repository,
+ _("Checking object directories"), 256);
for_each_loose_file_in_objdir(path, fsck_loose, fsck_cruft, fsck_subdir,
&cb_data);
@@ -879,7 +881,8 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)
if (show_progress) {
for (struct packed_git *p = get_all_packs(r); p; p = p->next)
pack_count++;
- progress = start_delayed_progress("Verifying reverse pack-indexes", pack_count);
+ progress = start_delayed_progress(the_repository,
+ "Verifying reverse pack-indexes", pack_count);
pack_count = 0;
}
@@ -989,7 +992,8 @@ int cmd_fsck(int argc,
total += p->num_objects;
}
- progress = start_progress(_("Checking objects"), total);
+ progress = start_progress(the_repository,
+ _("Checking objects"), total);
}
for (p = get_all_packs(the_repository); p;
p = p->next) {
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index d773809c4c96604b7668b48d3c231460bd94c79b..05691104c36b4f18bf1287acac3f2a3719ea0261 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -282,7 +282,8 @@ static unsigned check_objects(void)
max = get_max_object_index();
if (verbose)
- progress = start_delayed_progress(_("Checking objects"), max);
+ progress = start_delayed_progress(the_repository,
+ _("Checking objects"), max);
for (i = 0; i < max; i++) {
foreign_nr += check_object(get_indexed_object(i));
@@ -1249,6 +1250,7 @@ static void parse_pack_objects(unsigned char *hash)
if (verbose)
progress = start_progress(
+ the_repository,
progress_title ? progress_title :
from_stdin ? _("Receiving objects") : _("Indexing objects"),
nr_objects);
@@ -1329,7 +1331,8 @@ static void resolve_deltas(struct pack_idx_option *opts)
QSORT(ref_deltas, nr_ref_deltas, compare_ref_delta_entry);
if (verbose || show_resolving_progress)
- progress = start_progress(_("Resolving deltas"),
+ progress = start_progress(the_repository,
+ _("Resolving deltas"),
nr_ref_deltas + nr_ofs_deltas);
nr_dispatched = 0;
diff --git a/builtin/log.c b/builtin/log.c
index cb41a035c6ead57e4d466ae0042a40435ebcde2f..317335e60d450128685d9fce99eb8fe8f9860fd9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2495,7 +2495,8 @@ int cmd_format_patch(int argc,
rev.add_signoff = cfg.do_signoff;
if (show_progress)
- progress = start_delayed_progress(_("Generating patches"), total);
+ progress = start_delayed_progress(the_repository,
+ _("Generating patches"), total);
while (0 <= --nr) {
int shown;
display_progress(progress, total - nr);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1c3b8426515c42f14c536eea559dd377212855f6..d51c021d99d9f470c04b7ec52565ab2f4c1c19ae 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1264,7 +1264,8 @@ static void write_pack_file(void)
struct object_entry **write_order;
if (progress > pack_to_stdout)
- progress_state = start_progress(_("Writing objects"), nr_result);
+ progress_state = start_progress(the_repository,
+ _("Writing objects"), nr_result);
ALLOC_ARRAY(written_list, to_pack.nr_objects);
write_order = compute_write_order();
@@ -2400,7 +2401,8 @@ static void get_object_details(void)
struct object_entry **sorted_by_offset;
if (progress)
- progress_state = start_progress(_("Counting objects"),
+ progress_state = start_progress(the_repository,
+ _("Counting objects"),
to_pack.nr_objects);
CALLOC_ARRAY(sorted_by_offset, to_pack.nr_objects);
@@ -3220,7 +3222,8 @@ static void prepare_pack(int window, int depth)
unsigned nr_done = 0;
if (progress)
- progress_state = start_progress(_("Compressing objects"),
+ progress_state = start_progress(the_repository,
+ _("Compressing objects"),
nr_deltas);
QSORT(delta_list, n, type_size_sort);
ll_find_deltas(delta_list, n, window+1, depth, &nr_done);
@@ -3648,7 +3651,8 @@ static void add_objects_in_unpacked_packs(void);
static void enumerate_cruft_objects(void)
{
if (progress)
- progress_state = start_progress(_("Enumerating cruft objects"), 0);
+ progress_state = start_progress(the_repository,
+ _("Enumerating cruft objects"), 0);
add_objects_in_unpacked_packs();
add_unreachable_loose_objects();
@@ -3674,7 +3678,8 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs
revs.ignore_missing_links = 1;
if (progress)
- progress_state = start_progress(_("Enumerating cruft objects"), 0);
+ progress_state = start_progress(the_repository,
+ _("Enumerating cruft objects"), 0);
ret = add_unseen_recent_objects_to_traversal(&revs, cruft_expiration,
set_cruft_mtime, 1);
stop_progress(&progress_state);
@@ -3693,7 +3698,8 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs
if (prepare_revision_walk(&revs))
die(_("revision walk setup failed"));
if (progress)
- progress_state = start_progress(_("Traversing cruft objects"), 0);
+ progress_state = start_progress(the_repository,
+ _("Traversing cruft objects"), 0);
nr_seen = 0;
traverse_commit_list(&revs, show_cruft_commit, show_cruft_object, NULL);
@@ -4625,7 +4631,8 @@ int cmd_pack_objects(int argc,
prepare_packing_data(the_repository, &to_pack);
if (progress && !cruft)
- progress_state = start_progress(_("Enumerating objects"), 0);
+ progress_state = start_progress(the_repository,
+ _("Enumerating objects"), 0);
if (stdin_packs) {
/* avoids adding objects in excluded packs */
ignore_packed_keep_in_core = 1;
diff --git a/builtin/prune.c b/builtin/prune.c
index aeff9ca1b35c3f09c6bd334cbf52864df437fe05..1c357fffd8cde6816ecb598fb6588462f3074c09 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -64,7 +64,8 @@ static void perform_reachability_traversal(struct rev_info *revs)
return;
if (show_progress)
- progress = start_delayed_progress(_("Checking connectivity"), 0);
+ progress = start_delayed_progress(the_repository,
+ _("Checking connectivity"), 0);
mark_reachable_objects(revs, 1, expire, progress);
stop_progress(&progress);
initialized = 1;
diff --git a/builtin/remote.c b/builtin/remote.c
index b2b13a7dd2acd466081e7bb49186d45f167dce9f..95440bc9ff284eb109079e95f5a5f438040e7fd7 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -820,7 +820,8 @@ static int mv(int argc, const char **argv, const char *prefix,
* Count symrefs twice, since "renaming" them is done by
* deleting and recreating them in two separate passes.
*/
- progress = start_progress(_("Renaming remote references"),
+ progress = start_progress(the_repository,
+ _("Renaming remote references"),
rename.remote_branches->nr + rename.symrefs_nr);
}
for (i = 0; i < remote_branches.nr; i++) {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3196da7b2d23e33fafde57a1af1c42a5c985f0b0..8a7db9b546196c76ce8e374915fffa378f1199c5 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -735,7 +735,8 @@ int cmd_rev_list(int argc,
revs.limited = 1;
if (show_progress)
- progress = start_delayed_progress(show_progress, 0);
+ progress = start_delayed_progress(the_repository,
+ show_progress, 0);
if (use_bitmap_index) {
if (!try_bitmap_count(&revs, filter_provided_objects))
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 2197d6d9332117636f31abadb5aff70424ba305d..842a90353a50b6a4be863237d2804e5ef509d478 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -590,7 +590,8 @@ static void unpack_all(void)
use(sizeof(struct pack_header));
if (!quiet)
- progress = start_progress(_("Unpacking objects"), nr_objects);
+ progress = start_progress(the_repository,
+ _("Unpacking objects"), nr_objects);
CALLOC_ARRAY(obj_list, nr_objects);
begin_odb_transaction();
for (i = 0; i < nr_objects; i++) {
diff --git a/commit-graph.c b/commit-graph.c
index 0df66e5a243390bc1224b28e2b55c541f9d93fb1..2a2999a6b886905276a0c39dda6135f0c92aa361 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1534,6 +1534,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
if (ctx->report_progress)
ctx->progress = start_delayed_progress(
+ the_repository,
_("Loading known commits in commit graph"),
ctx->oids.nr);
for (i = 0; i < ctx->oids.nr; i++) {
@@ -1551,6 +1552,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
*/
if (ctx->report_progress)
ctx->progress = start_delayed_progress(
+ the_repository,
_("Expanding reachable commits in commit graph"),
0);
for (i = 0; i < ctx->oids.nr; i++) {
@@ -1571,6 +1573,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
if (ctx->report_progress)
ctx->progress = start_delayed_progress(
+ the_repository,
_("Clearing commit marks in commit graph"),
ctx->oids.nr);
for (i = 0; i < ctx->oids.nr; i++) {
@@ -1688,6 +1691,7 @@ static void compute_topological_levels(struct write_commit_graph_context *ctx)
if (ctx->report_progress)
info.progress = ctx->progress
= start_delayed_progress(
+ the_repository,
_("Computing commit graph topological levels"),
ctx->commits.nr);
@@ -1722,6 +1726,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
if (ctx->report_progress)
info.progress = ctx->progress
= start_delayed_progress(
+ the_repository,
_("Computing commit graph generation numbers"),
ctx->commits.nr);
@@ -1798,6 +1803,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)
if (ctx->report_progress)
progress = start_delayed_progress(
+ the_repository,
_("Computing commit changed paths Bloom filters"),
ctx->commits.nr);
@@ -1877,6 +1883,7 @@ int write_commit_graph_reachable(struct object_directory *odb,
data.commits = &commits;
if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
data.progress = start_delayed_progress(
+ the_repository,
_("Collecting referenced commits"), 0);
refs_for_each_ref(get_main_ref_store(the_repository), add_ref_to_set,
@@ -1908,7 +1915,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
"Finding commits for commit graph in %"PRIuMAX" packs",
pack_indexes->nr),
(uintmax_t)pack_indexes->nr);
- ctx->progress = start_delayed_progress(progress_title.buf, 0);
+ ctx->progress = start_delayed_progress(the_repository,
+ progress_title.buf, 0);
ctx->progress_done = 0;
}
for (i = 0; i < pack_indexes->nr; i++) {
@@ -1959,6 +1967,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
{
if (ctx->report_progress)
ctx->progress = start_delayed_progress(
+ the_repository,
_("Finding commits for commit graph among packed objects"),
ctx->approx_nr_objects);
for_each_packed_object(ctx->r, add_packed_commits, ctx,
@@ -1977,6 +1986,7 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
ctx->num_extra_edges = 0;
if (ctx->report_progress)
ctx->progress = start_delayed_progress(
+ the_repository,
_("Finding extra edges in commit graph"),
ctx->oids.nr);
oid_array_sort(&ctx->oids);
@@ -2136,6 +2146,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
get_num_chunks(cf)),
get_num_chunks(cf));
ctx->progress = start_delayed_progress(
+ the_repository,
progress_title.buf,
st_mult(get_num_chunks(cf), ctx->commits.nr));
}
@@ -2348,6 +2359,7 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx)
if (ctx->report_progress)
ctx->progress = start_delayed_progress(
+ the_repository,
_("Scanning merged commits"),
ctx->commits.nr);
@@ -2392,7 +2404,8 @@ static void merge_commit_graphs(struct write_commit_graph_context *ctx)
current_graph_number--;
if (ctx->report_progress)
- ctx->progress = start_delayed_progress(_("Merging commit-graph"), 0);
+ ctx->progress = start_delayed_progress(the_repository,
+ _("Merging commit-graph"), 0);
merge_commit_graph(ctx, g);
stop_progress(&ctx->progress);
@@ -2874,7 +2887,8 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW))
total += g->num_commits_in_base;
- progress = start_progress(_("Verifying commits in commit graph"),
+ progress = start_progress(the_repository,
+ _("Verifying commits in commit graph"),
total);
}
diff --git a/delta-islands.c b/delta-islands.c
index 1c465a6041914538bcb8be51c500653d8fa1a626..3aec43fada36f7052b825dcb2ac0e1ad79f028b7 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -267,7 +267,8 @@ void resolve_tree_islands(struct repository *r,
QSORT(todo, nr, tree_depth_compare);
if (progress)
- progress_state = start_progress(_("Propagating island marks"), nr);
+ progress_state = start_progress(the_repository,
+ _("Propagating island marks"), nr);
for (i = 0; i < nr; i++) {
struct object_entry *ent = todo[i].entry;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 10bb0321b10d5896aaa6a26a624d2066598bf51f..91b77993c7827f9ddc7b444b42f480b8209fd821 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -1567,6 +1567,7 @@ void diffcore_rename_extended(struct diff_options *options,
trace2_region_enter("diff", "inexact renames", options->repo);
if (options->show_rename_progress) {
progress = start_delayed_progress(
+ the_repository,
_("Performing inexact rename detection"),
(uint64_t)num_destinations * (uint64_t)num_sources);
}
diff --git a/entry.c b/entry.c
index 53a1c393582b2e6be4398985ca3c5f4fadece3cb..81b321e53d1b960976723ec1da49483bfd223ce8 100644
--- a/entry.c
+++ b/entry.c
@@ -188,7 +188,9 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)
dco->state = CE_RETRY;
if (show_progress)
- progress = start_delayed_progress(_("Filtering content"), dco->paths.nr);
+ progress = start_delayed_progress(the_repository,
+ _("Filtering content"),
+ dco->paths.nr);
while (dco->filters.nr > 0) {
for_each_string_list_item(filter, &dco->filters) {
struct string_list available_paths = STRING_LIST_INIT_DUP;
diff --git a/midx-write.c b/midx-write.c
index 0066594fa6310b37903972e150c7cd1d7e232c38..b3827b936bdb1df12c73fb7d9b98ff65fc875cc3 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1131,7 +1131,8 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
ctx.pack_paths_checked = 0;
if (flags & MIDX_PROGRESS)
- ctx.progress = start_delayed_progress(_("Adding packfiles to multi-pack-index"), 0);
+ ctx.progress = start_delayed_progress(r,
+ _("Adding packfiles to multi-pack-index"), 0);
else
ctx.progress = NULL;
@@ -1539,7 +1540,9 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
CALLOC_ARRAY(count, m->num_packs);
if (flags & MIDX_PROGRESS)
- progress = start_delayed_progress(_("Counting referenced objects"),
+ progress = start_delayed_progress(
+ r,
+ _("Counting referenced objects"),
m->num_objects);
for (i = 0; i < m->num_objects; i++) {
int pack_int_id = nth_midxed_pack_int_id(m, i);
@@ -1549,7 +1552,9 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
stop_progress(&progress);
if (flags & MIDX_PROGRESS)
- progress = start_delayed_progress(_("Finding and deleting unreferenced packfiles"),
+ progress = start_delayed_progress(
+ r,
+ _("Finding and deleting unreferenced packfiles"),
m->num_packs);
for (i = 0; i < m->num_packs; i++) {
char *pack_name;
diff --git a/midx.c b/midx.c
index f8a75cafd4efeabd273268ba04cddefaa434e5b2..d91088efb87ca0d59b1ad215556a8cfe1612b8af 100644
--- a/midx.c
+++ b/midx.c
@@ -907,7 +907,8 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
midx_report(_("incorrect checksum"));
if (flags & MIDX_PROGRESS)
- progress = start_delayed_progress(_("Looking for referenced packfiles"),
+ progress = start_delayed_progress(r,
+ _("Looking for referenced packfiles"),
m->num_packs + m->num_packs_in_base);
for (i = 0; i < m->num_packs + m->num_packs_in_base; i++) {
if (prepare_midx_pack(r, m, i))
@@ -927,7 +928,8 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
}
if (flags & MIDX_PROGRESS)
- progress = start_sparse_progress(_("Verifying OID order in multi-pack-index"),
+ progress = start_sparse_progress(r,
+ _("Verifying OID order in multi-pack-index"),
m->num_objects - 1);
for (curr = m; curr; curr = curr->base_midx) {
@@ -959,14 +961,17 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
}
if (flags & MIDX_PROGRESS)
- progress = start_sparse_progress(_("Sorting objects by packfile"),
+ progress = start_sparse_progress(r,
+ _("Sorting objects by packfile"),
m->num_objects);
display_progress(progress, 0); /* TODO: Measure QSORT() progress */
QSORT(pairs, m->num_objects, compare_pair_pos_vs_id);
stop_progress(&progress);
if (flags & MIDX_PROGRESS)
- progress = start_sparse_progress(_("Verifying object offsets"), m->num_objects);
+ progress = start_sparse_progress(r,
+ _("Verifying object offsets"),
+ m->num_objects);
for (i = 0; i < m->num_objects + m->num_objects_in_base; i++) {
struct object_id oid;
struct pack_entry e;
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 4f8be53c2bd75f83a0555e2a5510c2bbca07b36d..a06a1f35c619b3b01e63a506a6d4312e14cf181c 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -590,7 +590,8 @@ int bitmap_writer_build(struct bitmap_writer *writer)
int closed = 1; /* until proven otherwise */
if (writer->show_progress)
- writer->progress = start_progress("Building bitmaps",
+ writer->progress = start_progress(the_repository,
+ "Building bitmaps",
writer->selected_nr);
trace2_region_enter("pack-bitmap-write", "building_bitmaps_total",
the_repository);
@@ -710,7 +711,8 @@ void bitmap_writer_select_commits(struct bitmap_writer *writer,
}
if (writer->show_progress)
- writer->progress = start_progress("Selecting bitmap commits", 0);
+ writer->progress = start_progress(the_repository,
+ "Selecting bitmap commits", 0);
for (;;) {
struct commit *chosen = NULL;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index bceb6da042772df7afa741bafe7e4893473cbca8..48e3b99efb2f554f685392f5e23f47fb1954ffdc 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2578,7 +2578,9 @@ void test_bitmap_walk(struct rev_info *revs)
tdata.trees = ewah_to_bitmap(bitmap_git->trees);
tdata.blobs = ewah_to_bitmap(bitmap_git->blobs);
tdata.tags = ewah_to_bitmap(bitmap_git->tags);
- tdata.prg = start_progress("Verifying bitmap entries", result_popcnt);
+ tdata.prg = start_progress(revs->repo,
+ "Verifying bitmap entries",
+ result_popcnt);
tdata.seen = 0;
traverse_commit_list(revs, &test_show_commit, &test_show_object, &tdata);
diff --git a/preload-index.c b/preload-index.c
index ab94d6f39967ea4358f51ff8384aa60927bfe259..40ab2abafb8de500a5f2ec678a584a5fd5e1bc16 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -132,7 +132,9 @@ void preload_index(struct index_state *index,
memset(&pd, 0, sizeof(pd));
if (refresh_flags & REFRESH_PROGRESS && isatty(2)) {
- pd.progress = start_delayed_progress(_("Refreshing index"), index->cache_nr);
+ pd.progress = start_delayed_progress(the_repository,
+ _("Refreshing index"),
+ index->cache_nr);
pthread_mutex_init(&pd.mutex, NULL);
}
diff --git a/progress.c b/progress.c
index a8fdfceb5cd12879ebac80589bb5b5e8661c2627..8d5ae70f3a9ec7042bf12782190f6514014b0287 100644
--- a/progress.c
+++ b/progress.c
@@ -9,7 +9,6 @@
*/
#define GIT_TEST_PROGRESS_ONLY
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -37,6 +36,7 @@ struct throughput {
};
struct progress {
+ struct repository *repo;
const char *title;
uint64_t last_value;
uint64_t total;
@@ -254,10 +254,12 @@ void display_progress(struct progress *progress, uint64_t n)
display(progress, n, NULL);
}
-static struct progress *start_progress_delay(const char *title, uint64_t total,
+static struct progress *start_progress_delay(struct repository *r,
+ const char *title, uint64_t total,
unsigned delay, unsigned sparse)
{
struct progress *progress = xmalloc(sizeof(*progress));
+ progress->repo = r;
progress->title = title;
progress->total = total;
progress->last_value = -1;
@@ -270,7 +272,7 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,
progress->title_len = utf8_strwidth(title);
progress->split = 0;
set_progress_signal();
- trace2_region_enter("progress", title, the_repository);
+ trace2_region_enter("progress", title, r);
return progress;
}
@@ -284,14 +286,16 @@ static int get_default_delay(void)
return delay_in_secs;
}
-struct progress *start_delayed_progress(const char *title, uint64_t total)
+struct progress *start_delayed_progress(struct repository *r,
+ const char *title, uint64_t total)
{
- return start_progress_delay(title, total, get_default_delay(), 0);
+ return start_progress_delay(r, title, total, get_default_delay(), 0);
}
-struct progress *start_progress(const char *title, uint64_t total)
+struct progress *start_progress(struct repository *r,
+ const char *title, uint64_t total)
{
- return start_progress_delay(title, total, 0, 0);
+ return start_progress_delay(r, title, total, 0, 0);
}
/*
@@ -303,15 +307,17 @@ struct progress *start_progress(const char *title, uint64_t total)
* When "sparse" is set, stop_progress() will automatically force the done
* message to show 100%.
*/
-struct progress *start_sparse_progress(const char *title, uint64_t total)
+struct progress *start_sparse_progress(struct repository *r,
+ const char *title, uint64_t total)
{
- return start_progress_delay(title, total, 0, 1);
+ return start_progress_delay(r, title, total, 0, 1);
}
-struct progress *start_delayed_sparse_progress(const char *title,
+struct progress *start_delayed_sparse_progress(struct repository *r,
+ const char *title,
uint64_t total)
{
- return start_progress_delay(title, total, get_default_delay(), 1);
+ return start_progress_delay(r, title, total, get_default_delay(), 1);
}
static void finish_if_sparse(struct progress *progress)
@@ -341,14 +347,14 @@ static void force_last_update(struct progress *progress, const char *msg)
static void log_trace2(struct progress *progress)
{
- trace2_data_intmax("progress", the_repository, "total_objects",
+ trace2_data_intmax("progress", progress->repo, "total_objects",
progress->total);
if (progress->throughput)
- trace2_data_intmax("progress", the_repository, "total_bytes",
+ trace2_data_intmax("progress", progress->repo, "total_bytes",
progress->throughput->curr_total);
- trace2_region_leave("progress", progress->title, the_repository);
+ trace2_region_leave("progress", progress->title, progress->repo);
}
void stop_progress_msg(struct progress **p_progress, const char *msg)
diff --git a/progress.h b/progress.h
index 3a945637c81c22734b563325b66956ee4fb33b0b..ed068c7bab845b17f55d4ed8f0907497ad23fd47 100644
--- a/progress.h
+++ b/progress.h
@@ -3,6 +3,7 @@
#include "gettext.h"
struct progress;
+struct repository;
#ifdef GIT_TEST_PROGRESS_ONLY
@@ -14,10 +15,14 @@ void progress_test_force_update(void);
void display_throughput(struct progress *progress, uint64_t total);
void display_progress(struct progress *progress, uint64_t n);
-struct progress *start_progress(const char *title, uint64_t total);
-struct progress *start_sparse_progress(const char *title, uint64_t total);
-struct progress *start_delayed_progress(const char *title, uint64_t total);
-struct progress *start_delayed_sparse_progress(const char *title,
+struct progress *start_progress(struct repository *r,
+ const char *title, uint64_t total);
+struct progress *start_sparse_progress(struct repository *r,
+ const char *title, uint64_t total);
+struct progress *start_delayed_progress(struct repository *r,
+ const char *title, uint64_t total);
+struct progress *start_delayed_sparse_progress(struct repository *r,
+ const char *title,
uint64_t total);
void stop_progress_msg(struct progress **p_progress, const char *msg);
static inline void stop_progress(struct progress **p_progress)
diff --git a/prune-packed.c b/prune-packed.c
index d1c65ab10ed6004aae8265ce05f8db9ae3a33ad9..7dad2fc0c169cf456ba646536cddae399b0555bd 100644
--- a/prune-packed.c
+++ b/prune-packed.c
@@ -37,7 +37,8 @@ static int prune_object(const struct object_id *oid, const char *path,
void prune_packed_objects(int opts)
{
if (opts & PRUNE_PACKED_VERBOSE)
- progress = start_delayed_progress(_("Removing duplicate objects"), 256);
+ progress = start_delayed_progress(the_repository,
+ _("Removing duplicate objects"), 256);
for_each_loose_file_in_objdir(repo_get_object_directory(the_repository),
prune_object, NULL, prune_subdir, &opts);
diff --git a/pseudo-merge.c b/pseudo-merge.c
index 971f54cfe1a895aed00f6d0a65c62aafc83a0cc8..893b763fe45490875ea226eaffff0c7cb1dafb06 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -459,7 +459,8 @@ void select_pseudo_merges(struct bitmap_writer *writer)
return;
if (writer->show_progress)
- progress = start_progress("Selecting pseudo-merge commits",
+ progress = start_progress(the_repository,
+ "Selecting pseudo-merge commits",
writer->pseudo_merge_groups.nr);
refs_for_each_ref(get_main_ref_store(the_repository),
diff --git a/read-cache.c b/read-cache.c
index 15d79839c205176f9161f537aa706dac44b3023c..38c36caa7fef4d44da74c29e059839d88426df15 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1523,7 +1523,8 @@ int refresh_index(struct index_state *istate, unsigned int flags,
int t2_sum_scan = 0;
if (flags & REFRESH_PROGRESS && isatty(2))
- progress = start_delayed_progress(_("Refresh index"),
+ progress = start_delayed_progress(the_repository,
+ _("Refresh index"),
istate->cache_nr);
trace_performance_enter();
diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c
index 44be2645e9c7cb66e54b07832bd84f6efe6fc772..1f75b7bd199aff9e332c2601e444ce7b165e78c2 100644
--- a/t/helper/test-progress.c
+++ b/t/helper/test-progress.c
@@ -17,10 +17,14 @@
*
* See 't0500-progress-display.sh' for examples.
*/
+
+#define USE_THE_REPOSITORY_VARIABLE
#define GIT_TEST_PROGRESS_ONLY
+
#include "test-tool.h"
#include "parse-options.h"
#include "progress.h"
+#include "repository.h"
#include "strbuf.h"
#include "string-list.h"
@@ -64,7 +68,7 @@ int cmd__progress(int argc, const char **argv)
else
die("invalid input: '%s'", line.buf);
- progress = start_progress(title, total);
+ progress = start_progress(the_repository, title, total);
} else if (skip_prefix(line.buf, "progress ", (const char **) &end)) {
uint64_t item_count = strtoull(end, &end, 10);
if (*end != '\0')
diff --git a/unpack-trees.c b/unpack-trees.c
index b3be5d542f5fc5a02b8838101f7334ff44b2c626..334cb84f6531b588688d5a43c538c8d1a5f7e768 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -372,7 +372,8 @@ static struct progress *get_progress(struct unpack_trees_options *o,
total++;
}
- return start_delayed_progress(_("Updating files"), total);
+ return start_delayed_progress(the_repository,
+ _("Updating files"), total);
}
static void setup_collided_checkout_detection(struct checkout *state,
@@ -1773,6 +1774,7 @@ static int clear_ce_flags(struct index_state *istate,
strbuf_reset(&prefix);
if (show_progress)
istate->progress = start_delayed_progress(
+ the_repository,
_("Updating index flags"),
istate->cache_nr);
diff --git a/walker.c b/walker.c
index 7cc9dbea46d64d6bd3336025d640f284a6202157..1cf3da02193531a17fd11dbd2e8aadf36f38b200 100644
--- a/walker.c
+++ b/walker.c
@@ -172,7 +172,8 @@ static int loop(struct walker *walker)
uint64_t nr = 0;
if (walker->get_progress)
- progress = start_delayed_progress(_("Fetching objects"), 0);
+ progress = start_delayed_progress(the_repository,
+ _("Fetching objects"), 0);
while (process_queue) {
struct object *obj = process_queue->item;
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 02/14] pager: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 01/14] progress: stop using `the_repository` Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 12:17 ` shejialuo
2024-12-17 6:43 ` [PATCH 03/14] trace: " Patrick Steinhardt
` (13 subsequent siblings)
15 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "pager" subsystem by passing in a
repository when setting up the pager and when configuring it.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
add-patch.c | 2 +-
builtin/am.c | 4 ++--
builtin/blame.c | 2 +-
builtin/grep.c | 4 ++--
builtin/help.c | 4 ++--
builtin/log.c | 4 ++--
builtin/var.c | 2 +-
diff.c | 4 ++--
git.c | 8 ++++----
| 14 ++++++--------
| 7 ++++---
11 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 7b598e14df016402ca41e1998466b5ba45623f90..95c67d8c80c4f33bfa6180f69fdf2065d0b5b85c 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1464,7 +1464,7 @@ static int patch_update_file(struct add_p_state *s,
if (file_diff->hunk_nr) {
if (rendered_hunk_index != hunk_index) {
if (use_pager) {
- setup_pager();
+ setup_pager(the_repository);
sigchain_push(SIGPIPE, SIG_IGN);
}
render_hunk(s, hunk, 0, colored, &s->buf);
diff --git a/builtin/am.c b/builtin/am.c
index 1338b606febdde6700c573b45f89aa70785f54e8..27ccca8341feefcda5e1c4a850f14fce9e4deecb 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1786,7 +1786,7 @@ static int do_interactive(struct am_state *state)
}
strbuf_release(&msg);
} else if (*reply == 'v' || *reply == 'V') {
- const char *pager = git_pager(1);
+ const char *pager = git_pager(the_repository, 1);
struct child_process cp = CHILD_PROCESS_INIT;
if (!pager)
@@ -2246,7 +2246,7 @@ static int show_patch(struct am_state *state, enum resume_type resume_mode)
if (len < 0)
die_errno(_("failed to read '%s'"), patch_path);
- setup_pager();
+ setup_pager(the_repository);
write_in_full(1, sb.buf, sb.len);
strbuf_release(&sb);
return 0;
diff --git a/builtin/blame.c b/builtin/blame.c
index dd78288530f06efa99ec7a1ca767aa388805fd97..1f44c341c50ecd80fd76d2f055d488884cfcd20c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1202,7 +1202,7 @@ int cmd_blame(int argc,
stop_progress(&pi.progress);
if (!incremental)
- setup_pager();
+ setup_pager(the_repository);
else
goto cleanup;
diff --git a/builtin/grep.c b/builtin/grep.c
index d00ee76f24cfdea8ac15e08f812aa5868906940c..d1427290f773b6cec539fcd838ada2b61acb22c8 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1084,7 +1084,7 @@ int cmd_grep(int argc,
}
if (show_in_pager == default_pager)
- show_in_pager = git_pager(1);
+ show_in_pager = git_pager(the_repository, 1);
if (show_in_pager) {
opt.color = 0;
opt.name_only = 1;
@@ -1246,7 +1246,7 @@ int cmd_grep(int argc,
}
if (!show_in_pager && !opt.status_only)
- setup_pager();
+ setup_pager(the_repository);
die_for_incompatible_opt3(!use_index, "--no-index",
untracked, "--untracked",
diff --git a/builtin/help.c b/builtin/help.c
index 05136279cf7b1007ab754f5630c34536a5f9461f..c257079cebc3c09fb91f258c3b0148e2f204c0e7 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -658,7 +658,7 @@ int cmd_help(int argc,
case HELP_ACTION_ALL:
opt_mode_usage(argc, "--all", help_format);
if (verbose) {
- setup_pager();
+ setup_pager(the_repository);
list_all_cmds_help(show_external_commands,
show_aliases);
return 0;
@@ -692,7 +692,7 @@ int cmd_help(int argc,
return 0;
case HELP_ACTION_CONFIG:
opt_mode_usage(argc, "--config", help_format);
- setup_pager();
+ setup_pager(the_repository);
list_config_help(SHOW_CONFIG_HUMAN);
printf("\n%s\n", _("'git help config' for more information"));
return 0;
diff --git a/builtin/log.c b/builtin/log.c
index 317335e60d450128685d9fce99eb8fe8f9860fd9..3a6a3362f3c36ee257215e79cf40085f76c9efb3 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -369,7 +369,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
if (rev->line_level_traverse)
line_log_init(rev, line_cb.prefix, &line_cb.args);
- setup_pager();
+ setup_pager(the_repository);
}
static void cmd_log_init(int argc, const char **argv, const char *prefix,
@@ -2292,7 +2292,7 @@ int cmd_format_patch(int argc,
rev.commit_format = CMIT_FMT_MBOXRD;
if (use_stdout) {
- setup_pager();
+ setup_pager(the_repository);
} else if (!rev.diffopt.close_file) {
int saved;
diff --git a/builtin/var.c b/builtin/var.c
index 1449656cc924f849ee9c881c18b734a3ec55e9f3..50d024de66604ab9e8faa1bd59f662127bc62159 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -42,7 +42,7 @@ static char *sequence_editor(int ident_flag UNUSED)
static char *pager(int ident_flag UNUSED)
{
- const char *pgm = git_pager(1);
+ const char *pgm = git_pager(the_repository, 1);
if (!pgm)
pgm = "cat";
diff --git a/diff.c b/diff.c
index d28b4114c8dffba5cb5339152d198588550135a0..0822ae443361f80271c91a863a6f03b0e8c403c3 100644
--- a/diff.c
+++ b/diff.c
@@ -7386,6 +7386,6 @@ void setup_diff_pager(struct diff_options *opt)
* --exit-code" in hooks and other scripts, we do not do so.
*/
if (!opt->flags.exit_with_status &&
- check_pager_config("diff") != 0)
- setup_pager();
+ check_pager_config(the_repository, "diff") != 0)
+ setup_pager(the_repository);
}
diff --git a/git.c b/git.c
index 71d644dc1c59902b2f14c14914c78285ffa92638..d977ebc84cfba611c3e452cace3bda1ce13faf5d 100644
--- a/git.c
+++ b/git.c
@@ -125,7 +125,7 @@ static void commit_pager_choice(void)
setenv("GIT_PAGER", "cat", 1);
break;
case 1:
- setup_pager();
+ setup_pager(the_repository);
break;
default:
break;
@@ -136,7 +136,7 @@ void setup_auto_pager(const char *cmd, int def)
{
if (use_pager != -1 || pager_in_use())
return;
- use_pager = check_pager_config(cmd);
+ use_pager = check_pager_config(the_repository, cmd);
if (use_pager == -1)
use_pager = def;
commit_pager_choice();
@@ -462,7 +462,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
precompose_argv_prefix(argc, argv, NULL);
if (use_pager == -1 && run_setup &&
!(p->option & DELAY_PAGER_CONFIG))
- use_pager = check_pager_config(p->cmd);
+ use_pager = check_pager_config(the_repository, p->cmd);
if (use_pager == -1 && p->option & USE_PAGER)
use_pager = 1;
if (run_setup && startup_info->have_repository)
@@ -750,7 +750,7 @@ static void execv_dashed_external(const char **argv)
int status;
if (use_pager == -1 && !is_builtin(argv[0]))
- use_pager = check_pager_config(argv[0]);
+ use_pager = check_pager_config(the_repository, argv[0]);
commit_pager_choice();
strvec_pushf(&cmd.args, "git-%s", argv[0]);
--git a/pager.c b/pager.c
index 40b664f893c8ec61af0e6d00d58956f45b247d65..5531fff50eb73f7d22defcc7d8e752c94f741d66 100644
--- a/pager.c
+++ b/pager.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "config.h"
#include "editor.h"
@@ -84,7 +82,7 @@ static int core_pager_config(const char *var, const char *value,
return 0;
}
-const char *git_pager(int stdout_is_tty)
+const char *git_pager(struct repository *r, int stdout_is_tty)
{
const char *pager;
@@ -94,7 +92,7 @@ const char *git_pager(int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
- read_early_config(the_repository,
+ read_early_config(r,
core_pager_config, NULL);
pager = pager_program;
}
@@ -143,10 +141,10 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
pager_process->trace2_child_class = "pager";
}
-void setup_pager(void)
+void setup_pager(struct repository *r)
{
static int once = 0;
- const char *pager = git_pager(isatty(1));
+ const char *pager = git_pager(r, isatty(1));
if (!pager)
return;
@@ -293,7 +291,7 @@ static int pager_command_config(const char *var, const char *value,
}
/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
-int check_pager_config(const char *cmd)
+int check_pager_config(struct repository *r, const char *cmd)
{
struct pager_command_config_data data;
@@ -301,7 +299,7 @@ int check_pager_config(const char *cmd)
data.want = -1;
data.value = NULL;
- read_early_config(the_repository, pager_command_config, &data);
+ read_early_config(r, pager_command_config, &data);
if (data.value)
pager_program = data.value;
--git a/pager.h b/pager.h
index 103ecac476fdd6523930d7ff8fe542eeec038438..d070be6348971663d3ac913ed702a7ba541cb4e8 100644
--- a/pager.h
+++ b/pager.h
@@ -2,15 +2,16 @@
#define PAGER_H
struct child_process;
+struct repository;
-const char *git_pager(int stdout_is_tty);
-void setup_pager(void);
+const char *git_pager(struct repository *r, int stdout_is_tty);
+void setup_pager(struct repository *r);
void wait_for_pager(void);
int pager_in_use(void);
int term_columns(void);
void term_clear_line(void);
int decimal_width(uintmax_t);
-int check_pager_config(const char *cmd);
+int check_pager_config(struct repository *r, const char *cmd);
void prepare_pager_args(struct child_process *, const char *pager);
extern int pager_use_color;
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 03/14] trace: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 01/14] progress: stop using `the_repository` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 02/14] pager: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 04/14] serve: " Patrick Steinhardt
` (12 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "trace" subsystem by passing in a
repository when setting up tracing.
Adjust the only caller accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
git.c | 2 +-
trace.c | 9 ++++-----
trace.h | 4 +++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/git.c b/git.c
index d977ebc84cfba611c3e452cace3bda1ce13faf5d..a94dab3770251fa8b7b466daa85c2d61ca730670 100644
--- a/git.c
+++ b/git.c
@@ -467,7 +467,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
use_pager = 1;
if (run_setup && startup_info->have_repository)
/* get_git_dir() may set up repo, avoid that */
- trace_repo_setup();
+ trace_repo_setup(the_repository);
commit_pager_choice();
if (!help && p->option & NEED_WORK_TREE)
diff --git a/trace.c b/trace.c
index 2cfd25942ee725b022e8dea26096f01f2164a407..9b99460db82a28634c09260b42f311f7c69f4d59 100644
--- a/trace.c
+++ b/trace.c
@@ -21,7 +21,6 @@
* along with this program; if not, see <https://www.gnu.org/licenses/>.
*/
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -298,7 +297,7 @@ static const char *quote_crnl(const char *path)
return new_path.buf;
}
-void trace_repo_setup(void)
+void trace_repo_setup(struct repository *r)
{
const char *git_work_tree, *prefix = startup_info->prefix;
char *cwd;
@@ -308,14 +307,14 @@ void trace_repo_setup(void)
cwd = xgetcwd();
- if (!(git_work_tree = repo_get_work_tree(the_repository)))
+ if (!(git_work_tree = repo_get_work_tree(r)))
git_work_tree = "(null)";
if (!startup_info->prefix)
prefix = "(null)";
- trace_printf_key(&trace_setup_key, "setup: git_dir: %s\n", quote_crnl(repo_get_git_dir(the_repository)));
- trace_printf_key(&trace_setup_key, "setup: git_common_dir: %s\n", quote_crnl(repo_get_common_dir(the_repository)));
+ trace_printf_key(&trace_setup_key, "setup: git_dir: %s\n", quote_crnl(repo_get_git_dir(r)));
+ trace_printf_key(&trace_setup_key, "setup: git_common_dir: %s\n", quote_crnl(repo_get_common_dir(r)));
trace_printf_key(&trace_setup_key, "setup: worktree: %s\n", quote_crnl(git_work_tree));
trace_printf_key(&trace_setup_key, "setup: cwd: %s\n", quote_crnl(cwd));
trace_printf_key(&trace_setup_key, "setup: prefix: %s\n", quote_crnl(prefix));
diff --git a/trace.h b/trace.h
index d304d55aa1d706dc3f65bbf15c7b2506bc0e9499..9152fe9b3e565ee8dde1ddf185f30f021812a416 100644
--- a/trace.h
+++ b/trace.h
@@ -92,7 +92,9 @@ extern struct trace_key trace_default_key;
extern struct trace_key trace_perf_key;
extern struct trace_key trace_setup_key;
-void trace_repo_setup(void);
+struct repository;
+
+void trace_repo_setup(struct repository *r);
/**
* Checks whether the trace key is enabled. Used to prevent expensive
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 04/14] serve: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (2 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 03/14] trace: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 05/14] send-pack: " Patrick Steinhardt
` (11 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "serve" subsystem by passing in a
repository when advertising capabilities or serving requests.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/upload-pack.c | 6 ++++--
serve.c | 36 +++++++++++++++++-------------------
serve.h | 6 ++++--
t/helper/test-serve-v2.c | 7 +++++--
4 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
index dd63d6eadfe0028b3a62d7d14f777c919f567cc8..c2bbc035ab0c91baf5d66cee8bb370ecf1640505 100644
--- a/builtin/upload-pack.c
+++ b/builtin/upload-pack.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
#include "builtin.h"
#include "exec-cmd.h"
#include "gettext.h"
@@ -63,9 +65,9 @@ int cmd_upload_pack(int argc,
switch (determine_protocol_version_server()) {
case protocol_v2:
if (advertise_refs)
- protocol_v2_advertise_capabilities();
+ protocol_v2_advertise_capabilities(the_repository);
else
- protocol_v2_serve_loop(stateless_rpc);
+ protocol_v2_serve_loop(the_repository, stateless_rpc);
break;
case protocol_v1:
/*
diff --git a/serve.c b/serve.c
index c8694e375159ca0044cb045500954770e1e5cb93..f6dfe34a2bee6b24bedace2c272f8377dec9fb91 100644
--- a/serve.c
+++ b/serve.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "repository.h"
#include "config.h"
@@ -159,7 +157,7 @@ static struct protocol_capability capabilities[] = {
},
};
-void protocol_v2_advertise_capabilities(void)
+void protocol_v2_advertise_capabilities(struct repository *r)
{
struct strbuf capability = STRBUF_INIT;
struct strbuf value = STRBUF_INIT;
@@ -170,7 +168,7 @@ void protocol_v2_advertise_capabilities(void)
for (size_t i = 0; i < ARRAY_SIZE(capabilities); i++) {
struct protocol_capability *c = &capabilities[i];
- if (c->advertise(the_repository, &value)) {
+ if (c->advertise(r, &value)) {
strbuf_addstr(&capability, c->name);
if (value.len) {
@@ -214,20 +212,20 @@ static struct protocol_capability *get_capability(const char *key, const char **
return NULL;
}
-static int receive_client_capability(const char *key)
+static int receive_client_capability(struct repository *r, const char *key)
{
const char *value;
const struct protocol_capability *c = get_capability(key, &value);
- if (!c || c->command || !c->advertise(the_repository, NULL))
+ if (!c || c->command || !c->advertise(r, NULL))
return 0;
if (c->receive)
- c->receive(the_repository, value);
+ c->receive(r, value);
return 1;
}
-static int parse_command(const char *key, struct protocol_capability **command)
+static int parse_command(struct repository *r, const char *key, struct protocol_capability **command)
{
const char *out;
@@ -238,7 +236,7 @@ static int parse_command(const char *key, struct protocol_capability **command)
if (*command)
die("command '%s' requested after already requesting command '%s'",
out, (*command)->name);
- if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command || value)
+ if (!cmd || !cmd->advertise(r, NULL) || !cmd->command || value)
die("invalid command '%s'", out);
*command = cmd;
@@ -253,7 +251,7 @@ enum request_state {
PROCESS_REQUEST_DONE,
};
-static int process_request(void)
+static int process_request(struct repository *r)
{
enum request_state state = PROCESS_REQUEST_KEYS;
struct packet_reader reader;
@@ -278,8 +276,8 @@ static int process_request(void)
case PACKET_READ_EOF:
BUG("Should have already died when seeing EOF");
case PACKET_READ_NORMAL:
- if (parse_command(reader.line, &command) ||
- receive_client_capability(reader.line))
+ if (parse_command(r, reader.line, &command) ||
+ receive_client_capability(r, reader.line))
seen_capability_or_command = 1;
else
die("unknown capability '%s'", reader.line);
@@ -319,30 +317,30 @@ static int process_request(void)
if (!command)
die("no command requested");
- if (client_hash_algo != hash_algo_by_ptr(the_repository->hash_algo))
+ if (client_hash_algo != hash_algo_by_ptr(r->hash_algo))
die("mismatched object format: server %s; client %s",
- the_repository->hash_algo->name,
+ r->hash_algo->name,
hash_algos[client_hash_algo].name);
- command->command(the_repository, &reader);
+ command->command(r, &reader);
return 0;
}
-void protocol_v2_serve_loop(int stateless_rpc)
+void protocol_v2_serve_loop(struct repository *r, int stateless_rpc)
{
if (!stateless_rpc)
- protocol_v2_advertise_capabilities();
+ protocol_v2_advertise_capabilities(r);
/*
* If stateless-rpc was requested then exit after
* a single request/response exchange
*/
if (stateless_rpc) {
- process_request();
+ process_request(r);
} else {
for (;;)
- if (process_request())
+ if (process_request(r))
break;
}
}
diff --git a/serve.h b/serve.h
index f946cf904a242db5106625e280d7daa671348516..85bf73cfe53cb9cbb4b042c43a1f6a55338ad6ed 100644
--- a/serve.h
+++ b/serve.h
@@ -1,7 +1,9 @@
#ifndef SERVE_H
#define SERVE_H
-void protocol_v2_advertise_capabilities(void);
-void protocol_v2_serve_loop(int stateless_rpc);
+struct repository;
+
+void protocol_v2_advertise_capabilities(struct repository *r);
+void protocol_v2_serve_loop(struct repository *r, int stateless_rpc);
#endif /* SERVE_H */
diff --git a/t/helper/test-serve-v2.c b/t/helper/test-serve-v2.c
index 054cbcf5d83946b225774dc9da6b0ec1d112e79d..63a200b8d46f68bfd69f63f844977cc8e382bb32 100644
--- a/t/helper/test-serve-v2.c
+++ b/t/helper/test-serve-v2.c
@@ -1,6 +1,9 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
#include "test-tool.h"
#include "gettext.h"
#include "parse-options.h"
+#include "repository.h"
#include "serve.h"
#include "setup.h"
@@ -28,9 +31,9 @@ int cmd__serve_v2(int argc, const char **argv)
PARSE_OPT_KEEP_UNKNOWN_OPT);
if (advertise_capabilities)
- protocol_v2_advertise_capabilities();
+ protocol_v2_advertise_capabilities(the_repository);
else
- protocol_v2_serve_loop(stateless_rpc);
+ protocol_v2_serve_loop(the_repository, stateless_rpc);
return 0;
}
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 05/14] send-pack: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (3 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 04/14] serve: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 06/14] server-info: " Patrick Steinhardt
` (10 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "send-pack" subsystem by passing in a
repository when sending a packfile.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/send-pack.c | 2 +-
send-pack.c | 77 ++++++++++++++++++++++++++++-------------------------
send-pack.h | 3 ++-
transport.c | 2 +-
4 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 59b626aae8cd8291104d83b5ec201207c97715e8..8d461008e2e860a3de91a69412d66dcbde4d7b96 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -317,7 +317,7 @@ int cmd_send_pack(int argc,
set_ref_status_for_push(remote_refs, args.send_mirror,
args.force_update);
- ret = send_pack(&args, fd, conn, remote_refs, &extra_have);
+ ret = send_pack(the_repository, &args, fd, conn, remote_refs, &extra_have);
if (helper_status)
print_helper_status(remote_refs);
diff --git a/send-pack.c b/send-pack.c
index 7e8321368379efe2600a1f573e2e4cd5140a008d..772c7683a0157004ede74e56157e3a5d7f082e75 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "config.h"
#include "commit.h"
@@ -44,10 +42,11 @@ int option_parse_push_signed(const struct option *opt,
die("bad %s argument: %s", opt->long_name, arg);
}
-static void feed_object(const struct object_id *oid, FILE *fh, int negative)
+static void feed_object(struct repository *r,
+ const struct object_id *oid, FILE *fh, int negative)
{
if (negative &&
- !repo_has_object_file_with_flags(the_repository, oid,
+ !repo_has_object_file_with_flags(r, oid,
OBJECT_INFO_SKIP_FETCH_OBJECT |
OBJECT_INFO_QUICK))
return;
@@ -61,7 +60,8 @@ static void feed_object(const struct object_id *oid, FILE *fh, int negative)
/*
* Make a pack stream and spit it out into file descriptor fd
*/
-static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
+static int pack_objects(struct repository *r,
+ int fd, struct ref *refs, struct oid_array *advertised,
struct oid_array *negotiated,
struct send_pack_args *args)
{
@@ -74,7 +74,7 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
FILE *po_in;
int rc;
- trace2_region_enter("send_pack", "pack_objects", the_repository);
+ trace2_region_enter("send_pack", "pack_objects", r);
strvec_push(&po.args, "pack-objects");
strvec_push(&po.args, "--all-progress-implied");
strvec_push(&po.args, "--revs");
@@ -87,7 +87,7 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
strvec_push(&po.args, "-q");
if (args->progress)
strvec_push(&po.args, "--progress");
- if (is_repository_shallow(the_repository))
+ if (is_repository_shallow(r))
strvec_push(&po.args, "--shallow");
if (args->disable_bitmaps)
strvec_push(&po.args, "--no-use-bitmap-index");
@@ -104,15 +104,15 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
*/
po_in = xfdopen(po.in, "w");
for (size_t i = 0; i < advertised->nr; i++)
- feed_object(&advertised->oid[i], po_in, 1);
+ feed_object(r, &advertised->oid[i], po_in, 1);
for (size_t i = 0; i < negotiated->nr; i++)
- feed_object(&negotiated->oid[i], po_in, 1);
+ feed_object(r, &negotiated->oid[i], po_in, 1);
while (refs) {
if (!is_null_oid(&refs->old_oid))
- feed_object(&refs->old_oid, po_in, 1);
+ feed_object(r, &refs->old_oid, po_in, 1);
if (!is_null_oid(&refs->new_oid))
- feed_object(&refs->new_oid, po_in, 0);
+ feed_object(r, &refs->new_oid, po_in, 0);
refs = refs->next;
}
@@ -146,10 +146,10 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *advertised,
*/
if (rc > 128 && rc != 141)
error("pack-objects died of signal %d", rc - 128);
- trace2_region_leave("send_pack", "pack_objects", the_repository);
+ trace2_region_leave("send_pack", "pack_objects", r);
return -1;
}
- trace2_region_leave("send_pack", "pack_objects", the_repository);
+ trace2_region_leave("send_pack", "pack_objects", r);
return 0;
}
@@ -164,7 +164,8 @@ static int receive_unpack_status(struct packet_reader *reader)
return 0;
}
-static int receive_status(struct packet_reader *reader, struct ref *refs)
+static int receive_status(struct repository *r,
+ struct packet_reader *reader, struct ref *refs)
{
struct ref *hint;
int ret;
@@ -172,7 +173,7 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
int new_report = 0;
int once = 0;
- trace2_region_enter("send_pack", "receive_status", the_repository);
+ trace2_region_enter("send_pack", "receive_status", r);
hint = NULL;
ret = receive_unpack_status(reader);
while (1) {
@@ -221,10 +222,10 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
if (!strcmp(key, "refname"))
report->ref_name = xstrdup_or_null(val);
else if (!strcmp(key, "old-oid") && val &&
- !parse_oid_hex(val, &old_oid, &val))
+ !parse_oid_hex_algop(val, &old_oid, &val, r->hash_algo))
report->old_oid = oiddup(&old_oid);
else if (!strcmp(key, "new-oid") && val &&
- !parse_oid_hex(val, &new_oid, &val))
+ !parse_oid_hex_algop(val, &new_oid, &val, r->hash_algo))
report->new_oid = oiddup(&new_oid);
else if (!strcmp(key, "forced-update"))
report->forced_update = 1;
@@ -271,7 +272,7 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
new_report = 1;
}
}
- trace2_region_leave("send_pack", "receive_status", the_repository);
+ trace2_region_leave("send_pack", "receive_status", r);
return ret;
}
@@ -293,9 +294,9 @@ static int advertise_shallow_grafts_cb(const struct commit_graft *graft, void *c
return 0;
}
-static void advertise_shallow_grafts_buf(struct strbuf *sb)
+static void advertise_shallow_grafts_buf(struct repository *r, struct strbuf *sb)
{
- if (!is_repository_shallow(the_repository))
+ if (!is_repository_shallow(r))
return;
for_each_commit_graft(advertise_shallow_grafts_cb, sb);
}
@@ -426,13 +427,14 @@ static void reject_invalid_nonce(const char *nonce, int len)
}
}
-static void get_commons_through_negotiation(const char *url,
+static void get_commons_through_negotiation(struct repository *r,
+ const char *url,
const struct ref *remote_refs,
struct oid_array *commons)
{
struct child_process child = CHILD_PROCESS_INIT;
const struct ref *ref;
- int len = the_hash_algo->hexsz + 1; /* hash + NL */
+ int len = r->hash_algo->hexsz + 1; /* hash + NL */
int nr_negotiation_tip = 0;
child.git_cmd = 1;
@@ -466,7 +468,7 @@ static void get_commons_through_negotiation(const char *url,
break;
if (read_len != len)
die("invalid length read %d", read_len);
- if (parse_oid_hex(hex_hash, &oid, &end) || *end != '\n')
+ if (parse_oid_hex_algop(hex_hash, &oid, &end, r->hash_algo) || *end != '\n')
die("invalid hash");
oid_array_append(commons, &oid);
} while (1);
@@ -480,7 +482,8 @@ static void get_commons_through_negotiation(const char *url,
}
}
-int send_pack(struct send_pack_args *args,
+int send_pack(struct repository *r,
+ struct send_pack_args *args,
int fd[], struct child_process *conn,
struct ref *remote_refs,
struct oid_array *extra_have)
@@ -518,17 +521,17 @@ int send_pack(struct send_pack_args *args,
goto out;
}
- git_config_get_bool("push.negotiate", &push_negotiate);
+ repo_config_get_bool(r, "push.negotiate", &push_negotiate);
if (push_negotiate) {
- trace2_region_enter("send_pack", "push_negotiate", the_repository);
- get_commons_through_negotiation(args->url, remote_refs, &commons);
- trace2_region_leave("send_pack", "push_negotiate", the_repository);
+ trace2_region_enter("send_pack", "push_negotiate", r);
+ get_commons_through_negotiation(r, args->url, remote_refs, &commons);
+ trace2_region_leave("send_pack", "push_negotiate", r);
}
- if (!git_config_get_bool("push.usebitmaps", &use_bitmaps))
+ if (!repo_config_get_bool(r, "push.usebitmaps", &use_bitmaps))
args->disable_bitmaps = !use_bitmaps;
- git_config_get_bool("transfer.advertisesid", &advertise_sid);
+ repo_config_get_bool(r, "transfer.advertisesid", &advertise_sid);
/* Does the other end support the reporting? */
if (server_supports("report-status-v2"))
@@ -554,7 +557,7 @@ int send_pack(struct send_pack_args *args,
if (server_supports("push-options"))
push_options_supported = 1;
- if (!server_supports_hash(the_hash_algo->name, &object_format_supported))
+ if (!server_supports_hash(r->hash_algo->name, &object_format_supported))
die(_("the receiving end does not support this repository's hash algorithm"));
if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {
@@ -596,7 +599,7 @@ int send_pack(struct send_pack_args *args,
if (use_push_options)
strbuf_addstr(&cap_buf, " push-options");
if (object_format_supported)
- strbuf_addf(&cap_buf, " object-format=%s", the_hash_algo->name);
+ strbuf_addf(&cap_buf, " object-format=%s", r->hash_algo->name);
if (agent_supported)
strbuf_addf(&cap_buf, " agent=%s", git_user_agent_sanitized());
if (advertise_sid)
@@ -646,7 +649,7 @@ int send_pack(struct send_pack_args *args,
}
if (!args->dry_run)
- advertise_shallow_grafts_buf(&req_buf);
+ advertise_shallow_grafts_buf(r, &req_buf);
/*
* Finally, tell the other end!
@@ -686,7 +689,7 @@ int send_pack(struct send_pack_args *args,
}
if (args->stateless_rpc) {
- if (!args->dry_run && (cmds_sent || is_repository_shallow(the_repository))) {
+ if (!args->dry_run && (cmds_sent || is_repository_shallow(r))) {
packet_buf_flush(&req_buf);
send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
}
@@ -711,7 +714,7 @@ int send_pack(struct send_pack_args *args,
PACKET_READ_DIE_ON_ERR_PACKET);
if (need_pack_data && cmds_sent) {
- if (pack_objects(out, remote_refs, extra_have, &commons, args) < 0) {
+ if (pack_objects(r, out, remote_refs, extra_have, &commons, args) < 0) {
if (args->stateless_rpc)
close(out);
if (git_connection_is_socket(conn))
@@ -724,7 +727,7 @@ int send_pack(struct send_pack_args *args,
* we get one).
*/
if (status_report)
- receive_status(&reader, remote_refs);
+ receive_status(r, &reader, remote_refs);
if (use_sideband) {
close(demux.out);
@@ -743,7 +746,7 @@ int send_pack(struct send_pack_args *args,
packet_flush(out);
if (status_report && cmds_sent)
- ret = receive_status(&reader, remote_refs);
+ ret = receive_status(r, &reader, remote_refs);
else
ret = 0;
if (args->stateless_rpc)
diff --git a/send-pack.h b/send-pack.h
index 7edb80596c7b0edf607ea5cd0f01315106d02b73..d256715681b363d46db284b7ed22658960781571 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -6,6 +6,7 @@
struct child_process;
struct oid_array;
struct ref;
+struct repository;
/* Possible values for push_cert field in send_pack_args. */
#define SEND_PACK_PUSH_CERT_NEVER 0
@@ -35,7 +36,7 @@ struct option;
int option_parse_push_signed(const struct option *opt,
const char *arg, int unset);
-int send_pack(struct send_pack_args *args,
+int send_pack(struct repository *r, struct send_pack_args *args,
int fd[], struct child_process *conn,
struct ref *remote_refs, struct oid_array *extra_have);
diff --git a/transport.c b/transport.c
index 10d820c33353f695691506560f817c7515218139..81ae8243b9a32ab5289149e51a5a4b46340ac1fe 100644
--- a/transport.c
+++ b/transport.c
@@ -932,7 +932,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
break;
case protocol_v1:
case protocol_v0:
- ret = send_pack(&args, data->fd, data->conn, remote_refs,
+ ret = send_pack(the_repository, &args, data->fd, data->conn, remote_refs,
&data->extra_have);
break;
case protocol_unknown_version:
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 06/14] server-info: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (4 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 05/14] send-pack: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 12:31 ` shejialuo
2024-12-17 6:43 ` [PATCH 07/14] diagnose: " Patrick Steinhardt
` (9 subsequent siblings)
15 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "server-info" subsystem by passing in
a repository when updating server info and storing the repository in the
`update_info_ctx` structure to make it accessible to other functions.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/receive-pack.c | 2 +-
builtin/repack.c | 2 +-
builtin/update-server-info.c | 2 +-
server-info.c | 40 ++++++++++++++++++++++------------------
server-info.h | 4 +++-
5 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c2e9103f112c1e37e9f030308633daf49eec1ecf..191b5eeb34e6791776f93a3a9509efa887e4e087 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2628,7 +2628,7 @@ int cmd_receive_pack(int argc,
}
}
if (auto_update_server_info)
- update_server_info(0);
+ update_server_info(the_repository, 0);
clear_shallow_info(&si);
}
if (use_sideband)
diff --git a/builtin/repack.c b/builtin/repack.c
index 0c6dad7df47a1665026a348921c33b2067b59976..81d13630ea41f832bd0210971c13a82a3ddc0971 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -1565,7 +1565,7 @@ int cmd_repack(int argc,
}
if (run_update_server_info)
- update_server_info(0);
+ update_server_info(the_repository, 0);
if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0)) {
unsigned flags = 0;
diff --git a/builtin/update-server-info.c b/builtin/update-server-info.c
index 6769611a025d0d69bcd3dbbd06f5fed056262911..47a3f0bdd9c3498808b86c2e31f4489926226102 100644
--- a/builtin/update-server-info.c
+++ b/builtin/update-server-info.c
@@ -27,5 +27,5 @@ int cmd_update_server_info(int argc,
if (argc > 0)
usage_with_options(update_server_info_usage, options);
- return !!update_server_info(force);
+ return !!update_server_info(the_repository, force);
}
diff --git a/server-info.c b/server-info.c
index ef2f3f4b5c7b04c46520b2fd9d4352b806658f40..31c3fdc118447d42745362935e3483c59b7e0bc2 100644
--- a/server-info.c
+++ b/server-info.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -18,6 +17,7 @@
#include "tempfile.h"
struct update_info_ctx {
+ struct repository *repo;
FILE *cur_fp;
FILE *old_fp; /* becomes NULL if it differs from cur_fp */
struct strbuf cur_sb;
@@ -73,7 +73,7 @@ static int uic_printf(struct update_info_ctx *uic, const char *fmt, ...)
* it into place. The contents of the file come from "generate", which
* should return non-zero if it encounters an error.
*/
-static int update_info_file(char *path,
+static int update_info_file(struct repository *r, char *path,
int (*generate)(struct update_info_ctx *),
int force)
{
@@ -81,6 +81,7 @@ static int update_info_file(char *path,
struct tempfile *f = NULL;
int ret = -1;
struct update_info_ctx uic = {
+ .repo = r,
.cur_fp = NULL,
.old_fp = NULL,
.cur_sb = STRBUF_INIT,
@@ -152,7 +153,7 @@ static int add_info_ref(const char *path, const char *referent UNUSED, const str
void *cb_data)
{
struct update_info_ctx *uic = cb_data;
- struct object *o = parse_object(the_repository, oid);
+ struct object *o = parse_object(uic->repo, oid);
if (!o)
return -1;
@@ -160,7 +161,7 @@ static int add_info_ref(const char *path, const char *referent UNUSED, const str
return -1;
if (o->type == OBJ_TAG) {
- o = deref_tag(the_repository, o, path, 0);
+ o = deref_tag(uic->repo, o, path, 0);
if (o)
if (uic_printf(uic, "%s %s^{}\n",
oid_to_hex(&o->oid), path) < 0)
@@ -171,14 +172,14 @@ static int add_info_ref(const char *path, const char *referent UNUSED, const str
static int generate_info_refs(struct update_info_ctx *uic)
{
- return refs_for_each_ref(get_main_ref_store(the_repository),
+ return refs_for_each_ref(get_main_ref_store(uic->repo),
add_info_ref, uic);
}
-static int update_info_refs(int force)
+static int update_info_refs(struct repository *r, int force)
{
- char *path = git_pathdup("info/refs");
- int ret = update_info_file(path, generate_info_refs, force);
+ char *path = repo_git_path(r, "info/refs");
+ int ret = update_info_file(r, path, generate_info_refs, force);
free(path);
return ret;
}
@@ -284,14 +285,14 @@ static int compare_info(const void *a_, const void *b_)
return 1;
}
-static void init_pack_info(const char *infofile, int force)
+static void init_pack_info(struct repository *r, const char *infofile, int force)
{
struct packed_git *p;
int stale;
int i;
size_t alloc = 0;
- for (p = get_all_packs(the_repository); p; p = p->next) {
+ for (p = get_all_packs(r); p; p = p->next) {
/* we ignore things on alternate path since they are
* not available to the pullers in general.
*/
@@ -340,33 +341,36 @@ static int write_pack_info_file(struct update_info_ctx *uic)
return 0;
}
-static int update_info_packs(int force)
+static int update_info_packs(struct repository *r, int force)
{
char *infofile = mkpathdup("%s/info/packs",
- repo_get_object_directory(the_repository));
+ repo_get_object_directory(r));
int ret;
- init_pack_info(infofile, force);
- ret = update_info_file(infofile, write_pack_info_file, force);
+ init_pack_info(r, infofile, force);
+ ret = update_info_file(r, infofile, write_pack_info_file, force);
free_pack_info();
free(infofile);
return ret;
}
/* public */
-int update_server_info(int force)
+int update_server_info(struct repository *r, int force)
{
/* We would add more dumb-server support files later,
* including index of available pack files and their
* intended audiences.
*/
int errs = 0;
+ char *path;
- errs = errs | update_info_refs(force);
- errs = errs | update_info_packs(force);
+ errs = errs | update_info_refs(r, force);
+ errs = errs | update_info_packs(r, force);
/* remove leftover rev-cache file if there is any */
- unlink_or_warn(git_path("info/rev-cache"));
+ path = repo_git_path(r, "info/rev-cache");
+ unlink_or_warn(path);
+ free(path);
return errs;
}
diff --git a/server-info.h b/server-info.h
index 13bbde2c55fafe7bf07eb38377cec2f109de3164..e634d1722bdfaaa506777b4df370d908b05005e9 100644
--- a/server-info.h
+++ b/server-info.h
@@ -1,7 +1,9 @@
#ifndef SERVER_INFO_H
#define SERVER_INFO_H
+struct repository;
+
/* Dumb servers support */
-int update_server_info(int);
+int update_server_info(struct repository *r, int force);
#endif /* SERVER_INFO_H */
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 07/14] diagnose: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (5 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 06/14] server-info: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 08/14] mailinfo: " Patrick Steinhardt
` (8 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "diagnose" subsystem by passing in a
repository when generating a diagnostics archive.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/bugreport.c | 2 +-
builtin/diagnose.c | 4 +++-
diagnose.c | 15 ++++++++-------
diagnose.h | 5 ++++-
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index 7c2df035c9cff06f649c2b35394ea3db3d39f469..0ac59cc8dc5824fbc155674e9107bb845d8e054c 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c
@@ -167,7 +167,7 @@ int cmd_bugreport(int argc,
strbuf_addftime(&zip_path, option_suffix, localtime_r(&now, &tm), 0, 0);
strbuf_addstr(&zip_path, ".zip");
- if (create_diagnostics_archive(&zip_path, diagnose))
+ if (create_diagnostics_archive(the_repository, &zip_path, diagnose))
die_errno(_("unable to create diagnostics archive %s"), zip_path.buf);
strbuf_release(&zip_path);
diff --git a/builtin/diagnose.c b/builtin/diagnose.c
index 66a22d918e68a48a7c31ef3b75335f9a2bad7ab1..33c39bd5981f22d96b1b19e19ff83dec1c2873a6 100644
--- a/builtin/diagnose.c
+++ b/builtin/diagnose.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
#include "builtin.h"
#include "abspath.h"
#include "gettext.h"
@@ -58,7 +60,7 @@ int cmd_diagnose(int argc,
}
/* Prepare diagnostics */
- if (create_diagnostics_archive(&zip_path, mode))
+ if (create_diagnostics_archive(the_repository, &zip_path, mode))
die_errno(_("unable to create diagnostics archive %s"),
zip_path.buf);
diff --git a/diagnose.c b/diagnose.c
index b11931df86c4ba84f7aec77cb7965083f5aa31fa..bd485effea22ce400c4300a1e085b45204a3b42e 100644
--- a/diagnose.c
+++ b/diagnose.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "diagnose.h"
#include "compat/disk.h"
@@ -12,6 +10,7 @@
#include "object-store-ll.h"
#include "packfile.h"
#include "parse-options.h"
+#include "repository.h"
#include "write-or-die.h"
struct archive_dir {
@@ -179,7 +178,9 @@ static int add_directory_to_archiver(struct strvec *archiver_args,
return res;
}
-int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode)
+int create_diagnostics_archive(struct repository *r,
+ struct strbuf *zip_path,
+ enum diagnose_mode mode)
{
struct strvec archiver_args = STRVEC_INIT;
char **argv_copy = NULL;
@@ -218,7 +219,7 @@ int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode)
strbuf_addstr(&buf, "Collecting diagnostic info\n\n");
get_version_info(&buf, 1);
- strbuf_addf(&buf, "Repository root: %s\n", the_repository->worktree);
+ strbuf_addf(&buf, "Repository root: %s\n", r->worktree);
get_disk_info(&buf);
write_or_die(stdout_fd, buf.buf, buf.len);
strvec_pushf(&archiver_args,
@@ -227,7 +228,7 @@ int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode)
strbuf_reset(&buf);
strbuf_addstr(&buf, "--add-virtual-file=packs-local.txt:");
- dir_file_stats(the_repository->objects->odb, &buf);
+ dir_file_stats(r->objects->odb, &buf);
foreach_alt_odb(dir_file_stats, &buf);
strvec_push(&archiver_args, buf.buf);
@@ -250,13 +251,13 @@ int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode)
}
strvec_pushl(&archiver_args, "--prefix=",
- oid_to_hex(the_hash_algo->empty_tree), "--", NULL);
+ oid_to_hex(r->hash_algo->empty_tree), "--", NULL);
/* `write_archive()` modifies the `argv` passed to it. Let it. */
argv_copy = xmemdupz(archiver_args.v,
sizeof(char *) * archiver_args.nr);
res = write_archive(archiver_args.nr, (const char **)argv_copy, NULL,
- the_repository, NULL, 0);
+ r, NULL, 0);
if (res) {
error(_("failed to write archive"));
goto diagnose_cleanup;
diff --git a/diagnose.h b/diagnose.h
index f525219ab0cf9b36249892c847e731b952b081af..f7b38f49f5271ad00bf17d14e7b2aab3e9e174d5 100644
--- a/diagnose.h
+++ b/diagnose.h
@@ -4,6 +4,7 @@
#include "strbuf.h"
struct option;
+struct repository;
enum diagnose_mode {
DIAGNOSE_NONE,
@@ -13,6 +14,8 @@ enum diagnose_mode {
int option_parse_diagnose(const struct option *opt, const char *arg, int unset);
-int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode);
+int create_diagnostics_archive(struct repository *r,
+ struct strbuf *zip_path,
+ enum diagnose_mode mode);
#endif /* DIAGNOSE_H */
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 08/14] mailinfo: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (6 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 07/14] diagnose: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 09/14] credential: " Patrick Steinhardt
` (7 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "mailinfo" subsystem by passing in
a repository when setting up the mailinfo structure.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/am.c | 2 +-
builtin/mailinfo.c | 2 +-
mailinfo.c | 5 ++---
mailinfo.h | 4 +++-
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 27ccca8341feefcda5e1c4a850f14fce9e4deecb..e94d08e04b2387b6d45053e17c78c93038fa9531 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1211,7 +1211,7 @@ static int parse_mail(struct am_state *state, const char *mail)
int ret = 0;
struct mailinfo mi;
- setup_mailinfo(&mi);
+ setup_mailinfo(the_repository, &mi);
if (state->utf8)
mi.metainfo_charset = get_commit_output_encoding();
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index e17dec27b1df69c1e2a1020f245dd99c0973eaad..8de7ba7de1d6aadc610adec11e7394f7b1f538b9 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -83,7 +83,7 @@ int cmd_mailinfo(int argc,
OPT_END()
};
- setup_mailinfo(&mi);
+ setup_mailinfo(the_repository, &mi);
meta_charset.policy = CHARSET_DEFAULT;
argc = parse_options(argc, argv, prefix, options, mailinfo_usage, 0);
diff --git a/mailinfo.c b/mailinfo.c
index aa263bf490881daa915a03a681c00979f138d09b..7b001fa5dbd685cbe2b51c880e961e68461bef31 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -1269,7 +1268,7 @@ static int git_mailinfo_config(const char *var, const char *value,
return 0;
}
-void setup_mailinfo(struct mailinfo *mi)
+void setup_mailinfo(struct repository *r, struct mailinfo *mi)
{
memset(mi, 0, sizeof(*mi));
strbuf_init(&mi->name, 0);
@@ -1281,7 +1280,7 @@ void setup_mailinfo(struct mailinfo *mi)
mi->header_stage = 1;
mi->use_inbody_headers = 1;
mi->content_top = mi->content;
- git_config(git_mailinfo_config, mi);
+ repo_config(r, git_mailinfo_config, mi);
}
void clear_mailinfo(struct mailinfo *mi)
diff --git a/mailinfo.h b/mailinfo.h
index f2ffd0349e8007256f5b2118d41faf35a53edf0d..1f2066416578ace2fe6798e543959da43d718a5d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -5,6 +5,8 @@
#define MAX_BOUNDARIES 5
+struct repository;
+
enum quoted_cr_action {
quoted_cr_unset = -1,
quoted_cr_nowarn,
@@ -49,7 +51,7 @@ struct mailinfo {
};
int mailinfo_parse_quoted_cr_action(const char *actionstr, int *action);
-void setup_mailinfo(struct mailinfo *);
+void setup_mailinfo(struct repository *r, struct mailinfo *);
int mailinfo(struct mailinfo *, const char *msg, const char *patch);
void clear_mailinfo(struct mailinfo *);
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 09/14] credential: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (7 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 08/14] mailinfo: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 10/14] resolve-undo: " Patrick Steinhardt
` (6 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "credential" subsystem by passing in
a repository when filling, approving or rejecting credentials.
Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/credential.c | 6 +++---
credential.c | 34 +++++++++++++++++-----------------
credential.h | 11 +++++++----
http.c | 24 ++++++++++++------------
imap-send.c | 10 +++++-----
remote-curl.c | 4 ++--
6 files changed, 46 insertions(+), 43 deletions(-)
diff --git a/builtin/credential.c b/builtin/credential.c
index 14c8c6608b2fbd7091a11c14537d08783d5c281d..614b195b753ed8d0c2495ff9b26672250bf4edcb 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -32,15 +32,15 @@ int cmd_credential(int argc,
die("unable to read credential from stdin");
if (!strcmp(op, "fill")) {
- credential_fill(&c, 0);
+ credential_fill(the_repository, &c, 0);
credential_next_state(&c);
credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
} else if (!strcmp(op, "approve")) {
credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
- credential_approve(&c);
+ credential_approve(the_repository, &c);
} else if (!strcmp(op, "reject")) {
credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
- credential_reject(&c);
+ credential_reject(the_repository, &c);
} else {
usage(usage_msg);
}
diff --git a/credential.c b/credential.c
index a995031c5f5d842f8e82d5e44e4f7a3e51a9e815..b3f87b5b2f16c547d7889c7c60327cd75f3c27d9 100644
--- a/credential.c
+++ b/credential.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -166,7 +165,7 @@ static int match_partial_url(const char *url, void *cb)
return matches;
}
-static void credential_apply_config(struct credential *c)
+static void credential_apply_config(struct repository *r, struct credential *c)
{
char *normalized_url;
struct urlmatch_config config = URLMATCH_CONFIG_INIT;
@@ -191,7 +190,7 @@ static void credential_apply_config(struct credential *c)
credential_format(c, &url);
normalized_url = url_normalize(url.buf, &config.url);
- git_config(urlmatch_config_entry, &config);
+ repo_config(r, urlmatch_config_entry, &config);
string_list_clear(&config.vars, 1);
free(normalized_url);
urlmatch_config_release(&config);
@@ -254,34 +253,34 @@ static char *credential_ask_one(const char *what, struct credential *c,
return xstrdup(r);
}
-static int credential_getpass(struct credential *c)
+static int credential_getpass(struct repository *r, struct credential *c)
{
int interactive;
char *value;
- if (!git_config_get_maybe_bool("credential.interactive", &interactive) &&
+ if (!repo_config_get_maybe_bool(r, "credential.interactive", &interactive) &&
!interactive) {
- trace2_data_intmax("credential", the_repository,
+ trace2_data_intmax("credential", r,
"interactive/skipped", 1);
return -1;
}
- if (!git_config_get_string("credential.interactive", &value)) {
+ if (!repo_config_get_string(r, "credential.interactive", &value)) {
int same = !strcmp(value, "never");
free(value);
if (same) {
- trace2_data_intmax("credential", the_repository,
+ trace2_data_intmax("credential", r,
"interactive/skipped", 1);
return -1;
}
}
- trace2_region_enter("credential", "interactive", the_repository);
+ trace2_region_enter("credential", "interactive", r);
if (!c->username)
c->username = credential_ask_one("Username", c,
PROMPT_ASKPASS|PROMPT_ECHO);
if (!c->password)
c->password = credential_ask_one("Password", c,
PROMPT_ASKPASS);
- trace2_region_leave("credential", "interactive", the_repository);
+ trace2_region_leave("credential", "interactive", r);
return 0;
}
@@ -489,7 +488,8 @@ static int credential_do(struct credential *c, const char *helper,
return r;
}
-void credential_fill(struct credential *c, int all_capabilities)
+void credential_fill(struct repository *r,
+ struct credential *c, int all_capabilities)
{
int i;
@@ -499,7 +499,7 @@ void credential_fill(struct credential *c, int all_capabilities)
credential_next_state(c);
c->multistage = 0;
- credential_apply_config(c);
+ credential_apply_config(r, c);
if (all_capabilities)
credential_set_all_capabilities(c, CREDENTIAL_OP_INITIAL);
@@ -526,12 +526,12 @@ void credential_fill(struct credential *c, int all_capabilities)
c->helpers.items[i].string);
}
- if (credential_getpass(c) ||
+ if (credential_getpass(r, c) ||
(!c->username && !c->password && !c->credential))
die("unable to get password from user");
}
-void credential_approve(struct credential *c)
+void credential_approve(struct repository *r, struct credential *c)
{
int i;
@@ -542,20 +542,20 @@ void credential_approve(struct credential *c)
credential_next_state(c);
- credential_apply_config(c);
+ credential_apply_config(r, c);
for (i = 0; i < c->helpers.nr; i++)
credential_do(c, c->helpers.items[i].string, "store");
c->approved = 1;
}
-void credential_reject(struct credential *c)
+void credential_reject(struct repository *r, struct credential *c)
{
int i;
credential_next_state(c);
- credential_apply_config(c);
+ credential_apply_config(r, c);
for (i = 0; i < c->helpers.nr; i++)
credential_do(c, c->helpers.items[i].string, "erase");
diff --git a/credential.h b/credential.h
index 5f9e6ff2efef55fff8452fc6e50997a759a27118..1e6b0dc5b0f4654435c876b396ca0b6d833567ab 100644
--- a/credential.h
+++ b/credential.h
@@ -4,6 +4,8 @@
#include "string-list.h"
#include "strvec.h"
+struct repository;
+
/**
* The credentials API provides an abstracted way of gathering
* authentication credentials from the user.
@@ -65,7 +67,7 @@
* // Fill in the username and password fields by contacting
* // helpers and/or asking the user. The function will die if it
* // fails.
- * credential_fill(&c);
+ * credential_fill(repo, &c);
*
* // Otherwise, we have a username and password. Try to use it.
*
@@ -218,7 +220,8 @@ void credential_clear(struct credential *);
* If all_capabilities is set, this is an internal user that is prepared
* to deal with all known capabilities, and we should advertise that fact.
*/
-void credential_fill(struct credential *, int all_capabilities);
+void credential_fill(struct repository *, struct credential *,
+ int all_capabilities);
/**
* Inform the credential subsystem that the provided credentials
@@ -227,7 +230,7 @@ void credential_fill(struct credential *, int all_capabilities);
* that they may store the result to be used again. Any errors
* from helpers are ignored.
*/
-void credential_approve(struct credential *);
+void credential_approve(struct repository *, struct credential *);
/**
* Inform the credential subsystem that the provided credentials
@@ -239,7 +242,7 @@ void credential_approve(struct credential *);
* for another call to `credential_fill`). Any errors from helpers
* are ignored.
*/
-void credential_reject(struct credential *);
+void credential_reject(struct repository *, struct credential *);
/**
* Enable all of the supported credential flags in this credential.
diff --git a/http.c b/http.c
index c8fc15aa118d3b1c0e8db1c804948c92a7314b5c..f08b2ae47465332714494cd5ea054880d42702c1 100644
--- a/http.c
+++ b/http.c
@@ -609,7 +609,7 @@ static void init_curl_http_auth(CURL *result)
}
}
- credential_fill(&http_auth, 1);
+ credential_fill(the_repository, &http_auth, 1);
if (http_auth.password) {
if (always_auth_proactively()) {
@@ -652,7 +652,7 @@ static void init_curl_proxy_auth(CURL *result)
{
if (proxy_auth.username) {
if (!proxy_auth.password && !proxy_auth.credential)
- credential_fill(&proxy_auth, 1);
+ credential_fill(the_repository, &proxy_auth, 1);
set_proxyauth_name_password(result);
}
@@ -686,7 +686,7 @@ static int has_cert_password(void)
cert_auth.host = xstrdup("");
cert_auth.username = xstrdup("");
cert_auth.path = xstrdup(ssl_cert);
- credential_fill(&cert_auth, 0);
+ credential_fill(the_repository, &cert_auth, 0);
}
return 1;
}
@@ -700,7 +700,7 @@ static int has_proxy_cert_password(void)
proxy_cert_auth.host = xstrdup("");
proxy_cert_auth.username = xstrdup("");
proxy_cert_auth.path = xstrdup(http_proxy_ssl_cert);
- credential_fill(&proxy_cert_auth, 0);
+ credential_fill(the_repository, &proxy_cert_auth, 0);
}
return 1;
}
@@ -1784,9 +1784,9 @@ static int handle_curl_result(struct slot_results *results)
curl_errorstr, sizeof(curl_errorstr));
if (results->curl_result == CURLE_OK) {
- credential_approve(&http_auth);
- credential_approve(&proxy_auth);
- credential_approve(&cert_auth);
+ credential_approve(the_repository, &http_auth);
+ credential_approve(the_repository, &proxy_auth);
+ credential_approve(the_repository, &cert_auth);
return HTTP_OK;
} else if (results->curl_result == CURLE_SSL_CERTPROBLEM) {
/*
@@ -1795,7 +1795,7 @@ static int handle_curl_result(struct slot_results *results)
* with the certificate. So we reject the credential to
* avoid caching or saving a bad password.
*/
- credential_reject(&cert_auth);
+ credential_reject(the_repository, &cert_auth);
return HTTP_NOAUTH;
} else if (results->curl_result == CURLE_SSL_PINNEDPUBKEYNOTMATCH) {
return HTTP_NOMATCHPUBLICKEY;
@@ -1808,7 +1808,7 @@ static int handle_curl_result(struct slot_results *results)
credential_clear_secrets(&http_auth);
return HTTP_REAUTH;
}
- credential_reject(&http_auth);
+ credential_reject(the_repository, &http_auth);
if (always_auth_proactively())
http_proactive_auth = PROACTIVE_AUTH_NONE;
return HTTP_NOAUTH;
@@ -1822,7 +1822,7 @@ static int handle_curl_result(struct slot_results *results)
}
} else {
if (results->http_connectcode == 407)
- credential_reject(&proxy_auth);
+ credential_reject(the_repository, &proxy_auth);
if (!curl_errorstr[0])
strlcpy(curl_errorstr,
curl_easy_strerror(results->curl_result),
@@ -2210,7 +2210,7 @@ static int http_request_reauth(const char *url,
int ret;
if (always_auth_proactively())
- credential_fill(&http_auth, 1);
+ credential_fill(the_repository, &http_auth, 1);
ret = http_request(url, result, target, options);
@@ -2251,7 +2251,7 @@ static int http_request_reauth(const char *url,
BUG("Unknown http_request target");
}
- credential_fill(&http_auth, 1);
+ credential_fill(the_repository, &http_auth, 1);
ret = http_request(url, result, target, options);
}
diff --git a/imap-send.c b/imap-send.c
index 68ab1aea837c3549a56675407edae19e12c6bfdf..6c8f84e836bb4018d93520b6b5f96687c975d33a 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -922,7 +922,7 @@ static void server_fill_credential(struct imap_server_conf *srvc, struct credent
cred->username = xstrdup_or_null(srvc->user);
cred->password = xstrdup_or_null(srvc->pass);
- credential_fill(cred, 1);
+ credential_fill(the_repository, cred, 1);
if (!srvc->user)
srvc->user = xstrdup(cred->username);
@@ -1123,7 +1123,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
} /* !preauth */
if (cred.username)
- credential_approve(&cred);
+ credential_approve(the_repository, &cred);
credential_clear(&cred);
/* check the target mailbox exists */
@@ -1150,7 +1150,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
bail:
if (cred.username)
- credential_reject(&cred);
+ credential_reject(the_repository, &cred);
credential_clear(&cred);
out:
@@ -1492,9 +1492,9 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
if (cred.username) {
if (res == CURLE_OK)
- credential_approve(&cred);
+ credential_approve(the_repository, &cred);
else if (res == CURLE_LOGIN_DENIED)
- credential_reject(&cred);
+ credential_reject(the_repository, &cred);
}
credential_clear(&cred);
diff --git a/remote-curl.c b/remote-curl.c
index a24e3a8b9abcc9f01d862a18dbd237fdad8d3d3a..1273507a96cae97eeaac9a95a47618e5e4f72850 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -942,7 +942,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
do {
err = probe_rpc(rpc, &results);
if (err == HTTP_REAUTH)
- credential_fill(&http_auth, 0);
+ credential_fill(the_repository, &http_auth, 0);
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
@@ -1064,7 +1064,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
rpc->any_written = 0;
err = run_slot(slot, NULL);
if (err == HTTP_REAUTH && !large_request) {
- credential_fill(&http_auth, 0);
+ credential_fill(the_repository, &http_auth, 0);
curl_slist_free_all(headers);
goto retry;
}
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 10/14] resolve-undo: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (8 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 09/14] credential: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 11/14] tmp-objdir: " Patrick Steinhardt
` (5 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "resolve-undo" subsystem by passing
in the hash algorithm when reading or writing resolve-undo information.
While we could trivially update the caller to pass in the hash algorithm
used by the index itself, we instead pass in `the_hash_algo`. This is
mostly done to stay consistent with the rest of the code in that file,
which isn't prepared to handle arbitrary repositories, either.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
read-cache.c | 4 ++--
resolve-undo.c | 14 +++++++-------
resolve-undo.h | 6 ++++--
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 38c36caa7fef4d44da74c29e059839d88426df15..d54be2c17268728caf3e1e3ef94a5832d2cbfa60 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1754,7 +1754,7 @@ static int read_index_extension(struct index_state *istate,
istate->cache_tree = cache_tree_read(data, sz);
break;
case CACHE_EXT_RESOLVE_UNDO:
- istate->resolve_undo = resolve_undo_read(data, sz);
+ istate->resolve_undo = resolve_undo_read(data, sz, the_hash_algo);
break;
case CACHE_EXT_LINK:
if (read_link_extension(istate, data, sz))
@@ -3033,7 +3033,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
istate->resolve_undo) {
strbuf_reset(&sb);
- resolve_undo_write(&sb, istate->resolve_undo);
+ resolve_undo_write(&sb, istate->resolve_undo, the_hash_algo);
err = write_index_ext_header(f, eoie_c, CACHE_EXT_RESOLVE_UNDO,
sb.len) < 0;
hashwrite(f, sb.buf, sb.len);
diff --git a/resolve-undo.c b/resolve-undo.c
index b5a9dfb4acc51173b1d9b7b1b044a1fc611d3f74..52c45e5a49463656a48fc036749ee2749debaaac 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -34,7 +33,8 @@ void record_resolve_undo(struct index_state *istate, struct cache_entry *ce)
ui->mode[stage - 1] = ce->ce_mode;
}
-void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)
+void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo,
+ const struct git_hash_algo *algop)
{
struct string_list_item *item;
for_each_string_list_item(item, resolve_undo) {
@@ -50,18 +50,19 @@ void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)
for (i = 0; i < 3; i++) {
if (!ui->mode[i])
continue;
- strbuf_add(sb, ui->oid[i].hash, the_hash_algo->rawsz);
+ strbuf_add(sb, ui->oid[i].hash, algop->rawsz);
}
}
}
-struct string_list *resolve_undo_read(const char *data, unsigned long size)
+struct string_list *resolve_undo_read(const char *data, unsigned long size,
+ const struct git_hash_algo *algop)
{
struct string_list *resolve_undo;
size_t len;
char *endptr;
int i;
- const unsigned rawsz = the_hash_algo->rawsz;
+ const unsigned rawsz = algop->rawsz;
CALLOC_ARRAY(resolve_undo, 1);
resolve_undo->strdup_strings = 1;
@@ -96,8 +97,7 @@ struct string_list *resolve_undo_read(const char *data, unsigned long size)
continue;
if (size < rawsz)
goto error;
- oidread(&ui->oid[i], (const unsigned char *)data,
- the_repository->hash_algo);
+ oidread(&ui->oid[i], (const unsigned char *)data, algop);
size -= rawsz;
data += rawsz;
}
diff --git a/resolve-undo.h b/resolve-undo.h
index 89a32272620f2b9e93102039204c72c7886306b3..7ed11a1c59b8192df9bdc9bdad560e06d9f0b9e5 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -14,8 +14,10 @@ struct resolve_undo_info {
};
void record_resolve_undo(struct index_state *, struct cache_entry *);
-void resolve_undo_write(struct strbuf *, struct string_list *);
-struct string_list *resolve_undo_read(const char *, unsigned long);
+void resolve_undo_write(struct strbuf *, struct string_list *,
+ const struct git_hash_algo *algop);
+struct string_list *resolve_undo_read(const char *, unsigned long,
+ const struct git_hash_algo *algop);
void resolve_undo_clear_index(struct index_state *);
int unmerge_index_entry(struct index_state *, const char *, struct resolve_undo_info *, unsigned);
void unmerge_index(struct index_state *, const struct pathspec *, unsigned);
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 11/14] tmp-objdir: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (9 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 10/14] resolve-undo: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 12/14] add-interactive: " Patrick Steinhardt
` (4 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "tmp-objdir" subsystem by passing
in the repostiroy when creating a new temporary object directory.
While we could trivially update the caller to pass in the hash algorithm
used by the index itself, we instead pass in `the_hash_algo`. This is
mostly done to stay consistent with the rest of the code in that file,
which isn't prepared to handle arbitrary repositories, either.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/receive-pack.c | 2 +-
bulk-checkin.c | 2 +-
log-tree.c | 2 +-
tmp-objdir.c | 15 ++++++++-------
tmp-objdir.h | 5 +++--
5 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 191b5eeb34e6791776f93a3a9509efa887e4e087..56347a79633505efe8dc05acf1583b4c9995eefe 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2239,7 +2239,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)
strvec_push(&child.args, alt_shallow_file);
}
- tmp_objdir = tmp_objdir_create("incoming");
+ tmp_objdir = tmp_objdir_create(the_repository, "incoming");
if (!tmp_objdir) {
if (err_fd > 0)
close(err_fd);
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 4a70a70a951cfd1a488339a33bf3a76b5152a344..f7b15e3999f2c7e3fdb0d7bde01975ae4449bda3 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -333,7 +333,7 @@ void prepare_loose_object_bulk_checkin(void)
if (!odb_transaction_nesting || bulk_fsync_objdir)
return;
- bulk_fsync_objdir = tmp_objdir_create("bulk-fsync");
+ bulk_fsync_objdir = tmp_objdir_create(the_repository, "bulk-fsync");
if (bulk_fsync_objdir)
tmp_objdir_replace_primary_odb(bulk_fsync_objdir, 0);
}
diff --git a/log-tree.c b/log-tree.c
index d08eb672a933900558b305b31860f6753a470bf0..8b184d6776344bbc2ba4c00a9d50b44db17f2ede 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1042,7 +1042,7 @@ static int do_remerge_diff(struct rev_info *opt,
* into the alternative object store list as the primary.
*/
if (opt->remerge_diff && !opt->remerge_objdir) {
- opt->remerge_objdir = tmp_objdir_create("remerge-diff");
+ opt->remerge_objdir = tmp_objdir_create(the_repository, "remerge-diff");
if (!opt->remerge_objdir)
return error(_("unable to create temporary object directory"));
tmp_objdir_replace_primary_odb(opt->remerge_objdir, 1);
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 659fcdcc2954ed392f9e241667ea6a7d2c79b828..0ea078a5c5f4eb1ff465aa64ae962a0ccd44fb8b 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "tmp-objdir.h"
#include "abspath.h"
@@ -16,6 +14,7 @@
#include "repository.h"
struct tmp_objdir {
+ struct repository *repo;
struct strbuf path;
struct strvec env;
struct object_directory *prev_odb;
@@ -116,7 +115,8 @@ static int setup_tmp_objdir(const char *root)
return ret;
}
-struct tmp_objdir *tmp_objdir_create(const char *prefix)
+struct tmp_objdir *tmp_objdir_create(struct repository *r,
+ const char *prefix)
{
static int installed_handlers;
struct tmp_objdir *t;
@@ -125,6 +125,7 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
BUG("only one tmp_objdir can be used at a time");
t = xcalloc(1, sizeof(*t));
+ t->repo = r;
strbuf_init(&t->path, 0);
strvec_init(&t->env);
@@ -134,7 +135,7 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
* them.
*/
strbuf_addf(&t->path, "%s/tmp_objdir-%s-XXXXXX",
- repo_get_object_directory(the_repository), prefix);
+ repo_get_object_directory(r), prefix);
if (!mkdtemp(t->path.buf)) {
/* free, not destroy, as we never touched the filesystem */
@@ -154,7 +155,7 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
}
env_append(&t->env, ALTERNATE_DB_ENVIRONMENT,
- absolute_path(repo_get_object_directory(the_repository)));
+ absolute_path(repo_get_object_directory(r)));
env_replace(&t->env, DB_ENVIRONMENT, absolute_path(t->path.buf));
env_replace(&t->env, GIT_QUARANTINE_ENVIRONMENT,
absolute_path(t->path.buf));
@@ -273,14 +274,14 @@ int tmp_objdir_migrate(struct tmp_objdir *t)
return 0;
if (t->prev_odb) {
- if (the_repository->objects->odb->will_destroy)
+ if (t->repo->objects->odb->will_destroy)
BUG("migrating an ODB that was marked for destruction");
restore_primary_odb(t->prev_odb, t->path.buf);
t->prev_odb = NULL;
}
strbuf_addbuf(&src, &t->path);
- strbuf_addstr(&dst, repo_get_object_directory(the_repository));
+ strbuf_addstr(&dst, repo_get_object_directory(t->repo));
ret = migrate_paths(&src, &dst, 0);
diff --git a/tmp-objdir.h b/tmp-objdir.h
index 237d96b66050c82340af3c18b531bd5c877c15ab..fceda14979648f50bb28b6c527889b12d334b098 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -11,7 +11,7 @@
* Example:
*
* struct child_process child = CHILD_PROCESS_INIT;
- * struct tmp_objdir *t = tmp_objdir_create("incoming");
+ * struct tmp_objdir *t = tmp_objdir_create(repo, "incoming");
* strvec_push(&child.args, cmd);
* strvec_pushv(&child.env, tmp_objdir_env(t));
* if (!run_command(&child)) && !tmp_objdir_migrate(t))
@@ -21,13 +21,14 @@
*
*/
+struct repository;
struct tmp_objdir;
/*
* Create a new temporary object directory with the specified prefix;
* returns NULL on failure.
*/
-struct tmp_objdir *tmp_objdir_create(const char *prefix);
+struct tmp_objdir *tmp_objdir_create(struct repository *r, const char *prefix);
/*
* Return a list of environment strings, suitable for use with
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 12/14] add-interactive: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (10 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 11/14] tmp-objdir: " Patrick Steinhardt
@ 2024-12-17 6:43 ` Patrick Steinhardt
2024-12-17 6:44 ` [PATCH 13/14] graph: " Patrick Steinhardt
` (3 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:43 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "add-interactive" subsystem by
reusing the repository we already have available via parameters or in
the `add_i_state` structure.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
add-interactive.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/add-interactive.c b/add-interactive.c
index d0f8c10e6fd460c67c564a1040d12780dfafbc69..97ff35b6f12a32e4154fbe4d195d444c0f71c347 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -72,14 +71,14 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)
s->use_color ? GIT_COLOR_RESET : "", COLOR_MAXLEN);
FREE_AND_NULL(s->interactive_diff_filter);
- git_config_get_string("interactive.difffilter",
- &s->interactive_diff_filter);
+ repo_config_get_string(r, "interactive.difffilter",
+ &s->interactive_diff_filter);
FREE_AND_NULL(s->interactive_diff_algorithm);
- git_config_get_string("diff.algorithm",
- &s->interactive_diff_algorithm);
+ repo_config_get_string(r, "diff.algorithm",
+ &s->interactive_diff_algorithm);
- git_config_get_bool("interactive.singlekey", &s->use_single_key);
+ repo_config_get_bool(r, "interactive.singlekey", &s->use_single_key);
if (s->use_single_key)
setbuf(stdin, NULL);
}
@@ -535,7 +534,7 @@ static int get_modified_files(struct repository *r,
size_t *binary_count)
{
struct object_id head_oid;
- int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(r),
"HEAD", RESOLVE_REF_READING,
&head_oid, NULL);
struct collection_status s = { 0 };
@@ -560,7 +559,7 @@ static int get_modified_files(struct repository *r,
s.skip_unseen = filter && i;
opt.def = is_initial ?
- empty_tree_oid_hex(the_repository->hash_algo) : oid_to_hex(&head_oid);
+ empty_tree_oid_hex(r->hash_algo) : oid_to_hex(&head_oid);
repo_init_revisions(r, &rev, NULL);
setup_revisions(0, NULL, &rev, &opt);
@@ -765,7 +764,7 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,
size_t count, i, j;
struct object_id oid;
- int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(s->r),
"HEAD", RESOLVE_REF_READING,
&oid,
NULL);
@@ -996,7 +995,7 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps,
ssize_t count, i;
struct object_id oid;
- int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+ int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(s->r),
"HEAD", RESOLVE_REF_READING,
&oid,
NULL);
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 13/14] graph: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (11 preceding siblings ...)
2024-12-17 6:43 ` [PATCH 12/14] add-interactive: " Patrick Steinhardt
@ 2024-12-17 6:44 ` Patrick Steinhardt
2024-12-17 6:44 ` [PATCH 14/14] match-trees: " Patrick Steinhardt
` (2 subsequent siblings)
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:44 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "graph" subsystem by reusing the
repository we already have available via `struct rev_info`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
graph.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/graph.c b/graph.c
index 52205f75c37cf3467ac5e9de1574425d6c203359..26f6fbf000aef5ef1fa46aeed5851a8fbb2d8fe3 100644
--- a/graph.c
+++ b/graph.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -351,7 +350,7 @@ struct git_graph *graph_init(struct rev_info *opt)
if (!column_colors) {
char *string;
- if (git_config_get_string("log.graphcolors", &string)) {
+ if (repo_config_get_string(opt->repo, "log.graphcolors", &string)) {
/* not configured -- use default */
graph_set_column_colors(column_colors_ansi,
column_colors_ansi_max);
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 14/14] match-trees: stop using `the_repository`
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (12 preceding siblings ...)
2024-12-17 6:44 ` [PATCH 13/14] graph: " Patrick Steinhardt
@ 2024-12-17 6:44 ` Patrick Steinhardt
2024-12-17 12:45 ` [PATCH 00/14] Stop using `the_repository` in some trivial cases shejialuo
2025-01-07 11:41 ` Karthik Nayak
15 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-17 6:44 UTC (permalink / raw)
To: git
Stop using `the_repository` in the "match-trees" subsystem by passing
down the already-available repository parameters to internal functions
as required.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
match-trees.c | 50 +++++++++++++++++++++++++++-----------------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/match-trees.c b/match-trees.c
index a1c8b91eaef8fa7c1714e6c2b4ec78ac365ae99d..ef14ceb594c72ada250013b2b4653cfaa822bf3b 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -8,6 +7,7 @@
#include "tree.h"
#include "tree-walk.h"
#include "object-store-ll.h"
+#include "repository.h"
static int score_missing(unsigned mode)
{
@@ -54,14 +54,15 @@ static int score_matches(unsigned mode1, unsigned mode2)
return score;
}
-static void *fill_tree_desc_strict(struct tree_desc *desc,
+static void *fill_tree_desc_strict(struct repository *r,
+ struct tree_desc *desc,
const struct object_id *hash)
{
void *buffer;
enum object_type type;
unsigned long size;
- buffer = repo_read_object_file(the_repository, hash, &type, &size);
+ buffer = repo_read_object_file(r, hash, &type, &size);
if (!buffer)
die("unable to read tree (%s)", oid_to_hex(hash));
if (type != OBJ_TREE)
@@ -80,12 +81,13 @@ static int base_name_entries_compare(const struct name_entry *a,
/*
* Inspect two trees, and give a score that tells how similar they are.
*/
-static int score_trees(const struct object_id *hash1, const struct object_id *hash2)
+static int score_trees(struct repository *r,
+ const struct object_id *hash1, const struct object_id *hash2)
{
struct tree_desc one;
struct tree_desc two;
- void *one_buf = fill_tree_desc_strict(&one, hash1);
- void *two_buf = fill_tree_desc_strict(&two, hash2);
+ void *one_buf = fill_tree_desc_strict(r, &one, hash1);
+ void *two_buf = fill_tree_desc_strict(r, &two, hash2);
int score = 0;
for (;;) {
@@ -133,7 +135,8 @@ static int score_trees(const struct object_id *hash1, const struct object_id *ha
/*
* Match one itself and its subtrees with two and pick the best match.
*/
-static void match_trees(const struct object_id *hash1,
+static void match_trees(struct repository *r,
+ const struct object_id *hash1,
const struct object_id *hash2,
int *best_score,
char **best_match,
@@ -141,7 +144,7 @@ static void match_trees(const struct object_id *hash1,
int recurse_limit)
{
struct tree_desc one;
- void *one_buf = fill_tree_desc_strict(&one, hash1);
+ void *one_buf = fill_tree_desc_strict(r, &one, hash1);
while (one.size) {
const char *path;
@@ -152,7 +155,7 @@ static void match_trees(const struct object_id *hash1,
elem = tree_entry_extract(&one, &path, &mode);
if (!S_ISDIR(mode))
goto next;
- score = score_trees(elem, hash2);
+ score = score_trees(r, elem, hash2);
if (*best_score < score) {
free(*best_match);
*best_match = xstrfmt("%s%s", base, path);
@@ -160,7 +163,7 @@ static void match_trees(const struct object_id *hash1,
}
if (recurse_limit) {
char *newbase = xstrfmt("%s%s/", base, path);
- match_trees(elem, hash2, best_score, best_match,
+ match_trees(r, elem, hash2, best_score, best_match,
newbase, recurse_limit - 1);
free(newbase);
}
@@ -175,7 +178,8 @@ static void match_trees(const struct object_id *hash1,
* A tree "oid1" has a subdirectory at "prefix". Come up with a tree object by
* replacing it with another tree "oid2".
*/
-static int splice_tree(const struct object_id *oid1, const char *prefix,
+static int splice_tree(struct repository *r,
+ const struct object_id *oid1, const char *prefix,
const struct object_id *oid2, struct object_id *result)
{
char *subpath;
@@ -194,7 +198,7 @@ static int splice_tree(const struct object_id *oid1, const char *prefix,
if (*subpath)
subpath++;
- buf = repo_read_object_file(the_repository, oid1, &type, &sz);
+ buf = repo_read_object_file(r, oid1, &type, &sz);
if (!buf)
die("cannot read tree %s", oid_to_hex(oid1));
init_tree_desc(&desc, oid1, buf, sz);
@@ -232,15 +236,15 @@ static int splice_tree(const struct object_id *oid1, const char *prefix,
oid_to_hex(oid1));
if (*subpath) {
struct object_id tree_oid;
- oidread(&tree_oid, rewrite_here, the_repository->hash_algo);
- status = splice_tree(&tree_oid, subpath, oid2, &subtree);
+ oidread(&tree_oid, rewrite_here, r->hash_algo);
+ status = splice_tree(r, &tree_oid, subpath, oid2, &subtree);
if (status)
return status;
rewrite_with = &subtree;
} else {
rewrite_with = oid2;
}
- hashcpy(rewrite_here, rewrite_with->hash, the_repository->hash_algo);
+ hashcpy(rewrite_here, rewrite_with->hash, r->hash_algo);
status = write_object_file(buf, sz, OBJ_TREE, result);
free(buf);
return status;
@@ -271,7 +275,7 @@ void shift_tree(struct repository *r,
if (!depth_limit)
depth_limit = 2;
- add_score = del_score = score_trees(hash1, hash2);
+ add_score = del_score = score_trees(r, hash1, hash2);
add_prefix = xcalloc(1, 1);
del_prefix = xcalloc(1, 1);
@@ -279,13 +283,13 @@ void shift_tree(struct repository *r,
* See if one's subtree resembles two; if so we need to prefix
* two with a few fake trees to match the prefix.
*/
- match_trees(hash1, hash2, &add_score, &add_prefix, "", depth_limit);
+ match_trees(r, hash1, hash2, &add_score, &add_prefix, "", depth_limit);
/*
* See if two's subtree resembles one; if so we need to
* pick only subtree of two.
*/
- match_trees(hash2, hash1, &del_score, &del_prefix, "", depth_limit);
+ match_trees(r, hash2, hash1, &del_score, &del_prefix, "", depth_limit);
/* Assume we do not have to do any shifting */
oidcpy(shifted, hash2);
@@ -306,7 +310,7 @@ void shift_tree(struct repository *r,
if (!*add_prefix)
goto out;
- splice_tree(hash1, add_prefix, hash2, shifted);
+ splice_tree(r, hash1, add_prefix, hash2, shifted);
out:
free(add_prefix);
@@ -340,16 +344,16 @@ void shift_tree_by(struct repository *r,
if (candidate == 3) {
/* Both are plausible -- we need to evaluate the score */
- int best_score = score_trees(hash1, hash2);
+ int best_score = score_trees(r, hash1, hash2);
int score;
candidate = 0;
- score = score_trees(&sub1, hash2);
+ score = score_trees(r, &sub1, hash2);
if (score > best_score) {
candidate = 1;
best_score = score;
}
- score = score_trees(&sub2, hash1);
+ score = score_trees(r, &sub2, hash1);
if (score > best_score)
candidate = 2;
}
@@ -365,7 +369,7 @@ void shift_tree_by(struct repository *r,
* shift tree2 down by adding shift_prefix above it
* to match tree1.
*/
- splice_tree(hash1, shift_prefix, hash2, shifted);
+ splice_tree(r, hash1, shift_prefix, hash2, shifted);
else
/*
* shift tree2 up by removing shift_prefix from it
--
2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 02/14] pager: stop using `the_repository`
2024-12-17 6:43 ` [PATCH 02/14] pager: " Patrick Steinhardt
@ 2024-12-17 12:17 ` shejialuo
2024-12-31 6:55 ` Karthik Nayak
0 siblings, 1 reply; 26+ messages in thread
From: shejialuo @ 2024-12-17 12:17 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On Tue, Dec 17, 2024 at 07:43:49AM +0100, Patrick Steinhardt wrote:
> Stop using `the_repository` in the "pager" subsystem by passing in a
> repository when setting up the pager and when configuring it.
>
> Adjust callers accordingly by using `the_repository`. While there may be
> some callers that have a repository available in their context, this
> trivial conversion allows for easier verification and bubbles up the use
> of `the_repository` by one level.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> add-patch.c | 2 +-
> builtin/am.c | 4 ++--
> builtin/blame.c | 2 +-
> builtin/grep.c | 4 ++--
> builtin/help.c | 4 ++--
> builtin/log.c | 4 ++--
> builtin/var.c | 2 +-
> diff.c | 4 ++--
> git.c | 8 ++++----
> pager.c | 14 ++++++--------
> pager.h | 7 ++++---
> 11 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 05136279cf7b1007ab754f5630c34536a5f9461f..c257079cebc3c09fb91f258c3b0148e2f204c0e7 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -658,7 +658,7 @@ int cmd_help(int argc,
> case HELP_ACTION_ALL:
> opt_mode_usage(argc, "--all", help_format);
> if (verbose) {
> - setup_pager();
> + setup_pager(the_repository);
It's possible we run "git help" outside of the repository. Here we still
pass "the_repository" to the "setup_pager", it may be a little strange.
But later we will use the "repo" parameter instead of the global
variable "the_repository", so this is OK.
> list_all_cmds_help(show_external_commands,
> show_aliases);
> return 0;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 06/14] server-info: stop using `the_repository`
2024-12-17 6:43 ` [PATCH 06/14] server-info: " Patrick Steinhardt
@ 2024-12-17 12:31 ` shejialuo
0 siblings, 0 replies; 26+ messages in thread
From: shejialuo @ 2024-12-17 12:31 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On Tue, Dec 17, 2024 at 07:43:53AM +0100, Patrick Steinhardt wrote:
> Stop using `the_repository` in the "server-info" subsystem by passing in
> a repository when updating server info and storing the repository in the
> `update_info_ctx` structure to make it accessible to other functions.
>
> Adjust callers accordingly by using `the_repository`. While there may be
> some callers that have a repository available in their context, this
> trivial conversion allows for easier verification and bubbles up the use
> of `the_repository` by one level.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/receive-pack.c | 2 +-
> builtin/repack.c | 2 +-
> builtin/update-server-info.c | 2 +-
> server-info.c | 40 ++++++++++++++++++++++------------------
> server-info.h | 4 +++-
> 5 files changed, 28 insertions(+), 22 deletions(-)
>
> -int update_server_info(int force)
> +int update_server_info(struct repository *r, int force)
> {
> /* We would add more dumb-server support files later,
> * including index of available pack files and their
> * intended audiences.
> */
> int errs = 0;
> + char *path;
>
> - errs = errs | update_info_refs(force);
> - errs = errs | update_info_packs(force);
> + errs = errs | update_info_refs(r, force);
> + errs = errs | update_info_packs(r, force);
>
> /* remove leftover rev-cache file if there is any */
> - unlink_or_warn(git_path("info/rev-cache"));
> + path = repo_git_path(r, "info/rev-cache");
> + unlink_or_warn(path);
> + free(path);
>
The original "git_path" will be returned from the static string buffers,
so there is no need to free. But "repo_git_path" will return a allocated
string, the caller need to explicit free this. Make sense.
> return errs;
> }
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/14] Stop using `the_repository` in some trivial cases
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (13 preceding siblings ...)
2024-12-17 6:44 ` [PATCH 14/14] match-trees: " Patrick Steinhardt
@ 2024-12-17 12:45 ` shejialuo
2024-12-27 14:26 ` Patrick Steinhardt
2025-01-07 11:41 ` Karthik Nayak
15 siblings, 1 reply; 26+ messages in thread
From: shejialuo @ 2024-12-17 12:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On Tue, Dec 17, 2024 at 07:43:47AM +0100, Patrick Steinhardt wrote:
> Hi,
>
> this small patch series performs some refactorings to stop using
> `the_repository` in several subsystems. There wasn't really any
> criterium for which subsystems I picked, except that all of them have
> been trivial to convert.
>
> In this patch series I'm merely bubbling up `the_repository` one more
> layer even though some calling contexts already have a repository
> available. For the sake of triviality I decided not to handle these
> cases though and instead let a future patch series worry about them.
>
Actually, I am excited to see that we remove the global variable
"the_repository" in some subsystems because I have seen every patch with
"<subsystem>: stop using `the_repository`".
By this, we make the problem smaller, which is good. I have read through
all the patches, which looks to me.
Thanks,
Jialuo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/14] Stop using `the_repository` in some trivial cases
2024-12-17 12:45 ` [PATCH 00/14] Stop using `the_repository` in some trivial cases shejialuo
@ 2024-12-27 14:26 ` Patrick Steinhardt
0 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2024-12-27 14:26 UTC (permalink / raw)
To: shejialuo; +Cc: git
On Tue, Dec 17, 2024 at 08:45:54PM +0800, shejialuo wrote:
> On Tue, Dec 17, 2024 at 07:43:47AM +0100, Patrick Steinhardt wrote:
> > Hi,
> >
> > this small patch series performs some refactorings to stop using
> > `the_repository` in several subsystems. There wasn't really any
> > criterium for which subsystems I picked, except that all of them have
> > been trivial to convert.
> >
> > In this patch series I'm merely bubbling up `the_repository` one more
> > layer even though some calling contexts already have a repository
> > available. For the sake of triviality I decided not to handle these
> > cases though and instead let a future patch series worry about them.
> >
>
> Actually, I am excited to see that we remove the global variable
> "the_repository" in some subsystems because I have seen every patch with
> "<subsystem>: stop using `the_repository`".
>
> By this, we make the problem smaller, which is good. I have read through
> all the patches, which looks to me.
Thanks for your review!
Patrick
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/14] progress: stop using `the_repository`
2024-12-17 6:43 ` [PATCH 01/14] progress: stop using `the_repository` Patrick Steinhardt
@ 2024-12-31 6:42 ` Karthik Nayak
2025-01-06 20:57 ` Toon Claes
1 sibling, 0 replies; 26+ messages in thread
From: Karthik Nayak @ 2024-12-31 6:42 UTC (permalink / raw)
To: Patrick Steinhardt, git
[-- Attachment #1: Type: text/plain, Size: 912 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Stop using `the_repository` in the "progress" subsystem by passing in a
> repository when initializing `struct progress`. Furthermore, store a
> pointer to the repository in that struct so that we can pass it to the
> trace2 API when logging information.
>
So that's the only usage of `the_repository` in the progress subsystem
currently. Makes sense.
> Adjust callers accordingly by using `the_repository`. While there may be
> some callers that have a repository available in their context, this
> trivial conversion allows for easier verification and bubbles up the use
> of `the_repository` by one level.
>
I think this is a good approach. I would be vary if we ended up
introducing more instances of the `USE_THE_REPOSITORY_VARIABLE`
definition. But that's not the case here, we only do so in
`t/helper/test-progress.c`, which is expected. The patch looks good!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/14] pager: stop using `the_repository`
2024-12-17 12:17 ` shejialuo
@ 2024-12-31 6:55 ` Karthik Nayak
0 siblings, 0 replies; 26+ messages in thread
From: Karthik Nayak @ 2024-12-31 6:55 UTC (permalink / raw)
To: shejialuo, Patrick Steinhardt; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]
shejialuo <shejialuo@gmail.com> writes:
> On Tue, Dec 17, 2024 at 07:43:49AM +0100, Patrick Steinhardt wrote:
>> Stop using `the_repository` in the "pager" subsystem by passing in a
>> repository when setting up the pager and when configuring it.
>>
>> Adjust callers accordingly by using `the_repository`. While there may be
>> some callers that have a repository available in their context, this
>> trivial conversion allows for easier verification and bubbles up the use
>> of `the_repository` by one level.
>>
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>> ---
>> add-patch.c | 2 +-
>> builtin/am.c | 4 ++--
>> builtin/blame.c | 2 +-
>> builtin/grep.c | 4 ++--
>> builtin/help.c | 4 ++--
>> builtin/log.c | 4 ++--
>> builtin/var.c | 2 +-
>> diff.c | 4 ++--
>> git.c | 8 ++++----
>> pager.c | 14 ++++++--------
>> pager.h | 7 ++++---
>> 11 files changed, 27 insertions(+), 28 deletions(-)
>>
>> diff --git a/builtin/help.c b/builtin/help.c
>> index 05136279cf7b1007ab754f5630c34536a5f9461f..c257079cebc3c09fb91f258c3b0148e2f204c0e7 100644
>> --- a/builtin/help.c
>> +++ b/builtin/help.c
>> @@ -658,7 +658,7 @@ int cmd_help(int argc,
>> case HELP_ACTION_ALL:
>> opt_mode_usage(argc, "--all", help_format);
>> if (verbose) {
>> - setup_pager();
>> + setup_pager(the_repository);
>
> It's possible we run "git help" outside of the repository. Here we still
> pass "the_repository" to the "setup_pager", it may be a little strange.
> But later we will use the "repo" parameter instead of the global
> variable "the_repository", so this is OK.
>
Well, the crux here is that the pager only requires the repository for
calling `read_early_config()` in `config.c`. And that function is used
to read if there is pager config, since git allows you to setup the
pager via config. It also checks if the repo variable is set, so this
would work as expected outside of a git repository.
>> list_all_cmds_help(show_external_commands,
>> show_aliases);
>> return 0;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/14] progress: stop using `the_repository`
2024-12-17 6:43 ` [PATCH 01/14] progress: stop using `the_repository` Patrick Steinhardt
2024-12-31 6:42 ` Karthik Nayak
@ 2025-01-06 20:57 ` Toon Claes
2025-01-07 7:19 ` Patrick Steinhardt
1 sibling, 1 reply; 26+ messages in thread
From: Toon Claes @ 2025-01-06 20:57 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Karthik Nayak
Patrick Steinhardt <ps@pks.im> writes:
> Stop using `the_repository` in the "progress" subsystem by passing in a
> repository when initializing `struct progress`. Furthermore, store a
> pointer to the repository in that struct so that we can pass it to the
> trace2 API when logging information.
>
> Adjust callers accordingly by using `the_repository`. While there may be
> some callers that have a repository available in their context, this
> trivial conversion allows for easier verification and bubbles up the use
> of `the_repository` by one level.
I'm not sure I agree here. Below I've marked all places where I think we
are able to get the repo from somewhere else than `the_repository`. For
example, looking at diffcore-rename.c, the already present calls to
trace2_*() use `options->repo`, why shouldn't we do the same?
I understand what your angle is, you want to bubble up `the_repository`
and make the changes easier to reason about, but it feels to me we're
creating extra work. If most people disagree with me, I'm happy to take
your approach.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/blame.c | 4 +++-
> builtin/commit-graph.c | 1 +
> builtin/fsck.c | 12 ++++++++----
> builtin/index-pack.c | 7 +++++--
> builtin/log.c | 3 ++-
> builtin/pack-objects.c | 21 ++++++++++++++-------
> builtin/prune.c | 3 ++-
> builtin/remote.c | 3 ++-
> builtin/rev-list.c | 3 ++-
> builtin/unpack-objects.c | 3 ++-
> commit-graph.c | 20 +++++++++++++++++---
> delta-islands.c | 3 ++-
> diffcore-rename.c | 1 +
> entry.c | 4 +++-
> midx-write.c | 11 ++++++++---
> midx.c | 13 +++++++++----
> pack-bitmap-write.c | 6 ++++--
> pack-bitmap.c | 4 +++-
> preload-index.c | 4 +++-
> progress.c | 34 ++++++++++++++++++++--------------
> progress.h | 13 +++++++++----
> prune-packed.c | 3 ++-
> pseudo-merge.c | 3 ++-
> read-cache.c | 3 ++-
> t/helper/test-progress.c | 6 +++++-
> unpack-trees.c | 4 +++-
> walker.c | 3 ++-
> 27 files changed, 136 insertions(+), 59 deletions(-)
>
> [snip]
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 0196c54eb68ee54c22de72d64b3f31602594e50b..7a4dcb0716052ff1b9236ea66b8901960fe1c55d 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -197,7 +197,8 @@ static int traverse_reachable(void)
> unsigned int nr = 0;
> int result = 0;
> if (show_progress)
> - progress = start_delayed_progress(_("Checking connectivity"), 0);
> + progress = start_delayed_progress(the_repository,
> + _("Checking connectivity"), 0);
> while (pending.nr) {
> result |= traverse_one_object(object_array_pop(&pending));
> display_progress(progress, ++nr);
> @@ -703,7 +704,8 @@ static void fsck_object_dir(const char *path)
> fprintf_ln(stderr, _("Checking object directory"));
>
> if (show_progress)
> - progress = start_progress(_("Checking object directories"), 256);
> + progress = start_progress(the_repository,
> + _("Checking object directories"), 256);
>
> for_each_loose_file_in_objdir(path, fsck_loose, fsck_cruft, fsck_subdir,
> &cb_data);
> @@ -879,7 +881,8 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)
> if (show_progress) {
> for (struct packed_git *p = get_all_packs(r); p; p = p->next)
> pack_count++;
> - progress = start_delayed_progress("Verifying reverse pack-indexes", pack_count);
> + progress = start_delayed_progress(the_repository,
I think we should use `r` here.
> + "Verifying reverse pack-indexes", pack_count);
> pack_count = 0;
> }
>
> [snip]
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 0df66e5a243390bc1224b28e2b55c541f9d93fb1..2a2999a6b886905276a0c39dda6135f0c92aa361 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1534,6 +1534,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
>
> if (ctx->report_progress)
> ctx->progress = start_delayed_progress(
Shall we use `ctx->r` here? And same for all other changes in this file.
> + the_repository,
> _("Loading known commits in commit graph"),
> ctx->oids.nr);
> for (i = 0; i < ctx->oids.nr; i++) {
> @@ -1551,6 +1552,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
> */
> if (ctx->report_progress)
> ctx->progress = start_delayed_progress(
> + the_repository,
> _("Expanding reachable commits in commit graph"),
> 0);
> for (i = 0; i < ctx->oids.nr; i++) {
> @@ -1571,6 +1573,7 @@ static void close_reachable(struct write_commit_graph_context *ctx)
>
> if (ctx->report_progress)
> ctx->progress = start_delayed_progress(
> + the_repository,
> _("Clearing commit marks in commit graph"),
> ctx->oids.nr);
> for (i = 0; i < ctx->oids.nr; i++) {
> @@ -1688,6 +1691,7 @@ static void compute_topological_levels(struct write_commit_graph_context *ctx)
> if (ctx->report_progress)
> info.progress = ctx->progress
> = start_delayed_progress(
> + the_repository,
> _("Computing commit graph topological levels"),
> ctx->commits.nr);
>
> @@ -1722,6 +1726,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
> if (ctx->report_progress)
> info.progress = ctx->progress
> = start_delayed_progress(
> + the_repository,
> _("Computing commit graph generation numbers"),
> ctx->commits.nr);
>
> @@ -1798,6 +1803,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)
>
> if (ctx->report_progress)
> progress = start_delayed_progress(
> + the_repository,
> _("Computing commit changed paths Bloom filters"),
> ctx->commits.nr);
>
> @@ -1877,6 +1883,7 @@ int write_commit_graph_reachable(struct object_directory *odb,
> data.commits = &commits;
> if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
> data.progress = start_delayed_progress(
> + the_repository,
> _("Collecting referenced commits"), 0);
>
> refs_for_each_ref(get_main_ref_store(the_repository), add_ref_to_set,
> @@ -1908,7 +1915,8 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
> "Finding commits for commit graph in %"PRIuMAX" packs",
> pack_indexes->nr),
> (uintmax_t)pack_indexes->nr);
> - ctx->progress = start_delayed_progress(progress_title.buf, 0);
> + ctx->progress = start_delayed_progress(the_repository,
> + progress_title.buf, 0);
> ctx->progress_done = 0;
> }
> for (i = 0; i < pack_indexes->nr; i++) {
> @@ -1959,6 +1967,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
> {
> if (ctx->report_progress)
> ctx->progress = start_delayed_progress(
> + the_repository,
> _("Finding commits for commit graph among packed objects"),
> ctx->approx_nr_objects);
> for_each_packed_object(ctx->r, add_packed_commits, ctx,
> @@ -1977,6 +1986,7 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
> ctx->num_extra_edges = 0;
> if (ctx->report_progress)
> ctx->progress = start_delayed_progress(
> + the_repository,
> _("Finding extra edges in commit graph"),
> ctx->oids.nr);
> oid_array_sort(&ctx->oids);
> @@ -2136,6 +2146,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
> get_num_chunks(cf)),
> get_num_chunks(cf));
> ctx->progress = start_delayed_progress(
> + the_repository,
> progress_title.buf,
> st_mult(get_num_chunks(cf), ctx->commits.nr));
> }
> @@ -2348,6 +2359,7 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx)
>
> if (ctx->report_progress)
> ctx->progress = start_delayed_progress(
> + the_repository,
> _("Scanning merged commits"),
> ctx->commits.nr);
>
> @@ -2392,7 +2404,8 @@ static void merge_commit_graphs(struct write_commit_graph_context *ctx)
> current_graph_number--;
>
> if (ctx->report_progress)
> - ctx->progress = start_delayed_progress(_("Merging commit-graph"), 0);
> + ctx->progress = start_delayed_progress(the_repository,
> + _("Merging commit-graph"), 0);
>
> merge_commit_graph(ctx, g);
> stop_progress(&ctx->progress);
> @@ -2874,7 +2887,8 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
> if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW))
> total += g->num_commits_in_base;
>
> - progress = start_progress(_("Verifying commits in commit graph"),
> + progress = start_progress(the_repository,
> + _("Verifying commits in commit graph"),
> total);
> }
>
> diff --git a/delta-islands.c b/delta-islands.c
> index 1c465a6041914538bcb8be51c500653d8fa1a626..3aec43fada36f7052b825dcb2ac0e1ad79f028b7 100644
> --- a/delta-islands.c
> +++ b/delta-islands.c
> @@ -267,7 +267,8 @@ void resolve_tree_islands(struct repository *r,
> QSORT(todo, nr, tree_depth_compare);
>
> if (progress)
> - progress_state = start_progress(_("Propagating island marks"), nr);
> + progress_state = start_progress(the_repository,
Here we can use `r`.
> + _("Propagating island marks"), nr);
>
> for (i = 0; i < nr; i++) {
> struct object_entry *ent = todo[i].entry;
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 10bb0321b10d5896aaa6a26a624d2066598bf51f..91b77993c7827f9ddc7b444b42f480b8209fd821 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -1567,6 +1567,7 @@ void diffcore_rename_extended(struct diff_options *options,
> trace2_region_enter("diff", "inexact renames", options->repo);
> if (options->show_rename_progress) {
> progress = start_delayed_progress(
> + the_repository,
Can we use `options->repo` here?
If we do, we ideally should also replace occurrences of
`the_repository->hash_algo` in this function, although I realize that's
outside the scope of this commit.
> _("Performing inexact rename detection"),
> (uint64_t)num_destinations * (uint64_t)num_sources);
> }
>
> [snip]
>
> diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> index 4f8be53c2bd75f83a0555e2a5510c2bbca07b36d..a06a1f35c619b3b01e63a506a6d4312e14cf181c 100644
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -590,7 +590,8 @@ int bitmap_writer_build(struct bitmap_writer *writer)
> int closed = 1; /* until proven otherwise */
>
> if (writer->show_progress)
> - writer->progress = start_progress("Building bitmaps",
> + writer->progress = start_progress(the_repository,
Unsure, but can we use `writer->to_pack->repo`? Although I see some
trace2_* functions also use `the_repository`, so consistency in this
function would be nice.
> + "Building bitmaps",
> writer->selected_nr);
> trace2_region_enter("pack-bitmap-write", "building_bitmaps_total",
> the_repository);
> @@ -710,7 +711,8 @@ void bitmap_writer_select_commits(struct bitmap_writer *writer,
> }
>
> if (writer->show_progress)
> - writer->progress = start_progress("Selecting bitmap commits", 0);
> + writer->progress = start_progress(the_repository,
Same, can we use `writer->to_pack->repo`?
> + "Selecting bitmap commits", 0);
>
> for (;;) {
> struct commit *chosen = NULL;
>
> [snip]
>
> diff --git a/preload-index.c b/preload-index.c
> index ab94d6f39967ea4358f51ff8384aa60927bfe259..40ab2abafb8de500a5f2ec678a584a5fd5e1bc16 100644
> --- a/preload-index.c
> +++ b/preload-index.c
> @@ -132,7 +132,9 @@ void preload_index(struct index_state *index,
>
> memset(&pd, 0, sizeof(pd));
> if (refresh_flags & REFRESH_PROGRESS && isatty(2)) {
> - pd.progress = start_delayed_progress(_("Refreshing index"), index->cache_nr);
> + pd.progress = start_delayed_progress(the_repository,
Can we use `index->repo` here?
> [snip]
>
> diff --git a/pseudo-merge.c b/pseudo-merge.c
> index 971f54cfe1a895aed00f6d0a65c62aafc83a0cc8..893b763fe45490875ea226eaffff0c7cb1dafb06 100644
> --- a/pseudo-merge.c
> +++ b/pseudo-merge.c
> @@ -459,7 +459,8 @@ void select_pseudo_merges(struct bitmap_writer *writer)
> return;
>
> if (writer->show_progress)
> - progress = start_progress("Selecting pseudo-merge commits",
> + progress = start_progress(the_repository,
Also a candidate for `writer->to_pack->repo`?
> + "Selecting pseudo-merge commits",
> writer->pseudo_merge_groups.nr);
>
> refs_for_each_ref(get_main_ref_store(the_repository),
> diff --git a/read-cache.c b/read-cache.c
> index 15d79839c205176f9161f537aa706dac44b3023c..38c36caa7fef4d44da74c29e059839d88426df15 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1523,7 +1523,8 @@ int refresh_index(struct index_state *istate, unsigned int flags,
> int t2_sum_scan = 0;
>
> if (flags & REFRESH_PROGRESS && isatty(2))
> - progress = start_delayed_progress(_("Refresh index"),
> + progress = start_delayed_progress(the_repository,
I think also `istate->repo` would work here.
> + _("Refresh index"),
> istate->cache_nr);
>
> trace_performance_enter();
>
> [snip]
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index b3be5d542f5fc5a02b8838101f7334ff44b2c626..334cb84f6531b588688d5a43c538c8d1a5f7e768 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -372,7 +372,8 @@ static struct progress *get_progress(struct unpack_trees_options *o,
> total++;
> }
>
> - return start_delayed_progress(_("Updating files"), total);
> + return start_delayed_progress(the_repository,
Maybe also use `index->repo` here?
> + _("Updating files"), total);
> }
>
> static void setup_collided_checkout_detection(struct checkout *state,
> @@ -1773,6 +1774,7 @@ static int clear_ce_flags(struct index_state *istate,
> strbuf_reset(&prefix);
> if (show_progress)
> istate->progress = start_delayed_progress(
> + the_repository,
> _("Updating index flags"),
> istate->cache_nr);
>
> diff --git a/walker.c b/walker.c
> index 7cc9dbea46d64d6bd3336025d640f284a6202157..1cf3da02193531a17fd11dbd2e8aadf36f38b200 100644
> --- a/walker.c
> +++ b/walker.c
> @@ -172,7 +172,8 @@ static int loop(struct walker *walker)
> uint64_t nr = 0;
>
> if (walker->get_progress)
> - progress = start_delayed_progress(_("Fetching objects"), 0);
> + progress = start_delayed_progress(the_repository,
> + _("Fetching objects"), 0);
>
> while (process_queue) {
> struct object *obj = process_queue->item;
>
> --
> 2.48.0.rc0.184.g0fc57dec57.dirty
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/14] progress: stop using `the_repository`
2025-01-06 20:57 ` Toon Claes
@ 2025-01-07 7:19 ` Patrick Steinhardt
0 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2025-01-07 7:19 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Karthik Nayak
On Mon, Jan 06, 2025 at 09:57:26PM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Stop using `the_repository` in the "progress" subsystem by passing in a
> > repository when initializing `struct progress`. Furthermore, store a
> > pointer to the repository in that struct so that we can pass it to the
> > trace2 API when logging information.
> >
> > Adjust callers accordingly by using `the_repository`. While there may be
> > some callers that have a repository available in their context, this
> > trivial conversion allows for easier verification and bubbles up the use
> > of `the_repository` by one level.
>
> I'm not sure I agree here. Below I've marked all places where I think we
> are able to get the repo from somewhere else than `the_repository`. For
> example, looking at diffcore-rename.c, the already present calls to
> trace2_*() use `options->repo`, why shouldn't we do the same?
>
> I understand what your angle is, you want to bubble up `the_repository`
> and make the changes easier to reason about, but it feels to me we're
> creating extra work. If most people disagree with me, I'm happy to take
> your approach.
The problem is that this could lead to a change in behaviour, as the repo
we have available may or may not be the same as `the_repository`. So
without auditing every single callsite I have no way of knowing, and
that audit is quite involved when it touches a lot of subsystems at
once.
That's why I'm instead pushing it further down the road: we know that
injecting `the_repository` will yield the exact same behaviour as
before, and we only need to audit a single subsystem, namely the one
that we're currently converting. So it's one more step overall, but by
separating mechanical from non-mechanical changes it makes the steps
simpler overall.
Patrick
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/14] Stop using `the_repository` in some trivial cases
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
` (14 preceding siblings ...)
2024-12-17 12:45 ` [PATCH 00/14] Stop using `the_repository` in some trivial cases shejialuo
@ 2025-01-07 11:41 ` Karthik Nayak
2025-01-07 21:12 ` Toon Claes
15 siblings, 1 reply; 26+ messages in thread
From: Karthik Nayak @ 2025-01-07 11:41 UTC (permalink / raw)
To: Patrick Steinhardt, git
[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this small patch series performs some refactorings to stop using
> `the_repository` in several subsystems. There wasn't really any
> criterium for which subsystems I picked, except that all of them have
> been trivial to convert.
>
> In this patch series I'm merely bubbling up `the_repository` one more
> layer even though some calling contexts already have a repository
> available. For the sake of triviality I decided not to handle these
> cases though and instead let a future patch series worry about them.
>
> This series is built on v2.48.0-rc0 with ps/build-sign-compare at
> e03d2a9ccb (t/helper: don't depend on implicit wraparound, 2024-12-06)
> merged into it. There's a single merge conflict with 'seen' that can be
> solved like this:
I went through half of the patches a week ago, but got back to reading
through the series today.
The approach here is to simply bubble up the usage of `the_repository`
to upper layers and use `the_repository` there. The alternative approach
would be to try and resolve the dependency on the upper layers and not
use `the_repository`. This approach seems much safer. The patches look
good to me.
Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/14] Stop using `the_repository` in some trivial cases
2025-01-07 11:41 ` Karthik Nayak
@ 2025-01-07 21:12 ` Toon Claes
2025-01-07 21:15 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Toon Claes @ 2025-01-07 21:12 UTC (permalink / raw)
To: Karthik Nayak, Patrick Steinhardt, git
Karthik Nayak <karthik.188@gmail.com> writes:
> I went through half of the patches a week ago, but got back to reading
> through the series today.
I've also been reading through the whole series today, and the changes
are trivial and look good to me.
> The approach here is to simply bubble up the usage of `the_repository`
> to upper layers and use `the_repository` there. The alternative approach
> would be to try and resolve the dependency on the upper layers and not
> use `the_repository`. This approach seems much safer. The patches look
> good to me.
I took me a while to get into the mindset of taking this approach, but
after chatting with Patrick I've changed my mind and agree with this
approach. The goal of this series is to eliminate the use of
`the_repository` in the mentioned subsystems. Simply bubbling up the use
of that variable to the callers of those subsystems is very trivial and
safe to do.
--
Toon
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/14] Stop using `the_repository` in some trivial cases
2025-01-07 21:12 ` Toon Claes
@ 2025-01-07 21:15 ` Junio C Hamano
0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2025-01-07 21:15 UTC (permalink / raw)
To: Toon Claes; +Cc: Karthik Nayak, Patrick Steinhardt, git
Toon Claes <toon@iotcl.com> writes:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> I went through half of the patches a week ago, but got back to reading
>> through the series today.
>
> I've also been reading through the whole series today, and the changes
> are trivial and look good to me.
>
>> The approach here is to simply bubble up the usage of `the_repository`
>> to upper layers and use `the_repository` there. The alternative approach
>> would be to try and resolve the dependency on the upper layers and not
>> use `the_repository`. This approach seems much safer. The patches look
>> good to me.
>
> I took me a while to get into the mindset of taking this approach, but
> after chatting with Patrick I've changed my mind and agree with this
> approach. The goal of this series is to eliminate the use of
> `the_repository` in the mentioned subsystems. Simply bubbling up the use
> of that variable to the callers of those subsystems is very trivial and
> safe to do.
Yup. That way, the conversion would be bug-to-bug compatible, which
is much better than a rewrite that improves some parts while by
mistake breaks the existing code.
Thanks, all. Let's mark it for 'next'.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2025-01-07 21:15 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-17 6:43 [PATCH 00/14] Stop using `the_repository` in some trivial cases Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 01/14] progress: stop using `the_repository` Patrick Steinhardt
2024-12-31 6:42 ` Karthik Nayak
2025-01-06 20:57 ` Toon Claes
2025-01-07 7:19 ` Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 02/14] pager: " Patrick Steinhardt
2024-12-17 12:17 ` shejialuo
2024-12-31 6:55 ` Karthik Nayak
2024-12-17 6:43 ` [PATCH 03/14] trace: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 04/14] serve: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 05/14] send-pack: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 06/14] server-info: " Patrick Steinhardt
2024-12-17 12:31 ` shejialuo
2024-12-17 6:43 ` [PATCH 07/14] diagnose: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 08/14] mailinfo: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 09/14] credential: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 10/14] resolve-undo: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 11/14] tmp-objdir: " Patrick Steinhardt
2024-12-17 6:43 ` [PATCH 12/14] add-interactive: " Patrick Steinhardt
2024-12-17 6:44 ` [PATCH 13/14] graph: " Patrick Steinhardt
2024-12-17 6:44 ` [PATCH 14/14] match-trees: " Patrick Steinhardt
2024-12-17 12:45 ` [PATCH 00/14] Stop using `the_repository` in some trivial cases shejialuo
2024-12-27 14:26 ` Patrick Steinhardt
2025-01-07 11:41 ` Karthik Nayak
2025-01-07 21:12 ` Toon Claes
2025-01-07 21:15 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).