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!