| Message ID | a217e9a0640b45d21ef971d6e91cee3f1993f383.1656535245.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bundle URIs: design doc and initial git fetch --bundle-uri implementation | expand |
On 2022.06.29 20:40, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <derrickstolee@github.com> > > When fetching from a bundle URI, the branches of that bundle are stored > in a different ref namespace: refs/bundles/. This namespace is intended > to assist with later 'git fetch' negotiations with a Git server, > allowing the client to advertise which data it already has from a bundle > URI. > > These references can be confusing for a user when they appear as a > decoration in 'git log' output. Add "refs/bundles/" to the multi-valued > log.excludeDecoration config value. This is similar to the way > "refs/prefetch/" is hidden by background prefetch operations in 'git > maintenance' as added by 96eaffebb (maintenance: set > log.excludeDecoration durin prefetch, 2021-01-19). > > Signed-off-by: Derrick Stolee <derrickstolee@github.com> > --- > Documentation/fetch-options.txt | 3 ++- > bundle-uri.c | 7 +++++++ > t/t5558-fetch-bundle-uri.sh | 12 +++++++++--- > 3 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 09bd1feeed8..8b801bcc2f3 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -321,4 +321,5 @@ endif::git-pull[] > --bundle-uri=<uri>:: > Instead of fetching from a remote, fetch a bundle from the given > `<uri>` and unbundle the data into the local repository. The refs > - in the bundle will be stored under the `refs/bundle/*` namespace. > + in the bundle will be stored under the hidden `refs/bundle/*` > + namespace. > diff --git a/bundle-uri.c b/bundle-uri.c > index 4c793843a2a..6e0f1cb06fd 100644 > --- a/bundle-uri.c > +++ b/bundle-uri.c > @@ -1,6 +1,7 @@ > #include "cache.h" > #include "bundle-uri.h" > #include "bundle.h" > +#include "config.h" > #include "object-store.h" > #include "refs.h" > #include "run-command.h" > @@ -153,6 +154,12 @@ int fetch_bundle_uri(struct repository *r, const char *uri) > if ((result = unbundle_from_file(r, filename.buf))) > goto cleanup; > > + git_config_set_multivar_gently("log.excludedecoration", > + "refs/bundle/", > + "refs/bundle/", > + CONFIG_FLAGS_FIXED_VALUE | > + CONFIG_FLAGS_MULTI_REPLACE); > + I dislike the idea of modifying the user's config as a side effect here. Since it's a cosmetic issue, can we drop this patch and figure out better default values for log.excludedecoration? > cleanup: > unlink(filename.buf); > strbuf_release(&filename); > diff --git a/t/t5558-fetch-bundle-uri.sh b/t/t5558-fetch-bundle-uri.sh > index 919db6f4551..bed4cc3e99c 100755 > --- a/t/t5558-fetch-bundle-uri.sh > +++ b/t/t5558-fetch-bundle-uri.sh > @@ -28,7 +28,9 @@ test_expect_success 'fetch file bundle' ' > git -C fetch-to fetch --bundle-uri="$(pwd)/fetch-from/B.bundle" && > git -C fetch-to rev-parse refs/bundles/topic >actual && > git -C fetch-from rev-parse topic >expect && > - test_cmp expect actual > + test_cmp expect actual && > + > + test_config -C fetch-to log.excludedecoration refs/bundle/ > ' > > test_expect_success 'fetch file:// bundle' ' > @@ -36,7 +38,9 @@ test_expect_success 'fetch file:// bundle' ' > git -C fetch-file fetch --bundle-uri="file://$(pwd)/fetch-from/B.bundle" && > git -C fetch-file rev-parse refs/bundles/topic >actual && > git -C fetch-from rev-parse topic >expect && > - test_cmp expect actual > + test_cmp expect actual && > + > + test_config -C fetch-file log.excludedecoration refs/bundle/ > ' > > ######################################################################### > @@ -62,7 +66,9 @@ test_expect_success 'fetch HTTP bundle' ' > git -C fetch-http fetch --bundle-uri="$HTTPD_URL/B.bundle" && > git -C fetch-http rev-parse refs/bundles/topic >actual && > git -C fetch-from rev-parse topic >expect && > - test_cmp expect actual > + test_cmp expect actual && > + > + test_config log.excludedecoration refs/bundle/ > ' > > # Do not add tests here unless they use the HTTP server, as they will > -- > gitgitgadget
On 7/21/2022 5:47 PM, Josh Steadmon wrote: > On 2022.06.29 20:40, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <derrickstolee@github.com> >> >> When fetching from a bundle URI, the branches of that bundle are stored >> in a different ref namespace: refs/bundles/. This namespace is intended >> to assist with later 'git fetch' negotiations with a Git server, >> allowing the client to advertise which data it already has from a bundle >> URI. >> >> These references can be confusing for a user when they appear as a >> decoration in 'git log' output. Add "refs/bundles/" to the multi-valued >> log.excludeDecoration config value. This is similar to the way >> "refs/prefetch/" is hidden by background prefetch operations in 'git >> maintenance' as added by 96eaffebb (maintenance: set >> log.excludeDecoration durin prefetch, 2021-01-19). >> + git_config_set_multivar_gently("log.excludedecoration", >> + "refs/bundle/", >> + "refs/bundle/", >> + CONFIG_FLAGS_FIXED_VALUE | >> + CONFIG_FLAGS_MULTI_REPLACE); >> + > > I dislike the idea of modifying the user's config as a side effect here. > Since it's a cosmetic issue, can we drop this patch and figure out > better default values for log.excludedecoration? You're right. log.excludeDecoration was initially created precisely for enabling it within the prefetch maintenance task, but it would be better to change which decoration set is shown by default. I'll pull this patch out of this series and bring out another one that rethinks this entire space. Thanks, -Stolee
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 09bd1feeed8..8b801bcc2f3 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -321,4 +321,5 @@ endif::git-pull[] --bundle-uri=<uri>:: Instead of fetching from a remote, fetch a bundle from the given `<uri>` and unbundle the data into the local repository. The refs - in the bundle will be stored under the `refs/bundle/*` namespace. + in the bundle will be stored under the hidden `refs/bundle/*` + namespace. diff --git a/bundle-uri.c b/bundle-uri.c index 4c793843a2a..6e0f1cb06fd 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -1,6 +1,7 @@ #include "cache.h" #include "bundle-uri.h" #include "bundle.h" +#include "config.h" #include "object-store.h" #include "refs.h" #include "run-command.h" @@ -153,6 +154,12 @@ int fetch_bundle_uri(struct repository *r, const char *uri) if ((result = unbundle_from_file(r, filename.buf))) goto cleanup; + git_config_set_multivar_gently("log.excludedecoration", + "refs/bundle/", + "refs/bundle/", + CONFIG_FLAGS_FIXED_VALUE | + CONFIG_FLAGS_MULTI_REPLACE); + cleanup: unlink(filename.buf); strbuf_release(&filename); diff --git a/t/t5558-fetch-bundle-uri.sh b/t/t5558-fetch-bundle-uri.sh index 919db6f4551..bed4cc3e99c 100755 --- a/t/t5558-fetch-bundle-uri.sh +++ b/t/t5558-fetch-bundle-uri.sh @@ -28,7 +28,9 @@ test_expect_success 'fetch file bundle' ' git -C fetch-to fetch --bundle-uri="$(pwd)/fetch-from/B.bundle" && git -C fetch-to rev-parse refs/bundles/topic >actual && git -C fetch-from rev-parse topic >expect && - test_cmp expect actual + test_cmp expect actual && + + test_config -C fetch-to log.excludedecoration refs/bundle/ ' test_expect_success 'fetch file:// bundle' ' @@ -36,7 +38,9 @@ test_expect_success 'fetch file:// bundle' ' git -C fetch-file fetch --bundle-uri="file://$(pwd)/fetch-from/B.bundle" && git -C fetch-file rev-parse refs/bundles/topic >actual && git -C fetch-from rev-parse topic >expect && - test_cmp expect actual + test_cmp expect actual && + + test_config -C fetch-file log.excludedecoration refs/bundle/ ' ######################################################################### @@ -62,7 +66,9 @@ test_expect_success 'fetch HTTP bundle' ' git -C fetch-http fetch --bundle-uri="$HTTPD_URL/B.bundle" && git -C fetch-http rev-parse refs/bundles/topic >actual && git -C fetch-from rev-parse topic >expect && - test_cmp expect actual + test_cmp expect actual && + + test_config log.excludedecoration refs/bundle/ ' # Do not add tests here unless they use the HTTP server, as they will