Re: [RFC/PATCH] Introduce branch.<name>.pushremote

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2013, 20:00 UTC
Message-ID: <7vhallw946.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALkWK0kp5SvbeTQgLRKWUAT_koSi0wyp6iPTC324iH8+D8oeJg@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 11 quoted lines

> Junio C Hamano wrote:
>
>> In other words, does it make sense to read branch.$name.pushremote
>> for all the other irrelevant branches?
>>
>> In yet other words, perhaps adding pushremote_name to the branch
>> structure is unneeded, and you only need this single global
>> variable?
>
> Frankly, I'm unhappy with this global.  Setting a global here and
> subsequently reading it in pushremote_get() feels flaky.

Why unhappy?

This is expressed as a per-branch configuration, but in realitiy the configuration on any branch that is not the current one does *not* matter an iota.

An API call find_pushremote_for_branch() that takes a branch name and returns the pushremote for that arbitrary branch has no use in your designed feature. find_pushremote() that tells you where a lazy "git push" goes under the conditoin is the only thing you need in your design. You can still implement it without globals, e.g.

	struct bp { const char *branch; const char *remotename; };

        static int pushremote(const char *var, const char *val, void *cb)
	{
		struct bp *bp = cb;
	        const char *name, *key;
                int namelen;

		if (!parse_config_key(var, "branch", &name, &namelen, &key) &&
                    namelen == strlen(bp->branch) &&
                    !memcmp(name, bp->branch, name, namelen) &&
		    !strcmp(key, "remotepush")) {
			git_config_string(&bp->remotename, var, val);
		}
		return 0;
	}

	char *find_pushremote(void) {
		struct bp bp;

                bp.branch = get_current_branch();
                bp.remotename = NULL;
                git_config(pushremote, &bp);
                return bp.remotename;
	}

And if you want to take default.pushremote that changes a lazy "git push" to go to somewhere other than "origin", you make the pushremote() callback notice that variable, add one element "defaultremote" to struct "bp" to record that value, and then make find_pushremote() do

	return bp.remotename ? bp.remotename : bp.defaultremote;

Sometimes "global variable" feels disturbing because then we cannot run more than one user at the same time. One caller may want to know the value for "foo" branch while another want it for "bar" branch.

But there is no reason for anybody to want to know the value for the
variables branch.$name.pushremote for all defined $name's at the
same time, exactly because this matters only for the current branch,
no?