Hi,
this is the third version of my patch series that introduces the
`USE_THE_REPOSITORY_VARIABLE` macro. When unset, this will cause us to
hide `the_repository`, `the_hash_algo` and several other functions that
implicitly rely on those global variables from our headers. This is a
first step towards fully getting rid of this global state in favor of
passing it down explicitly via function parameters.
Changes compared to v2:
- Note in a commit message that we aim to have a faithful conversion
when introducing a `struct git_hash_algo` parameter to functions. So
even in case the calling context has a `struct git_hash_algo`
available via a local repository, we still use `the_repository` such
that there cannot be a change in behaviour here. Fixing those sites
will be left for a future patch series such that we can avoid any
kind of regressions caused by this comparatively large refactoring.
I also adapted some conversions to fully follow through with this
intent.
- Fix an issue with sparse by adding another `extern` declaration of
`the_repository` to "repository.c".
Thanks!
Patrick
Patrick Steinhardt (20):
hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions
hash: require hash algorithm in `hasheq()`, `hashcmp()` and
`hashclr()`
hash: require hash algorithm in `oidread()` and `oidclr()`
global: ensure that object IDs are always padded
hash: convert `oidcmp()` and `oideq()` to compare whole hash
hash: make `is_null_oid()` independent of `the_repository`
hash: require hash algorithm in `is_empty_{blob,tree}_oid()`
hash: require hash algorithm in `empty_tree_oid_hex()`
global: introduce `USE_THE_REPOSITORY_VARIABLE` macro
refs: avoid include cycle with "repository.h"
hash-ll: merge with "hash.h"
http-fetch: don't crash when parsing packfile without a repo
oidset: pass hash algorithm when parsing file
protocol-caps: use hash algorithm from passed-in repository
replace-object: use hash algorithm from passed-in repository
compat/fsmonitor: fix socket path in networked SHA256 repos
t/helper: use correct object hash in partial-clone helper
t/helper: fix segfault in "oid-array" command without repository
t/helper: remove dependency on `the_repository` in "proc-receive"
hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE`
add-interactive.c | 4 +-
add-patch.c | 4 +-
apply.c | 4 +-
apply.h | 2 +-
archive-tar.c | 3 +
archive-zip.c | 3 +
archive.c | 2 +
attr.c | 2 +
bisect.c | 2 +
blame.c | 4 +-
bloom.c | 1 +
branch.c | 2 +
builtin.h | 8 +
builtin/am.c | 8 +-
builtin/blame.c | 3 +-
builtin/fast-export.c | 2 +-
builtin/fast-import.c | 43 ++-
builtin/fetch-pack.c | 4 +-
builtin/index-pack.c | 11 +-
builtin/log.c | 4 +-
builtin/merge.c | 7 +-
builtin/notes.c | 2 +-
builtin/pack-objects.c | 3 +-
builtin/pack-redundant.c | 10 +-
builtin/patch-id.c | 6 +-
builtin/pull.c | 6 +-
builtin/receive-pack.c | 4 +-
builtin/replace.c | 2 +-
builtin/rm.c | 2 +-
builtin/tag.c | 2 +-
builtin/unpack-objects.c | 9 +-
builtin/update-ref.c | 8 +-
bulk-checkin.c | 3 +
bundle-uri.c | 2 +
bundle.c | 2 +
cache-tree.c | 7 +-
checkout.c | 2 +
checkout.h | 2 +-
chunk-format.c | 2 +
chunk-format.h | 2 +-
combine-diff.c | 2 +
commit-graph.c | 22 +-
commit-graph.h | 2 +
commit-reach.c | 2 +
commit.c | 2 +
common-main.c | 2 +
compat/fsmonitor/fsm-ipc-darwin.c | 7 +-
compat/sha1-chunked.c | 2 +-
compat/win32/trace2_win32_process_info.c | 2 +
config.c | 3 +
connected.c | 2 +
convert.c | 2 +
convert.h | 2 +-
csum-file.c | 9 +-
csum-file.h | 2 +-
delta-islands.c | 2 +
diagnose.c | 2 +
diff-lib.c | 7 +-
diff.c | 9 +-
diff.h | 2 +-
diffcore-break.c | 3 +
diffcore-rename.c | 7 +-
diffcore.h | 2 +-
dir.c | 9 +-
dir.h | 2 +-
entry.c | 2 +
environment.c | 3 +
fetch-pack.c | 2 +
fmt-merge-msg.c | 2 +
fsck.c | 5 +-
fsmonitor-ipc.c | 2 +
git.c | 2 +
hash-ll.h | 310 ----------------
hash-lookup.c | 5 +-
hash.h | 366 ++++++++++++++++---
help.c | 2 +
hex.c | 8 +-
hex.h | 28 +-
http-backend.c | 2 +
http-fetch.c | 8 +-
http-push.c | 5 +-
http-walker.c | 6 +-
http.c | 2 +
list-objects-filter-options.c | 2 +
list-objects-filter.c | 2 +
list-objects.c | 2 +
log-tree.c | 2 +
loose.c | 2 +
loose.h | 2 +
ls-refs.c | 2 +
mailmap.c | 2 +
match-trees.c | 6 +-
merge-blobs.c | 2 +
merge-ort.c | 2 +
merge-ort.h | 2 +-
merge-recursive.c | 3 +
merge.c | 2 +
midx-write.c | 2 +
midx.c | 5 +-
negotiator/default.c | 2 +
negotiator/skipping.c | 2 +
notes-cache.c | 2 +
notes-merge.c | 8 +-
notes-utils.c | 2 +
notes.c | 14 +-
object-file-convert.c | 6 +-
object-file.c | 19 +-
object-name.c | 2 +
object.c | 2 +
object.h | 2 +-
oid-array.c | 2 +
oidmap.h | 2 +-
oidset.c | 8 +-
oidset.h | 4 +-
oidtree.c | 4 +-
oidtree.h | 2 +-
oss-fuzz/fuzz-commit-graph.c | 2 +
pack-bitmap-write.c | 6 +-
pack-bitmap.c | 5 +-
pack-check.c | 7 +-
pack-revindex.c | 2 +
pack-write.c | 5 +-
packfile.c | 20 +-
packfile.h | 2 +
parallel-checkout.c | 8 +-
parse-options-cb.c | 2 +
path.c | 3 +
pathspec.c | 2 +
pretty.c | 2 +
progress.c | 2 +
promisor-remote.c | 2 +
protocol-caps.c | 5 +-
range-diff.c | 2 +
reachable.c | 2 +
read-cache-ll.h | 2 +-
read-cache.c | 21 +-
rebase-interactive.c | 2 +
ref-filter.c | 2 +
reflog-walk.c | 2 +
reflog.c | 2 +
refs.c | 8 +-
refs.h | 8 +-
refs/files-backend.c | 8 +-
refs/packed-backend.c | 8 +-
refs/ref-cache.h | 2 +-
refs/reftable-backend.c | 39 +-
refspec.c | 2 +
reftable/dump.c | 2 +-
reftable/reftable-record.h | 2 +-
reftable/system.h | 2 +-
remote-curl.c | 2 +
remote.c | 10 +-
remote.h | 2 +-
replace-object.c | 2 +-
repository.c | 8 +
repository.h | 9 +-
rerere.c | 2 +
reset.c | 2 +
reset.h | 2 +-
resolve-undo.c | 5 +-
resolve-undo.h | 2 +-
revision.c | 2 +
run-command.c | 2 +
scalar.c | 2 +
send-pack.c | 2 +
sequencer.c | 8 +-
serve.c | 4 +-
server-info.c | 2 +
setup.c | 2 +
shallow.c | 2 +
split-index.c | 4 +-
split-index.h | 2 +-
streaming.c | 3 +
submodule-config.c | 4 +-
submodule.c | 8 +-
t/helper/test-bitmap.c | 2 +
t/helper/test-bloom.c | 2 +
t/helper/test-cache-tree.c | 2 +
t/helper/test-dump-cache-tree.c | 2 +
t/helper/test-dump-fsmonitor.c | 2 +
t/helper/test-dump-split-index.c | 2 +
t/helper/test-dump-untracked-cache.c | 2 +
t/helper/test-find-pack.c | 2 +
t/helper/test-fsmonitor-client.c | 2 +
t/helper/test-hash-speed.c | 2 +-
t/helper/test-lazy-init-name-hash.c | 2 +
t/helper/test-match-trees.c | 2 +
t/helper/test-oid-array.c | 4 +
t/helper/test-oidmap.c | 2 +
t/helper/test-pack-mtimes.c | 2 +
t/helper/test-partial-clone.c | 2 +-
t/helper/test-proc-receive.c | 9 +-
t/helper/test-reach.c | 2 +
t/helper/test-read-cache.c | 2 +
t/helper/test-read-graph.c | 2 +
t/helper/test-read-midx.c | 2 +
t/helper/test-ref-store.c | 2 +
t/helper/test-repository.c | 2 +
t/helper/test-revision-walking.c | 2 +
t/helper/test-scrap-cache-tree.c | 2 +
t/helper/test-sha1.c | 2 +-
t/helper/test-sha256.c | 2 +-
t/helper/test-submodule-config.c | 4 +-
t/helper/test-submodule-nested-repo-config.c | 2 +
t/helper/test-submodule.c | 2 +
t/helper/test-trace2.c | 2 +
t/helper/test-write-cache.c | 2 +
t/t0064-oid-array.sh | 18 +
t/t5550-http-fetch-dumb.sh | 6 +
t/test-lib-functions.sh | 5 +
t/unit-tests/lib-oid.h | 2 +-
t/unit-tests/t-example-decorate.c | 2 +
tag.c | 2 +
tmp-objdir.c | 2 +
transport-helper.c | 2 +
transport.c | 2 +
tree-diff.c | 1 +
tree-walk.c | 6 +-
tree-walk.h | 2 +-
tree.c | 2 +
unpack-trees.c | 2 +
upload-pack.c | 2 +
walker.c | 2 +
worktree.c | 2 +
wt-status.c | 6 +-
xdiff-interface.c | 2 +
xdiff-interface.h | 2 +-
227 files changed, 1002 insertions(+), 617 deletions(-)
delete mode 100644 hash-ll.h
Range-diff against v2:
1: d2154e8c45 = 1: d2154e8c45 hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions
2: aa468c3d88 ! 2: c481479598 hash: require hash algorithm in `hasheq()`, `hashcmp()` and `hashclr()`
@@ Commit message
As those functions are now independent of `the_repository`, we can move
them from "hash.h" to "hash-ll.h".
+ Note that both in this and subsequent commits in this series we always
+ just pass `the_repository->hash_algo` as input even if it is obvious
+ that there is a repository in the context that we should be using the
+ hash from instead. This is done to be on the safe side and not introduce
+ any regressions. All callsites should eventually be amended to use a
+ repo passed via parameters, but this is outside the scope of this patch
+ series.
+
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
## builtin/index-pack.c ##
@@ pack-check.c: static int verify_packfile(struct repository *r,
r->hash_algo->final_fn(hash, &ctx);
pack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL);
- if (!hasheq(hash, pack_sig))
-+ if (!hasheq(hash, pack_sig, r->hash_algo))
++ if (!hasheq(hash, pack_sig, the_repository->hash_algo))
err = error("%s pack checksum mismatch",
p->pack_name);
- if (!hasheq(index_base + index_size - r->hash_algo->hexsz, pack_sig))
+ if (!hasheq(index_base + index_size - r->hash_algo->hexsz, pack_sig,
-+ r->hash_algo))
++ the_repository->hash_algo))
err = error("%s pack checksum does not match its index",
p->pack_name);
unuse_pack(w_curs);
3: 403ea4485b ! 3: 226173a92b hash: require hash algorithm in `oidread()` and `oidclr()`
@@ commit-graph.c: static struct tree *load_tree_for_commit(struct repository *r,
st_mult(GRAPH_DATA_WIDTH, graph_pos - g->num_commits_in_base);
- oidread(&oid, commit_data);
-+ oidread(&oid, commit_data, r->hash_algo);
++ oidread(&oid, commit_data, the_repository->hash_algo);
set_commit_tree(c, lookup_tree(r, &oid));
return c->maybe_tree;
@@ commit-graph.c: int write_commit_graph(struct object_directory *odb,
struct object_id oid;
- oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_len, i));
+ oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_len, i),
-+ r->hash_algo);
++ the_repository->hash_algo);
oid_array_append(&ctx->oids, &oid);
}
}
@@ commit-graph.c: static int verify_one_commit_graph(struct repository *r,
- oidread(&cur_oid, g->chunk_oid_lookup + st_mult(g->hash_len, i));
+ oidread(&cur_oid, g->chunk_oid_lookup + st_mult(g->hash_len, i),
-+ r->hash_algo);
++ the_repository->hash_algo);
if (i && oidcmp(&prev_oid, &cur_oid) >= 0)
graph_report(_("commit-graph has incorrect OID order: %s then %s"),
4: fa263d6b07 = 4: 3947c0c04d global: ensure that object IDs are always padded
5: a7df209bda = 5: 91eb94bc0b hash: convert `oidcmp()` and `oideq()` to compare whole hash
6: 9058837c93 = 6: 9ae29e1912 hash: make `is_null_oid()` independent of `the_repository`
7: d26584dc8f = 7: da48d5fdea hash: require hash algorithm in `is_empty_{blob,tree}_oid()`
8: 4858cca25f = 8: 1cf25bec3e hash: require hash algorithm in `empty_tree_oid_hex()`
9: cb3694ad0e ! 9: 7e023a335f global: introduce `USE_THE_REPOSITORY_VARIABLE` macro
@@ remote.c
#include "abspath.h"
#include "config.h"
+ ## repository.c ##
+@@
+ #include "promisor-remote.h"
+ #include "refs.h"
+
++/*
++ * We do not define `USE_THE_REPOSITORY_VARIABLE` in this file because we do
++ * not want to rely on functions that implicitly use `the_repository`. This
++ * means that the `extern` declaration of `the_repository` isn't visible here,
++ * which makes sparse unhappy. We thus declare it here.
++ */
++extern struct repository *the_repository;
++
+ /* The main repository */
+ static struct repository the_repo;
+ struct repository *the_repository = &the_repo;
+
## repository.h ##
@@ repository.h: struct repository {
unsigned different_commondir:1;
10: 4492548209 = 10: 74c88ebc39 refs: avoid include cycle with "repository.h"
11: f3cbc4b9f9 = 11: fe4550ba0c hash-ll: merge with "hash.h"
12: 9178098dd7 = 12: 30babd8a67 http-fetch: don't crash when parsing packfile without a repo
13: 0b4436c32b = 13: fa41a85c7e oidset: pass hash algorithm when parsing file
14: c7abfbc489 = 14: 0b42208e2f protocol-caps: use hash algorithm from passed-in repository
15: 9ae4fdb8f1 = 15: e1f4bffea1 replace-object: use hash algorithm from passed-in repository
16: 3ceb726655 = 16: 5b8f981ea2 compat/fsmonitor: fix socket path in networked SHA256 repos
17: 74e5489bd0 = 17: ad83b17ad0 t/helper: use correct object hash in partial-clone helper
18: 470aea1fc8 = 18: a488363bcb t/helper: fix segfault in "oid-array" command without repository
19: 1f0682fc7d = 19: 5de7a01af5 t/helper: remove dependency on `the_repository` in "proc-receive"
20: 16fb86c2b2 = 20: 436ffc0570 hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE`
--
2.45.2.457.g8d94cfb545.dirty
Attachment:
signature.asc
Description: PGP signature
