Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Over the time the default value for --thin has been switched between
> on and off. As of now it's always on, even if --no-thin is given.
> Correct the code to respect --no-thin.
>
> receive-pack learns about --no-thin only for testing purposes, hence
> no document update.
Please name it "--reject-thin-pack-for-testing" to make it crystal
clear that a documentation patch to "document undocumented option"
is unwanted.
> diff --git a/builtin/push.c b/builtin/push.c
> index 04f0eaf..333a1fb 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -15,7 +15,7 @@ static const char * const push_usage[] = {
> NULL,
> };
>
> -static int thin;
> +static int thin = 1;
> static int deleterefs;
> static const char *receivepack;
> static int verbosity;
> @@ -313,8 +313,7 @@ static int push_with_options(struct transport *transport, int flags)
> if (receivepack)
> transport_set_option(transport,
> TRANS_OPT_RECEIVEPACK, receivepack);
> - if (thin)
> - transport_set_option(transport, TRANS_OPT_THIN, "yes");
> + transport_set_option(transport, TRANS_OPT_THIN, thin ? "yes" : NULL);
>
> if (verbosity > 0)
> fprintf(stderr, _("Pushing to %s\n"), transport->url);
Hmm, I am utterly confused.
How does the original code have thin as an non-overridable default?
The variable is initialized to 0, parse-options specifies it as
OPT_BOOLEAN, and TRANS_OPT_THIN is set only if "thin" is set.
Updated code flips the default to "1" but unconditionally uses
TRANS_OPT_THIN to set it to either "yes" or NULL. While it is not
wrong per-se, do_push() (i.e. the caller of this function) grabs the
transport from transport_get() which initializes the transport with
the thin option disabled by default, so it is not immediately
obvious to me why "always call TRANS_OPT_THIN but set it explicitly
to NULL when --no-thin is asked" is necessary or improvement.
Puzzled....
--
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