gmane.comp.version-control.git

Urgent please. (Similar surname with my client)

Ching Chan — 2012-05-24T20:47:34

Hello,
I am Ching Chan, a principal Assurance Manager in one of the banks in  
Malaysia.
I am contacting you in regards to the 12 million United States dollars  
belonging to my deceased client.The bank are trying to confiscate the  
money and they have given me an ultimatum to privide a member of his  
family and all effort to provide his relatives have been  
unsuccessful.Since you have the same surname with my client I would  
like to present you as the rightful person to claim the money.Please  
reply For further information and instructions.

Thanks,
Ching Chan
Malaysia

Re: Git is on TWITTER !!!!!!!!!!!!

Miles Bader — 2012-05-25T04:22:41

Sometimes it's just for the amusement value ...

Maybe better illustrated by gregkh (the linux guy), who [at one time,
maybe not now] had his bash history piped to his twitter feed!  Mostly
completely useless, but kind of a funny comment on twitter...

-miles

Re: Git hangs at “Writing objects: 11%”

Timothy Normand Miller — 2012-05-25T01:41:55

On May 24, 2012, at 8:51 PM, Jeff King wrote:


Thank you for helping!


If the local repo I'm pushing to is given by pathname, it's fine.  If I'm pushing to another account via ssh on the localhost, it has the same hangup.

All fetches work fine.


There's this one CentOS 6 machine where I know for a fact that git hasn't been updated since I was last able to push this particular repo.  That machine has git version 1.7.1.  That's where I first encountered the problem.  So I copied the repo to my Mac (version 1.7.8.6), but it freezes there at 11%.  Then I tried copying it to the same machine that hosts the repo (Gentoo Linux with git version 1.7.8.6), and push from there (still using ssh, but locally), and it also freezes at 11%.

So there's something wonky about the repo that git doesn't like, although it only has the problem over ssh.  I haven't tried git's native protocol.


First, I ran the push and then killed it, and I got this:
http://www.cse.ohio-state.edu/~millerti/foo.out

But then I realized that you'd want to see where it hung up, so I can it again and then sent the file before killing the push:
http://www.cse.ohio-state.edu/~millerti/foo2.out



I really don't know how to interpret the trace.


Try this:
http://www.cse.ohio-state.edu/~millerti/revue_strace3.txt

This one was from earlier in the day.  It's from a push from the same machine that hosts the repo.


That would make things a lot easier, but I'm not at liberty to share it.


Thanks again!

Re: remove_duplicates() in builtin/fetch-pack.c is O(N^2)

Martin Fick — 2012-05-25T01:32:36

wrote:

Yeah, I thought about that.  Could you just right the new 
packed-ref file with the new refs and the old refs which 
were in the file already, then just delete any loose refs 
which were ones which were just added by this operation, 
only if they have not changed?

This way, if someone else modifies one of the same refs, 
they could just win?

-Martin

Re: remove_duplicates() in builtin/fetch-pack.c is O(N^2)

Jeff King — 2012-05-25T01:04:34


Hmm. Yeah, clone will always write a packed-refs file, but I think "git
fetch" will always write loose refs, under the assumption that the
former will be getting a lot more refs than the latter. But of course
that is only a guess. It would be nice if fetch could fetch straight
into packed refs if we are getting more than N items.

We'd have to give some thought to potential race conditions, though.
Usually pack-refs isn't modifying the ref, so it can just write out the
value to the packed-refs file, then delete the loose ref if nobody has
touched it since we wrote. But here we're combining it with a
modification, so I suspect there would be a race with another process
trying to modify it.


Yeah, I wouldn't be surprised if this thrashes your disk. Writing
hundreds of thousands of 40-byte files is one of the most awful loads
for many filesystems, since each file gets its own inode. I haven't
tried btrfs, but my impression is that it can magically pack the data
from many files into one node.

-Peff

Re: remove_duplicates() in builtin/fetch-pack.c is O(N^2)

Martin Fick — 2012-05-25T00:54:56

wrote:

Mostly true, except for one strange case still I think?

When cloning a gerrit repo, users to not get the changes 
since they are not under refs/heads but refs/changes.  So 
later, if they choose to fetch refs/changes/*, all of those
new incoming refs are loose.  Yes, someone should pack those 
refs right away, but I think it actually churns the hell out 
of my disk and takes a significant amount of time during the 
initial fetch.  I am not certain about this, and the 
behavior may depend on the filesystem in use, but I think 
that this time might even be asynchronous (journals and 
all), it feels like my disk keeps churning for a while even 
after this is over.  I believe that this might still be the 
worst case left with refs, and it can be pretty bad,

-Martin

Re: Git hangs at “Writing objects: 11%”

Jeff King — 2012-05-25T00:51:56


I'd first start with trying to remove as many variables as possible.
Does the problem only happen over ssh, or does it also happen when
pushing across a pipe to a repo on the local machine? If so, does it
also happen during a fetch of the same data?

If you can reproduce it at will (which it sounds like you can), you
could also try some older or newer git versions to see if they work any
better. If you can find a working version, it might be worth trying to
bisect and find the commit that introduces the breakage.

If the problem still exists in the latest version, then I'd start by
stracing as much as possible. On the client side, you can use "strace
-f" to see what all of the processes are doing; you'll probably also
want to pass:

  --receive-pack='strace -f -o foo.out git-receive-pack'

to git-push to ask the remote side to strace. There's a reasonable
chance you'll simply see that the client side is waiting on the server
side for I/O, so you'll want to know what the server side is doing.

I see you posted an strace snippet of a process waiting in select() on
stack overflow. It's hard to tell what's going on from there, though,
because we can't see which processes are which (we see the pids, but we
don't know which programs they're running, or where the commands go). A
full strace log would help a lot (if it's long and you need a place to
post it, try something like https://gist.github.com).

And finally, if the repo is something you can make public, I can try to
reproduce on my machine. That might tell us if the problem is with your
repo, or something else about your machines or setup.

-Peff

Re: remove_duplicates() in builtin/fetch-pack.c is O(N^2)

Jeff King — 2012-05-25T00:39:20


No worries. I am pretty confident that the patches I supplied so far are
a good thing whether they help your case or not. So if you come back in
a month and show that they solved all your problems, then great. And if
they don't, then it will just show us what new problems we have to work
on. :)


Yes, exclusively warm. And all of the refs were packed, which makes the
warm/cold difference less interesting (it's one 30MB or so file).  I
don't think there's much point in thinking about the performance of 400K
loose refs (which would be absolutely horrific cold-cache on most
traditional filesystems). If you have that many, you would want to keep
the bulk of them packed.

-Peff

Re: t4014 broken by 43ae9f47ab: format-patch: use default email for generating message ids

Jeff King — 2012-05-25T00:34:57


Yeah. I'd be fine with more tightening, but I wanted to just catch the
most common uncontroversial problem in the initial round. We can build
on top later if we want.


Yeah, I considered that, but got nervous thinking about the same can of
worms. It is not like people are complaining now, so I'd rather leave
it.

-Peff

Re: remove_duplicates() in builtin/fetch-pack.c is O(N^2)

Martin Fick — 2012-05-25T00:17:45

Peff,

I have not been ignoring you, I am sorry that I have not 
replied yet.  Unfortunately, I am having a very hard time 
getting conclusive tests with my large repo.  I making
plenty of mistakes in what I think I am testing I believe,
but also I am having a hard time getting reproducible 
results so far.  And some tests take quite a while, so it is 
not always obvious what I might be doing wrong.

I will let you know more when I figure out what I am doing 
wrong, but please know that I have been doing a lot of 
testing and plan to post some results eventually.

Were your tests mostly warm cache tests?

-Martin

Re: t4014 broken by 43ae9f47ab: format-patch: use default email for generating message ids

Junio C Hamano — 2012-05-25T00:08:15


All looked pretty straightforward and cleanly done.

We might want to further tighten 6/7 to verify user-supplied (i.e. non
default) e-mail for sanity, as I agree with the comment below --- lines of
that patch.

Also the check might want to be further tightened in the RFC 822/2822/5322
sense, but getting it correct will open a huge can of worms; I think the
check in 6/7 is a good place to stop, at least for now.

Thanks.

Re: [PATCH 1/2] git-p4: Test changelists touching two branches

Junio C Hamano — 2012-05-25T00:02:16


If you mean this part, the parentheses to throw you into a subprocess are
required.  Otherwise, a failure in any of these three p4 commands will
leave you in $cli directory, causing the next test to start in a directory
that it does not expect.

[PATCH 7/7] format-patch: do not use bogus email addresses in message ids

Jeff King — 2012-05-24T23:32:52

We can ask git_committer_info to be strict about coming up
with an email, which will die automatically on a poorly
configured machine. This is better than letting invalid
message-ids into the wild.

Signed-off-by: Jeff King <peff< at >peff.net>
---
 builtin/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index d86bca3..906dca4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
< at >< at > -741,7 +741,7 < at >< at > static void gen_message_id(struct rev_info *info, char *base)
 struct strbuf buf = STRBUF_INIT;
 strbuf_addf(&buf, "%s.%lu.git.%s", base,
     (unsigned long) time(NULL),
-    git_committer_info(IDENT_NO_NAME|IDENT_NO_DATE));
+    git_committer_info(IDENT_NO_NAME|IDENT_NO_DATE|IDENT_STRICT));
 info->message_id = strbuf_detach(&buf, NULL);
 }

[PATCH 6/7] ident: reject bogus email addresses with IDENT_STRICT

Jeff King — 2012-05-24T23:32:37

If we come up with a hostname like "foo.(none)" because the
user's machine is not fully qualified, we should reject this
in strict mode (e.g., when we are making a commit object),
just as we reject an empty gecos username.

Signed-off-by: Jeff King <peff< at >peff.net>
---
Note that in conjunction with the previous patch, you can no longer "git
commit" with such a bogus address. I think this is a good thing.

However, it's possible some old-timers might disagree. I remember Linus
a long time ago mentioning that using the machine-name in the committer
line was a _feature_. These days he seems to set user.email to a real
address, though (and I think that is sane these days, because other
tools really want to do use identities as more than just a token. E.g.,
email tools, gpg-signing, etc).

 ident.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ident.c b/ident.c
index c42258f..ca098d9 100644
--- a/ident.c
+++ b/ident.c
< at >< at > -288,6 +288,12 < at >< at > const char *fmt_ident(const char *name, const char *email,
 name = pw->pw_name;
 }
 
+if (strict && email == git_default_email.buf &&
+    !strstr(email, "(none)")) {
+fputs(env_hint, stderr);
+die("unable to auto-detect email address (got '%s')", email);
+}
+
 if (want_date) {
 if (date_str && date_str[0]) {
 if (parse_date(date_str, date, sizeof(date)) < 0)

[PATCH 5/7] ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT

Jeff King — 2012-05-24T23:28:40

Callers who ask for ERROR_ON_NO_NAME are not so much
concerned that the name will be blank (because, after all,
we will fall back to using the username), but rather it is a
check to make sure that low-quality identities do not end up
in things like commit messages or emails (whereas it is OK
for them to end up in things like reflogs).

When future commits add more quality checks on the identity,
each of these callers would want to use those checks, too.
Rather than modify each of them later to add a new flag,
let's refactor the flag.

Signed-off-by: Jeff King <peff< at >peff.net>
---
 builtin/commit.c | 3 +--
 builtin/log.c    | 2 +-
 builtin/merge.c  | 4 ++--
 builtin/tag.c    | 2 +-
 builtin/var.c    | 4 ++--
 cache.h          | 2 +-
 commit.c         | 4 ++--
 gpg-interface.c  | 2 +-
 ident.c          | 6 +++---
 9 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a2ec73d..f43eaaf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
< at >< at > -526,8 +526,7 < at >< at > static void determine_author_info(struct strbuf *author_ident)
 
 if (force_date)
 date = force_date;
-strbuf_addstr(author_ident, fmt_ident(name, email, date,
-      IDENT_ERROR_ON_NO_NAME));
+strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT));
 if (!split_ident_line(&author, author_ident->buf, author_ident->len)) {
 export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0);
 export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0);
diff --git a/builtin/log.c b/builtin/log.c
index 4538309..d86bca3 100644
--- a/builtin/log.c
+++ b/builtin/log.c
< at >< at > -1147,7 +1147,7 < at >< at > int cmd_format_patch(int argc, const char **argv, const char *prefix)
 if (do_signoff) {
 const char *committer;
 const char *endpos;
-committer = git_committer_info(IDENT_ERROR_ON_NO_NAME);
+committer = git_committer_info(IDENT_STRICT);
 endpos = strchr(committer, '>');
 if (!endpos)
 die(_("bogus committer info %s"), committer);
diff --git a/builtin/merge.c b/builtin/merge.c
index 470fc57..dd50a0c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
< at >< at > -1447,7 +1447,7 < at >< at > int cmd_merge(int argc, const char **argv, const char *prefix)
 refresh_cache(REFRESH_QUIET);
 if (allow_trivial && !fast_forward_only) {
 /* See if it is really trivial. */
-git_committer_info(IDENT_ERROR_ON_NO_NAME);
+git_committer_info(IDENT_STRICT);
 printf(_("Trying really trivial in-index merge...\n"));
 if (!read_tree_trivial(common->item->object.sha1,
        head_commit->object.sha1,
< at >< at > -1490,7 +1490,7 < at >< at > int cmd_merge(int argc, const char **argv, const char *prefix)
 die(_("Not possible to fast-forward, aborting."));
 
 /* We are going to make a new commit. */
-git_committer_info(IDENT_ERROR_ON_NO_NAME);
+git_committer_info(IDENT_STRICT);
 
 /*
  * At this point, we need a real merge.  No matter what strategy
diff --git a/builtin/tag.c b/builtin/tag.c
index 4fb6bd7..7b1be85 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
< at >< at > -332,7 +332,7 < at >< at > static void create_tag(const unsigned char *object, const char *tag,
   sha1_to_hex(object),
   typename(type),
   tag,
-  git_committer_info(IDENT_ERROR_ON_NO_NAME));
+  git_committer_info(IDENT_STRICT));
 
 if (header_len > sizeof(header_buf) - 1)
 die(_("tag header too big."));
diff --git a/builtin/var.c b/builtin/var.c
index 99d068a..aedbb53 100644
--- a/builtin/var.c
+++ b/builtin/var.c
< at >< at > -11,7 +11,7 < at >< at > static const char *editor(int flag)
 {
 const char *pgm = git_editor();
 
-if (!pgm && flag & IDENT_ERROR_ON_NO_NAME)
+if (!pgm && flag & IDENT_STRICT)
 die("Terminal is dumb, but EDITOR unset");
 
 return pgm;
< at >< at > -55,7 +55,7 < at >< at > static const char *read_var(const char *var)
 val = NULL;
 for (ptr = git_vars; ptr->read; ptr++) {
 if (strcmp(var, ptr->name) == 0) {
-val = ptr->read(IDENT_ERROR_ON_NO_NAME);
+val = ptr->read(IDENT_STRICT);
 break;
 }
 }
diff --git a/cache.h b/cache.h
index 4a76c23..06413e1 100644
--- a/cache.h
+++ b/cache.h
< at >< at > -887,7 +887,7 < at >< at > unsigned long approxidate_careful(const char *, int *);
 unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
-#define IDENT_ERROR_ON_NO_NAME 1
+#define IDENT_STRICT       1
 #define IDENT_NO_DATE       2
 #define IDENT_NO_NAME       4
 extern const char *git_author_info(int);
diff --git a/commit.c b/commit.c
index 9ed36c7..8248a99 100644
--- a/commit.c
+++ b/commit.c
< at >< at > -1154,9 +1154,9 < at >< at > int commit_tree_extended(const struct strbuf *msg, unsigned char *tree,
 
 /* Person/date information */
 if (!author)
-author = git_author_info(IDENT_ERROR_ON_NO_NAME);
+author = git_author_info(IDENT_STRICT);
 strbuf_addf(&buffer, "author %s\n", author);
-strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_ERROR_ON_NO_NAME));
+strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
 if (!encoding_is_utf8)
 strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
 
diff --git a/gpg-interface.c b/gpg-interface.c
index 09ab64a..0863c61 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
< at >< at > -30,7 +30,7 < at >< at > const char *get_signing_key(void)
 {
 if (configured_signing_key)
 return configured_signing_key;
-return git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
 }
 
 /*
diff --git a/ident.c b/ident.c
index 8b5080d..c42258f 100644
--- a/ident.c
+++ b/ident.c
< at >< at > -267,7 +267,7 < at >< at > const char *fmt_ident(const char *name, const char *email,
 {
 static struct strbuf ident = STRBUF_INIT;
 char date[50];
-int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);
+int strict = (flag & IDENT_STRICT);
 int want_date = !(flag & IDENT_NO_DATE);
 int want_name = !(flag & IDENT_NO_NAME);
 
< at >< at > -279,7 +279,7 < at >< at > const char *fmt_ident(const char *name, const char *email,
 if (want_name && !*name) {
 struct passwd *pw;
 
-if (error_on_no_name) {
+if (strict) {
 if (name == git_default_name.buf)
 fputs(env_hint, stderr);
 die("empty ident name (for <%s>) not allowed", email);
< at >< at > -314,7 +314,7 < at >< at > const char *fmt_ident(const char *name, const char *email,
 
 const char *fmt_name(const char *name, const char *email)
 {
-return fmt_ident(name, email, NULL, IDENT_ERROR_ON_NO_NAME | IDENT_NO_DATE);
+return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE);
 }
 
 const char *git_author_info(int flag)

[PATCH 4/7] format-patch: use GIT_COMMITTER_EMAIL in message ids

Jeff King — 2012-05-24T23:28:25

Before commit 43ae9f4, we generated the tail of a message id
by calling git_committer_info and parsing the email out of
the result. 43ae9f4 changed to use ident_default_email
directly, so we didn't have to bother with parsing. As a
side effect, it meant we no longer used GIT_COMMITTER_EMAIL
at all.

In general, this is probably reasonable behavior. Either the
default email is sane on your system, or you are using
user.email to provide something sane. The exception is if
you rely on GIT_COMMITTER_EMAIL being set all the time to
override the bogus generated email.

This is unlikely to match anybody's real-life setup, but we
do use it in the test environment. And furthermore, it's
what we have always done, and the change in 43ae9f4 was
about cleaning up, not fixing any bug; we should be
conservative and keep the behavior identical.

Signed-off-by: Jeff King <peff< at >peff.net>
---
This one should fix Michael's test failure. Let me know if it doesn't.

Arguably the call to ident_default_email() in http-push.c should be
converted in the same way. I'm unclear on how that value is actually
used, so it may not matter at all.

 builtin/log.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8010a40..4538309 100644
--- a/builtin/log.c
+++ b/builtin/log.c
< at >< at > -740,7 +740,8 < at >< at > static void gen_message_id(struct rev_info *info, char *base)
 {
 struct strbuf buf = STRBUF_INIT;
 strbuf_addf(&buf, "%s.%lu.git.%s", base,
-    (unsigned long) time(NULL), ident_default_email());
+    (unsigned long) time(NULL),
+    git_committer_info(IDENT_NO_NAME|IDENT_NO_DATE));
 info->message_id = strbuf_detach(&buf, NULL);
 }

[PATCH 3/7] ident: let callers omit name with fmt_indent

Jeff King — 2012-05-24T23:27:24

Most callers want to see all of "$name <$email> $date", but
a few want only limited parts, omitting the date, or even
the name. We already have IDENT_NO_DATE to handle the date
part, but there's not a good option for getting just the
email. Callers have to done one of:

  1. Call ident_default_email; this does not respect
     environment variables, nor does it promise to trim
     whitespace or other crud from the result.

  2. Call git_{committer,author}_info; this returns the name
     and email, leaving the caller to parse out the wanted
     bits.

This patch adds IDENT_NO_NAME; it stops short of adding
IDENT_NO_EMAIL, as no callers want it (nor are likely to),
and it complicates the error handling of the function.

When no name is requested, the angle brackets (<>) around
the email address are also omitted.

Signed-off-by: Jeff King <peff< at >peff.net>
---
I worked this up with IDENT_NO_EMAIL, too, but it really just made the
function harder to read. And I doubt any callers are going to want it.

 cache.h |  1 +
 ident.c | 14 +++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 0ac0d57..4a76c23 100644
--- a/cache.h
+++ b/cache.h
< at >< at > -889,6 +889,7 < at >< at > enum date_mode parse_date_format(const char *format);
 
 #define IDENT_ERROR_ON_NO_NAME 1
 #define IDENT_NO_DATE       2
+#define IDENT_NO_NAME       4
 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
 extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
diff --git a/ident.c b/ident.c
index 59beef2..8b5080d 100644
--- a/ident.c
+++ b/ident.c
< at >< at > -269,13 +269,14 < at >< at > const char *fmt_ident(const char *name, const char *email,
 char date[50];
 int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);
 int want_date = !(flag & IDENT_NO_DATE);
+int want_name = !(flag & IDENT_NO_NAME);
 
-if (!name)
+if (want_name && !name)
 name = ident_default_name();
 if (!email)
 email = ident_default_email();
 
-if (!*name) {
+if (want_name && !*name) {
 struct passwd *pw;
 
 if (error_on_no_name) {
< at >< at > -297,10 +298,13 < at >< at > const char *fmt_ident(const char *name, const char *email,
 }
 
 strbuf_reset(&ident);
-strbuf_addstr_without_crud(&ident, name);
-strbuf_addstr(&ident, " <");
+if (want_name) {
+strbuf_addstr_without_crud(&ident, name);
+strbuf_addstr(&ident, " <");
+}
 strbuf_addstr_without_crud(&ident, email);
-strbuf_addch(&ident, '>');
+if (want_name)
+strbuf_addch(&ident, '>');
 if (want_date) {
 strbuf_addch(&ident, ' ');
 strbuf_addstr_without_crud(&ident, date);

[PATCH 2/7] ident: refactor NO_DATE flag in fmt_ident

Jeff King — 2012-05-24T23:26:50

As a short-hand, we extract this flag into the local
variable "name_addr_only". It's more accurate to simply
negate this and refer to it as "want_date", which will be
less confusing when we add more NO_* flags.

While we're touching this part of the code, let's move the
call to ident_default_date() only when we are actually going
to use it, not when we have NO_DATE set, or when we get a
date from the environment.

Signed-off-by: Jeff King <peff< at >peff.net>
---
Just refactoring for the next patch...

 ident.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ident.c b/ident.c
index f5160e1..59beef2 100644
--- a/ident.c
+++ b/ident.c
< at >< at > -268,7 +268,7 < at >< at > const char *fmt_ident(const char *name, const char *email,
 static struct strbuf ident = STRBUF_INIT;
 char date[50];
 int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);
-int name_addr_only = (flag & IDENT_NO_DATE);
+int want_date = !(flag & IDENT_NO_DATE);
 
 if (!name)
 name = ident_default_name();
< at >< at > -287,10 +287,13 < at >< at > const char *fmt_ident(const char *name, const char *email,
 name = pw->pw_name;
 }
 
-strcpy(date, ident_default_date());
-if (!name_addr_only && date_str && date_str[0]) {
-if (parse_date(date_str, date, sizeof(date)) < 0)
-die("invalid date format: %s", date_str);
+if (want_date) {
+if (date_str && date_str[0]) {
+if (parse_date(date_str, date, sizeof(date)) < 0)
+die("invalid date format: %s", date_str);
+}
+else
+strcpy(date, ident_default_date());
 }
 
 strbuf_reset(&ident);
< at >< at > -298,7 +301,7 < at >< at > const char *fmt_ident(const char *name, const char *email,
 strbuf_addstr(&ident, " <");
 strbuf_addstr_without_crud(&ident, email);
 strbuf_addch(&ident, '>');
-if (!name_addr_only) {
+if (want_date) {
 strbuf_addch(&ident, ' ');
 strbuf_addstr_without_crud(&ident, date);
 }

[PATCH 1/7] ident: refactor empty ident error message

Jeff King — 2012-05-24T23:26:32

There's on point in printing the name, since it is by
definition the empty string if we have reached this code
path. Instead, let's be more clear that we are complaining
about the empty name, but still show the email address that
it is attached to (since that may provide some context to
the user).

Signed-off-by: Jeff King <peff< at >peff.net>
---
As I mentioned in the cover letter, this one is optional. But I think it
makes sense.

 ident.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ident.c b/ident.c
index e279039..f5160e1 100644
--- a/ident.c
+++ b/ident.c
< at >< at > -281,7 +281,7 < at >< at > const char *fmt_ident(const char *name, const char *email,
 if (error_on_no_name) {
 if (name == git_default_name.buf)
 fputs(env_hint, stderr);
-die("empty ident %s <%s> not allowed", name, email);
+die("empty ident name (for <%s>) not allowed", email);
 }
 pw = xgetpwuid_self();
 name = pw->pw_name;

Re: t4014 broken by 43ae9f47ab: format-patch: use default email for generating message ids

Jeff King — 2012-05-24T23:25:15


Here it is.

  [1/7]: ident: refactor empty ident error message

This one is only tangentially related. I was going to touch the message
more in patch 3, but decided not to (details in that patch).

  [2/7]: ident: refactor NO_DATE flag in fmt_ident
  [3/7]: ident: let callers omit name with fmt_indent
  [4/7]: format-patch: use GIT_COMMITTER_EMAIL in message ids

These ones should fix Michael's failing test and restore the original
behavior.

  [5/7]: ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT
  [6/7]: ident: reject bogus email addresses with IDENT_STRICT
  [7/7]: format-patch: do not use bogus email addresses in message ids

These ones prevent bogus message ids from being generated at all
(which is an improvement over the previous state).

-Peff

Re: [PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs

Jon Seymour — 2012-05-24T23:07:11

Mmmm. Probably shouldn't be doing these _all_ of these edits on each
iteration of the while loop, methinks.

I'll write a test, and fix.

jon.