"Xing Xin" <bupt_xingxin@xxxxxxx> writes:
> Personally I think data from bundles and data received via network
> should be treated equally.
Yup, that is not personal ;-) but is universally accepted as a good
discipline. In the case of bundle-uri, the bundle came over the
network so it is even more true that they should be treated the
same.
> For "fetch-pack" we now have some configs
> such as "fetch.fsckobjects" and "transfer.fsckobjects" to decide the
> behavior, these configs are invisible when we are fetching bundles.
When fetching over network, transport.c:fetch_refs_via_pack() calls
fetch_pack.c:fetch_pack(), which eventually calls get_pack() and the
configuration variables are honored there. It appears that the
transport layer is unaware of the .fsckobjects configuration knobs.
When fetching from a bundle, transport.c:fetch_refs_from_bundle()
calls bundle.c:unbundle(). This function has three callers, i.e.
"git bundle unbundle", normal fetching from a bundle, and more
recently added bundle-uri codepaths.
I think one reasonable approach to take is to add an extra parameter
that takes one of three values: (never, use-config, always), and
conditionally add "--fsck-objects" to the command line of the
index-pack. Teach "git bundle unbundle" the "--fsck-objects" option
so that it can pass 'never' or 'always' from the command line, and
pass 'use-config' from the code paths for normal fetching from a
budnle and bundle-uri.
To implement use-config, you'd probably need to refactor a small
part of fetch-pack.c:get_pack()
if (fetch_fsck_objects >= 0
? fetch_fsck_objects
: transfer_fsck_objects >= 0
? transfer_fsck_objects
: 0)
fsck_objects = 1;
into a public function (to support a caller like unbundle() that
comes from sideways, the new function may also need to call
fetch_pack_setup() to prime them).
A patch series may take a structure like so:
* define enum { UNBUNDLE_FSCK_NEVER, UNBUNDLE_FSCK_ALWAYS } in
bundle.h, have bundle.c:unbundle() accept a new parameter of that
type, and conditionally add "--fsck-objects" to its call to
"index-pack". "git bundle unbundle" can pass 'never' to its
invocation to unbundle() as an easy way to test it. For the
other two callers, we can start by passing 'always'.
* (optional) teach "git bundle unbundle" a new "--fsck-objects"
option to allow passing 'always' to its call to unbundle(). With
that, add tests to feed it a bundle with questionable objects in
it and make sure that unbundling notices.
* refactor fetch-pack.c:get_pack() to make the fetch-then-transfer
configuration logic available to external callers.
* Add UNBUNDLE_FSCK_USE_CONFIG to the enum, enhance unbundle() to
react to the value by calling the helper function you introduced
in the previous step.
Thanks.
