* 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: 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: [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: [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: [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: [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: [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
* [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
* [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 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 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 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 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
* 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
* 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: [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 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: [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: 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 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
* 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] git-p4: import utf16 file properly
From: Chris Li @ 2011-09-14 18:39 UTC (permalink / raw)
To: Luke Diamand; +Cc: git, Pete Wyckoff, Junio C Hamano
In-Reply-To: <CANeU7QnW5kSni0W9M9q-FTWv4p_qc67LG3mA6BQj_U-wxNuZeQ@mail.gmail.com>
On Wed, Sep 14, 2011 at 11:29 AM, Chris Li <git@chrisli.org> wrote:
>> 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 did a little bit research and found this:
http://www.perforce.com/perforce/doc.current/manuals/p4guide/ab_filetypes.html
RCS keyword expand should only happen for "+k" or "+ko" modifiers.
There for, "utf16" files without modifier should not be converted.
In that regard, the patch is correct. But both the original and patched version
did not handle "utf16+k" type of files. I still consider it as a separate issue.
Chris
^ permalink raw reply
* Re: [PATCH 3/7] refactor argv_array into generic code
From: Junio C Hamano @ 2011-09-14 18:42 UTC (permalink / raw)
To: Jeff King; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215757.GC24490@sigill.intra.peff.net>
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 ;-).
^ permalink raw reply
* Re: Fwd: [Survey] Signed push
From: Linus Torvalds @ 2011-09-14 18:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git
In-Reply-To: <7vk49bui4f.fsf@alter.siamese.dyndns.org>
On Wed, Sep 14, 2011 at 10:52 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Now people do "git merge" in scripts etc, so we can't fix it ;-(
>
> Yes you can, by teaching "git merge -e" to open an editor ;-).
>
> In the meantime, "commit --amend" is your friend.
The problem with both of those are human: because it's extra work,
it's not going to get done.
In contrast, if it's extra work to *avoid* editing the merge message,
people probably would see the editor pop up, and start adding a few
small notes.
That's why "default behavior" matters so much. It's not that it's not
*possible* to edit a merge message, it's that nobody ever does!
Linus
^ permalink raw reply
* Re: [PATCH] fetch: avoid quadratic loop checking for updated submodules
From: Jeff King @ 2011-09-14 18:29 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, git
In-Reply-To: <4E70F224.2080401@web.de>
On Wed, Sep 14, 2011 at 08:27:48PM +0200, Jens Lehmann wrote:
> Am 14.09.2011 00:17, schrieb Jeff King:
> > One thing that could make it slightly less expensive (but I wouldn't
> > worry about implementing until it becomes an issue): you do a full diff
> [...]
> I think this would work for the functionality which is implemented right
> now. But with Frederik's gitfile work I hope to enable git to support
> submodules being moved around in the work tree rather soonish. Then we
> would be blind for any changes in the new location. Until it hurts us
> I'd prefer to stay with the correct version, even if it is a bit slower.
Agreed. Let's worry about it if and when it becomes a performance issue.
Thanks for putting up my with my total cluelessness regarding what's
happening in submodules. ;)
-Peff
^ 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