* git bundler service
From: Tomas Carnecky @ 2011-09-14 18:48 UTC (permalink / raw)
To: git
I do a fair amount of end-user support in our official IRC channel and
every now and then someone with a really slow or unreliable internet
connection stumbles over the fact that git can't resume clones. In the
past I would real quick clone the repo and make a bundle for them (if
the repo was public). Now I made a service out of it:
https://bundler.caurea.org/. The site is really simple: you enter a git
url and after a while you'll be able to download the bundle. Bundles are
cached for a week. Each page has instructions how to convert the bundle
into a git repository as you would get it via a regular git clone
(example: https://bundler.caurea.org/bundle/4e709b2ab3f4d18746000002).
In theory it should be possible use the service from the commandline,
for example to create a wrapper around git clone which does all the
things automagically. If somebody is interested in doing that, please
contact me and I'll send you the API documentation.
tom
^ permalink raw reply
* Re: [PATCH 3/7] refactor argv_array into generic code
From: Junio C Hamano @ 2011-09-14 18:51 UTC (permalink / raw)
To: Jeff King; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <7v4o0fufsn.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> The submodule code recently grew generic code to build a
>> dynamic argv array.
>
> That has not yet happened as far as I am concerned X-<.
>
> I do not want the clean-up to depend on the uncooked submodule code and
> that was why I said this is not urgent.
>
> Will think about what to do next. Eh, rather, will backburner thinking
> about it for now ;-).
Ahh, I thought this "submodule" was about enhancing the on-demand stuff,
but you meant your fix to the "other half". I should have looked before
responding; sorry.
Will queue directly on top of that fix and all will be well.
Thanks.
^ permalink raw reply
* Re: [PATCH 7/7] run_hook: use argv_array API
From: Junio C Hamano @ 2011-09-14 18:54 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215824.GG24490@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> @@ -609,26 +610,23 @@ int finish_async(struct async *async)
> int run_hook(const char *index_file, const char *name, ...)
> {
> struct child_process hook;
> - const char **argv = NULL, *env[2];
> + struct argv_array argv = ARGV_ARRAY_INIT;
> + const char *p, *env[2];
Given that in argv-array.h you define it as
#define ARGV_ARRAY_INIT { empty_argv, 0, 0 };
the above will introduce decl-after-stmt.
^ permalink raw reply
* Re: [PATCH] git-p4: import utf16 file properly
From: Luke Diamand @ 2011-09-14 18:56 UTC (permalink / raw)
To: Chris Li; +Cc: git, Pete Wyckoff, Junio C Hamano
In-Reply-To: <CANeU7QnW5kSni0W9M9q-FTWv4p_qc67LG3mA6BQj_U-wxNuZeQ@mail.gmail.com>
On 14/09/11 19:29, Chris Li wrote:
> On Wed, Sep 14, 2011 at 12:55 AM, Luke Diamand<luke@diamand.org> wrote:
>> On 13/09/11 22:33, Chris Li wrote:
>>> The fix is simple, just ask perforce to print the depot
>>> file into a real file. This way perforce will not performe
>>> the utf16 to utf8 conversion. Git can import the exact same
>>> file as perforce checkout.
>>
>> Does this change do the right thing with RCS keywords in UTF16 files?
>
> I don't know what is the rules about the RCS keyword in UTF16 files.
> I look at the current git-p4, it does not do any keyword replacement in
> utf16 files. So this patch did not change that. It should be a separate issue.
>
> The way I see it, this patch is a straight enhancement compare to the
> current git-p4 because the current git-p4 *corrupts* the utf16 files.
>
>>
>> If p4CmdList() fails, e.g. due to running out of diskspace, will this just
>> happily import a truncated/corrupt file?
>
> Good point. I add the error check and attach the new patch.
>
>> (And I could be wrong about this, but does you patch have newline damage? It
>> didn't seem to apply for me).
Looks good to me. I think you're right about the RCS keywords not being
relevant here.
>
> Gmail dmage the white space. I should always use the attachment.
> Does the attached patch work for you?
>
> Thanks
>
> Chris
^ permalink raw reply
* Re: [PATCH 7/7] run_hook: use argv_array API
From: Jeff King @ 2011-09-14 18:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <7vty8ft0oc.fsf@alter.siamese.dyndns.org>
On Wed, Sep 14, 2011 at 11:54:11AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > @@ -609,26 +610,23 @@ int finish_async(struct async *async)
> > int run_hook(const char *index_file, const char *name, ...)
> > {
> > struct child_process hook;
> > - const char **argv = NULL, *env[2];
> > + struct argv_array argv = ARGV_ARRAY_INIT;
> > + const char *p, *env[2];
>
> Given that in argv-array.h you define it as
>
> #define ARGV_ARRAY_INIT { empty_argv, 0, 0 };
>
> the above will introduce decl-after-stmt.
Oops. Can you squash in removing the semicolon from the macro
definition?
-Peff
^ permalink raw reply
* Re: [nit] diff func headers ignore context
From: Junio C Hamano @ 2011-09-14 19:01 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20110913220421.GA24549@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Sep 13, 2011 at 05:58:25PM -0400, Jeff King wrote:
>
>> @@ -609,26 +610,23 @@ int finish_async(struct async *async)
>> int run_hook(const char *index_file, const char *name, ...)
>> {
>> struct child_process hook;
>> - const char **argv = NULL, *env[2];
>> + struct argv_array argv = ARGV_ARRAY_INIT;
>
> I find this diff function header pretty confusing. Of course we're not
> in finish_async, as you can see by the fact that the context contains
> the start of run_hook.
>
> I don't think this is something that can be solved with xfuncname
> config; we would have to teach xdiff to look at context lines when
> picking a header line.
>
> Am I the only one who finds this confusing? Can anyone think of a reason
> to keep showing finish_async in this example?
Would this be sufficient? Instead of looking for the first line that
matches the "beginning" pattern going backwards starting from one line
before the displayed context, we start our examination at the first line
shown in the context.
xdiff/xemit.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 277e2ee..5f9c0e0 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
if (xecfg->flags & XDL_EMIT_FUNCNAMES) {
long l;
- for (l = s1 - 1; l >= 0 && l > funclineprev; l--) {
+ for (l = s1; l >= 0 && l > funclineprev; l--) {
const char *rec;
long reclen = xdl_get_rec(&xe->xdf1, l, &rec);
long newfunclen = ff(rec, reclen, funcbuf,
^ permalink raw reply related
* Re: [PATCH 1/3] make-static: master
From: Junio C Hamano @ 2011-09-14 19:03 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Ramsay Jones, GIT Mailing-list
In-Reply-To: <4E704EA2.8060307@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Am 9/14/2011 1:46, schrieb Junio C Hamano:
>> This kind of breakage report was exactly what I was looking for by
>> merging it early to 'next'. Hopefully no other (function / platform) combo
>> has such dependencies...
>
> There is! prepare_git_cmd is used from the Windows section in run-command.c.
> Therefore, the following hunks of the patch should be reverted.
Sorry, I do not think it should just be "reverted".
Instead, a new comment should be added to explain why it needs to be
exposed, which was the _whole_ point of the message you are replying to,
in which I showed how it should be done by an example.
^ permalink raw reply
* Re: git bundler service
From: Junio C Hamano @ 2011-09-14 19:13 UTC (permalink / raw)
To: Tomas Carnecky; +Cc: git
In-Reply-To: <4E70F6E8.80408@dbservice.com>
Tomas Carnecky <tom@dbservice.com> writes:
> I do a fair amount of end-user support in our official IRC channel and
> every now and then someone with a really slow or unreliable internet
> connection stumbles over the fact that git can't resume clones. In the
> past I would real quick clone the repo and make a bundle for them (if
> the repo was public). Now I made a service out of it:
> https://bundler.caurea.org/. The site is really simple: you enter a
> git url and after a while you'll be able to download the
> bundle.
Interesting, as I was talking with others on a related solution to a
similar issue of priming the well for large clones.
The basic idea is to allow a site automatically to redirect fresh-clone
clients to a URL to HTTP reachable mirror network, and force them to grab
a bundle, extract it and then re-contact the server for only incremental
updates relative to the bundle (and of course that exchange would happen
inside the updated client without end-user intervention).
^ permalink raw reply
* [PATCH 0/5] credential helper super fun pak
From: Jeff King @ 2011-09-14 19:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Brian Gernhardt
Here's a mixed bag of bugfix commits to go on top of what's in
jk/http-auth-keyring.
[1/5]: docs: minor tweaks to credentials API
Clarifications in response to comments from Junio on the original
series.
[2/5]: credential-cache: fix expiration calculation corner cases
This fixes the failures in t0300 on OS X.
[3/5]: t0300: make askpass tests a little more robust
[4/5]: t0300: make alternate username tests more robust
These two just make the tests more rigorous so that patch 5/5
can actually demonstrate the failure it's fixing.
[5/5]: credential-store: use a better storage format
This makes multiple usernames on the same host work with
credential-store (they already work with credential-cache).
-Peff
^ permalink raw reply
* [PATCH 1/5] docs: minor tweaks to credentials API
From: Jeff King @ 2011-09-14 19:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
These are in response to Junio's comments:
1. Clarify how credential rejection works (first we call
helpers, then we clear fields, so helpers get fields).
2. Clarify quoting of keys/values returned from helpers.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/technical/api-credentials.txt | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index 335a007..b2eb24c 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -48,10 +48,11 @@ Functions
`credential_reject`::
Inform the credential subsystem that the provided credentials
- have been rejected. This will clear the username and password
- fields in `struct credential`, as well as notify any helpers of
- the rejection (which may, for example, purge the invalid
- credentials from storage).
+ have been rejected. This will notify any helpers of the
+ rejection (which allows them to, for example, purge the invalid
+ credentials from storage), and then clear the username and
+ password fields in `struct credential`. It can then be
+ `credential_fill`-ed again.
`credential_getpass`::
@@ -98,8 +99,11 @@ command line:
The helper should produce a list of items on stdout, each followed by a
newline character. Each item should consist of a key-value pair, separated
-by an `=` (equals) sign. The value may contain any bytes except a
-newline. When reading the response, git understands the following keys:
+by an `=` (equals) sign. The key can contain any bytes except `=` or
+newline. The value may contain any bytes except a newline. In both
+cases, all bytes are treated as-is (i.e., there is no quoting, and one
+cannot transmit a value with newline in it). When reading the response,
+git understands the following keys:
`username`::
--
1.7.6.252.ge9c18
^ permalink raw reply related
* [PATCH 2/5] credential-cache: fix expiration calculation corner cases
From: Jeff King @ 2011-09-14 19:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
The main credential-cache daemon loop calls poll to wait for
a client or to trigger the expiration of credentials. When
the last credential we hold expires, we exit.
However, there is a corner case: when we first start up, we
have no credentials, and are waiting for a client to
provide us with one. In this case, we ended up handing
complete junk for the timeout argument to poll(). On some
systems, this caused us to just wait a long time for the
client (which usually showed up within a second or so). On
OS X, however, the system quite reasonably complained about
our junk value with EINVAL.
Fixing this is pretty straightforward; we just notice that
we have no entries to compare against. However, that bug was
covering up another one: our expiration calculation didn't
give clients a chance to actually connect and provide us
with a credential before we decided that we should exit
because we weren't holding any credentials!
The new algorithm is:
1. Sleep until it's time to expire the most recent
credential.
2. If we don't have any credentials yet, wait 30 seconds
for a client to contact us and give us one.
3. After expiring the last credential, wait 30 seconds for
a client to provide us with one.
Technically only parts (1) and (2) are needed to implement
the original intended behavior.
But (3) is a minor optimization that is made easy by the new
code. When a client gives us a credential, then removes it
(e.g., because it had a bogus password), and then gives us
another one, we used to exit, forcing the client to start a
new daemon instance. Instead, we can just reuse the existing
daemon instance.
Signed-off-by: Jeff King <peff@peff.net>
---
credential-cache--daemon.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index f520347..d6769b1 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -57,20 +57,33 @@ static void remove_credential(const struct credential *c)
static int check_expirations(void)
{
+ static unsigned long wait_for_entry_until;
int i = 0;
unsigned long now = time(NULL);
unsigned long next = (unsigned long)-1;
+ /*
+ * Initially give the client 30 seconds to actually contact us
+ * and store a credential before we decide there's no point in
+ * keeping the daemon around.
+ */
+ if (!wait_for_entry_until)
+ wait_for_entry_until = now + 30;
+
while (i < entries_nr) {
if (entries[i].expiration <= now) {
entries_nr--;
- if (!entries_nr)
- return 0;
free(entries[i].item.description);
free(entries[i].item.unique);
free(entries[i].item.username);
free(entries[i].item.password);
memcpy(&entries[i], &entries[entries_nr], sizeof(*entries));
+ /*
+ * Stick around 30 seconds in case a new credential
+ * shows up (e.g., because we just removed a failed
+ * one, and we will soon get the correct one).
+ */
+ wait_for_entry_until = now + 30;
}
else {
if (entries[i].expiration < next)
@@ -79,6 +92,12 @@ static int check_expirations(void)
}
}
+ if (!entries_nr) {
+ if (wait_for_entry_until <= now)
+ return 0;
+ next = wait_for_entry_until;
+ }
+
return next - now;
}
--
1.7.6.252.ge9c18
^ permalink raw reply related
* [PATCH 3/5] t0300: make askpass tests a little more robust
From: Jeff King @ 2011-09-14 19:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
Our fake askpass always just returned "askpass-result";
let's actually have it return a different fake value for
usernames and passwords, to be sure values are getting
routed correctly.
All tests should still pass.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0300-credentials.sh | 77 ++++++++++++++++++++++++-----------------------
1 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 5d54976..09d5b15 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -52,7 +52,8 @@ test_expect_success 'setup helper scripts' '
cat >askpass <<-\EOF &&
#!/bin/sh
echo >&2 askpass: $*
- echo askpass-result
+ what=`echo $1 | tr A-Z a-z | tr -cd a-z`
+ echo "askpass-$what"
EOF
chmod +x askpass &&
GIT_ASKPASS=askpass &&
@@ -155,8 +156,8 @@ test_expect_success 'do not bother completing already-full credential' '
# askpass helper is run, we know the internal getpass is working.
test_expect_success 'empty methods falls back to internal getpass' '
check <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
@@ -166,7 +167,7 @@ test_expect_success 'empty methods falls back to internal getpass' '
test_expect_success 'internal getpass does not ask for known username' '
check --username=foo <<-\EOF
username=foo
- password=askpass-result
+ password=askpass-password
--
askpass: Password:
EOF
@@ -176,7 +177,7 @@ test_expect_success 'internal getpass can pull from config' '
git config credential.foo.username configured-username
check --unique=foo <<-\EOF
username=configured-username
- password=askpass-result
+ password=askpass-password
--
askpass: Password:
EOF
@@ -185,15 +186,15 @@ test_expect_success 'internal getpass can pull from config' '
test_expect_success 'credential-cache caches password' '
test_when_finished "git credential-cache --exit" &&
check --unique=host cache <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
check --unique=host cache <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
EOF
'
@@ -201,15 +202,15 @@ test_expect_success 'credential-cache caches password' '
test_expect_success 'credential-cache requires matching unique token' '
test_when_finished "git credential-cache --exit" &&
check --unique=host cache <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
check --unique=host2 cache <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
@@ -219,15 +220,15 @@ test_expect_success 'credential-cache requires matching unique token' '
test_expect_success 'credential-cache requires matching usernames' '
test_when_finished "git credential-cache --exit" &&
check --unique=host cache <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
check --unique=host --username=other cache <<-\EOF
username=other
- password=askpass-result
+ password=askpass-password
--
askpass: Password:
EOF
@@ -236,16 +237,16 @@ test_expect_success 'credential-cache requires matching usernames' '
test_expect_success 'credential-cache times out' '
test_when_finished "git credential-cache --exit || true" &&
check --unique=host "cache --timeout=1" <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
sleep 2 &&
check --unique=host cache <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
@@ -255,18 +256,18 @@ test_expect_success 'credential-cache times out' '
test_expect_success 'credential-cache removes rejected credentials' '
test_when_finished "git credential-cache --exit || true" &&
check --unique=host cache <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
- check --reject --unique=host --username=askpass-result cache <<-\EOF &&
+ check --reject --unique=host --username=askpass-username cache <<-\EOF &&
--
EOF
check --unique=host cache <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
@@ -276,15 +277,15 @@ test_expect_success 'credential-cache removes rejected credentials' '
test_expect_success 'credential-store stores password' '
test_when_finished "rm -f .git-credentials" &&
check --unique=host store <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
check --unique=host store <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
EOF
'
@@ -292,15 +293,15 @@ test_expect_success 'credential-store stores password' '
test_expect_success 'credential-store requires matching unique token' '
test_when_finished "rm -f .git-credentials" &&
check --unique=host store <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
check --unique=host2 store <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
@@ -310,18 +311,18 @@ test_expect_success 'credential-store requires matching unique token' '
test_expect_success 'credential-store removes rejected credentials' '
test_when_finished "rm -f .git-credentials" &&
check --unique=host store <<-\EOF &&
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
EOF
- check --reject --unique=host --username=askpass-result store <<-\EOF &&
+ check --reject --unique=host --username=askpass-username store <<-\EOF &&
--
EOF
check --unique=host store <<-\EOF
- username=askpass-result
- password=askpass-result
+ username=askpass-username
+ password=askpass-password
--
askpass: Username:
askpass: Password:
--
1.7.6.252.ge9c18
^ permalink raw reply related
* [PATCH 4/5] t0300: make alternate username tests more robust
From: Jeff King @ 2011-09-14 19:18 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
One of the tests is to make sure that if we have a cached
credential "user/pass", that we ignore it when asking for
"--username=other", and instead get a new password. But
since askpass returns only a single fake value, we couldn't
differentiate between using the cached password and the new
one.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0300-credentials.sh | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 09d5b15..41ab8cc 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -53,7 +53,11 @@ test_expect_success 'setup helper scripts' '
#!/bin/sh
echo >&2 askpass: $*
what=`echo $1 | tr A-Z a-z | tr -cd a-z`
- echo "askpass-$what"
+ if test -f "askpass-$what"; then
+ cat "askpass-$what"
+ else
+ echo "askpass-$what"
+ fi
EOF
chmod +x askpass &&
GIT_ASKPASS=askpass &&
@@ -226,9 +230,11 @@ test_expect_success 'credential-cache requires matching usernames' '
askpass: Username:
askpass: Password:
EOF
+ test_when_finished "rm -f askpass-password" &&
+ echo other-password >askpass-password &&
check --unique=host --username=other cache <<-\EOF
username=other
- password=askpass-password
+ password=other-password
--
askpass: Password:
EOF
--
1.7.6.252.ge9c18
^ permalink raw reply related
* [PATCH 5/5] credential-store: use a better storage format
From: Jeff King @ 2011-09-14 19:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
In the name of simplicity, the credential-store helper
re-used git's config file format for on-disk storage of
credentials. The scheme looked something like this:
[credential "unique-context"]
username = foo
password = bar
This works fine if you have only one credential for each
context. But there's no good way to store separate passwords
for user1@unique-context versus user2@unique-context. For
end users, this meant that:
1. Trying to access user2@host when you already have a
cached credential for user1@host would return user2's
username with user1's password.
2. The stored result would also mix the credentials.
3. Rejection would remove all entries for the host,
regardless of username.
You can hack around it by assuming that order is important,
and that:
[credential "unique-context"]
username = user1
password = pass1
username = user2
password = pass2
refers to two distinct credentials. But the config code
isn't well setup for that. You have to keep track of the
last username field you parsed, and assume the next password
field after it is associated with it. Which isn't too bad.
But removing just one credential (which we need to do for
--reject) is impossible. You can say "delete the username
field that is user2", but there is no way to say "delete
the password field that comes right after the username field
that is user2" (it's tempting to find the password for user2
and say "delete the password field that is pass2", but
that's not strictly correct either).
Instead, let's define a new, very simple storage format.
Each line is a credential containing three tokens: unique,
username, and password. Each is separated by a space, and
shell-quoted to avoid ambiguity.
There's now an additional test that we can differentiate the credentials
correctly (matching the similar test we already have for
credential-cache). The test for removing credentials is also enhanced to
check that removing one credential leaves an unrelated one stored.
Signed-off-by: Jeff King <peff@peff.net>
---
credential-store.c | 119 ++++++++++++++++++++++++++++++++++++++++++------
t/t0300-credentials.sh | 38 ++++++++++++++-
2 files changed, 140 insertions(+), 17 deletions(-)
diff --git a/credential-store.c b/credential-store.c
index 8ab8582..aae8e0c 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -2,40 +2,129 @@
#include "credential.h"
#include "string-list.h"
#include "parse-options.h"
+#include "quote.h"
+
+static struct lock_file credential_lock;
+
+static int parse_credential_file(const char *fn,
+ struct credential *c,
+ int (*match_cb)(const char *username,
+ const char *password,
+ struct credential *c),
+ int (*other_cb)(const char *buf))
+{
+ FILE *fh;
+ struct strbuf buf = STRBUF_INIT;
+ const char **argv = NULL;
+ int alloc = 0;
+ int retval = 0;
+
+ fh = fopen(fn, "r");
+ if (!fh)
+ return errno == ENOENT ? 0 : -1;
+
+ while (strbuf_getwholeline(&buf, fh, '\n') != EOF) {
+ int nr = 0;
+ char *pristine = xstrdup(buf.buf);
+
+ strbuf_trim(&buf);
+ if (!sq_dequote_to_argv(buf.buf, &argv, &nr, &alloc) &&
+ nr == 3 &&
+ !strcmp(c->unique, argv[0]) &&
+ (!c->username || !strcmp(c->username, argv[1]))) {
+ if (match_cb(argv[1], argv[2], c) < 0) {
+ retval = -1;
+ break;
+ }
+ }
+ else if (other_cb) {
+ if (other_cb(pristine) < 0) {
+ retval = -1;
+ break;
+ }
+ }
+ free(pristine);
+ }
+
+ free(argv);
+ strbuf_release(&buf);
+ fclose(fh);
+ return retval;
+}
+
+
+static int copy_credential(const char *username, const char *password,
+ struct credential *c)
+{
+ if (!c->username)
+ c->username = xstrdup(username);
+ free(c->password);
+ c->password = xstrdup(password);
+ return 0;
+}
static int lookup_credential(const char *fn, struct credential *c)
{
- config_exclusive_filename = fn;
- credential_from_config(c);
+ if (!c->unique)
+ return 0;
+ parse_credential_file(fn, c, copy_credential, NULL);
return c->username && c->password;
}
-static void store_item(const char *fn, const char *unique,
- const char *item, const char *value)
+static int skip_match(const char *username, const char *password,
+ struct credential *c)
{
- struct strbuf key = STRBUF_INIT;
+ return 0;
+}
- if (!unique)
- return;
+static int print_entry(const char *buf)
+{
+ return write_in_full(credential_lock.fd, buf, strlen(buf));
+}
- config_exclusive_filename = fn;
+static int rewrite_credential_file(const char *fn, struct credential *c,
+ int replace)
+{
umask(077);
+ if (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)
+ return -1;
+ if (parse_credential_file(fn, c, skip_match, print_entry) < 0) {
+ rollback_lock_file(&credential_lock);
+ return -1;
+ }
+ if (replace) {
+ struct strbuf buf = STRBUF_INIT;
+ int r;
- strbuf_addf(&key, "credential.%s.%s", unique, item);
- git_config_set(key.buf, value);
- strbuf_release(&key);
+ sq_quote_buf(&buf, c->unique);
+ strbuf_addch(&buf, ' ');
+ sq_quote_buf(&buf, c->username);
+ strbuf_addch(&buf, ' ');
+ sq_quote_buf(&buf, c->password);
+ strbuf_addch(&buf, '\n');
+
+ r = write_in_full(credential_lock.fd, buf.buf, buf.len);
+ strbuf_release(&buf);
+ if (r < 0) {
+ rollback_lock_file(&credential_lock);
+ return -1;
+ }
+ }
+ return commit_lock_file(&credential_lock);
}
static void store_credential(const char *fn, struct credential *c)
{
- store_item(fn, c->unique, "username", c->username);
- store_item(fn, c->unique, "password", c->password);
+ if (!c->unique || !c->username || !c->password)
+ return;
+ rewrite_credential_file(fn, c, 1);
}
static void remove_credential(const char *fn, struct credential *c)
{
- store_item(fn, c->unique, "username", NULL);
- store_item(fn, c->unique, "password", NULL);
+ if (!c->unique)
+ return;
+ rewrite_credential_file(fn, c, 0);
}
int main(int argc, const char **argv)
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 41ab8cc..613c3fb 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -314,6 +314,30 @@ test_expect_success 'credential-store requires matching unique token' '
EOF
'
+test_expect_success 'credential-store requires matching usernames' '
+ test_when_finished "rm -f .git-credentials" &&
+ check --unique=host store <<-\EOF &&
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username:
+ askpass: Password:
+ EOF
+ test_when_finished "rm -f askpass-password" &&
+ echo other-password >askpass-password &&
+ check --unique=host --username=other store <<-\EOF &&
+ username=other
+ password=other-password
+ --
+ askpass: Password:
+ EOF
+ check --unique=host --username=askpass-username store <<-\EOF
+ username=askpass-username
+ password=askpass-password
+ --
+ EOF
+'
+
test_expect_success 'credential-store removes rejected credentials' '
test_when_finished "rm -f .git-credentials" &&
check --unique=host store <<-\EOF &&
@@ -323,16 +347,26 @@ test_expect_success 'credential-store removes rejected credentials' '
askpass: Username:
askpass: Password:
EOF
+ check --unique=host --username=other store <<-\EOF &&
+ username=other
+ password=askpass-password
+ --
+ askpass: Password:
+ EOF
check --reject --unique=host --username=askpass-username store <<-\EOF &&
--
EOF
- check --unique=host store <<-\EOF
+ check --unique=host --username=askpass-username store <<-\EOF &&
username=askpass-username
password=askpass-password
--
- askpass: Username:
askpass: Password:
EOF
+ check --unique=host --username=other store <<-\EOF
+ username=other
+ password=askpass-password
+ --
+ EOF
'
test_done
--
1.7.6.252.ge9c18
^ permalink raw reply related
* Re: [Survey] Signed push
From: Andy Lutomirski @ 2011-09-14 19:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git, linux-kernel
In-Reply-To: <7vaaa8xufi.fsf@alter.siamese.dyndns.org>
On 09/13/2011 09:45 AM, Junio C Hamano wrote:
>
> An alternative that I am considering is to let the requester say this
> instead:
>
> are available in the git repository at:
> git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>
> without adding the extra line.
>
> That is, to allow fetching the history up to an explicitly named commit
> object. This would only involve a change to fetch-pack at the receiving
> end; just match the commit object name given from the command line against
> the ls-remote response and ask upload-pack to give the history leading to
> it. The released versions of Git already will happily oblige, as long as
> the commit object named in the request message still sits at the tip of
> the intended branch.
I would love this feature on the pull/fetch interface, but for a
completely different reason. Sometimes I want to pull a particular
object (usually a commit, but sometimes just a tree or blob) from
*myself*, and having to stick it on a branch is annoying.
One use-case is when applying a patch in git's extended format. If I
know where it came from, I ought to be able to pull the blobs it depends
on to enable three-way merge. I think that this is essentially
impossible remotely right now.
Of course, merging with the result of the pull will result in terrible
automatically-generated messages, but it's easy to fix that up manually.
This is one thing that I think Mercurial handles better than git. (And
apologies for the noise if I've missed a way to do this with current
git. I've looked, but maybe I missed some magic way to do this.)
--Andy
^ permalink raw reply
* Re: [PATCH 7/7] run_hook: use argv_array API
From: Junio C Hamano @ 2011-09-14 20:01 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Christian Couder, Jens Lehmann, git
In-Reply-To: <20110914185658.GA21352@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Sep 14, 2011 at 11:54:11AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > @@ -609,26 +610,23 @@ int finish_async(struct async *async)
>> > int run_hook(const char *index_file, const char *name, ...)
>> > {
>> > struct child_process hook;
>> > - const char **argv = NULL, *env[2];
>> > + struct argv_array argv = ARGV_ARRAY_INIT;
>> > + const char *p, *env[2];
>>
>> Given that in argv-array.h you define it as
>>
>> #define ARGV_ARRAY_INIT { empty_argv, 0, 0 };
>>
>> the above will introduce decl-after-stmt.
>
> Oops. Can you squash in removing the semicolon from the macro
> definition?
Yes, I did. Thanks.
^ permalink raw reply
* Re: [PATCH 1/3] make-static: master
From: Junio C Hamano @ 2011-09-14 20:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ramsay Jones, GIT Mailing-list
In-Reply-To: <7vlitrt08n.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> Am 9/14/2011 1:46, schrieb Junio C Hamano:
>>> This kind of breakage report was exactly what I was looking for by
>>> merging it early to 'next'. Hopefully no other (function / platform) combo
>>> has such dependencies...
>>
>> There is! prepare_git_cmd is used from the Windows section in run-command.c.
>> Therefore, the following hunks of the patch should be reverted.
Thanks; this fix-up will be queued on top.
-- >8 --
Subject: [PATCH] exec_cmd.c: prepare_git_cmd() is sometimes used
Add warning to prevent people from making the same mistake.
Noticed by Johannes Sixt.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
exec_cmd.c | 7 ++++++-
exec_cmd.h | 1 +
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 1dddbf4..9c784db 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -113,7 +113,12 @@ void setup_path(void)
strbuf_release(&new_path);
}
-static const char **prepare_git_cmd(const char **argv)
+/*
+ * This symbol may be unreferenced from outside this file in some
+ * builds, but run-command.c on Windows does use it. Do not make it
+ * static without checking!
+ */
+const char **prepare_git_cmd(const char **argv)
{
int argc;
const char **nargv;
diff --git a/exec_cmd.h b/exec_cmd.h
index f5d2cdd..e2b546b 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -5,6 +5,7 @@ extern void git_set_argv_exec_path(const char *exec_path);
extern const char *git_extract_argv0_path(const char *path);
extern const char *git_exec_path(void);
extern void setup_path(void);
+extern const char **prepare_git_cmd(const char **argv);
extern int execv_git_cmd(const char **argv); /* NULL terminated */
extern int execl_git_cmd(const char *cmd, ...);
extern const char *system_path(const char *path);
--
1.7.7.rc1.1.g1e5814
^ permalink raw reply related
* Re: [Survey] Signed push
From: Junio C Hamano @ 2011-09-14 20:40 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: Linus Torvalds, git
In-Reply-To: <4E7101F3.1090204@mit.edu>
Andy Lutomirski <luto@MIT.EDU> writes:
>> An alternative that I am considering is to let the requester say this
>> instead:
>>
>> are available in the git repository at:
>> git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>>
>> without adding the extra line.
>>
>> That is, to allow fetching the history up to an explicitly named commit
>> object. This would only involve a change to fetch-pack at the receiving
>> end; just match the commit object name given from the command line against
>> the ls-remote response and ask upload-pack to give the history leading to
>> it....
>
> I would love this feature on the pull/fetch interface, but for a
> completely different reason. Sometimes I want to pull a particular
> object (usually a commit, but sometimes just a tree or blob) from
> *myself*, and having to stick it on a branch is annoying.
I am afraind that it is not going to happen; see
http://article.gmane.org/gmane.comp.version-control.git/181317
for a rationale.
^ permalink raw reply
* Re: [Survey] Signed push
From: Andrew Lutomirski @ 2011-09-14 20:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vwrdasvr7.fsf@alter.siamese.dyndns.org>
On Wed, Sep 14, 2011 at 1:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Andy Lutomirski <luto@MIT.EDU> writes:
>
>>> An alternative that I am considering is to let the requester say this
>>> instead:
>>>
>>> are available in the git repository at:
>>> git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>>>
>>> without adding the extra line.
>>>
>>> That is, to allow fetching the history up to an explicitly named commit
>>> object. This would only involve a change to fetch-pack at the receiving
>>> end; just match the commit object name given from the command line against
>>> the ls-remote response and ask upload-pack to give the history leading to
>>> it....
>>
>> I would love this feature on the pull/fetch interface, but for a
>> completely different reason. Sometimes I want to pull a particular
>> object (usually a commit, but sometimes just a tree or blob) from
>> *myself*, and having to stick it on a branch is annoying.
>
> I am afraind that it is not going to happen; see
>
> http://article.gmane.org/gmane.comp.version-control.git/181317
>
> for a rationale.
>
Do you mean that it's a security feature? What if a .git/config
option existed to allow this use? Or even a git upload-pack option
that turned it *on* and was stripped by git-shell?
--Andy
^ permalink raw reply
* Re: Fwd: [Survey] Signed push
From: Sam Vilain @ 2011-09-14 20:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vobynui8a.fsf@alter.siamese.dyndns.org>
[re-send as text-only. sorry, new system]
On 9/14/11 10:49 AM, Junio C Hamano wrote:
>> The extra line in the pull request is cheap - it's not like we need to
>> > ration them. The above format, in contrast, requires that the person
>> > doing the*pull* have a recent enough git client, otherwise the merge
>> > commit message will be just horrible.
> In a re-roll patch I've added ";# branch-name" at the end of that line for
> people with older git, but existing git wouldn't allow you to fetch anything
> but refs so you won't risk getting "just horrible" merge message;-)
>
If the system is watertight enough, then you could have verified pushes
ONLY under some new refspace, like:
refs/forks/forkid/branch
"forkid" is set up when you choose to "follow" someone's pushes. ie,
you say something like:
git fork follow linustorvalds@osdl.org
<http://pgp.mit.edu:11371/pks/lookup?op=vindex&search=0x17762C4676E21CBB>
"torvalds@osdl.org" here representing a PGP key ID search string. It
could be instead, "0x17762c4676e21cbb"
And then the "refs/forks/linus/xxx" space gets populated as signed
pushes are seen on the network (perhaps when fetching from a pull
request, or perhaps via a service). A configuration option can be set
to warn if you are not merging a signed branch. If the new "pull
request/push" object is a distinct object type or tag object, then it
could be the destination of the 'refs/forks/...' ref, and the branch
desription and hence the default merge message be retrieved from that.
Sam
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2011, #04; Mon, 12)
From: Junio C Hamano @ 2011-09-14 20:57 UTC (permalink / raw)
To: Jonathon Mah; +Cc: Dan McGee, David Aguilar, git
In-Reply-To: <D3CA81F2-647B-4AD0-A4FC-4C22772FD791@JonathonMah.com>
Jonathon Mah <me@JonathonMah.com> writes:
> On 2011-09-12, at 14:59, Junio C Hamano wrote:
>
>>> [Stalled]
>>>
>>> * jm/mergetool-pathspec (2011-06-22) 2 commits
>>> - mergetool: Don't assume paths are unmerged
>>> - mergetool: Add tests for filename with whitespace
>>
>> What's the plan for this series? Do we still want to pursue it within the
>> timeframe for the next round?
>>
>> Is there any mergetool/difftool expert who volunteers to help moving this
>> topic forward?
>
> I'd love this to stay alive. As I've mentioned before, my relationship
> with shell is tenuous. My biggest problem is...
Hopefully volunteers can help moving this forward with "coding".
http://thread.gmane.org/gmane.comp.version-control.git/176215
http://thread.gmane.org/gmane.comp.version-control.git/176216
There were issues with the patches that were _not_ about the coding but
about the documentation and design.
Topics with a stale version in 'pu' that are stalled are not even alive--
they are zombies that need to be wiped at some point, or replaced with
updated series, whichever comes first.
^ permalink raw reply
* Re: [Survey] Signed push
From: Jonathan Nieder @ 2011-09-14 21:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CACsJy8Dwu2U-7eEZU-VYmcrA7JwtvUkJS5SywXjZWoE1twchhQ@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> On Wed, Sep 14, 2011 at 2:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> An alternative that I am considering is to let the requester say this
>> instead:
>>
>> are available in the git repository at:
>> git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
[...]
> Stupid question, if we agree to go with signed push, can we also sign
> pull requests and verify them when we pull? I suppose most of the
> time, pulling can be done automatically by extracting pull url from
> the request. This would make pull/push both signed.
>
> BTW, there's a third way (rsync is obsolete) to carry changes away in
> human-unreadable way: bundles. Should we also sign the bundles too (I
> guess we could just do the same as in signed push).
If I understand you correctly, then ordinary PGP email signing[1]
should work for that already. In your first example, the receiver can
make sure whatever process grabs a pull request verifies it, and in
the second example, the receiver checks the signature on her email
before saving a bundle and passing it to "git fetch".
[1] http://www.phildev.net/pgp/gpgmua.html
^ permalink raw reply
* Re: Helping on Git development
From: Jonathan Nieder @ 2011-09-14 21:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Ardill, Eduardo D'Avila, git, git
In-Reply-To: <7vehzjugdz.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> As to contributing to the project, right now, I think we have enough
> people who want to write code and documentation for Git, but what we lack
> are bandwidth to (this is not meant to be an exhaustive list):
[...]
> - distilling random wishes from the end user community while winnowing
> chaffs that are unrealistic or do not fit well with the grand scheme of
> things, to come up with a concrete proposal and a patch series
[...]
> - dig list archives to point people at age-old discussions to non-issues
> that have long been resolved to squelch noise; and
>
> - remind original submitter, people who were involved in the discussion,
> and people who should have been involved but who weren't, of a worthy
> but stalled topics from time to time.
I also should (reluctantly) mention that the Debian bug tracker has
been accepting bugs from outsiders and provides a service like this.
Caveats:
- it only tracks bugs that affect Debian (usually meaning
platform-independent bugs). Occasionally bugs from Windows users
have been reported there and it's been okay.
- the interface might seem quirky if you're not used to it.
Documentation is at http://www.debian.org/Bugs/
- no guarantee of a quick response. When there is a response,
usually it is "here are some thoughts; now let's take this to
git@vger.kernel.org".
- if the bugtracker gets swamped with reports from outside without
manpower to match, the policy re bugs from outside Debian might
change.
Bug listing: [1].
To subscribe to receive bug reports by email: [2].
Thoughts welcome, as always.
Jonathan
[1] http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=git;include=tags:upstream;exclude=tags:fixed-upstream;exclude=tags:moreinfo;exclude=severity:wishlist
[2] http://www.debian.org/doc/manuals/developers-reference/resources.html#pts-commands
Summary: an email to pts@qa.debian.org whose body contains the two
lines "subscribe git", "keyword git = bts" would do the trick.
^ permalink raw reply
* Re: [PATCH 0/5] credential helper super fun pak
From: Junio C Hamano @ 2011-09-14 21:37 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Brian Gernhardt
In-Reply-To: <20110914191704.GA23201@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Here's a mixed bag of bugfix commits to go on top of what's in
> jk/http-auth-keyring.
Heh, it indeed is a super-fun-pak. Will queue.
^ permalink raw reply
* Re: [PATCH] contrib: add a credential helper for Mac OS X's keychain
From: John Szakmeister @ 2011-09-14 22:23 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <CAG+J_Dw-vf7FtyT-vPpj-LHBo0rCBJi39bHh=8vWjc52QBMM2A@mail.gmail.com>
On Wed, Sep 14, 2011 at 9:31 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
[snip]
> Nope, you snipped out too much context. That command is turned into a
> string and then sent to /usr/bin/security on its stdin. It is
> absolutely not passed on the command-line.
Bah. I see it now. Thanks.
-John
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox