"Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:
> static void upload_pack(void)
> {
> + git_config(upload_pack_config, NULL);
> + if (!configured_advertise)
> + push_advertise("refs/*");
> +
> + head_ref(scan_ref, NULL);
> + for_each_ref(scan_ref, NULL);
> +
> reset_timeout();
> - head_ref(send_ref, NULL);
> - for_each_ref(send_ref, NULL);
> - packet_flush(1);
> + send_refs();
> receive_needs();
> if (want_obj.nr) {
> get_common_commits();
> @@ -652,6 +764,7 @@ int main(int argc, char **argv)
>
> git_extract_argv0_path(argv[0]);
> read_replace_refs = 0;
> + push_advertise("HEAD");
>
> for (i = 1; i < argc; i++) {
> char *arg = argv[i];
> @@ -667,6 +780,10 @@ int main(int argc, char **argv)
> daemon_mode = 1;
> continue;
> }
> + if (!prefixcmp(arg, "--expand=")) {
> + push_advertise(arg + 9);
> + continue;
> + }
> if (!strcmp(arg, "--")) {
> i++;
> break;
This arrangement of push_advertise() calls are rather curious. I think
your design guidelines are:
- We do want to advertise HEAD, so it can (and should) be unconditional;
- We may want to restrict with configuration, or use "refs/*" as the
fallback default;
- We may want to (extend|replace) advertised set configured in the
configuration file.
I would naively expect the above to be implemented in this order:
- In main, first thing is to do the git_config() bit; if to_advertise is
non-empty after git_config() returns, we have seen upload.advertise.
Otherwise we haven't. Push "refs/*" in the latter case.
- Then, parse the command line. Do we see "--expand="? If so:
. When handling the first "--expand=", clear to_advertise list. This
step is optional---necessary only if we want to make config variable
override-able ;-);
. Then, push the pattern in.
- And finally push "HEAD" in unconditionally.
Which would mean a few things:
- Your version always extends what is read from the configuration with
--expand=, but I think replacing would be the right thing to do.
- configured_advertise global variable can go.
- Instead, to implement "--expand= replaces", we need a local variable in
main() to remember if we already cleared to_advertise of the patterns
read from the configuration file, and use it in the part that parses
"--expand=".
But I may not be thinking clearly; I was up all night because I couldn't
sleep under loud noises of fire-fighting helicopters...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html