Avoid the_repository in merge-ort and replay
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v3 Latest5 of 6 patches 4 repliesIn review
- v26 patches 2 repliesSuperseded
- v15 patches 9 repliesSuperseded
Changes in v3 from the cover letter
* In first patch, actually avoid the_repository when attempting to remove check against the_repository
* Fix commit message of patch 3 due to the new patch 1.
* Slight tweak to commit message of patch 6.
5 of 6 patches are in this archive. The cover letter announces 6.
Patches in v3 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/6 | merge,diff: remove the_repository check before prefetching blobsdiff.c diffcore-break.c diffcore-rename.c +1 more | +5 | 0 replies | |
| 2/6 | merge-ort: pass repository to write_tree()merge-ort.c | +8 | 0 replies | |
| 3/6 | merge-ort: replace the_repository with opt->repomerge-ort.c | +21 | 0 replies | |
| 4/6 | merge-ort: replace the_hash_algo with opt->repo->hash_algomerge-ort.c | +13 | 0 replies | |
| 5/6 | merge-ort: prevent the_repository from coming backmerge-ort.c | +8 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v3 yet.
Cover letter [PATCH v3 0/6] Avoid the_repository in merge-ort and replay
Changes since v2:
* In first patch, actually avoid the_repository when attempting to remove check against the_repository
* Fix commit message of patch 3 due to the new patch 1.
* Slight tweak to commit message of patch 6.
Changes since v1:
* Add a preparatory patch removing the_repository check from blob prefetching in both merge-ort and diff*; it's no longer necessary
* Fix casing mismatch
* Simplify the hammer a bit based on the new first patch, but add some simple comments explaining it
Remove explicit uses of the_repository and the_hash_algo from merge-ort, and since this has now been done multiple times for both merge-ort and replay, implement a small measure to prevent them from returning to either merge-ort or replay.
See
https://lore.kernel.org/git/CABPp-BH7E1Bh2g0vR3T4NEsv34DvFQPzMuJSsqtOAaWY-fFCxg@mail.gmail.com/
and
https://lore.kernel.org/git/CABPp-BFuwvqiCTCCpoyT6em9_1-qrgPWHWhrufQ3UuZ+Kfkb6A@mail.gmail.com/
for recent discussions on these.
As noted in the comments on v1, I actually do not know why prefetch_for_content_merges() needs to use the_repository. When I introduced it back in 2bff554b23e8 (merge-ort: add prefetching for content merges, 2021-06-22), I was just looking at diffcore_std() and trying to mimic how it did the prefetch, and it has such a comparison. If anyone knows why diffcore_std() needs to compare against the_repository, I'd love to hear...
Series overview: Patches 1-3: Mostly mechanical removal of existing uses Patches 4-5: Simple hammer to prevent the problem from returning
Elijah Newren (6):
merge,diff: remove the_repository check before prefetching blobs
merge-ort: pass repository to write_tree()
merge-ort: replace the_repository with opt->repo
merge-ort: replace the_hash_algo with opt->repo->hash_algo
merge-ort: prevent the_repository from coming back
replay: prevent the_repository from coming back
diff.c | 2 +- diffcore-break.c | 2 +- diffcore-rename.c | 4 +- merge-ort.c | 94 +++++++++++++++++++++++++---------------------- replay.c | 6 +++ 5 files changed, 61 insertions(+), 47 deletions(-)
base-commit: 73fd77805fc6406f31c36212846d9e2541d19321
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2048%2Fnewren%2Favoid_the_repository-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2048/newren/avoid_the_repository-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2048
Range-diff vs v2:
1: 7155a0da6f ! 1: e75334a5cf merge,diff: remove the_repository check before prefetching blobs
@@ diff.c: void diffcore_std(struct diff_options *options)
* decides that it needs inexact rename detection.
*/
- if (options->repo == the_repository && repo_has_promisor_remote(the_repository) &&
-+ if (repo_has_promisor_remote(the_repository) &&
++ if (repo_has_promisor_remote(options->repo) &&
(options->output_format & output_formats_to_prefetch ||
options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
diff_queued_diff_prefetch(options->repo);
@@ diffcore-break.c: static int should_break(struct repository *r,
return 0; /* they are the same */
- if (r == the_repository && repo_has_promisor_remote(the_repository)) {
-+ if (repo_has_promisor_remote(the_repository)) {
++ if (repo_has_promisor_remote(r)) {
options.missing_object_cb = diff_queued_diff_prefetch;
options.missing_object_data = r;
}
@@ diffcore-rename.c: static int find_basename_matches(struct diff_options *options
}
- if (options->repo == the_repository && repo_has_promisor_remote(the_repository)) {
-+ if (repo_has_promisor_remote(the_repository)) {
++ if (repo_has_promisor_remote(options->repo)) {
dpf_options.missing_object_cb = basename_prefetch;
dpf_options.missing_object_data = &prefetch_options;
}
@@ diffcore-rename.c: void diffcore_rename_extended(struct diff_options *options,
/* Finish setting up dpf_options */
prefetch_options.skip_unmodified = skip_unmodified;
- if (options->repo == the_repository && repo_has_promisor_remote(the_repository)) {
-+ if (repo_has_promisor_remote(the_repository)) {
++ if (repo_has_promisor_remote(options->repo)) {
dpf_options.missing_object_cb = inexact_prefetch;
dpf_options.missing_object_data = &prefetch_options;
}
@@ merge-ort.c: static void prefetch_for_content_merges(struct merge_options *opt,
struct oid_array to_fetch = OID_ARRAY_INIT;
- if (opt->repo != the_repository || !repo_has_promisor_remote(the_repository))
-+ if (!repo_has_promisor_remote(the_repository))
++ if (!repo_has_promisor_remote(opt->repo))
return;
for (e = &plist->items[plist->nr-1]; e >= plist->items; --e) {
2: 911cba991b = 2: a9a9d422a3 merge-ort: pass repository to write_tree()
3: 68af47ed18 ! 3: 4ebfcb08a5 merge-ort: replace the_repository with opt->repo @@ Commit message
merge-ort: replace the_repository with opt->repo
We have a perfectly valid repository available and do not need to use
- the_repository, except for one location in
- prefetch_for_content_merges().
+ the_repository.
@@ merge-ort.c: static int read_oid_strbuf(struct merge_options *opt,
if (!buf) {
path_msg(opt, ERROR_OBJECT_READ_FAILED, 0,
path, NULL, NULL, NULL,
-@@ merge-ort.c: static void prefetch_for_content_merges(struct merge_options *opt,
- struct string_list_item *e;
- struct oid_array to_fetch = OID_ARRAY_INIT;
-
-- if (!repo_has_promisor_remote(the_repository))
-+ if (!repo_has_promisor_remote(opt->repo))
- return;
-
- for (e = &plist->items[plist->nr-1]; e >= plist->items; --e) {
@@ merge-ort.c: static int checkout(struct merge_options *opt,
unpack_opts.verbose_update = (opt->verbosity > 2);
unpack_opts.fn = twoway_merge;
4: bfa68716af = 4: 09076d81b6 merge-ort: replace the_hash_algo with opt->repo->hash_algo 5: 932d945c9b = 5: 42a2576878 merge-ort: prevent the_repository from coming back 6: 67db46f34f ! 6: 0654d04584 replay: prevent the_repository from coming back
@@ Commit message
coming back.
Define the_repository to make it a compilation error so that they don't
- come back any more.
+ come back any more; the repo parameter plumbed through the various
+ functions can be used instead.
Signature · 1 line
gitgitgadget