Thanks everyone for your comments. Changes from v2:
Patch 1:
- I took a look at head_ref_namespaced() was hoping that it could be
refactored to meet my new needs, but I don't think it's feasible.
head_ref_namespaced() seems to have a precise use - for when we
already have a callback that we're using with
for_each_namespaced_ref(), and we want to use it with the HEAD ref as
well. Instead what I did is to eliminate the use of
head_ref_namespaced() here, and used send_possibly_unborn_head() for
both the regular and the new unborn case.
- Renamed null_oid to avoid shadowing, as pointed out by Junio.
- Changed to a boolean lsrefs.allowUnborn, so no more advertise/allow
distinction. Currently it still defaults to unallowed. Peff, what did
you mean in [1] by the following:
So it seems like you're saying that it should default to "on", but at
the same time you are talking about enabling the feature (which seems
to imply switching it from "off" to "on"). (Also, note that this is a
server-side thing; on the client-side, Git will always use what the
server gives and there is no option to control this.)
- Added documentation of lsrefs.allowUnborn, which I forgot.
Patch 2:
- Used Junio's suggested commit message.
Patch 3:
- No changes except what was necessary due to the config option change.
[1] https://lore.kernel.org/git/X9xJLWdFJfNJTn0p@coredump.intra.peff.net/
Jonathan Tan (3):
ls-refs: report unborn targets of symrefs
connect, transport: add no-op arg for future patch
clone: respect remote unborn HEAD
Documentation/config.txt | 2 +
Documentation/config/init.txt | 2 +-
Documentation/config/lsrefs.txt | 3 ++
Documentation/technical/protocol-v2.txt | 10 ++++-
builtin/clone.c | 19 +++++++--
builtin/fetch-pack.c | 3 +-
builtin/fetch.c | 2 +-
builtin/ls-remote.c | 2 +-
builtin/remote.c | 2 +-
connect.c | 29 ++++++++++++--
ls-refs.c | 51 +++++++++++++++++++++++--
ls-refs.h | 1 +
remote.h | 3 +-
serve.c | 2 +-
t/t5606-clone-options.sh | 7 ++--
t/t5702-protocol-v2.sh | 9 +++++
transport-helper.c | 7 +++-
transport-internal.h | 13 +++----
transport.c | 29 ++++++++------
transport.h | 7 +++-
20 files changed, 160 insertions(+), 43 deletions(-)
create mode 100644 Documentation/config/lsrefs.txt
Range-diff against v2:
1: 7f5b50c7b2 ! 1: 7d20ec323a ls-refs: report unborn targets of symrefs
@@ Commit message
Currently, symrefs that have unborn targets (such as in this case) are
not communicated by the protocol. Teach Git to advertise and support the
- "unborn" feature in "ls-refs" (guarded by the lsrefs.unborn config).
- This feature indicates that "ls-refs" supports the "unborn" argument;
- when it is specified, "ls-refs" will send the HEAD symref with the name
- of its unborn target.
+ "unborn" feature in "ls-refs" (guarded by the lsrefs.allowunborn
+ config). This feature indicates that "ls-refs" supports the "unborn"
+ argument; when it is specified, "ls-refs" will send the HEAD symref with
+ the name of its unborn target.
This change is only for protocol v2. A similar change for protocol v0
would require independent protocol design (there being no analogous
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
+ ## Documentation/config.txt ##
+@@ Documentation/config.txt: include::config/interactive.txt[]
+
+ include::config/log.txt[]
+
++include::config/lsrefs.txt[]
++
+ include::config/mailinfo.txt[]
+
+ include::config/mailmap.txt[]
+
+ ## Documentation/config/lsrefs.txt (new) ##
+@@
++lsrefs.allowUnborn::
++ Allow the server to send information about unborn symrefs during the
++ protocol v2 ref advertisement.
+
## Documentation/technical/protocol-v2.txt ##
@@ Documentation/technical/protocol-v2.txt: ls-refs takes in the following arguments:
When specified, only references having a prefix matching one of
@@ ls-refs.c: static int send_ref(const char *refname, const struct object_id *oid,
+ struct strbuf namespaced = STRBUF_INIT;
+ struct object_id oid;
+ int flag;
-+ int null_oid;
++ int oid_is_null;
+
+ strbuf_addf(&namespaced, "%sHEAD", get_git_namespace());
+ resolve_ref_unsafe(namespaced.buf, 0, &oid, &flag);
-+ null_oid = is_null_oid(&oid);
-+ if (!null_oid || (data->symrefs && (flag & REF_ISSYMREF)))
-+ send_ref(namespaced.buf, null_oid ? NULL : &oid, flag, data);
++ oid_is_null = is_null_oid(&oid);
++ if (!oid_is_null ||
++ (data->unborn && data->symrefs && (flag & REF_ISSYMREF)))
++ send_ref(namespaced.buf, oid_is_null ? NULL : &oid, flag, data);
+ strbuf_release(&namespaced);
+}
+
@@ ls-refs.c: static int send_ref(const char *refname, const struct object_id *oid,
+{
+ struct ls_refs_data *data = cb_data;
+
-+ if (!strcmp("lsrefs.unborn", var))
-+ data->allow_unborn = !strcmp(value, "allow") ||
-+ !strcmp(value, "advertise");
++ if (!strcmp("lsrefs.allowunborn", var))
++ data->allow_unborn = git_config_bool(var, value);
++
/*
* We only serve fetches over v2 for now, so respect only "uploadpack"
* config. This may need to eventually be expanded to "receive", but we
@@ ls-refs.c: int ls_refs(struct repository *r, struct strvec *keys,
die(_("expected flush after ls-refs arguments"));
- head_ref_namespaced(send_ref, &data);
-+ if (data.unborn)
-+ send_possibly_unborn_head(&data);
-+ else
-+ head_ref_namespaced(send_ref, &data);
++ send_possibly_unborn_head(&data);
for_each_namespaced_ref(send_ref, &data);
packet_flush(1);
strvec_clear(&data.prefixes);
@@ ls-refs.c: int ls_refs(struct repository *r, struct strvec *keys,
+int ls_refs_advertise(struct repository *r, struct strbuf *value)
+{
+ if (value) {
-+ char *str = NULL;
++ int allow_unborn_value;
+
-+ if (!repo_config_get_string(the_repository, "lsrefs.unborn",
-+ &str) &&
-+ !strcmp("advertise", str)) {
++ if (!repo_config_get_bool(the_repository,
++ "lsrefs.allowunborn",
++ &allow_unborn_value) &&
++ allow_unborn_value)
+ strbuf_addstr(value, "unborn");
-+ free(str);
-+ }
+ }
+
+ return 1;
2: e24fb6d746 ! 2: b5a78857eb connect, transport: add no-op arg for future patch
@@ Metadata
## Commit message ##
connect, transport: add no-op arg for future patch
- A future patch will require transport_get_remote_refs() and
- get_remote_refs() to gain a new argument. Add the argument in this
- patch, with no effect on execution, so that the future patch only needs
- to concern itself with new logic.
+ In a future patch we plan to return the name of an unborn current branch
+ from deep in the callchain to a caller via a new pointer parameter that
+ points at a variable in the caller when the caller calls
+ get_remote_refs() and transport_get_remote_refs(). Add the parameter to
+ functions involved in the callchain, but no caller passes an actual
+ argument yet in this step. Thus, the future patch only needs to concern
+ itself with new logic.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
3: 6fcb3b16ce ! 3: c2303dc976 clone: respect remote unborn HEAD
@@ t/t5606-clone-options.sh: test_expect_success 'redirected clone -v does show pro
test_expect_success 'chooses correct default initial branch name' '
- git init --bare empty &&
+ git -c init.defaultBranch=foo init --bare empty &&
-+ test_config -C empty lsrefs.unborn advertise &&
++ test_config -C empty lsrefs.allowUnborn true &&
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
git -c init.defaultBranch=up clone empty whats-up &&
- test refs/heads/up = $(git -C whats-up symbolic-ref HEAD) &&
@@ t/t5702-protocol-v2.sh: test_expect_success 'clone with file:// using protocol v
+test_expect_success 'clone of empty repo propagates name of default branch' '
+ git -c init.defaultBranch=mydefaultbranch init file_empty_parent &&
-+ test_config -C file_empty_parent lsrefs.unborn advertise &&
++ test_config -C file_empty_parent lsrefs.allowUnborn true &&
+
+ git -c init.defaultBranch=main -c protocol.version=2 \
+ clone "file://$(pwd)/file_empty_parent" file_empty_child &&
+ grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+'
-+
-+test_expect_success '...but not if it is not advertised' '
-+ test_config -C file_empty_parent lsrefs.unborn none &&
-+
-+ git -c init.defaultBranch=main -c protocol.version=2 \
-+ clone "file://$(pwd)/file_empty_parent" file_empty_child_2 &&
-+ grep "refs/heads/main" file_empty_child_2/.git/HEAD
-+'
+
test_expect_success 'fetch with file:// using protocol v2' '
test_when_finished "rm -f log" &&