Git development
 help / color / mirror / Atom feed
* [PATCH] git-fast-import(1): remove duplicate "--done" option
From: John Keeping @ 2013-01-05 16:06 UTC (permalink / raw)
  To: git; +Cc: Jonathan Nieder, Eric S. Raymond, Sverre Rabbelier

The "--done" option to git-fast-import is documented twice in its manual
page.  Combine the best bits of each description, keeping the location
of the instance that was added first.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
I'm guessing that the reason the option was documented again (in commit
3266de10) is because the options are not in an obvious order.  There
does seem to be some grouping of the options by type, but without
subheadings I wonder if it would make more sense to just put them all in
alphabetical order?

 Documentation/git-fast-import.txt | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 68bca1a..4ef5721 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -39,10 +39,6 @@ OPTIONS
 	See ``Date Formats'' below for details about which formats
 	are supported, and their syntax.
 
--- done::
-	Terminate with error if there is no 'done' command at the
-	end of the stream.
-
 --force::
 	Force updating modified existing branches, even if doing
 	so would cause commits to be lost (as the new commit does
@@ -108,7 +104,8 @@ OPTIONS
 	output.
 
 --done::
-	Require a `done` command at the end of the stream.
+	Terminate with error if there is no 'done' command at the
+	end of the stream.
 	This option might be useful for detecting errors that
 	cause the frontend to terminate before it has started to
 	write a stream.
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH] Documentation: fix man page dependency on asciidoc.conf
From: John Keeping @ 2013-01-05 16:00 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

When building manual pages, the source text is transformed to XML with
AsciiDoc before the man pages are generated from the XML with xmlto.

Fix the dependency in the Makefile so that the XML files are rebuilt
when asciidoc.conf changes and not just the manual pages from unchanged
XML.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 Documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e53d333..71d2b4a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -178,7 +178,7 @@ all: html man
 
 html: $(DOC_HTML)
 
-$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7): asciidoc.conf
+$(DOC_HTML) $(MAN_XML): asciidoc.conf
 
 man: man1 man5 man7
 man1: $(DOC_MAN1)
-- 
1.8.0.2

^ permalink raw reply related

* Re: Lockless Refs?  (Was [PATCH] refs: do not use cached refs in repack_without_ref)
From: Jeff King @ 2013-01-05 16:12 UTC (permalink / raw)
  To: Martin Fick; +Cc: Michael Haggerty, Shawn Pearce, git, Junio C Hamano
In-Reply-To: <201212310330.53835.mfick@codeaurora.org>

On Mon, Dec 31, 2012 at 03:30:53AM -0700, Martin Fick wrote:

> The general approach is to setup a transaction and either 
> commit or abort it.  A transaction can be setup by renaming 
> an appropriately setup directory to the "ref.lock" name.  If 
> the rename succeeds, the transaction is begun.  Any actor can 
> abort the transaction (up until it is committed) by simply 
> deleting the "ref.lock" directory, so it is not at risk of 
> going stale.

Deleting a directory is not atomic, as you first have to remove the
contents, putting it into a potentially inconsistent state. I'll assume
you deal with that later...

> One important piece of the transaction is the use of uuids.  
> The uuids provide a mechanism to tie the atomic commit pieces 
> to the transactions and thus to prevent long sleeping process 
> from inadvertently performing actions which could be out of 
> date when they wake finally up.

Has this been a problem for you in practice? Avoiding this is one of the
reasons that git does not take out long locks; instead, it takes the
lock only at the moment it is ready to write, and aborts if it has been
updated since the longer-term operation began. This has its own problems
(you might do a lot of work only to have your operation aborted), but I
am not sure that your proposal improves on that.

Your proposal does sound like it could potentially improve robustness
when killing stale transactions (i.e., you know that the transaction
originator will never wake up and think it still has the lock). But
again, is that a problem in practice? Git typically holds ref locks for
a few syscalls. If you are conservative about leaving potentially stale
locks in place (e.g., give them a few minutes to complete before
assuming they are now bogus), you will not run into that problem.

The more conservative you are about treating a lock as stale, of course,
the less performant you will be in the face of stale locks. But since
they're the exception, that isn't a big problem in practice (at least it
has not been for me).

> In each case, the atomic 
> commit piece is the renaming of a file.   For the create and 
> update pieces, a file is renamed from the "ref.lock" dir to 
> the "ref" file resulting in an update to the sha for the ref.

I think we've had problems with cross-directory renames on some
filesystems, but I don't recall the details. I know that Coda does not
like cross-directory links, but cross-directory renames are OK (and in
fact we fall back to the latter when the former does not work).

Ah, here we go: 5723fe7 (Avoid cross-directory renames and linking on
object creation, 2008-06-14). Looks like NFS is the culprit.

> In the case of a delete, the actor may verify that "ref" 
> currently contains the sha to "prune" if it needs to, and 
> then renames the "ref" file to "ref.lock/uuid/delete". On 
> success, the ref was deleted.
> 
> Whether successful or not, the actor may now simply delete 
> the "ref.lock" directory, clearing the way for a new 
> transaction.  Any other actor may delete this directory at 
> any time also, likely either on conflict (if they are 
> attempting to initiate a transaction), or after a grace 
> period just to cleanup the FS.  Any actor may also safely 
> cleanup the tmp directories, preferably also after a grace 
> period.

Hmm. So what happens to the "delete" file when the ref.lock directory is
being deleted? Presumably deleting the ref.lock directory means doing it
recursively (which is non-atomic). But then why are we keeping the
delete file at all, if we're just about to remove it?

What happens if another process wants to cancel a transaction that is
partially done? That is, the ref.lock directory is created, but it
contains the uuid subdir? It sounds like it's OK to just delete from it,
and the original process will then fail at its rename?

> One neat part about this scheme is that I believe it would be 
> backwards compatible with the current locking mechanism since 
> the transaction directory will simply appear to be a lock to 
> older clients.  And the old lock file should continue to lock 
> out these newer transactions.

Yeah, I don't see anything that would prevent that. The current code
only cares about open("$ref.lock", O_EXCL). But that should be correct
and atomic with respect to the renamed directories. You are depending on
atomic directory renames. Those aren't used anywhere yet in git, as far
as I know. So that may run into problems (but there's no reason this
system couldn't be optional for filesystems that are more abled, and
other systems could fall back to the straight-file locking).


So in response to your question, no, I don't see any real showstoppers
here. And unlike the "all refs are files in a directory" scheme, it's
confined to writing, which solves the readdir() atomicity questions I
had there.

I still can't say I'm super excited about it, just because it seems like
a solution in search of a problem that I have not experienced myself.
But if it's solving a problem for you, I don't want to discourage you
from pursuing it.

> To be honest, I suspect I missed something obvious because 
> this seems almost too simple to work.  I am ashamed that it 
> took me so long to come up with (of course, I will be even 
> more ashamed :( when it is shown to be flawed!)  This scheme 
> also feels extensible. if there are no obvious flaws in it, I 
> will try to post solutions for ref packing and for multiple 
> repository/ref transactions also soon.

Fixing the minor remaining races on ref packing would be nice, but I do
not think those are a problem of the on-disk lock representation. The
reason they are not fixed now is from an attempt to keep lock contention
low between unrelated updates (something which we should probably give
up on in favor of correctness; but that is orthogonal to whether we do
it with the existing file locks or with a new system).

A much stronger fix for that would be to record deletes in the loose ref
storage so that we don't have to update the packed-refs file as often
(because it is inherently a lock bottleneck, and because it is wasteful
when you have a very large number of refs). But that means dealing with
directory/file conflicts between deleted and existing branches, which is
a pain.

-Peff

^ permalink raw reply

* Re: [PATCH] git-fast-import(1): remove duplicate "--done" option
From: Eric S. Raymond @ 2013-01-05 16:26 UTC (permalink / raw)
  To: John Keeping; +Cc: git, Jonathan Nieder, Sverre Rabbelier
In-Reply-To: <20130105160652.GE6440@serenity.lan>

John Keeping <john@keeping.me.uk>:
> I'm guessing that the reason the option was documented again (in commit
> 3266de10) is because the options are not in an obvious order.  There
> does seem to be some grouping of the options by type, but without
> subheadings I wonder if it would make more sense to just put them all in
> alphabetical order?

+1

This duplication originated with me. I'll apologize with a 
reordering patch.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* [PATCH] Alphabetize the fast-import options, following a suggestion on the list.
From: Eric S. Raymond @ 2013-01-05 16:44 UTC (permalink / raw)
  To: git

---
 Documentation/git-fast-import.txt | 94 +++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 49 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 68bca1a..d006bcf 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -33,24 +33,9 @@ the frontend program in use.
 
 OPTIONS
 -------
---date-format=<fmt>::
-	Specify the type of dates the frontend will supply to
-	fast-import within `author`, `committer` and `tagger` commands.
-	See ``Date Formats'' below for details about which formats
-	are supported, and their syntax.
-
--- done::
-	Terminate with error if there is no 'done' command at the
-	end of the stream.
-
---force::
-	Force updating modified existing branches, even if doing
-	so would cause commits to be lost (as the new commit does
-	not contain the old commit).
-
---max-pack-size=<n>::
-	Maximum size of each output packfile.
-	The default is unlimited.
+--active-branches=<n>::
+	Maximum number of branches to maintain active at once.
+	See ``Memory Utilization'' below for details.  Default is 5.
 
 --big-file-threshold=<n>::
 	Maximum size of a blob that fast-import will attempt to
@@ -58,13 +43,27 @@ OPTIONS
 	(512 MiB).  Some importers may wish to lower this on systems
 	with constrained memory.
 
+--cat-blob-fd=<fd>::
+	Write responses to `cat-blob` and `ls` queries to the
+	file descriptor <fd> instead of `stdout`.  Allows `progress`
+	output intended for the end-user to be separated from other
+	output.
+
+--date-format=<fmt>::
+	Specify the type of dates the frontend will supply to
+	fast-import within `author`, `committer` and `tagger` commands.
+	See ``Date Formats'' below for details about which formats
+	are supported, and their syntax.
+
 --depth=<n>::
 	Maximum delta depth, for blob and tree deltification.
 	Default is 10.
 
---active-branches=<n>::
-	Maximum number of branches to maintain active at once.
-	See ``Memory Utilization'' below for details.  Default is 5.
+--done::
+	Terminate with error if there is no 'done' command at the end
+	of the stream.  This option might be useful for detecting
+	errors that cause the frontend to terminate before it has
+	started to write a stream.
 
 --export-marks=<file>::
 	Dumps the internal marks table to <file> when complete.
@@ -75,6 +74,20 @@ OPTIONS
 	at checkpoint (or completion) the same path can also be
 	safely given to \--import-marks.
 
+--export-pack-edges=<file>::
+	After creating a packfile, print a line of data to
+	<file> listing the filename of the packfile and the last
+	commit on each branch that was written to that packfile.
+	This information may be useful after importing projects
+	whose total object set exceeds the 4 GiB packfile limit,
+	as these commits can be used as edge points during calls
+	to 'git pack-objects'.
+
+--force::
+	Force updating modified existing branches, even if doing
+	so would cause commits to be lost (as the new commit does
+	not contain the old commit).
+
 --import-marks=<file>::
 	Before processing any input, load the marks specified in
 	<file>.  The input file must exist, must be readable, and
@@ -87,13 +100,9 @@ OPTIONS
 	Like --import-marks but instead of erroring out, silently
 	skips the file if it does not exist.
 
---relative-marks::
-	After specifying --relative-marks the paths specified
-	with --import-marks= and --export-marks= are relative
-	to an internal directory in the current repository.
-	In git-fast-import this means that the paths are relative
-	to the .git/info/fast-import directory. However, other
-	importers may use a different location.
+--max-pack-size=<n>::
+	Maximum size of each output packfile.
+	The default is unlimited.
 
 --no-relative-marks::
 	Negates a previous --relative-marks. Allows for combining
@@ -101,32 +110,19 @@ OPTIONS
 	--(no-)-relative-marks with the --(import|export)-marks=
 	options.
 
---cat-blob-fd=<fd>::
-	Write responses to `cat-blob` and `ls` queries to the
-	file descriptor <fd> instead of `stdout`.  Allows `progress`
-	output intended for the end-user to be separated from other
-	output.
-
---done::
-	Require a `done` command at the end of the stream.
-	This option might be useful for detecting errors that
-	cause the frontend to terminate before it has started to
-	write a stream.
-
---export-pack-edges=<file>::
-	After creating a packfile, print a line of data to
-	<file> listing the filename of the packfile and the last
-	commit on each branch that was written to that packfile.
-	This information may be useful after importing projects
-	whose total object set exceeds the 4 GiB packfile limit,
-	as these commits can be used as edge points during calls
-	to 'git pack-objects'.
-
 --quiet::
 	Disable all non-fatal output, making fast-import silent when it
 	is successful.  This option disables the output shown by
 	\--stats.
 
+--relative-marks::
+	After specifying --relative-marks the paths specified
+	with --import-marks= and --export-marks= are relative
+	to an internal directory in the current repository.
+	In git-fast-import this means that the paths are relative
+	to the .git/info/fast-import directory. However, other
+	importers may use a different location.
+
 --stats::
 	Display some basic statistics about the objects fast-import has
 	created, the packfiles they were stored into, and the
-- 
1.8.1



-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

"Gun control" is a job-safety program for criminals.

^ permalink raw reply related

* Re: [LHF] making t5000 "tar xf" tests more lenient
From: René Scharfe @ 2013-01-05 18:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwqw7mb09.fsf@alter.siamese.dyndns.org>

Am 24.12.2012 21:49, schrieb Junio C Hamano:
> I've been running testsuite on a few platforms that are unfamiliar
> to me, and was bitten by BSD implementation of tar that do not grok
> the extended pax headers.  I've already fixed one in t9502 [*1*]
> where we produce a tarball with "git archive" and then try to
> validate it with the platform implementation of "tar" so that the
> test won't barf when it sees the extended pax header.
> 
> t5000 is littered with similar tests that break unnecessarily with
> BSD implementations.  I think what it wants to test are:
> 
>   - "archive" produces tarball, and "tar" can extract from it.  If
>     your "tar" does not understand pax header, you may get it as an
>     extra file that shouldn't be there, but that should not cause the
>     test to fail---in real life, people without a pax-aware "tar"
>     will just ignore and remove the header and can go on.
> 
>   - "get-tar-commmit-id" can inspect a tarball produced by "archive"
>     to recover the object name of the commit even on a platform
>     without a pax-aware "tar".
> 
> Perhaps t5000 can be restructured like so:
> 
>   - create a tarball with the commit-id and test with
>     "get-tar-commit-id" to validate it; also create a tarball out of
>     a tree to make sure it does not have commit-id and check with
>     "get-tar-commit-id".  Do this on any and all platform, even on
>     the ones without a pax-aware "tar".
> 
>   - check platform implementation of "tar" early to see if extracting
>     a simple output from "git archive" results in an extra pax header
>     file.  If so, remember this fact and produce any and all tarballs
>     used in the remainder of the test by forcing ^{tree}.
> 
> so that people on platforms without pax-aware "tar" do not have to
> install GNU tar only to pass this test.
> 
> It would be a good exercise during the holiday week for somebody on
> BSD (it seems NetBSD is more troublesome than OpenBSD) to come up
> with a patch to help users on these platforms.

I got around to building a virtual machine with NetBSD 6.0.1, which
involved a bit of cursing because networking only seems to work when I
turn off ACPI and SMP -- and running tests is a lot more pleasant with
more than one CPU.

Anyway, I don't think the pax headers are to blame here.  The patch below
fixes the tar failures for me, but I'm not sure why.  There must be
something special about some (not all!) directory entries with trailing
slashes after directory names.

Several ZIP tests still fail, however, because NetBSD's unzip doesn't
seem to support (our flavour of) symlinks and streamed files.

I'll take a deeper look into it over the weekend.

René


---
 archive-tar.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/archive-tar.c b/archive-tar.c
index 0ba3f25..fd2d3e8 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -215,6 +215,8 @@ static int write_tar_entry(struct archiver_args *args,
 	if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
 		*header.typeflag = TYPEFLAG_DIR;
 		mode = (mode | 0777) & ~tar_umask;
+		if (pathlen && path[pathlen - 1] == '/')
+			pathlen--;
 	} else if (S_ISLNK(mode)) {
 		*header.typeflag = TYPEFLAG_LNK;
 		mode |= 0777;
-- 
1.7.12

^ permalink raw reply related

* Re: [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Junio C Hamano @ 2013-01-05 19:38 UTC (permalink / raw)
  To: Anand Kumria; +Cc: Paul Mackerras, git
In-Reply-To: <CAM1C4Gm_ea8DgrVhnp_MHmqaF6pyDe98EDA_BPkjvc8M5AO6FQ@mail.gmail.com>

Anand Kumria <wildfire@progsoc.org> writes:

> Sorry, I didn't know that gitk had been split back out (and
> Documentation/gitk.txt still mentions it is part of the git suite).

It is not "split back" at all, and it won't be.  From "git" user's
point of view it is part of the suite.

Gitk however is still a viable freestanding project, so it would be
selfish for me to take a patch to gitk-git/gitk directly to my tree,
as the patch will not be able to flow back to the standalone gitk
project. Hence we always let patches go through Paul's tree and then
I pull from him.

^ permalink raw reply

* Re: [PATCH 00/10] push: switch default from "matching" to "simple"
From: Matthieu Moy @ 2013-01-05 19:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

> We promised to change the behaviour of lazy "git push [there]" that
> does not say what to push on the command line from "matching" to
> "simple" in Git 2.0.  Even though the top-level RelNotes symbolic
> link points at 1.8.2, we can change our mind to tag 2.0 at the end
> of this cycle.

I had a quick look at the series, it sounds straightforward and correct.
Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [LHF] making t5000 "tar xf" tests more lenient
From: Junio C Hamano @ 2013-01-05 19:43 UTC (permalink / raw)
  To: René Scharfe; +Cc: git
In-Reply-To: <50E8722B.8010408@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Anyway, I don't think the pax headers are to blame here.  The patch below
> fixes the tar failures for me, but I'm not sure why.  There must be
> something special about some (not all!) directory entries with trailing
> slashes after directory names.
>
> Several ZIP tests still fail, however, because NetBSD's unzip doesn't
> seem to support (our flavour of) symlinks and streamed files.

Just FYI, I found that unzip that comes with base distro and the one
you can add via pkg_add (or pkgin) are different, and the latter
seemed to behave better.

^ permalink raw reply

* Re: [PATCH] run-command: encode signal death as a positive integer
From: Johannes Sixt @ 2013-01-05 19:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Bart Trojanowski
In-Reply-To: <20130105144949.GA24479@sigill.intra.peff.net>

Am 05.01.2013 15:49, schrieb Jeff King:
> On Sat, Jan 05, 2013 at 09:03:16AM -0500, Jeff King wrote:
> 
>> In fact, I really wonder if this code from wait_or_whine is actually
>> correct:
>>
>>   code = WTERMSIG(status);
>>   /*
>>    * This return value is chosen so that code & 0xff
>>    * mimics the exit code that a POSIX shell would report for
>>    * a program that died from this signal.
>>    */
>>   code -= 128;
> 
> After looking at it some more, it is correct, but I think we could make
> life slightly easier for callers. See the patch below.  I've tried to
> re-state the somewhat rambling argument from my previous email;
> hopefully it makes sense.
> 
> -- >8 --
> Subject: [PATCH] run-command: encode signal death as a positive integer
> 
> When a sub-command dies due to a signal, we encode the
> signal number into the numeric exit status as "signal -
> 128". This is easy to identify (versus a regular positive
> error code), and when cast to an unsigned integer (e.g., by
> feeding it to exit), matches what a POSIX shell would return
> when reporting a signal death in $? or through its own exit
> code.
> 
> So we have a negative value inside the code, but once it
> passes across an exit() barrier, it looks positive (and any
> code we receive from a sub-shell will have the positive
> form). E.g., death by SIGPIPE (signal 13) will look like
> -115 to us in inside git, but will end up as 141 when we
> call exit() with it. And a program killed by SIGPIPE but run
> via the shell will come to us with an exit code of 141.
> 
> Unfortunately, this means that when the "use_shell" option
> is set, we need to be on the lookout for _both_ forms. We
> might or might not have actually invoked the shell (because
> we optimize out some useless shell calls). If we didn't invoke
> the shell, we will will see the sub-process's signal death
> directly, and run-command converts it into a negative value.
> But if we did invoke the shell, we will see the shell's
> 128+signal exit status. To be thorough, we would need to
> check both, or cast the value to an unsigned char (after
> checking that it is not -1, which is a magic error value).
> 
> Fortunately, most callsites do not care at all whether the
> exit was from a code or from a signal; they merely check for
> a non-zero status, and sometimes propagate the error via
> exit(). But for the callers that do care, we can make life
> slightly easier by just using the consistent positive form.
> 
> This actually fixes two minor bugs:
> 
>   1. In launch_editor, we check whether the editor died from
>      SIGINT or SIGQUIT. But we checked only the negative
>      form, meaning that we would fail to notice a signal
>      death exit code which was propagated through the shell.
> 
>   2. In handle_alias, we assume that a negative return value
>      from run_command means that errno tells us something
>      interesting (like a fork failure, or ENOENT).
>      Otherwise, we simply propagate the exit code. Negative
>      signal death codes confuse us, and we print a useless
>      "unable to run alias 'foo': Success" message. By
>      encoding signal deaths using the positive form, the
>      existing code just propagates it as it would a normal
>      non-zero exit code.
> 
> The downside is that callers of run_command can no longer
> differentiate between a signal received directly by the
> sub-process, and one propagated. However, no caller
> currently cares, and since we already optimize out some
> calls to the shell under the hood, that distinction is not
> something that should be relied upon by callers.

The idea was initially to regard death by signal as an internal error.
But since (1) there are no callers that are really interested in the
difference and (2) we get it wrong in the shell case anyway, this change
makes total sense.

Acked-by: Johannes Sixt <j6t@kdbg.org>

> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/technical/api-run-command.txt | 6 ++----
>  editor.c                                    | 2 +-
>  run-command.c                               | 2 +-
>  3 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
> index f18b4f4..5d7d7f2 100644
> --- a/Documentation/technical/api-run-command.txt
> +++ b/Documentation/technical/api-run-command.txt
> @@ -55,10 +55,8 @@ The functions above do the following:
>    non-zero.
>  
>  . If the program terminated due to a signal, then the return value is the
> -  signal number - 128, ie. it is negative and so indicates an unusual
> -  condition; a diagnostic is printed. This return value can be passed to
> -  exit(2), which will report the same code to the parent process that a
> -  POSIX shell's $? would report for a program that died from the signal.
> +  signal number + 128, ie. the same value that a POSIX shell's $? would
> +  report.  A diagnostic is printed.
>  
>  
>  `start_async`::
> diff --git a/editor.c b/editor.c
> index 065a7ab..27bdecd 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -51,7 +51,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
>  		sigchain_push(SIGINT, SIG_IGN);
>  		sigchain_push(SIGQUIT, SIG_IGN);
>  		ret = finish_command(&p);
> -		sig = ret + 128;
> +		sig = ret - 128;
>  		sigchain_pop(SIGINT);
>  		sigchain_pop(SIGQUIT);
>  		if (sig == SIGINT || sig == SIGQUIT)
> diff --git a/run-command.c b/run-command.c
> index 757f263..cfb7274 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -249,7 +249,7 @@ static int wait_or_whine(pid_t pid, const char *argv0)
>  		 * mimics the exit code that a POSIX shell would report for
>  		 * a program that died from this signal.
>  		 */
> -		code -= 128;
> +		code += 128;
>  	} else if (WIFEXITED(status)) {
>  		code = WEXITSTATUS(status);
>  		/*
> 

^ permalink raw reply

* Re: [LHF] making t5000 "tar xf" tests more lenient
From: Junio C Hamano @ 2013-01-05 20:11 UTC (permalink / raw)
  To: René Scharfe; +Cc: git
In-Reply-To: <50E8722B.8010408@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Anyway, I don't think the pax headers are to blame here.

Hmph, I am reasonably sure I saw a test that created an archive from
a commit (hence with pax header), asked platform tar to either list
the contents or actually extracted to the filesystem, and tried to
ensure nothing but the paths in the repository existed in the
archive.  When the platform tar implementation treated the pax
header as an extra file, such a test sees something not in the
repository and fails.

This was on (and I am still on) NetBSD 6.0 not 6.0.1, by the way.

^ permalink raw reply

* [PATCH] clone: support atomic operation with --separate-git-dir
From: Jens Lehmann @ 2013-01-05 20:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Heiko Voigt, git, Manlio Perillo, W. Trevor King,
	Nguyen Thai Ngoc Duy
In-Reply-To: <50E83DAE.1080500@web.de>

Since b57fb80a7d (init, clone: support --separate-git-dir for .git file)
git clone supports the --separate-git-dir option to create the git dir
outside the work tree. But when that option is used, the git dir won't be
deleted in case the clone fails like it would be without this option. This
makes clone lose its atomicity as in case of a failure a partly set up git
dir is left behind. A real world example where this leads to problems is
when "git submodule update" fails to clone a submodule and later calls to
"git submodule update" stumble over the partially set up git dir and try
to revive the submodule from there, which then fails with a not very user
friendly error message.

Fix that by updating the junk_git_dir variable (used to remember if and
what git dir should be removed in case of failure) to the new value given
with the --seperate-git-dir option. Also add a test for this to t5600 (and
while at it fix the former last test to not cd into a directory to test
for its existence but use "test -d" instead).

Reported-by: Manlio Perillo <manlio.perillo@gmail.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---


Am 05.01.2013 15:50, schrieb Jens Lehmann:
> Am 05.01.2013 15:01, schrieb Jens Lehmann:
>> The reason seems to be that clone leaves a partial initialized .git
>> directory in case of connection problems. The next time submodule
>> update runs it tries to revive the submodule from .git/modules/<name>
>> but fails as there are no objects in it.
>>
>> This looks like a bug in clone to me, as it takes precautions to clean
>> up if something goes wrong but doesn't do that in this case. But while
>> glancing over the code I didn't find out what goes wrong here.
> 
> I dug a bit deeper here and found the reason: In remove_junk() of
> builtin/clone.c the junk_git_dir points to the gitfile in the
> submodules work tree, not the .git/modules/<name> directory where
> clone was told to put the git directory. Will see if I can come up
> with a patch soonish ...

And this fixes it for me. Manlio, it'd be great if you could test
this patch (but please not only remove .git/modules/<name> but also
the submodule work tree before doing that).


 builtin/clone.c               |  4 +++-
 t/t5600-clone-fail-cleanup.sh | 12 +++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index ec2f75b..8d23a62 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -771,8 +771,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		die(_("could not create leading directories of '%s'"), git_dir);

 	set_git_dir_init(git_dir, real_git_dir, 0);
-	if (real_git_dir)
+	if (real_git_dir) {
 		git_dir = real_git_dir;
+		junk_git_dir = real_git_dir;
+	}

 	if (0 <= option_verbosity) {
 		if (option_bare)
diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh
index ee06d28..4435693 100755
--- a/t/t5600-clone-fail-cleanup.sh
+++ b/t/t5600-clone-fail-cleanup.sh
@@ -37,6 +37,16 @@ test_expect_success \

 test_expect_success \
     'successful clone must leave the directory' \
-    'cd bar'
+    'test -d bar'
+
+test_expect_success 'failed clone --separate-git-dir should not leave any directories' '
+	mkdir foo/.git/objects.bak/ &&
+	mv foo/.git/objects/* foo/.git/objects.bak/ &&
+	test_must_fail git clone --separate-git-dir gitdir foo worktree &&
+	test_must_fail test -e gitdir &&
+	test_must_fail test -e worktree &&
+	mv foo/.git/objects.bak/* foo/.git/objects/ &&
+	rmdir foo/.git/objects.bak
+'

 test_done
-- 
1.8.1.81.gb1e9864

^ permalink raw reply related

* RE: [PATCH v4] git-completion.bash: add support for path completion
From: Marc Khouzam @ 2013-01-05 20:23 UTC (permalink / raw)
  To: Junio C Hamano, Manlio Perillo
  Cc: git@vger.kernel.org, szeder@ira.uka.de,
	felipe.contreras@gmail.com
In-Reply-To: <7vehi0qh4x.fsf@alter.siamese.dyndns.org>


> Junio C Hamano <gitster@pobox.com> writes:
> 
>>  Marc Khouzam <marc.khouzam@ericsson.com> writes:
>>
>>> I've been playing with it but I'm not getting the expected
>>> behavior when I cd to a sub-directory.
>>
>> Thanks for testing.  Manlio?
> 
> Can you try the attached patch?

Thanks for this, it improves the situation dramatically.
I did further testing with your patch and found some less obvious
issues.  I didn't debug the script myself as I'm not that familiar with
it either, but I think the testcases below should help Manlio or
someone else look into some regressions.

1- Using .. or . breaks completion when after the '/':
> cd git/contrib
> git rm ../contrib/completion/<tab>
../contrib/completion/ion.bash  ../contrib/completion/ion.tcsh  ../contrib/completion/ion.zsh   ../contrib/completion/sh
It looks like the space taken by the path where we are located, eats up the same number of characters from the file name, e.g.,
   ../contrib/completion/ion.bash
   ../contrib/git-completion.bash

2- Maybe related to problem 1.  Using .. breaks completion in other ways:
> cd git/contrib
> git rm ../con<tab>
../config.c       ../config.mak.in  ../configure.ac   ../connect.c      ../connected.c    ../connected.h    ../convert.c      ../convert.h
Notice that 'contrib' is not proposed.
Don't be fooled by the fact that
> git rm ../cont<tab>
will complete to 
> git rm ../contrib
as it is only because the completion script returned nothing, and bash file-completion 
kicked-in instead (it fooled me :)).

3- Also probably related to problems 1 and 2.  Using absolute paths behaves wierdly and 
worse than before:
git rm /home/marc/git/git/con<tab>
will give
git rm /home/marc/git/git/config-
although there is the 'contrib' dir that should be shown, amongst others.
Seems like the same problem as above with 'git rm ../con<tab>'

In my opinion, the above three cases are regressions.

4- Completion choices include their entire path, which is not what bash does by default.  For example:
> cd git/contrib
> ls completion/git-<tab>
git-completion.bash  git-completion.tcsh  git-completion.zsh   git-prompt.sh
but
> git rm completion/git-<tab>
completion/git-completion.bash  completion/git-completion.tcsh  completion/git-completion.zsh   completion/git-prompt.sh
notice the extra 'completion/' before each completion.  This can get pretty large when completing with 
many directory prefixes.  The current tcsh completion has the same problem which I couldn't fix.  However, I am 
not sure if it can be fixed for bash.

I personally don't think this is regression, just an slight annoyance.

5- With this feature git-completion.bash will return directories as completions.  This is something
git-completion.tcsh is not handling very well.  I will post a patch to fix that.

Below are two suggestions that are in line with this effort but that are not regressions.

A) It would be nice if 
git commit -a <TAB>
also completed with untracked files

B) Now that, for example, 'git rm' completion is smart about path completion 
it would be nice to somehow not trigger bash default file completion
when 'git rm' does not report any completions.  

For example, if I have a file called zz.tar.gz (which is an ignored file) 
and I do 'git rm <tab>', I will get the proper list of files that can be
removed by git, excluding zz.tar.gz.  But if I complete
'git rm zz.tar.<tab>' then the completion script will return nothing,
since git cannot remove that ignored file, but we will then fall-back
to the bash default completion, which will complete the file to zz.tar.gz.

Although there are some issues, I think this feature will greatly benefit the user
and is worth the time needed to fix.

Thanks!

Marc

^ permalink raw reply

* [PATCH] status: report ignored yet tracked directories
From: Antoine Pelisse @ 2013-01-05 20:42 UTC (permalink / raw)
  To: tboegi, peff; +Cc: git, Antoine Pelisse
In-Reply-To: <20130105112432.GA14666@sigill.intra.peff.net>

Tracked directories (i.e. directories containing tracked files) that
are ignored must be reported as ignored if they contain untracked files.

Currently, tracked files or directories can't be reported untracked or ignored.
Remove that constraint when searching ignored files.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
Torsten, Jeff,

Can you please test this patch and tell me if this is better ? (t7061 is now
successful with core.ignorecase=true)

This patch applies on top of ap/status-ignored-in-ignored-directory (but
 should also apply cleanly on top of next for testing purpose).

Thanks,

 dir.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 9b80348..eefa8ab 100644
--- a/dir.c
+++ b/dir.c
@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)

 static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
 {
-	if (cache_name_exists(pathname, len, ignore_case))
+	if (!(dir->flags & DIR_SHOW_IGNORED) &&
+	    cache_name_exists(pathname, len, ignore_case))
 		return NULL;

 	ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
--
1.7.12.4.2.geb8c5b8.dirty

^ permalink raw reply related

* [Feature request] make git buildable from a separate directory
From: Manlio Perillo @ 2013-01-05 20:52 UTC (permalink / raw)
  To: git

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi.

Many C projects I have seen (based on autoconf, but not always - like
Nginx) allow the project to be build in a separate directory, in order
to avoid to pollute the working directory with compiled files.

Unfortunately this seems to not be possible with Git.
The Makefile seems quite complex to me, so I'm not sure to be able to
change it to do what I want, without breaking it.


Thanks  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDoknUACgkQscQJ24LbaUTw/QCdHbphkU3Mepo98D07yLaj3YyF
5I4Anii94QDHsC1zm2Jp1hy2X/JFa/NE
=vV1z
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH] clone: support atomic operation with --separate-git-dir
From: Manlio Perillo @ 2013-01-05 21:20 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Junio C Hamano, Heiko Voigt, git, W. Trevor King,
	Nguyen Thai Ngoc Duy
In-Reply-To: <50E88A40.9010904@web.de>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 05/01/2013 21:17, Jens Lehmann ha scritto:
> Since b57fb80a7d (init, clone: support --separate-git-dir for .git file)
> git clone supports the --separate-git-dir option to create the git dir
> outside the work tree. But when that option is used, the git dir won't be
> deleted in case the clone fails like it would be without this option. This
> makes clone lose its atomicity as in case of a failure a partly set up git
> dir is left behind. A real world example where this leads to problems is
> when "git submodule update" fails to clone a submodule and later calls to
> "git submodule update" stumble over the partially set up git dir and try
> to revive the submodule from there, which then fails with a not very user
> friendly error message.
> 
> Fix that by updating the junk_git_dir variable (used to remember if and
> what git dir should be removed in case of failure) to the new value given
> with the --seperate-git-dir option. Also add a test for this to t5600 (and
> while at it fix the former last test to not cd into a directory to test
> for its existence but use "test -d" instead).
> 
> Reported-by: Manlio Perillo <manlio.perillo@gmail.com>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
> [...]
> And this fixes it for me. Manlio, it'd be great if you could test
> this patch (but please not only remove .git/modules/<name> but also
> the submodule work tree before doing that).
> 

I can confirm that the patch solves the problem I reported.


Thanks   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDomR4ACgkQscQJ24LbaUQszACfV42L9Xcy+mme6RY/vY+K2H4T
QDAAoIIupUSjwv6qUgzUMQV1aNplrWJD
=uN3W
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: Suggested improvements to the git-p4 documentation (branch-related)
From: Pete Wyckoff @ 2013-01-05 21:25 UTC (permalink / raw)
  To: Olivier Delalleau; +Cc: git
In-Reply-To: <CAFXk4bqt_pMVDtVKF-JiQuGbSpy2+_rGOg5RTTE+0pNKFcZh3w@mail.gmail.com>

shish@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500:
> While struggling to get git-p4 to work properly with branches, I
> thought the documentation on http://git-scm.com/docs/git-p4 could use
> some improvements:

Thanks, I definitely appreciate the constructive comments here.

> 1. At the end of the "Branch detection" section, the following
> commands are provided (for when you want to explicitly provide branch
> mappings to git-p4):
> 
> git config git-p4.branchList main:branch1
> git p4 clone --detect-branches //depot@all
> 
> The second command should end with a dot (".") because the first
> command only works if you are already in a git-initialized folder.
> Thus I would also suggest to add "git init" as first command to type.

That is confusing.  I'll make it this:

    git init depot
    cd depot
    git config git-p4.branchList main:branch1
    git p4 clone --detect-branches //depot@all .

> 2. Even though having a "main" branch is standard in Perforce, it
> would be worth mentioning what happens when you don't: there is a
> message "Could not detect main branch. No checkout/master branch
> created" output by the "git p4 clone" command. However, it will still
> work if you manually set the master branch ("git checkout -b master
> remotes/p4/my_custom_main_branch").

This feels like a bug to me, and indeed I had an old patch series
that planned to fix it.  Let me knock that into shape, instead of
changing the documentation.  It will automatically do the
checkout step you did.

> 3. I don't know what I missed for that one, but I haven't been able to
> get the example for the --branch option to work. It says that after
> "git init", we can import a p4 branch with:
> 
> git p4 sync --branch=refs/remotes/p4/proj2 //depot/proj2
> 
> However, after doing this, followed by "git checkout -b proj2
> remotes/p4/proj2", I am unable to properly use "git p4 sync" or "git
> p4 submit" from this branch, as git complains about a missing
> refs/remotes/p4/master.

Yes, also annoying.  I have a failing test case for this, but
haven't fixed it yet.  The idea is that "git p4 sync --branch=proj2"
will sync refs/remotes/p4/proj2.  If there is no p4/master, and
you don't specify --branch, it will fail with a more useful error
message.

For submit, there is code that walks from your current branch
back in history until it finds a commit on a known p4 remote
branch.  This is sort of like the merge-base calculation in git,
but restricted to a linear history.  I haven't tested that
recently, but will add a test and fix it if needed too.


Please do feel welcome to to rearrange or expand the
documentation so it makes more sense, if you are so inspired.

		-- Pete

^ permalink raw reply

* [PATCH 00/10] Log mailmap optimization
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git

Hi,

Here is a reroll of ap/log-mailmap.
The idea is to use another preparatory step:
Allow case-insensitive and length search in list_lookup

We can now search for mapping name and email without any copy.
Of course a copy is then necessary to store the info, but we no
longer need any copy to look-up the mapping (useful for replacing or not
before grep).

Thanks,

Antoine Pelisse (10):
  list_lookup: create case and length search
  Use split_ident_line to parse author and committer
  mailmap: remove email copy and length limitation
  mailmap: simplify map_user() interface
  mailmap: add mailmap structure to rev_info and pp
  pretty: use mailmap to display username and email
  log: add --use-mailmap option
  test: add test for --use-mailmap option
  log: grep author/committer using mailmap
  log: add log.mailmap configuration option

 Documentation/config.txt  |   4 +
 Documentation/git-log.txt |   5 ++
 builtin/blame.c           | 183 ++++++++++++++++++++++------------------------
 builtin/log.c             |  16 +++-
 builtin/shortlog.c        |  54 ++++----------
 commit.h                  |   1 +
 log-tree.c                |   1 +
 mailmap.c                 |  55 +++++---------
 mailmap.h                 |   4 +-
 pretty.c                  | 114 ++++++++++++++++-------------
 revision.c                |  54 ++++++++++++++
 revision.h                |   1 +
 string-list.c             |  30 ++++++--
 string-list.h             |   2 +
 t/t4203-mailmap.sh        |  56 ++++++++++++++
 15 files changed, 349 insertions(+), 231 deletions(-)

--
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply

* [PATCH 01/10] list_lookup: create case and length search
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

Create a new function to look-up a string in a string_list, but:
 - add a new parameter to ignore case differences
 - add a length parameter to search for a substring

The idea is to avoid several copies (lowering a string before searching
it when we just want to ignore case), or copying a substring of a bigger
string to search it in the string_list

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 string-list.c | 30 ++++++++++++++++++++++++------
 string-list.h |  2 ++
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/string-list.c b/string-list.c
index 397e6cf..f06e110 100644
--- a/string-list.c
+++ b/string-list.c
@@ -4,13 +4,21 @@
 /* if there is no exact match, point to the index where the entry could be
  * inserted */
 static int get_entry_index(const struct string_list *list, const char *string,
-		int *exact_match)
+		int *exact_match, int case_sensitive, size_t n)
 {
 	int left = -1, right = list->nr;
 
 	while (left + 1 < right) {
+		int compare;
 		int middle = (left + right) / 2;
-		int compare = strcmp(string, list->items[middle].string);
+		if (case_sensitive)
+			compare = strncmp(string, list->items[middle].string, n);
+		else
+			compare = strncasecmp(string, list->items[middle].string, n);
+		/* Make sure our string is not a substring of item string */
+		if (!compare && n != -1)
+			if (list->items[middle].string[n] != '\0')
+				compare = -1;
 		if (compare < 0)
 			right = middle;
 		else if (compare > 0)
@@ -29,7 +37,7 @@ static int get_entry_index(const struct string_list *list, const char *string,
 static int add_entry(int insert_at, struct string_list *list, const char *string)
 {
 	int exact_match = 0;
-	int index = insert_at != -1 ? insert_at : get_entry_index(list, string, &exact_match);
+	int index = insert_at != -1 ? insert_at : get_entry_index(list, string, &exact_match, 1, -1);
 
 	if (exact_match)
 		return -1 - index;
@@ -70,7 +78,7 @@ struct string_list_item *string_list_insert_at_index(struct string_list *list,
 int string_list_has_string(const struct string_list *list, const char *string)
 {
 	int exact_match;
-	get_entry_index(list, string, &exact_match);
+	get_entry_index(list, string, &exact_match, 1, -1);
 	return exact_match;
 }
 
@@ -78,7 +86,7 @@ int string_list_find_insert_index(const struct string_list *list, const char *st
 				  int negative_existing_index)
 {
 	int exact_match;
-	int index = get_entry_index(list, string, &exact_match);
+	int index = get_entry_index(list, string, &exact_match, 1, -1);
 	if (exact_match)
 		index = -1 - (negative_existing_index ? index : 0);
 	return index;
@@ -86,7 +94,17 @@ int string_list_find_insert_index(const struct string_list *list, const char *st
 
 struct string_list_item *string_list_lookup(struct string_list *list, const char *string)
 {
-	int exact_match, i = get_entry_index(list, string, &exact_match);
+	int exact_match, i = get_entry_index(list, string, &exact_match, 1, -1);
+	if (!exact_match)
+		return NULL;
+	return list->items + i;
+}
+
+struct string_list_item *string_list_lookup_extended(struct string_list *list,
+    const char *string, int case_sensitive, size_t n)
+{
+	int exact_match, i = get_entry_index(list, string, &exact_match,
+					     case_sensitive, n);
 	if (!exact_match)
 		return NULL;
 	return list->items + i;
diff --git a/string-list.h b/string-list.h
index c50b0d0..4f5ae19 100644
--- a/string-list.h
+++ b/string-list.h
@@ -62,6 +62,8 @@ struct string_list_item *string_list_insert(struct string_list *list, const char
 struct string_list_item *string_list_insert_at_index(struct string_list *list,
 						     int insert_at, const char *string);
 struct string_list_item *string_list_lookup(struct string_list *list, const char *string);
+struct string_list_item *string_list_lookup_extended(struct string_list *list,
+    const char *string, int case_sensitive, size_t n);
 
 /*
  * Remove all but the first of consecutive entries with the same
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related

* [PATCH 02/10] Use split_ident_line to parse author and committer
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

Currently blame.c::get_acline, pretty.c::pp_user_info() and
shortlog.c::insert_one_record are parsing author name, email, time and
tz themselves.

Use ident.c::split_ident_line for better code reuse.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 builtin/blame.c    | 59 +++++++++++++++++++-----------------------------------
 builtin/shortlog.c | 36 +++++++++------------------------
 pretty.c           | 35 +++++++++++++++++++-------------
 3 files changed, 52 insertions(+), 78 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index cfae569..dd4aff9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1343,8 +1343,9 @@ static void get_ac_line(const char *inbuf, const char *what,
 			int mail_len, char *mail,
 			unsigned long *time, const char **tz)
 {
-	int len, tzlen, maillen;
-	char *tmp, *endp, *timepos, *mailpos;
+	struct ident_split ident;
+	int len, tzlen, maillen, namelen;
+	char *tmp, *endp, *mailpos;
 
 	tmp = strstr(inbuf, what);
 	if (!tmp)
@@ -1355,7 +1356,10 @@ static void get_ac_line(const char *inbuf, const char *what,
 		len = strlen(tmp);
 	else
 		len = endp - tmp;
-	if (person_len <= len) {
+	if (person_len <= len)
+		goto error_out;
+
+	if (split_ident_line(&ident, tmp, len)) {
 	error_out:
 		/* Ugh */
 		*tz = "(unknown)";
@@ -1364,47 +1368,26 @@ static void get_ac_line(const char *inbuf, const char *what,
 		*time = 0;
 		return;
 	}
-	memcpy(person, tmp, len);
 
-	tmp = person;
-	tmp += len;
-	*tmp = 0;
-	while (person < tmp && *tmp != ' ')
-		tmp--;
-	if (tmp <= person)
-		goto error_out;
-	*tz = tmp+1;
-	tzlen = (person+len)-(tmp+1);
+	namelen = ident.name_end - ident.name_begin;
+	memcpy(person, ident.name_begin, namelen);
+	person[namelen] = 0;
 
-	*tmp = 0;
-	while (person < tmp && *tmp != ' ')
-		tmp--;
-	if (tmp <= person)
-		goto error_out;
-	*time = strtoul(tmp, NULL, 10);
-	timepos = tmp;
+	maillen = ident.mail_end - ident.mail_begin + 2;
+	memcpy(mail, ident.mail_begin - 1, maillen);
+	mail[maillen] = 0;
 
-	*tmp = 0;
-	while (person < tmp && !(*tmp == ' ' && tmp[1] == '<'))
-		tmp--;
-	if (tmp <= person)
-		return;
-	mailpos = tmp + 1;
-	*tmp = 0;
-	maillen = timepos - tmp;
-	memcpy(mail, mailpos, maillen);
+	*time = strtoul(ident.date_begin, NULL, 10);
 
-	if (!mailmap.nr)
-		return;
+	tzlen = ident.tz_end - ident.tz_begin;
 
-	/*
-	 * mailmap expansion may make the name longer.
-	 * make room by pushing stuff down.
-	 */
-	tmp = person + person_len - (tzlen + 1);
-	memmove(tmp, *tz, tzlen);
+	/* Place tz at the end of person */
+	*tz = tmp = person + person_len - (tzlen + 1);
+	memcpy(tmp, ident.tz_begin, tzlen);
 	tmp[tzlen] = 0;
-	*tz = tmp;
+
+	if (!mailmap.nr)
+		return;
 
 	/*
 	 * Now, convert both name and e-mail using mailmap
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index b316cf3..03c6cd7 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -40,40 +40,24 @@ static void insert_one_record(struct shortlog *log,
 	char emailbuf[1024];
 	size_t len;
 	const char *eol;
-	const char *boemail, *eoemail;
 	struct strbuf subject = STRBUF_INIT;
+	struct ident_split ident;
 
-	boemail = strchr(author, '<');
-	if (!boemail)
-		return;
-	eoemail = strchr(boemail, '>');
-	if (!eoemail)
+	if (split_ident_line(&ident, author, strlen(author)))
 		return;
 
 	/* copy author name to namebuf, to support matching on both name and email */
-	memcpy(namebuf, author, boemail - author);
-	len = boemail - author;
-	while (len > 0 && isspace(namebuf[len-1]))
-		len--;
+	len = ident.name_end - ident.name_begin;
+	memcpy(namebuf, ident.name_begin, len);
 	namebuf[len] = 0;
 
 	/* copy email name to emailbuf, to allow email replacement as well */
-	memcpy(emailbuf, boemail+1, eoemail - boemail);
-	emailbuf[eoemail - boemail - 1] = 0;
-
-	if (!map_user(&log->mailmap, emailbuf, sizeof(emailbuf), namebuf, sizeof(namebuf))) {
-		while (author < boemail && isspace(*author))
-			author++;
-		for (len = 0;
-		     len < sizeof(namebuf) - 1 && author + len < boemail;
-		     len++)
-			namebuf[len] = author[len];
-		while (0 < len && isspace(namebuf[len-1]))
-			len--;
-		namebuf[len] = '\0';
-	}
-	else
-		len = strlen(namebuf);
+	len = ident.mail_end - ident.mail_begin;
+	memcpy(emailbuf, ident.mail_begin, len);
+	emailbuf[len] = 0;
+
+	map_user(&log->mailmap, emailbuf, sizeof(emailbuf), namebuf, sizeof(namebuf));
+	len = strlen(namebuf);
 
 	if (log->email) {
 		size_t room = sizeof(namebuf) - len - 1;
diff --git a/pretty.c b/pretty.c
index 5bdc2e7..350d1df 100644
--- a/pretty.c
+++ b/pretty.c
@@ -387,29 +387,36 @@ void pp_user_info(const struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
 {
+	struct ident_split ident;
+	int linelen, namelen;
+	char *line_end, *date;
 	int max_length = 78; /* per rfc2822 */
-	char *date;
-	int namelen;
 	unsigned long time;
 	int tz;
 
 	if (pp->fmt == CMIT_FMT_ONELINE)
 		return;
-	date = strchr(line, '>');
-	if (!date)
+
+	line_end = strchr(line, '\n');
+	if (!line_end) {
+		line_end = strchr(line, '\0');
+		if (!line_end)
+			return;
+	}
+
+	linelen = ++line_end - line;
+	if (split_ident_line(&ident, line, linelen))
 		return;
-	namelen = ++date - line;
-	time = strtoul(date, &date, 10);
+
+	namelen = ident.mail_end - ident.name_begin + 1;
+	time = strtoul(ident.date_begin, &date, 10);
 	tz = strtol(date, NULL, 10);
 
 	if (pp->fmt == CMIT_FMT_EMAIL) {
-		char *name_tail = strchr(line, '<');
 		int display_name_length;
-		if (!name_tail)
-			return;
-		while (line < name_tail && isspace(name_tail[-1]))
-			name_tail--;
-		display_name_length = name_tail - line;
+
+		display_name_length = ident.name_end - ident.name_begin;
+
 		strbuf_addstr(sb, "From: ");
 		if (needs_rfc2047_encoding(line, display_name_length, RFC2047_ADDRESS)) {
 			add_rfc2047(sb, line, display_name_length,
@@ -427,10 +434,10 @@ void pp_user_info(const struct pretty_print_context *pp,
 		}
 		if (namelen - display_name_length + last_line_length(sb) > max_length) {
 			strbuf_addch(sb, '\n');
-			if (!isspace(name_tail[0]))
+			if (!isspace(ident.name_end[0]))
 				strbuf_addch(sb, ' ');
 		}
-		strbuf_add(sb, name_tail, namelen - display_name_length);
+		strbuf_add(sb, ident.name_end, namelen - display_name_length);
 		strbuf_addch(sb, '\n');
 	} else {
 		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related

* [PATCH 03/10] mailmap: remove email copy and length limitation
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

In map_user(), the new string_list_lookup_extended() allows us to remove
the copy of email buffer to lower it.
This also removes the limitation on the length of the copy buffer and
simplifies the function.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 mailmap.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index ea4b471..9db8a1b 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -193,8 +193,7 @@ int map_user(struct string_list *map,
 	char *end_of_email;
 	struct string_list_item *item;
 	struct mailmap_entry *me;
-	char buf[1024], *mailbuf;
-	int i;
+	size_t maillen;
 
 	/* figure out space requirement for email */
 	end_of_email = strchr(email, '>');
@@ -204,18 +203,11 @@ int map_user(struct string_list *map,
 		if (!end_of_email)
 			return 0;
 	}
-	if (end_of_email - email + 1 < sizeof(buf))
-		mailbuf = buf;
-	else
-		mailbuf = xmalloc(end_of_email - email + 1);
-
-	/* downcase the email address */
-	for (i = 0; i < end_of_email - email; i++)
-		mailbuf[i] = tolower(email[i]);
-	mailbuf[i] = 0;
-
-	debug_mm("map_user: map '%s' <%s>\n", name, mailbuf);
-	item = string_list_lookup(map, mailbuf);
+
+	maillen = end_of_email - email;
+
+	debug_mm("map_user: map '%s' <%.*s>\n", name, maillen, email);
+	item = string_list_lookup_extended(map, email, 0, maillen);
 	if (item != NULL) {
 		me = (struct mailmap_entry *)item->util;
 		if (me->namemap.nr) {
@@ -226,8 +218,6 @@ int map_user(struct string_list *map,
 				item = subitem;
 		}
 	}
-	if (mailbuf != buf)
-		free(mailbuf);
 	if (item != NULL) {
 		struct mailmap_info *mi = (struct mailmap_info *)item->util;
 		if (mi->name == NULL && (mi->email == NULL || maxlen_email == 0)) {
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related

* [PATCH 06/10] pretty: use mailmap to display username and email
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

Use the mailmap information to display the correct
username and email address in all log commands.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 pretty.c | 58 +++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/pretty.c b/pretty.c
index dffcade..622275c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -387,9 +387,13 @@ void pp_user_info(const struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
 {
+	struct strbuf name;
+	struct strbuf mail;
 	struct ident_split ident;
-	int linelen, namelen;
+	int linelen;
 	char *line_end, *date;
+	const char *mailbuf, *namebuf;
+	size_t namelen, maillen;
 	int max_length = 78; /* per rfc2822 */
 	unsigned long time;
 	int tz;
@@ -408,42 +412,54 @@ void pp_user_info(const struct pretty_print_context *pp,
 	if (split_ident_line(&ident, line, linelen))
 		return;
 
-	namelen = ident.mail_end - ident.name_begin + 1;
+
+	mailbuf = ident.mail_begin;
+	maillen = ident.mail_end - ident.mail_begin;
+	namebuf = ident.name_begin;
+	namelen = ident.name_end - ident.name_begin;
+
+	if (pp->mailmap)
+		map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
+
+	strbuf_init(&mail, 0);
+	strbuf_init(&name, 0);
+
+	strbuf_add(&mail, mailbuf, maillen);
+	strbuf_add(&name, namebuf, namelen);
+
+	namelen = name.len + mail.len + 3; /* ' ' + '<' + '>' */
 	time = strtoul(ident.date_begin, &date, 10);
 	tz = strtol(date, NULL, 10);
 
 	if (pp->fmt == CMIT_FMT_EMAIL) {
-		int display_name_length;
-
-		display_name_length = ident.name_end - ident.name_begin;
-
 		strbuf_addstr(sb, "From: ");
-		if (needs_rfc2047_encoding(line, display_name_length, RFC2047_ADDRESS)) {
-			add_rfc2047(sb, line, display_name_length,
-						encoding, RFC2047_ADDRESS);
+		if (needs_rfc2047_encoding(name.buf, name.len, RFC2047_ADDRESS)) {
+			add_rfc2047(sb, name.buf, name.len,
+				    encoding, RFC2047_ADDRESS);
 			max_length = 76; /* per rfc2047 */
-		} else if (needs_rfc822_quoting(line, display_name_length)) {
+		} else if (needs_rfc822_quoting(name.buf, name.len)) {
 			struct strbuf quoted = STRBUF_INIT;
-			add_rfc822_quoted(&quoted, line, display_name_length);
+			add_rfc822_quoted(&quoted, name.buf, name.len);
 			strbuf_add_wrapped_bytes(sb, quoted.buf, quoted.len,
 							-6, 1, max_length);
 			strbuf_release(&quoted);
 		} else {
-			strbuf_add_wrapped_bytes(sb, line, display_name_length,
-							-6, 1, max_length);
+			strbuf_add_wrapped_bytes(sb, name.buf, name.len,
+						 -6, 1, max_length);
 		}
-		if (namelen - display_name_length + last_line_length(sb) > max_length) {
+		if (namelen - name.len + last_line_length(sb) > max_length)
 			strbuf_addch(sb, '\n');
-			if (!isspace(ident.name_end[0]))
-				strbuf_addch(sb, ' ');
-		}
-		strbuf_add(sb, ident.name_end, namelen - display_name_length);
-		strbuf_addch(sb, '\n');
+
+		strbuf_addf(sb, " <%s>\n", mail.buf);
 	} else {
-		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
+		strbuf_addf(sb, "%s: %.*s%s <%s>\n", what,
 			      (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0,
-			      "    ", namelen, line);
+			      "    ", name.buf, mail.buf);
 	}
+
+	strbuf_release(&mail);
+	strbuf_release(&name);
+
 	switch (pp->fmt) {
 	case CMIT_FMT_MEDIUM:
 		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, pp->date_mode));
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related

* [PATCH 05/10] mailmap: add mailmap structure to rev_info and pp
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

the mailmap string_list structure filled with mailmap
information is passed along from rev_info to pretty_print_context
to provide mailmap information to pretty print each commits
with the correct username and email.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 commit.h   | 1 +
 log-tree.c | 1 +
 revision.h | 1 +
 3 files changed, 3 insertions(+)

diff --git a/commit.h b/commit.h
index b6ad8f3..7f8f987 100644
--- a/commit.h
+++ b/commit.h
@@ -89,6 +89,7 @@ struct pretty_print_context {
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
 	const char *output_encoding;
+	struct string_list *mailmap;
 };
 
 struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 4f86def..92254fd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)
 	ctx.preserve_subject = opt->preserve_subject;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
+	ctx.mailmap = opt->mailmap;
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
diff --git a/revision.h b/revision.h
index 059bfff..83a79f5 100644
--- a/revision.h
+++ b/revision.h
@@ -143,6 +143,7 @@ struct rev_info {
 	const char	*subject_prefix;
 	int		no_inline;
 	int		show_log_size;
+	struct string_list *mailmap;
 
 	/* Filter by commit log message */
 	struct grep_opt	grep_filter;
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related

* [PATCH 04/10] mailmap: simplify map_user() interface
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

The new string_list_lookup_extended() allows us to simplify map_user(),
mostly to avoid copies of string buffers. It also simplifies caller
functions.

map_user() directly receive pointers and length from the commit buffer
as mail and name. If mapping of the user and mail can be done, the
pointer is updated to a new location. Lengths are also updated if
necessary.

The caller of map_user() can then copy the new email and name if
necessary.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 builtin/blame.c    | 156 +++++++++++++++++++++++++++--------------------------
 builtin/shortlog.c |  32 +++++------
 mailmap.c          |  43 +++++++--------
 mailmap.h          |   4 +-
 pretty.c           |  35 +++++-------
 5 files changed, 126 insertions(+), 144 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index dd4aff9..86450e3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1321,31 +1321,31 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
  * Information on commits, used for output.
  */
 struct commit_info {
-	const char *author;
-	const char *author_mail;
+	struct strbuf author;
+	struct strbuf author_mail;
 	unsigned long author_time;
-	const char *author_tz;
+	struct strbuf author_tz;
 
 	/* filled only when asked for details */
-	const char *committer;
-	const char *committer_mail;
+	struct strbuf committer;
+	struct strbuf committer_mail;
 	unsigned long committer_time;
-	const char *committer_tz;
+	struct strbuf committer_tz;
 
-	const char *summary;
+	struct strbuf summary;
 };
 
 /*
  * Parse author/committer line in the commit object buffer
  */
 static void get_ac_line(const char *inbuf, const char *what,
-			int person_len, char *person,
-			int mail_len, char *mail,
-			unsigned long *time, const char **tz)
+	struct strbuf *name, struct strbuf *mail,
+	unsigned long *time, struct strbuf *tz)
 {
 	struct ident_split ident;
-	int len, tzlen, maillen, namelen;
-	char *tmp, *endp, *mailpos;
+	size_t len, maillen, namelen;
+	char *tmp, *endp;
+	const char *tmpname, *tmpmail;
 
 	tmp = strstr(inbuf, what);
 	if (!tmp)
@@ -1356,51 +1356,61 @@ static void get_ac_line(const char *inbuf, const char *what,
 		len = strlen(tmp);
 	else
 		len = endp - tmp;
-	if (person_len <= len)
-		goto error_out;
 
 	if (split_ident_line(&ident, tmp, len)) {
 	error_out:
 		/* Ugh */
-		*tz = "(unknown)";
-		strcpy(person, *tz);
-		strcpy(mail, *tz);
+		tmp = "(unknown)";
+		strbuf_addstr(name, tmp);
+		strbuf_addstr(mail, tmp);
+		strbuf_addstr(tz, tmp);
 		*time = 0;
 		return;
 	}
 
 	namelen = ident.name_end - ident.name_begin;
-	memcpy(person, ident.name_begin, namelen);
-	person[namelen] = 0;
+	tmpname = ident.name_begin;
 
-	maillen = ident.mail_end - ident.mail_begin + 2;
-	memcpy(mail, ident.mail_begin - 1, maillen);
-	mail[maillen] = 0;
+	maillen = ident.mail_end - ident.mail_begin;
+	tmpmail = ident.mail_begin;
 
 	*time = strtoul(ident.date_begin, NULL, 10);
 
-	tzlen = ident.tz_end - ident.tz_begin;
-
-	/* Place tz at the end of person */
-	*tz = tmp = person + person_len - (tzlen + 1);
-	memcpy(tmp, ident.tz_begin, tzlen);
-	tmp[tzlen] = 0;
-
-	if (!mailmap.nr)
-		return;
+	len = ident.tz_end - ident.tz_begin;
+	strbuf_add(tz, ident.tz_begin, len);
 
 	/*
 	 * Now, convert both name and e-mail using mailmap
 	 */
-	if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
-		/* Add a trailing '>' to email, since map_user returns plain emails
-		   Note: It already has '<', since we replace from mail+1 */
-		mailpos = memchr(mail, '\0', mail_len);
-		if (mailpos && mailpos-mail < mail_len - 1) {
-			*mailpos = '>';
-			*(mailpos+1) = '\0';
-		}
-	}
+	map_user(&mailmap, &tmpmail, &maillen,
+		 &tmpname, &namelen);
+
+	strbuf_addf(mail, "<%.*s>", (int)maillen, tmpmail);
+	strbuf_add(name, tmpname, namelen);
+}
+
+static void commit_info_init(struct commit_info *ci)
+{
+
+	strbuf_init(&ci->author, 0);
+	strbuf_init(&ci->author_mail, 0);
+	strbuf_init(&ci->author_tz, 0);
+	strbuf_init(&ci->committer, 0);
+	strbuf_init(&ci->committer_mail, 0);
+	strbuf_init(&ci->committer_tz, 0);
+	strbuf_init(&ci->summary, 0);
+}
+
+static void commit_info_destroy(struct commit_info *ci)
+{
+
+	strbuf_release(&ci->author);
+	strbuf_release(&ci->author_mail);
+	strbuf_release(&ci->author_tz);
+	strbuf_release(&ci->committer);
+	strbuf_release(&ci->committer_mail);
+	strbuf_release(&ci->committer_tz);
+	strbuf_release(&ci->summary);
 }
 
 static void get_commit_info(struct commit *commit,
@@ -1410,11 +1420,8 @@ static void get_commit_info(struct commit *commit,
 	int len;
 	const char *subject, *encoding;
 	char *reencoded, *message;
-	static char author_name[1024];
-	static char author_mail[1024];
-	static char committer_name[1024];
-	static char committer_mail[1024];
-	static char summary_buf[1024];
+
+	commit_info_init(ret);
 
 	/*
 	 * We've operated without save_commit_buffer, so
@@ -1432,11 +1439,8 @@ static void get_commit_info(struct commit *commit,
 	encoding = get_log_output_encoding();
 	reencoded = logmsg_reencode(commit, encoding);
 	message   = reencoded ? reencoded : commit->buffer;
-	ret->author = author_name;
-	ret->author_mail = author_mail;
 	get_ac_line(message, "\nauthor ",
-		    sizeof(author_name), author_name,
-		    sizeof(author_mail), author_mail,
+		    &ret->author, &ret->author_mail,
 		    &ret->author_time, &ret->author_tz);
 
 	if (!detailed) {
@@ -1444,21 +1448,16 @@ static void get_commit_info(struct commit *commit,
 		return;
 	}
 
-	ret->committer = committer_name;
-	ret->committer_mail = committer_mail;
 	get_ac_line(message, "\ncommitter ",
-		    sizeof(committer_name), committer_name,
-		    sizeof(committer_mail), committer_mail,
+		    &ret->committer, &ret->committer_mail,
 		    &ret->committer_time, &ret->committer_tz);
 
-	ret->summary = summary_buf;
 	len = find_commit_subject(message, &subject);
-	if (len && len < sizeof(summary_buf)) {
-		memcpy(summary_buf, subject, len);
-		summary_buf[len] = 0;
-	} else {
-		sprintf(summary_buf, "(%s)", sha1_to_hex(commit->object.sha1));
-	}
+	if (len)
+		strbuf_add(&ret->summary, subject, len);
+	else
+		strbuf_addf(&ret->summary, "(%s)", sha1_to_hex(commit->object.sha1));
+
 	free(reencoded);
 }
 
@@ -1487,15 +1486,15 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)
 
 	suspect->commit->object.flags |= METAINFO_SHOWN;
 	get_commit_info(suspect->commit, &ci, 1);
-	printf("author %s\n", ci.author);
-	printf("author-mail %s\n", ci.author_mail);
+	printf("author %s\n", ci.author.buf);
+	printf("author-mail %s\n", ci.author_mail.buf);
 	printf("author-time %lu\n", ci.author_time);
-	printf("author-tz %s\n", ci.author_tz);
-	printf("committer %s\n", ci.committer);
-	printf("committer-mail %s\n", ci.committer_mail);
+	printf("author-tz %s\n", ci.author_tz.buf);
+	printf("committer %s\n", ci.committer.buf);
+	printf("committer-mail %s\n", ci.committer_mail.buf);
 	printf("committer-time %lu\n", ci.committer_time);
-	printf("committer-tz %s\n", ci.committer_tz);
-	printf("summary %s\n", ci.summary);
+	printf("committer-tz %s\n", ci.committer_tz.buf);
+	printf("summary %s\n", ci.summary.buf);
 	if (suspect->commit->object.flags & UNINTERESTING)
 		printf("boundary\n");
 	if (suspect->previous) {
@@ -1503,6 +1502,9 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)
 		printf("previous %s ", sha1_to_hex(prev->commit->object.sha1));
 		write_name_quoted(prev->path, stdout, '\n');
 	}
+
+	commit_info_destroy(&ci);
+
 	return 1;
 }
 
@@ -1689,11 +1691,11 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 		if (opt & OUTPUT_ANNOTATE_COMPAT) {
 			const char *name;
 			if (opt & OUTPUT_SHOW_EMAIL)
-				name = ci.author_mail;
+				name = ci.author_mail.buf;
 			else
-				name = ci.author;
+				name = ci.author.buf;
 			printf("\t(%10s\t%10s\t%d)", name,
-			       format_time(ci.author_time, ci.author_tz,
+			       format_time(ci.author_time, ci.author_tz.buf,
 					   show_raw_time),
 			       ent->lno + 1 + cnt);
 		} else {
@@ -1712,14 +1714,14 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 				const char *name;
 				int pad;
 				if (opt & OUTPUT_SHOW_EMAIL)
-					name = ci.author_mail;
+					name = ci.author_mail.buf;
 				else
-					name = ci.author;
+					name = ci.author.buf;
 				pad = longest_author - utf8_strwidth(name);
 				printf(" (%s%*s %10s",
 				       name, pad, "",
 				       format_time(ci.author_time,
-						   ci.author_tz,
+						   ci.author_tz.buf,
 						   show_raw_time));
 			}
 			printf(" %*d) ",
@@ -1734,6 +1736,8 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 
 	if (sb->final_buf_size && cp[-1] != '\n')
 		putchar('\n');
+
+	commit_info_destroy(&ci);
 }
 
 static void output(struct scoreboard *sb, int option)
@@ -1858,9 +1862,9 @@ static void find_alignment(struct scoreboard *sb, int *option)
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci, 1);
 			if (*option & OUTPUT_SHOW_EMAIL)
-				num = utf8_strwidth(ci.author_mail);
+				num = utf8_strwidth(ci.author_mail.buf);
 			else
-				num = utf8_strwidth(ci.author);
+				num = utf8_strwidth(ci.author.buf);
 			if (longest_author < num)
 				longest_author = num;
 		}
@@ -1872,6 +1876,8 @@ static void find_alignment(struct scoreboard *sb, int *option)
 			longest_dst_lines = num;
 		if (largest_score < ent_score(sb, e))
 			largest_score = ent_score(sb, e);
+
+		commit_info_destroy(&ci);
 	}
 	max_orig_digits = decimal_width(longest_src_lines);
 	max_digits = decimal_width(longest_dst_lines);
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 03c6cd7..1eeed0f 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -36,36 +36,28 @@ static void insert_one_record(struct shortlog *log,
 	const char *dot3 = log->common_repo_prefix;
 	char *buffer, *p;
 	struct string_list_item *item;
-	char namebuf[1024];
-	char emailbuf[1024];
-	size_t len;
+	const char *mailbuf, *namebuf;
+	size_t namelen, maillen;
 	const char *eol;
 	struct strbuf subject = STRBUF_INIT;
+	struct strbuf namemailbuf = STRBUF_INIT;
 	struct ident_split ident;
 
 	if (split_ident_line(&ident, author, strlen(author)))
 		return;
 
-	/* copy author name to namebuf, to support matching on both name and email */
-	len = ident.name_end - ident.name_begin;
-	memcpy(namebuf, ident.name_begin, len);
-	namebuf[len] = 0;
+	namebuf = ident.name_begin;
+	mailbuf = ident.mail_begin;
+	namelen = ident.name_end - ident.name_begin;
+	maillen = ident.mail_end - ident.mail_begin;
 
-	/* copy email name to emailbuf, to allow email replacement as well */
-	len = ident.mail_end - ident.mail_begin;
-	memcpy(emailbuf, ident.mail_begin, len);
-	emailbuf[len] = 0;
+	map_user(&log->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
+	strbuf_add(&namemailbuf, namebuf, namelen);
 
-	map_user(&log->mailmap, emailbuf, sizeof(emailbuf), namebuf, sizeof(namebuf));
-	len = strlen(namebuf);
+	if (log->email)
+		strbuf_addf(&namemailbuf, " <%.*s>", (int)maillen, mailbuf);
 
-	if (log->email) {
-		size_t room = sizeof(namebuf) - len - 1;
-		int maillen = strlen(emailbuf);
-		snprintf(namebuf + len, room, " <%.*s>", maillen, emailbuf);
-	}
-
-	item = string_list_insert(&log->list, namebuf);
+	item = string_list_insert(&log->list, namemailbuf.buf);
 	if (item->util == NULL)
 		item->util = xcalloc(1, sizeof(struct string_list));
 
diff --git a/mailmap.c b/mailmap.c
index 9db8a1b..6c5e204 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -188,49 +188,42 @@ void clear_mailmap(struct string_list *map)
 }
 
 int map_user(struct string_list *map,
-	     char *email, int maxlen_email, char *name, int maxlen_name)
+			 const char **email, size_t *emaillen,
+			 const char **name, size_t *namelen)
 {
-	char *end_of_email;
 	struct string_list_item *item;
 	struct mailmap_entry *me;
-	size_t maillen;
-
-	/* figure out space requirement for email */
-	end_of_email = strchr(email, '>');
-	if (!end_of_email) {
-		/* email passed in might not be wrapped in <>, but end with a \0 */
-		end_of_email = memchr(email, '\0', maxlen_email);
-		if (!end_of_email)
-			return 0;
-	}
-
-	maillen = end_of_email - email;
 
-	debug_mm("map_user: map '%s' <%.*s>\n", name, maillen, email);
-	item = string_list_lookup_extended(map, email, 0, maillen);
+	debug_mm("map_user: map '%.*s' <%.*s>\n",
+		 *name, *namelen, *emaillen, *email);
+	item = string_list_lookup_extended(map, *email, 0, *emaillen);
 	if (item != NULL) {
 		me = (struct mailmap_entry *)item->util;
 		if (me->namemap.nr) {
 			/* The item has multiple items, so we'll look up on name too */
 			/* If the name is not found, we choose the simple entry      */
-			struct string_list_item *subitem = string_list_lookup(&me->namemap, name);
+			struct string_list_item *subitem = string_list_lookup_extended(
+					&me->namemap, *name, 1, *namelen);
 			if (subitem)
 				item = subitem;
 		}
 	}
 	if (item != NULL) {
 		struct mailmap_info *mi = (struct mailmap_info *)item->util;
-		if (mi->name == NULL && (mi->email == NULL || maxlen_email == 0)) {
+		if (mi->name == NULL && mi->email == NULL) {
 			debug_mm("map_user:  -- (no simple mapping)\n");
 			return 0;
 		}
-		if (maxlen_email && mi->email)
-			strlcpy(email, mi->email, maxlen_email);
-		else
-			*end_of_email = '\0';
-		if (maxlen_name && mi->name)
-			strlcpy(name, mi->name, maxlen_name);
-		debug_mm("map_user:  to '%s' <%s>\n", name, mi->email ? mi->email : "");
+		if (mi->email) {
+				*email = mi->email;
+				*emaillen = strlen(*email);
+		}
+		if (mi->name) {
+				*name = mi->name;
+				*namelen = strlen(*name);
+		}
+		debug_mm("map_user:  to '%.*s' <.*%s>\n", *namelen, *name,
+				 *emaillen, *email);
 		return 1;
 	}
 	debug_mm("map_user:  --\n");
diff --git a/mailmap.h b/mailmap.h
index d5c3664..ed7c93b 100644
--- a/mailmap.h
+++ b/mailmap.h
@@ -4,7 +4,7 @@
 int read_mailmap(struct string_list *map, char **repo_abbrev);
 void clear_mailmap(struct string_list *map);
 
-int map_user(struct string_list *mailmap,
-	     char *email, int maxlen_email, char *name, int maxlen_name);
+int map_user(struct string_list *map,
+			 const char **email, size_t *emaillen, const char **name, size_t *namelen);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 350d1df..dffcade 100644
--- a/pretty.c
+++ b/pretty.c
@@ -593,7 +593,8 @@ char *logmsg_reencode(const struct commit *commit,
 	return out;
 }
 
-static int mailmap_name(char *email, int email_len, char *name, int name_len)
+static int mailmap_name(const char **email, size_t *email_len,
+			const char **name, size_t *name_len)
 {
 	static struct string_list *mail_map;
 	if (!mail_map) {
@@ -610,36 +611,26 @@ static size_t format_person_part(struct strbuf *sb, char part,
 	const int placeholder_len = 2;
 	int tz;
 	unsigned long date = 0;
-	char person_name[1024];
-	char person_mail[1024];
 	struct ident_split s;
-	const char *name_start, *name_end, *mail_start, *mail_end;
+	const char *name, *mail;
+	size_t maillen, namelen;
 
 	if (split_ident_line(&s, msg, len) < 0)
 		goto skip;
 
-	name_start = s.name_begin;
-	name_end = s.name_end;
-	mail_start = s.mail_begin;
-	mail_end = s.mail_end;
-
-	if (part == 'N' || part == 'E') { /* mailmap lookup */
-		snprintf(person_name, sizeof(person_name), "%.*s",
-			 (int)(name_end - name_start), name_start);
-		snprintf(person_mail, sizeof(person_mail), "%.*s",
-			 (int)(mail_end - mail_start), mail_start);
-		mailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));
-		name_start = person_name;
-		name_end = name_start + strlen(person_name);
-		mail_start = person_mail;
-		mail_end = mail_start +  strlen(person_mail);
-	}
+	name = s.name_begin;
+	namelen = s.name_end - s.name_begin;
+	mail = s.mail_begin;
+	maillen = s.mail_end - s.mail_begin;
+
+	if (part == 'N' || part == 'E') /* mailmap lookup */
+		mailmap_name(&mail, &maillen, &name, &namelen);
 	if (part == 'n' || part == 'N') {	/* name */
-		strbuf_add(sb, name_start, name_end-name_start);
+		strbuf_add(sb, name, namelen);
 		return placeholder_len;
 	}
 	if (part == 'e' || part == 'E') {	/* email */
-		strbuf_add(sb, mail_start, mail_end-mail_start);
+		strbuf_add(sb, mail, maillen);
 		return placeholder_len;
 	}
 
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related

* [PATCH 08/10] test: add test for --use-mailmap option
From: Antoine Pelisse @ 2013-01-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <1357421206-5014-1-git-send-email-apelisse@gmail.com>

The new option '--use-mailmap' can be used to make
sure that mailmap file is used to convert name
when running log commands.

The test is simple and checks that the Author line
is correctly replaced when running log.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 t/t4203-mailmap.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 1f182f6..db043dc 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -239,6 +239,20 @@ test_expect_success 'Log output (complex mapping)' '
 	test_cmp expect actual
 '
 
+cat >expect <<\EOF
+Author: CTO <cto@company.xx>
+Author: Santa Claus <santa.claus@northpole.xx>
+Author: Santa Claus <santa.claus@northpole.xx>
+Author: Other Author <other@author.xx>
+Author: Other Author <other@author.xx>
+Author: Some Dude <some@dude.xx>
+Author: A U Thor <author@example.com>
+EOF
+test_expect_success 'Log output with --use-mailmap' '
+	git log --use-mailmap | grep Author >actual &&
+	test_cmp expect actual
+'
+
 # git blame
 cat >expect <<\EOF
 ^OBJI (A U Thor     DATE 1) one
-- 
1.7.12.4.3.g2036a08.dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox