Fwd: git clone does not respect command line options

Hi!

I'm trying to use git in an integration test and i'm having trouble
with configuration options.

On windows developer machines we use wincred as our credenital helper
and thus have it set in ~/.gitconfig

For the integration test that is no use as it will make testing
unauthorized logging in impossible.

Since there is no way of disabling configuration options on the
command line i tried setting it to store with a file I could delete.
So in front of every command we insert `-c credential.helper="store
--file=creds.txt"`. In the end the command line looks like:

git -c credential.helper="store --file=creds.txt" clone
http://admin:admin@oururl@20000/TestRepo.git

I see the file creds.txt being created containing only
http://admin:admin@oururl@20000/TestRepo.git but the credenital at the
same time appears in the windows credential store.

Can anybody else confirm this?

Thank you.

On Fri, Feb 26, 2016 at 12:17:49PM +0530, Guilherme wrote:

I'm trying to use git in an integration test and i'm having trouble
with configuration options.

On windows developer machines we use wincred as our credenital helper
and thus have it set in ~/.gitconfig

For the integration test that is no use as it will make testing
unauthorized logging in impossible.

Since there is no way of disabling configuration options on the
command line i tried setting it to store with a file I could delete.
So in front of every command we insert `-c credential.helper="store
--file=creds.txt"`. In the end the command line looks like:

git -c credential.helper="store --file=creds.txt" clone
http://admin:admin@oururl@20000/TestRepo.git

I see the file creds.txt being created containing only
http://admin:admin@oururl@20000/TestRepo.git but the credenital at the
same time appears in the windows credential store.

Can anybody else confirm this?

That's behaving as expected. Unfortunately, you cannot currently do what
you want easily; there is no way to "unset" a multi-valued config
variable (like credential.helper) with a later one. Git will ask both
configured helpers for the password, and will store a successful result
in both.

The simplest way I can think of to work around it is to point your $HOME
elsewhere[1] during the integration test, so that it does not read your
regular ~/gitconfig.

-Peff

[1] Actually, that is what I would do on a Unix system. I have no idea
    how the home directory is determined on Windows.

Thanks for the quick reply.

Is there any documentation on which variables are muli-valued?

git -c credential.helper="store --file=creds" config --get credential.helper

only returns one value.

How can i even know if there are multiple set. I mean someone might
have just created an extra credential.helper in `--system` that I'm
not expecting...




On Fri, Feb 26, 2016 at 1:04 PM, Jeff King [off-list ref] wrote:

On Fri, Feb 26, 2016 at 12:17:49PM +0530, Guilherme wrote:

quoted

I'm trying to use git in an integration test and i'm having trouble
with configuration options.

On windows developer machines we use wincred as our credenital helper
and thus have it set in ~/.gitconfig

For the integration test that is no use as it will make testing
unauthorized logging in impossible.

Since there is no way of disabling configuration options on the
command line i tried setting it to store with a file I could delete.
So in front of every command we insert `-c credential.helper="store
--file=creds.txt"`. In the end the command line looks like:

git -c credential.helper="store --file=creds.txt" clone
http://admin:admin@oururl@20000/TestRepo.git

I see the file creds.txt being created containing only
http://admin:admin@oururl@20000/TestRepo.git but the credenital at the
same time appears in the windows credential store.

Can anybody else confirm this?

That's behaving as expected. Unfortunately, you cannot currently do what
you want easily; there is no way to "unset" a multi-valued config
variable (like credential.helper) with a later one. Git will ask both
configured helpers for the password, and will store a successful result
in both.

The simplest way I can think of to work around it is to point your $HOME
elsewhere[1] during the integration test, so that it does not read your
regular ~/gitconfig.

-Peff

[1] Actually, that is what I would do on a Unix system. I have no idea
    how the home directory is determined on Windows.

On Fri, Feb 26, 2016 at 01:16:39PM +0530, Guilherme wrote:

Is there any documentation on which variables are muli-valued?

There's no central registry. It's often mentioned in the documentation
for a particular config option, but it looks like the credential.*
config is not very clear about this.

There aren't very many of them. I think credential.* is one set. The
remote.*.fetch/push refspecs are another. I don't think there are any
others used by git itself, but I may just be forgetting them.

git -c credential.helper="store --file=creds" config --get credential.helper

only returns one value.

How can i even know if there are multiple set. I mean someone might
have just created an extra credential.helper in `--system` that I'm
not expecting...

Right. The "git-config" program doesn't know about the semantics of
particular values (remember that in the early days, there were many
porcelains which built on top of git, and they could all store their own
config). Using "--get" implements "last one wins" semantics, which
is what most config variables want. You can use "--get-all" to see all
instances of a multi-valued variable.

The usability on all of this is obviously pretty horrible, but it's hard
to change at this point without breaking backwards compatibility.

-Peff

On Thu, Feb 25, 2016 at 11:59 PM, Jeff King [off-list ref] wrote:

Right. The "git-config" program doesn't know about the semantics of
particular values (remember that in the early days, there were many
porcelains which built on top of git, and they could all store their own
config). Using "--get" implements "last one wins" semantics, which
is what most config variables want. You can use "--get-all" to see all
instances of a multi-valued variable.

And note that several libraries of hooks and git extensions store
configuration there as well, not just traditional porcelain. (Though
maybe that is considered porcelain? Not really sure on the term here).
I do this myself for several custom git hooks.

Thanks,
Jake

On Fri, Feb 26, 2016 at 12:15:46AM -0800, Jacob Keller wrote:

On Thu, Feb 25, 2016 at 11:59 PM, Jeff King [off-list ref] wrote:

quoted

Right. The "git-config" program doesn't know about the semantics of
particular values (remember that in the early days, there were many
porcelains which built on top of git, and they could all store their own
config). Using "--get" implements "last one wins" semantics, which
is what most config variables want. You can use "--get-all" to see all
instances of a multi-valued variable.

And note that several libraries of hooks and git extensions store
configuration there as well, not just traditional porcelain. (Though
maybe that is considered porcelain? Not really sure on the term here).
I do this myself for several custom git hooks.

Thanks, I meant to add "and it is unclear these days how many addons are
still using this feature".

I mentioned ugliness and backwards compatibility earlier.  I think
having a meaning-agnostic git-config command is still a reasonable thing
these days. But given how few multi-valued variables there are, it might
have been worth designing them differently, so that everything is
last-one-wins.

As an alternative, it would be nice to have some config syntax for
"clear the list". Maybe something like an empty string, which I think
has no meaning for the current multi-valued variables (at least not for
credential helpers or refspecs). That would allow something like:

  git -c credential.helper= clone ...

to do what you'd expect.

-Peff

On Fri, Feb 26, 2016 at 3:24 PM, Jeff King [off-list ref] wrote:

As an alternative, it would be nice to have some config syntax for
"clear the list". Maybe something like an empty string, which I think
has no meaning for the current multi-valued variables (at least not for
credential helpers or refspecs). That would allow something like:

  git -c credential.helper= clone ...

to do what you'd expect.

I've been thinking of -= instead. It's unambiguous. And you can use
wildcards on both sides. "credential.helper -= *" means delete that
key, "credential.* -= *" deletes all credential.* keys.
credential.helper -= abc only deletes it if the previous value is abc.
-- 
Duy

On Fri, Feb 26, 2016 at 03:34:53PM +0700, Duy Nguyen wrote:

On Fri, Feb 26, 2016 at 3:24 PM, Jeff King [off-list ref] wrote:

quoted

As an alternative, it would be nice to have some config syntax for
"clear the list". Maybe something like an empty string, which I think
has no meaning for the current multi-valued variables (at least not for
credential helpers or refspecs). That would allow something like:

  git -c credential.helper= clone ...

to do what you'd expect.

I've been thinking of -= instead. It's unambiguous. And you can use
wildcards on both sides. "credential.helper -= *" means delete that
key, "credential.* -= *" deletes all credential.* keys.
credential.helper -= abc only deletes it if the previous value is abc.

But there you're inventing new syntax, so you'd need to invent new
syntax inside the config file, too. And you'd need to somehow
communicate to the consumers of the config values that the value is
"unset". So for config callbacks inside of git, they need to take more
than just the key/value pair (or we'd have to read all of the config and
pre-process it). Ditto for git-config. How do we show in the output of
--get-all that the list was reset? Or again, we could pre-process
completely in git-config (which would probably mean using a new option,
--get-list or something, instead of --get-all).

By contrast, I think my suggestion can be implemented as:

diff --git a/credential.c b/credential.c
index 7d6501d..aa99666 100644
--- a/credential.c
+++ b/credential.c

@@ -63,9 +63,12 @@ static int credential_config_callback(const char *var, const char *value,
 		key = dot + 1;
 	}
 
-	if (!strcmp(key, "helper"))
-		string_list_append(&c->helpers, value);
-	else if (!strcmp(key, "username")) {
+	if (!strcmp(key, "helper")) {
+		if (*value)
+			string_list_append(&c->helpers, value);
+		else
+			string_list_clear(&c->helpers, 0);
+	} else if (!strcmp(key, "username")) {
 		if (!c->username)
 			c->username = xstrdup(value);
 	}

The big downside is that each consumer of the value needs to learn this
trick. But as I said, I think there aren't very many.

Don't get me wrong; I think your suggestion is a little cleaner. If we
were designing the config system from scratch, I'd probably favor a
single query-able tree rather than the callback system, and do things
like list-processing centrally. But given the history, I'm not sure if
it's worth it now.

-Peff