On Fri, Jun 21, 2024 at 11:23 AM Toon Claes <toon@xxxxxxxxx> wrote:
>
> A bundle URI can serve a gitformat-bundle(5) or a bundle list. This
> plain text file is in the Git config format containing other bundle
> URIs. To avoid these bundle lists to nest too deep, we've set a limit
> with `max_bundle_uri_depth`.
Yeah, max_bundle_uri_depth seems to be hardcoded to 4.
> Although, when walk through the tree of
s/walk/walking/
> bundles, the current depth is incremented in download_bundle_list() and
> then calls download_bundle_to_file(), which also increments the depth.
s/and then calls/which then calls/
> Remove the increment in download_bundle_to_file().
The increment is removed by replacing:
fetch_bundle_uri_internal( ..., ctx->depth + 1, ...)
with:
fetch_bundle_uri_internal( ..., ctx->depth, ...)
in download_bundle_to_file(). Ok.
It looks like there is another similar call to that function like this:
fetch_bundle_uri_internal( ... , ctx.depth + 1, ... )
in fetch_bundles_by_token() though.
There ctx.depth is initialized to 0 before the call, so it looks like
it could work, but fetch_bundle_uri_internal() can call
fetch_bundle_list_in_config_format() which can call
download_bundle_list() which, as we saw above, still increases the
depth by 1.
So even if download_bundle_list() then calls download_bundle_to_file()
without increasing the depth, I am not sure it works well in all
cases. At least I think a bit more explanations might be needed.
> +test_expect_success 'clone bundle list (file, above max depth)' '
> + cat >bundle-list-1 <<-EOF &&
> + [bundle]
> + version = 1
> + mode = any
> +
> + [bundle "bundle-list-2"]
> + uri = file://$(pwd)/bundle-list-2
> + EOF
> +
> + cat >bundle-list-2 <<-EOF &&
> + [bundle]
> + version = 1
> + mode = any
> +
> + [bundle "bundle-list-3"]
> + uri = file://$(pwd)/bundle-list-3
> + EOF
> +
> + cat >bundle-list-3 <<-EOF &&
> + [bundle]
> + version = 1
> + mode = any
> +
> + [bundle "bundle-list-4"]
> + uri = file://$(pwd)/bundle-list-4
> + EOF
> +
> + cat >bundle-list-4 <<-EOF &&
> + [bundle]
> + version = 1
> + mode = any
> +
> + [bundle "bundle-0"]
> + uri = file://$(pwd)/clone-from/bundle-0.bundle
Is there a reason why it's not more like:
[bundle "bundle-list-5"]
uri = file://$(pwd)/bundle-list-5
?
> + EOF
It looks like the above is the setup part of the following tests, so
it could perhaps be moved into a separate `test_expect_success 'setup
deep clone bundle list'` test.
> + git clone --bundle-uri="file://$(pwd)/bundle-list-1" \
> + clone-from clone-too-deep 2>err &&
> + ! grep "fatal" err &&
> + grep "warning: exceeded bundle URI recursion limit" err &&
> +
> + git -C clone-from for-each-ref --format="%(objectname)" >oids &&
> + git -C clone-too-deep cat-file --batch-check <oids &&
> +
> + git -C clone-too-deep for-each-ref --format="%(refname)" >refs &&
> + ! grep "refs/bundles/" refs
> +'
> +
> +test_expect_success 'clone bundle list (file, below max depth)' '
> + git clone --bundle-uri="file://$(pwd)/bundle-list-2" \
> + clone-from clone-max-depth 2>err &&
> + ! grep "fatal" err &&
> + ! grep "warning: exceeded bundle URI recursion limit" err &&
> +
> + git -C clone-from for-each-ref --format="%(objectname)" >oids &&
> + git -C clone-max-depth cat-file --batch-check <oids &&
> +
> + git -C clone-max-depth for-each-ref --format="%(refname)" >refs &&
> + ! grep "refs/bundles/" refs
> +'
Thanks!
