Re: [PATCH v7 0/1] refs.c: SSE4.2 optimizations for check_refname_component

2014-06-09 Thread Junio C Hamano
Johannes Sixt  writes:

> And I get this when I compile on Windows with msysgit:
>
> CC abspath.o
> In file included from git-compat-util.h:694,
>  from cache.h:4,
>  from abspath.c:1:
> compat/cpuid.h: In function 'processor_supports_sse42':
> compat/cpuid.h:11: warning: implicit declaration of function '__cpuid'
> abspath.c: At top level:
> compat/cpuid.h:8: warning: 'processor_supports_sse42' defined but not used
> abspath.c: In function 'processor_supports_sse42':
> compat/cpuid.h:11: warning: 'eax' is used uninitialized in this function
> compat/cpuid.h:11: warning: 'ebx' is used uninitialized in this function
> compat/cpuid.h:11: warning: 'ecx' is used uninitialized in this function
> compat/cpuid.h:11: warning: 'edx' is used uninitialized in this function
>
> Perhaps our gcc is too old?

Maybe.

In any case, it is a good indication that it probably is a good idea
to start with an optional USE_SSE42 (not !NO_SSE42 or HAVE_SSE42) so
that it is clear to anybody that those with SSE42 does not have to
use this compilation option.  Once the code proves itself, we can
consider turning it on by default when able, but it seems that it is
a bit too premature for that (not that the code itself is premature
in the original author's environment, but its portability has not
been quite ready for everybody yet, it seems).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 10/20] git-submodule.sh: avoid "test -a/-o "

2014-06-09 Thread Johannes Sixt
Am 6/10/2014 1:23, schrieb Junio C Hamano:
> Elia Pinto  writes:
> 
>> @@ -1059,13 +1059,17 @@ cmd_summary() {
>>  while read mod_src mod_dst sha1_src sha1_dst status sm_path
>>  do
>>  # Always show modules deleted or type-changed 
>> (blob<->module)
>> -test $status = D -o $status = T && echo "$sm_path" && 
>> continue
>> +case "$status" in
>> +[DT])
>> +printf '%s\n' "$sm_path" &&
>> +continue
>> +esac
> 
> I think this conversion is wrong and causes parse error.  The
> surrounding code cannot be seen in the context of thsi patch, but
> looks somewhat like this:
> 
>   modules=$( 
>case "$status" in
>[DT])
>...
>esac
> )
> 
> Perhaps you would need to spell it with the extra opening
> parenthesis, like so:
> 
>   case string in
> ([DT])
>   ...
>   esac
> 
> or something.

Do you just think that it causes parse errors or did you actually observe
them? Because I think that no parse error should occur.

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] t9001: avoid not portable '\n' with sed

2014-06-09 Thread Torsten Bögershausen

On 06/10/2014 07:55 AM, Junio C Hamano wrote:

Torsten Bögershausen  writes:


t9001 used a '\n' in a sed expression to split one line into two lines,
but the usage of '\n' in the "replacement string" is not portable.

This looks peculiarly familiar; don't I already have it queued?

Yes, V2 is queued and in pu,and only the commit msg is changed
between V2 and V3.

I think that V3 explains the difference between POSIX sed and
gnu sed much better, and does reflect all the comments from
the list, which otherwise may be lost.
And I suspect that not only the sed under Mac OS X does not
handle this very '\n' different from gnu sed, or in other words,
more platforms may not have a gnu sed and may need the fix.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 0/1] refs.c: SSE4.2 optimizations for check_refname_component

2014-06-09 Thread Johannes Sixt
Am 6/10/2014 1:05, schrieb Junio C Hamano:
> Junio C Hamano  writes:
> 
>> David Turner  writes:
>>
>>> Since Junio has picked up the first patch from previous versions of
>>> this series, I'm just going to send the second (SSE) one.  I decided
>>> not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses
>>> the former convention (for instance, that's what GIT_PARSE_WITH
>>> generates).
>>
>> Yeah but NO_FROTZ is used only when FROTZ is something everybody is
>> expected to have (e.g. it's in posix, people ought to have it, but
>> we do support those who don't), isn't it?  For a very arch specific
>> stuff like sse42, I'd feel better to make it purely opt-in by
>> forcing people to explicitly say HAVE_SSE42 to enable it.
> 
> Just FYI: I am getting
> 
> compat/cpuid.h:8:12: error: 'processor_supports_sse42' defined but
> not used [-Werror=unused-function]
> cc1: all warnings being treated as errors
> 
> while building 'pu'; I'll have to rebuild 'pu' without this patch
> before I can push the day's result out.

And I get this when I compile on Windows with msysgit:

CC abspath.o
In file included from git-compat-util.h:694,
 from cache.h:4,
 from abspath.c:1:
compat/cpuid.h: In function 'processor_supports_sse42':
compat/cpuid.h:11: warning: implicit declaration of function '__cpuid'
abspath.c: At top level:
compat/cpuid.h:8: warning: 'processor_supports_sse42' defined but not used
abspath.c: In function 'processor_supports_sse42':
compat/cpuid.h:11: warning: 'eax' is used uninitialized in this function
compat/cpuid.h:11: warning: 'ebx' is used uninitialized in this function
compat/cpuid.h:11: warning: 'ecx' is used uninitialized in this function
compat/cpuid.h:11: warning: 'edx' is used uninitialized in this function

Perhaps our gcc is too old?

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] t9001: avoid not portable '\n' with sed

2014-06-09 Thread Junio C Hamano
Torsten Bögershausen  writes:

> t9001 used a '\n' in a sed expression to split one line into two lines,
> but the usage of '\n' in the "replacement string" is not portable.

This looks peculiarly familiar; don't I already have it queued?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t5551: fix the 50,000 tag test

2014-06-09 Thread Junio C Hamano
Torsten Bögershausen  writes:

> Thanks for digging, as we now know better:
> do you want to squeeze in someting like this:
>
> s/Since the day/Since commit 5e2c7cd2/ 

I usually do not dig only to give suggestions to others, which will
risk the time I spent to go to total waste.  When I find the log
message lacking, and if the patch looked otherwise OK enough, I do
the digging myself in order to tweak to reduce one round-trip
(otherwise I'd just discard and tell the submitter to do the
digging).  I may send out what I learned as a response, but that is
a mere "side effect"; the primary effect of queuing an improved
patch has often already happened when you see the result of my
digging.

I do not recall what I did for this particular patch, but you should
be able to fetch and run "git log origin/master..origin/pu" to find
out what I did ;-).

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git reset --hard with staged changes

2014-06-09 Thread Junio C Hamano
Dale Worley  writes:

> (As far as I can tell from Git's behavior, the definition of tracked file is
> "any file that is in the base commit or in the index".  Based on that
> definition, "git reset --hard" is working as documented.)

The book (whichever book you guys are talking about) is wrong, if it
considers only the paths in the HEAD commit tracked.  After the user
deliberately does "git add" a path not in HEAD, the user runs any
command (e.g. "git apply --index", "git cherry-pick --no-commit")
that may bring a path not in HEAD to the result without recording a
new commit that updates the HEAD, a new path is recorded in the
index and that path is considered "tracked" before the resulting
contents in the index is made into a commit.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] send-email: do not insert third header

2014-06-09 Thread Junio C Hamano
Junio C Hamano  writes:

> Stepan Kasal  writes:
>
>> It is sometimes desirable to insert several header lines at the top of
>> the body, e.g., if From or Date differs from the mail header.
>> (Linus even recommends to use this second header for all kernel
>> submissions.)
>>
>> send-email has a minimal support for this; make sure it is not applied
>> when there is a second header already inserted in the patch file.
>
> I have a slight suspicion that you are reading the recommendation
> wrong.  We do not recommend to record these in-body headers in the
> message of the commit object (the recommendation is to prepend
> in-body headers to the message of the commit object when sending it
> out for review---it pretty much assumes that the underlying commit
> does not have these in-body headers that are used only during the
> transit over e-mail forwarding chain).
>
> But your patch seems to assume that the input message to send-email
> already has the in-body header.  Doesn't that indicate a misuse of
> the tool, making this new "feature" smell more like a way to
> encourage such a misuse by covering up the result?
>
> I dunno.

I forgot to mention that possible enhancements. As you mentioned,
there is only a minimal support for this in send-email.  After you
committed somebody else's patch in your tree, format-patch will
produce the normal From: and Date: using the original author's
identity and timestamp, but send-email only uses the in-body header
for "From:" (but not "Date:") to propagate this information and only
when the author is different from yourself.  It is plausible that
two new options to tell send-email to optionally

 (1) propagate "Date:" in the output from format-patch as an in-body
 header; and

 (2) propagate "From:" in the output from format-patch as an in-body
 header even when you are sending out your own patch

would support the recent kernel submission convention better.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] completion: Handle '!f() { ... }; f' aliases

2014-06-09 Thread Junio C Hamano
Steffen Prohaska  writes:

> '!f() { ... }; f' is a recommended pattern to declare more complex
> aliases (see git wiki [1]).  This commit teaches the completion to
> handle them.

Hmm, I've never endorsed nor recommended such a notation myself ;-)
I tend to prefer writing it like so instead:

sh -c '...' -

so that I won't clobber "f" (or any other name).  I wonder if you
can help users of this other pattern as well.

> When determining which completion to use for an alias, the opening brace
> is now ignored in order to continue the search for a git command inside
> the function body.  For example, the alias '!f() { git commit ... }' now
> triggers commit completion.

I suspect that "scanning" is error-prone.  I like this one for its
cuteness very much, though:

> Furthermore, the null command ':' is now skipped, so that it can be used
> as a workaround to declare the desired completion style.  For example,
> the alias '!f() { : git commit ; if ...  ' now triggers commit
> completion.



> +test_expect_success 'completion uses  completion for alias !f() { 
> VAR=val git  ... }' '
> + test_config alias.co "!f() { VAR=val git checkout ... ; } f" &&

Is it only "f" that is completed, or can I spell it using another
arbitrary token, e.g.

test_config alias.co "!co () { git checkout ... } co"

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/15] commit: record buffer length in cache

2014-06-09 Thread Jeff King
On Tue, Jun 10, 2014 at 07:12:37AM +0200, Christian Couder wrote:

> From: Jeff King 
> >
> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
> > diff_options *opt,
> > ident, ident, path,
> > (!contents_from ? path :
> >  (!strcmp(contents_from, "-") ? "standard input" : 
> > contents_from)));
> > -   set_commit_buffer(commit, strbuf_detach(&msg, NULL));
> > +   set_commit_buffer(commit, msg.buf, msg.len);
> 
> I find the above strange. I would have done something like:
> 
> - set_commit_buffer(commit, strbuf_detach(&msg, NULL));
> + size_t size;
> + char *buf = strbuf_detach(&msg, &size);
> + set_commit_buffer(commit, buf, size);

It is a little strange. You can't do:

  set_commit_buffer(commit, strbuf_detach(&msg, NULL), msg.len);

because the second argument resets msg.len as a side effect. Doing it
your way is longer, but perhaps is a bit more canonical. In general, one
should call strbuf_detach to ensure that the buffer is allocated (and
does not point to slopbuf). That's guaranteed here, because we just put
contents into the buffer, but it's probably more hygienic to use the
more verbose form.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/15] commit: record buffer length in cache

2014-06-09 Thread Christian Couder
From: Jeff King 
>
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
> diff_options *opt,
>   ident, ident, path,
>   (!contents_from ? path :
>(!strcmp(contents_from, "-") ? "standard input" : 
> contents_from)));
> - set_commit_buffer(commit, strbuf_detach(&msg, NULL));
> + set_commit_buffer(commit, msg.buf, msg.len);

I find the above strange. I would have done something like:

-   set_commit_buffer(commit, strbuf_detach(&msg, NULL));
+   size_t size;
+   char *buf = strbuf_detach(&msg, &size);
+   set_commit_buffer(commit, buf, size);

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] t9001: avoid not portable '\n' with sed

2014-06-09 Thread Eric Sunshine
On Tue, Jun 10, 2014 at 12:07 AM, Torsten Bögershausen  wrote:
> t9001 used a '\n' in a sed expression to split one line into two lines,
> but the usage of '\n' in the "replacement string" is not portable.
>
> The '\n' can be used to match a newline in the "pattern space",
> but otherwise the meaning of '\n' is unspecified in POSIX.
>
> - Gnu versions of sed will treat '\n' as a newline character.
> - Other versions of sed (like /usr/bin/sed under Mac OS X)
>   simply ignore the '\' before the 'n', treating '\n' as 'n'.
>
> For reference see:
> pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
> http://www.gnu.org/software/sed/manual/sed.html
>
> As the test already requires perl as a prerequisite, use perl instead of sed.

René Scharfe pointed out this useful resource [1] for writing portable
'sed' when he fixed [2] a problem on NetBSD in a test I had written.

[1]: http://sed.sourceforge.net/sedfaq4.html
[2]: http://thread.gmane.org/gmane.comp.version-control.git/231654

> Signed-off-by: Torsten Bögershausen 
> ---
> Sending a V3 patch seems "spammish", but after re-reading all
> the comments I think that the commit msg should point out the difference
> between POSIX sed and gnu sed somewhat better.
>
>  t/t9001-send-email.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 64d9434..19a3ced 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1342,7 +1342,7 @@ test_cover_addresses () {
> git format-patch --cover-letter -2 -o outdir &&
> cover=`echo outdir/-*.patch` &&
> mv $cover cover-to-edit.patch &&
> -   sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch 
> >"$cover" &&
> +   perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" 
> cover-to-edit.patch >"$cover" &&
> git send-email \
>   --force \
>   --from="Example " \
> --
> 2.0.0.553.ged01b91
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] t9001: avoid not portable '\n' with sed

2014-06-09 Thread Torsten Bögershausen
t9001 used a '\n' in a sed expression to split one line into two lines,
but the usage of '\n' in the "replacement string" is not portable.

The '\n' can be used to match a newline in the "pattern space",
but otherwise the meaning of '\n' is unspecified in POSIX.

- Gnu versions of sed will treat '\n' as a newline character.
- Other versions of sed (like /usr/bin/sed under Mac OS X)
  simply ignore the '\' before the 'n', treating '\n' as 'n'.

For reference see:
pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
http://www.gnu.org/software/sed/manual/sed.html

As the test already requires perl as a prerequisite, use perl instead of sed.

Signed-off-by: Torsten Bögershausen 
---
Sending a V3 patch seems "spammish", but after re-reading all
the comments I think that the commit msg should point out the difference
between POSIX sed and gnu sed somewhat better.

 t/t9001-send-email.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 64d9434..19a3ced 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
git format-patch --cover-letter -2 -o outdir &&
cover=`echo outdir/-*.patch` &&
mv $cover cover-to-edit.patch &&
-   sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch 
>"$cover" &&
+   perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" 
cover-to-edit.patch >"$cover" &&
git send-email \
  --force \
  --from="Example " \
-- 
2.0.0.553.ged01b91

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t5551: fix the 50,000 tag test

2014-06-09 Thread Torsten Bögershausen
On 2014-06-09 21.16, Junio C Hamano wrote:

>> Since the day "git clone" printed "Cloning into 'too-many-refs'" to stderr,
> 
> Thanks.  It is 68b939b2 (clone: send diagnostic messages to stderr,
> 2013-09-18); before it we showed the message to the standard output
> stream instead.
> 
> Will queue.
Thanks for digging, as we now know better:
do you want to squeeze in someting like this:

s/Since the day/Since commit 5e2c7cd2/ 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] Add an explicit GIT_DIR to the list of excludes

2014-06-09 Thread Pasha Bolokhov
> On Thu, Jun 5, 2014 at 3:15 AM, Pasha Bolokhov  
> wrote:
>> +   /* only add it if GIT_DIR does not end with '.git' or '/.git' */
>> +   if (len < 4 || strcmp(n_git + len - 4, ".git") ||
>> +   (len > 4 && n_git[len - 5] != '/')) {
>
> Hmm.. should we exclude "foobar.git" as well?

Why wouldn't we? Everything that has basename ".git" is hard-wired
to be excluded, but everything else, including "foobar.git" should be
added to the excludes manually... How is it better than just "foobar"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/15] logmsg_reencode: return const buffer

2014-06-09 Thread Eric Sunshine
On Mon, Jun 9, 2014 at 2:10 PM, Jeff King  wrote:
> The return value from logmsg_reencode may be either a newly
> allocated buffer or a pointer to the existing commit->buffer.
> We would not want the caller to accidentally free() or
> modify the latter, so let's mark it as const.  We can cast
> away the constness in logmsg_free, but only once we have
> determined that it is a free-able buffer.
>
> Signed-off-by: Jeff King 
> ---
> index 71e2337..47e5b7a 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2788,7 +2788,7 @@ static int commit_match(struct commit *commit, struct 
> rev_info *opt)
>  {
> int retval;
> const char *encoding;
> -   char *message;
> +   const char *message;
> struct strbuf buf = STRBUF_INIT;
>
> if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)
> @@ -2830,12 +2830,18 @@ static int commit_match(struct commit *commit, struct 
> rev_info *opt)
> format_display_notes(commit->object.sha1, &buf, encoding, 1);
> }
>
> -   /* Find either in the original commit message, or in the temporary */
> +   /* Find either in the original commit message, or in the temporary.

Style:

/*
 * Find either...
 */

> +* Note that we cast away the constness of "message" here. It is
> +* const because it may come from the cached commit buffer. That's OK,
> +* because we know that it is modifiable heap memory, and that while
> +* grep_buffer may modify it for speed, it will restore any
> +* changes before returning.
> +*/
> if (buf.len)
> retval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);
> else
> retval = grep_buffer(&opt->grep_filter,
> -message, strlen(message));
> +(char *)message, strlen(message));
> strbuf_release(&buf);
> logmsg_free(message, commit);
> return retval;
> --
> 2.0.0.729.g520999f
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git reset --hard with staged changes

2014-06-09 Thread Dale Worley
From: Pierre-François CLEMENT  gmail.com>
> You create a new (untracked) file.
> You use git-reset's hard mode to go one commit back, the new
> (untracked) file's still there.
> You add/stage that new file.
> You use git-reset's hard mode again to go one commit back, and the new
> untracked file you just staged gets deleted.
> 
> Also, according to Git-scm
> (http://git-scm.com/book/en/Git-Basics-Recording-Changes-to-the-Repository):
> 
> "Tracked files are files that were in the last snapshot [...].
> Untracked files are everything else."
> 
> So it seems to me like staged untracked files shouldn't be considered
> as tracked files, and thus shouldn't be removed. Or maybe, git-reset's
> hard mode should always delete everything including untracked files?
> It would also make sense, given the numerous modes it has.

There's a core question that must be answered:  What, *exactly*, is a
"tracked file"?

If you look at that passage in the book, it continues:

"Tracked files are files that were in the last snapshot; they can be
unmodified, modified, or staged. Untracked files are everything else — any
files in your working directory that were not in your last snapshot and are
not in your staging area."

But if you look carefully, that passage gives two definitions of "untracked
files", and *they don't agree*, specifically in the case of a file that is
in the index but not in the base commit.  And that's the case we're considering.

To fix this, you've got to figure out what the definition of "tracked file"
is supposed to be, and then ensure that everything (code and documentation)
is consistent with that.

(As far as I can tell from Git's behavior, the definition of tracked file is
"any file that is in the base commit or in the index".  Based on that
definition, "git reset --hard" is working as documented.)

Dale


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/15] use get_commit_buffer everywhere

2014-06-09 Thread Jeff King
On Mon, Jun 09, 2014 at 08:02:24PM -0400, Jeff King wrote:

> I'm still confused and disturbed that my gcc is not noticing this
> obvious const violation. Hmm, shutting off ccache seems to make it work.
> Doubly disturbing.

Ah, mystery solved. It's a gcc bug:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60014

I get:

  $ gcc -c -Wall -Werror -DSHA1_HEADER='"block-sha1/sha1.h"' notes-merge.c
  notes-merge.c: In function ‘notes_merge_commit’:
  notes-merge.c:723:2: error: passing argument 2 of ‘strbuf_attach’
discards ‘const’ qualifier from pointer target type [-Werror]
  ...etc...

  $ gcc -E -Wall -Werror -DSHA1_HEADER='"block-sha1/sha1.h"' notes-merge.c 
>foo.c
  $ gcc -c -Wall -Werror -DSHA1_HEADER='"block-sha1/sha1.h"' foo.c
  [no warnings from either]

ccache uses the latter technique.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] rebase -i: Remember merge options beyond continue actions

2014-06-09 Thread Fabian Ruch
Hi Eric,

thanks a lot for the reference.

I added the Reported-by: and Signed-off-by: lines to the commit message.

   Fabian

-- >8 --
Subject: rebase -i: Remember merge options beyond continue actions

If the user explicitly specified a merge strategy or strategy options,
"rebase --interactive" started using the default merge strategy again
after "rebase --continue".

This problem gets fixed by this commit. Add test.

Since the "rebase" options "-s" and "-X" imply "--merge", we can simply
remove the "do_merge" guard in the interactive mode and always compile
the "cherry-pick" arguments from the "rebase" state variables "strategy"
and "strategy_opts".

Reported-by: Diogo de Campos 
Signed-off-by: Fabian Ruch 
---
 git-rebase--interactive.sh| 18 +++---
 t/t3404-rebase-interactive.sh | 16 
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..817c933 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -77,17 +77,13 @@ amend="$state_dir"/amend
 rewritten_list="$state_dir"/rewritten-list
 rewritten_pending="$state_dir"/rewritten-pending
 
-strategy_args=
-if test -n "$do_merge"
-then
-   strategy_args=${strategy:+--strategy=$strategy}
-   eval '
-   for strategy_opt in '"$strategy_opts"'
-   do
-   strategy_args="$strategy_args -X$(git rev-parse 
--sq-quote "${strategy_opt#--}")"
-   done
-   '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+   for strategy_opt in '"$strategy_opts"'
+   do
+   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
"${strategy_opt#--}")"
+   done
+'
 
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0023a5..73849f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
 '
 
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+   git checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&
+   git reset --hard HEAD^ &&
+   >breakpoint &&
+   git add breakpoint &&
+   git commit -m "breakpoint for interactive mode" &&
+   echo five >conflict &&
+   echo Z >file1 &&
+   git commit -a -m "one file conflict" &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 2" git rebase -i --strategy=recursive -Xours 
conflict-branch &&
+   git rebase --continue &&
+   test $(git show conflict-branch:conflict) = $(cat conflict) &&
+   test $(cat file1) = Z
+'
+
 test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD)
test_when_finished "git rebase --abort; git reset --hard $current_head; 
rm -f error" &&
-- 
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] rebase -i: Remember merge options beyond continue actions

2014-06-09 Thread Eric Sunshine
On Mon, Jun 9, 2014 at 8:02 PM, Fabian Ruch  wrote:
> If the user explicitly specified a merge strategy or strategy options,
> "rebase --interactive" started using the default merge strategy again
> after "rebase --continue".

For reference, this problem was reported as far back as 2013-08-09 [1].

[1]: http://article.gmane.org/gmane.comp.version-control.git/232013

> This problem gets fixed by this commit. Add test.
>
> Since the "rebase" options "-s" and "-X" imply "--merge", we can simply
> remove the "do_merge" guard in the interactive mode and always compile
> the "cherry-pick" arguments from the "rebase" state variables "strategy"
> and "strategy_opts".

Missing sign-off.

> ---
>  git-rebase--interactive.sh| 18 +++---
>  t/t3404-rebase-interactive.sh | 16 
>  2 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 6ec9d3c..817c933 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -77,17 +77,13 @@ amend="$state_dir"/amend
>  rewritten_list="$state_dir"/rewritten-list
>  rewritten_pending="$state_dir"/rewritten-pending
>
> -strategy_args=
> -if test -n "$do_merge"
> -then
> -   strategy_args=${strategy:+--strategy=$strategy}
> -   eval '
> -   for strategy_opt in '"$strategy_opts"'
> -   do
> -   strategy_args="$strategy_args -X$(git rev-parse 
> --sq-quote "${strategy_opt#--}")"
> -   done
> -   '
> -fi
> +strategy_args=${strategy:+--strategy=$strategy}
> +eval '
> +   for strategy_opt in '"$strategy_opts"'
> +   do
> +   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
> "${strategy_opt#--}")"
> +   done
> +'
>
>  GIT_CHERRY_PICK_HELP="$resolvemsg"
>  export GIT_CHERRY_PICK_HELP
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index c0023a5..73849f1 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
> test $(cat file1) = Z
>  '
>
> +test_expect_success 'interrupted rebase -i with --strategy and -X' '
> +   git checkout -b conflict-merge-use-theirs-interrupted conflict-branch 
> &&
> +   git reset --hard HEAD^ &&
> +   >breakpoint &&
> +   git add breakpoint &&
> +   git commit -m "breakpoint for interactive mode" &&
> +   echo five >conflict &&
> +   echo Z >file1 &&
> +   git commit -a -m "one file conflict" &&
> +   set_fake_editor &&
> +   FAKE_LINES="edit 1 2" git rebase -i --strategy=recursive -Xours 
> conflict-branch &&
> +   git rebase --continue &&
> +   test $(git show conflict-branch:conflict) = $(cat conflict) &&
> +   test $(cat file1) = Z
> +'
> +
>  test_expect_success 'rebase -i error on commits with \ in message' '
> current_head=$(git rev-parse HEAD)
> test_when_finished "git rebase --abort; git reset --hard 
> $current_head; rm -f error" &&
> --
> 2.0.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] rebase -i: Remember merge options beyond continue actions

2014-06-09 Thread Fabian Ruch
If the user explicitly specified a merge strategy or strategy options,
"rebase --interactive" started using the default merge strategy again
after "rebase --continue".

This problem gets fixed by this commit. Add test.

Since the "rebase" options "-s" and "-X" imply "--merge", we can simply
remove the "do_merge" guard in the interactive mode and always compile
the "cherry-pick" arguments from the "rebase" state variables "strategy"
and "strategy_opts".
---
 git-rebase--interactive.sh| 18 +++---
 t/t3404-rebase-interactive.sh | 16 
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..817c933 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -77,17 +77,13 @@ amend="$state_dir"/amend
 rewritten_list="$state_dir"/rewritten-list
 rewritten_pending="$state_dir"/rewritten-pending
 
-strategy_args=
-if test -n "$do_merge"
-then
-   strategy_args=${strategy:+--strategy=$strategy}
-   eval '
-   for strategy_opt in '"$strategy_opts"'
-   do
-   strategy_args="$strategy_args -X$(git rev-parse 
--sq-quote "${strategy_opt#--}")"
-   done
-   '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+   for strategy_opt in '"$strategy_opts"'
+   do
+   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
"${strategy_opt#--}")"
+   done
+'
 
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0023a5..73849f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
 '
 
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+   git checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&
+   git reset --hard HEAD^ &&
+   >breakpoint &&
+   git add breakpoint &&
+   git commit -m "breakpoint for interactive mode" &&
+   echo five >conflict &&
+   echo Z >file1 &&
+   git commit -a -m "one file conflict" &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 2" git rebase -i --strategy=recursive -Xours 
conflict-branch &&
+   git rebase --continue &&
+   test $(git show conflict-branch:conflict) = $(cat conflict) &&
+   test $(cat file1) = Z
+'
+
 test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD)
test_when_finished "git rebase --abort; git reset --hard $current_head; 
rm -f error" &&
-- 
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/15] use get_commit_buffer everywhere

2014-06-09 Thread Jeff King
On Mon, Jun 09, 2014 at 03:40:57PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > diff --git a/notes-merge.c b/notes-merge.c
> > index 94a1a8a..7885ab2 100644
> > --- a/notes-merge.c
> > +++ b/notes-merge.c
> > @@ -671,7 +671,8 @@ int notes_merge_commit(struct notes_merge_options *o,
> > DIR *dir;
> > struct dirent *e;
> > struct strbuf path = STRBUF_INIT;
> > -   char *msg = strstr(partial_commit->buffer, "\n\n");
> > +   const char *buffer = get_commit_buffer(partial_commit);
> > +   const char *msg = strstr(buffer, "\n\n");
> 
> This tightening causes...
> 
> > struct strbuf sb_msg = STRBUF_INIT;
> > int baselen;
> >  
> > @@ -720,6 +721,7 @@ int notes_merge_commit(struct notes_merge_options *o,
> > }
> >  
> > strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);
> 
> ...a new error here:
> 
> notes-merge.c:723:2: error: passing argument 2 of 'strbuf_attach'
> discards 'const' qualifier from pointer target type [-Werror]
> strbuf.h:19:13: note: expected 'void *' but argument is of type
> 'const char *'

That's weird. I compile with -Wall -Werror, and my gcc doesn't complain.
Hmph.

I agree it's not right, though. I think the original is questionable,
too. It takes a pointer into the middle of partial_commit->buffer and
attaches it to a strbuf. That's wrong because:

  1. It's pointing into the middle of an allocated buffer, not the
 beginning.

  2. We do not own partial_commit->buffer in the first place.

So any call to strbuf_detach on the result would be disastrous. The
compiler doesn't notice because of the const leak in strstr, and it
doesn't cause a bug in practice because the only use of the strbuf is to
pass it as a const to create_notes_commit.

I feel like the most elegant solution is for create_notes_commit to take
a buf/len pair rather than a strbuf, but it unfortunately is just
feeding that to commit_tree. Adjusting that code path would affect quite
a few other spots.

The other obvious option is actually populating the strbuf, but it feels
ugly to have to make a copy just to satisfy the function interface.

Maybe a cast and a warning comment are the least evil thing, as below? I
dunno, it feels pretty wrong.

diff --git a/notes-merge.c b/notes-merge.c
index 94a1a8a..1f3b309 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -671,7 +671,7 @@ int notes_merge_commit(struct notes_merge_options *o,
DIR *dir;
struct dirent *e;
struct strbuf path = STRBUF_INIT;
-   char *msg = strstr(partial_commit->buffer, "\n\n");
+   const char *msg = strstr(partial_commit->buffer, "\n\n");
struct strbuf sb_msg = STRBUF_INIT;
int baselen;
 
@@ -719,7 +719,15 @@ int notes_merge_commit(struct notes_merge_options *o,
strbuf_setlen(&path, baselen);
}
 
-   strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);
+   /*
+* This is a bit tricky. We should not be attaching msg, which
+* is not owned by us and is not even the start of a heap buffer, to a
+* strbuf. But the create_notes_commit interface really wants
+* a strbuf, even though it will only ever use it as a buf/len pair and
+* never modify it. So this is tentatively safe as long as nobody ever
+* modifies, detaches, or releases the strbuf.
+*/
+   strbuf_attach(&sb_msg, (char *)msg, strlen(msg), strlen(msg) + 1);
create_notes_commit(partial_tree, partial_commit->parents, &sb_msg,
result_sha1);
if (o->verbosity >= 4)

I'm still confused and disturbed that my gcc is not noticing this
obvious const violation. Hmm, shutting off ccache seems to make it work.
Doubly disturbing.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git reset --hard with staged changes

2014-06-09 Thread Junio C Hamano
Pierre-François CLEMENT  writes:

> Hm, I didn't think of "git apply --index"... Makes sense for this
> special use, but I'm not sure about the other use cases.

Try merging another branch that tracks a file your current branch
does not know about and ending up with conflicts during that merge.
Resetting the half-done result away must remove that new path from
your working tree and the index.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git] Re: [RFC PATCH] clone: add clone.recursesubmodules config option

2014-06-09 Thread W. Trevor King
On Mon, Jun 09, 2014 at 03:17:07PM +0200, Jens Lehmann wrote:
> And by the way: wouldn't it make more sense to tell the user /what/
> we do automatically? So maybe 'submodule.autoupdate' is a better
> name for the new switch?

Or autocheckout?  No need to preserve submodule-specific jargon when
we have a perfectly acceptable word for this in the core interface ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH] send-email: do not insert third header

2014-06-09 Thread Junio C Hamano
Stepan Kasal  writes:

> It is sometimes desirable to insert several header lines at the top of
> the body, e.g., if From or Date differs from the mail header.
> (Linus even recommends to use this second header for all kernel
> submissions.)
>
> send-email has a minimal support for this; make sure it is not applied
> when there is a second header already inserted in the patch file.

I have a slight suspicion that you are reading the recommendation
wrong.  We do not recommend to record these in-body headers in the
message of the commit object (the recommendation is to prepend
in-body headers to the message of the commit object when sending it
out for review---it pretty much assumes that the underlying commit
does not have these in-body headers that are used only during the
transit over e-mail forwarding chain).

But your patch seems to assume that the input message to send-email
already has the in-body header.  Doesn't that indicate a misuse of
the tool, making this new "feature" smell more like a way to
encourage such a misuse by covering up the result?

I dunno.

>
> Signed-off-by: Stepan Kasal 
> ---
>  git-send-email.perl | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 9949db0..891df13 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1456,7 +1456,9 @@ foreach my $t (@files) {
>   }
>  
>   if (defined $sauthor and $sauthor ne $sender) {
> - $message = "From: $author\n\n$message";
> + if ($message !~ m/^From: /) {
> + $message = "From: $author\n\n$message";
> + }
>   if (defined $author_encoding) {
>   if ($has_content_type) {
>   if ($body_encoding eq $author_encoding) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 7/7] tests: drop GIT_*_TIMING_TESTS environment variable support

2014-06-09 Thread Junio C Hamano
Two tests (t3302 and t3419) used to have their own environment
variable to trigger expensive tests without enabling expensive
tests in other scripts; a user could set GIT_NOTES_TIMING_TESTS
but not GIT_TEST_LONG and run the whole test suite and trigger
expensive tests only in t3302 but not other tests.  The same for
GIT_PATCHID_TIMING_TESTS in t3419.

While this may have seemed a good flexibility, in reality if you are
concentrating on a single test (e.g. t3302), you can just run that
single test with the GIT_TEST_LONG to trigger expensive tests.  It
does not seem worth forcing other people who may want to come up
with their own expesive tests to invent new environment variables by
keeping this convention.

Drop them.

Signed-off-by: Junio C Hamano 
---
 t/t3302-notes-index-expensive.sh | 2 --
 t/t3419-rebase-patch-id.sh   | 2 --
 2 files changed, 4 deletions(-)

diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index 8d44e04..7217c5e 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -7,8 +7,6 @@ test_description='Test commit notes index (expensive!)'
 
 . ./test-lib.sh
 
-test -n "$GIT_NOTES_TIMING_TESTS" && test_set_prereq EXPENSIVE
-
 create_repo () {
number_of_commits=$1
nr=0
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 9292b49..217dd79 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -4,8 +4,6 @@ test_description='git rebase - test patch id computation'
 
 . ./test-lib.sh
 
-test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE
-
 count () {
i=0
while test $i -lt $1
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/7] t3419: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite

2014-06-09 Thread Junio C Hamano
This was only necessary because do_tests helper the script defines
took its parameters in a wrong order.  Just pass an empty string (or
not passing the optional EXPENSIVE prerequisite) when running the
test with a light-weight set of parameters and have the shell do the
right thing when parsing test_expect_success helper.

Also update coding style while we are at it.

Signed-off-by: Junio C Hamano 
---
 t/t3419-rebase-patch-id.sh | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 08e30b3..9292b49 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -4,11 +4,9 @@ test_description='git rebase - test patch id computation'
 
 . ./test-lib.sh
 
-test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE
 
-count()
-{
+count () {
i=0
while test $i -lt $1
do
@@ -17,8 +15,7 @@ count()
done
 }
 
-scramble()
-{
+scramble () {
i=0
while read x
do
@@ -27,12 +24,11 @@ scramble()
echo "$x"
fi
i=$((($i+1) % 10))
-   done < "$1" > "$1.new"
+   done <"$1" >"$1.new"
mv -f "$1.new" "$1"
 }
 
-run()
-{
+run () {
echo \$ "$@"
/usr/bin/time "$@" >/dev/null
 }
@@ -42,10 +38,8 @@ test_expect_success 'setup' '
git tag root
 '
 
-do_tests()
-{
-   pr=$1
-   nlines=$2
+do_tests () {
+   nlines=$1 pr=${2-}
 
test_expect_success $pr "setup: $nlines lines" "
rm -f .gitattributes &&
@@ -102,7 +96,7 @@ do_tests()
"
 }
 
-do_tests NOT_EXPENSIVE 500
-do_tests EXPENSIVE 5
+do_tests 500
+do_tests 5 EXPENSIVE
 
 test_done
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 10/20] git-submodule.sh: avoid "test -a/-o "

2014-06-09 Thread Junio C Hamano
Elia Pinto  writes:

> @@ -1059,13 +1059,17 @@ cmd_summary() {
>   while read mod_src mod_dst sha1_src sha1_dst status sm_path
>   do
>   # Always show modules deleted or type-changed 
> (blob<->module)
> - test $status = D -o $status = T && echo "$sm_path" && 
> continue
> + case "$status" in
> + [DT])
> + printf '%s\n' "$sm_path" &&
> + continue
> + esac

I think this conversion is wrong and causes parse error.  The
surrounding code cannot be seen in the context of thsi patch, but
looks somewhat like this:

modules=$( 
   case "$status" in
   [DT])
   ...
   esac
    )

Perhaps you would need to spell it with the extra opening
parenthesis, like so:

case string in
([DT])
...
esac

or something.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/7] A few minor test-prereq updates

2014-06-09 Thread Junio C Hamano
While reviewing somebody's patch, I noticed that individual test
scripts set EXPENSIVE test prerequisite with copied-and-pasted
lines.  Here is a bit to update them, while fixing styles in old
test scripts that had these copied-and-pasted lines.

The last step discards support for GIT_{NOTES,PATCHID}_TIMING_TESTS
environment variables to enable EXPENSIVE tests in two test scripts,
which *is* a backward incompatible change and people may want to
argue against it (but I of course thought it is a good change and
that is why it is included).

Junio C Hamano (7):
  test: turn EXPENSIVE into a lazy prerequisite
  test: turn USR_BIN_TIME into a lazy prerequisite
  t3302: coding style updates
  t3302: do not chdir around in the primary test process
  t3302: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite
  t3419: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite
  tests: drop GIT_*_TIMING_TESTS environment variable support

 t/t0021-conversion.sh|   2 -
 t/t3302-notes-index-expensive.sh | 130 +--
 t/t3419-rebase-patch-id.sh   |  25 +++-
 t/t5551-http-fetch.sh|   2 -
 t/test-lib.sh|   8 +++
 5 files changed, 85 insertions(+), 82 deletions(-)

-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/20] contrib/examples/git-clone.sh: avoid "test -a/-o "

2014-06-09 Thread Junio C Hamano
Elia Pinto  writes:

> The construct is error-prone; "test" being built-in in most modern
> shells, the reason to avoid "test  && test " spawning
> one extra process by using a single "test  -a " no
> longer exists.
>
> Signed-off-by: Elia Pinto 
> ---
>  contrib/examples/git-clone.sh |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/examples/git-clone.sh b/contrib/examples/git-clone.sh
> index b4c9376..08cf246 100755
> --- a/contrib/examples/git-clone.sh
> +++ b/contrib/examples/git-clone.sh
> @@ -516,7 +516,7 @@ then
>  
>   case "$no_checkout" in
>   '')
> - test "z$quiet" = z -a "z$no_progress" = z && v=-v || v=
> + test "z$quiet" = z && test "z$no_progress" = z && v=-v || v=
>   git read-tree -m -u $v HEAD HEAD
>   esac
>  fi

Hmph.  If we want to see them both empty, 

test "$quiet,$no_progress" = ,

would have been a better way to spell this, but that is outside the
scope of this series.

But I wonder if we really want to update the contrib/examples/,
which is a record of how historically we have implemented various
scripted Porcelains using lower level plumbing commands.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/7] t3302: coding style updates

2014-06-09 Thread Junio C Hamano
Use "<<-END_OF_HERE_TEXT" to push the contents of here-text to the
right in order to show the loop structure better.

Use write_script when writing a script to be run.

Use "test" (not "[ ... ]") and avoid unnecessary ";" in the middle
of a line.

Signed-off-by: Junio C Hamano 
---
 t/t3302-notes-index-expensive.sh | 90 +---
 1 file changed, 47 insertions(+), 43 deletions(-)

diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index dc706ab..aa9dbd7 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -16,43 +16,43 @@ create_repo () {
test -d .git || {
git init &&
(
-   while [ $nr -lt $number_of_commits ]; do
+   while test $nr -lt $number_of_commits
+   do
nr=$(($nr+1))
mark=$(($nr+$nr))
notemark=$(($mark+1))
test_tick &&
-   cat < $GIT_COMMITTER_DATE
-data <> note_commit
+   cat <<-INPUT_END &&
+   commit refs/heads/master
+   mark :$mark
+   committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 
$GIT_COMMITTER_DATE
+   data <>note_commit
done &&
test_tick &&
-   cat < $GIT_COMMITTER_DATE
-data < 
$GIT_COMMITTER_DATE
+   data < output &&
+   git log | grep "^" >output &&
i=$count &&
-   while [ $i -gt 0 ]; do
+   while test $i -gt 0
+   do
echo "commit #$i" &&
echo "note for commit #$i" &&
-   i=$(($i-1));
-   done > expect &&
+   i=$(($i-1))
+   done >expect &&
test_cmp expect output
 }
 
-cat > time_notes << \EOF
+write_script time_notes <<\EOF
mode=$1
i=1
-   while [ $i -lt $2 ]; do
+   while test $i -lt $2
+   do
case $1 in
no-notes)
-   GIT_NOTES_REF=non-existing; export GIT_NOTES_REF
-   ;;
+   GIT_NOTES_REF=non-existing
+   export GIT_NOTES_REF
+   ;;
notes)
unset GIT_NOTES_REF
-   ;;
+   ;;
esac
-   git log >/dev/null
+   git log
i=$(($i+1))
-   done
+   done >/dev/null
 EOF
 
 time_notes () {
for mode in no-notes notes
do
echo $mode
-   /usr/bin/time "$SHELL_PATH" ../time_notes $mode $1
+   /usr/bin/time ../time_notes $mode $1
done
 }
 
@@ -118,7 +121,8 @@ do_tests () {
 }
 
 do_tests NOT_EXPENSIVE 10
-for count in 100 1000 1; do
+for count in 100 1000 1
+do
do_tests EXPENSIVE $count
 done
 
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/7] test: turn EXPENSIVE into a lazy prerequisite

2014-06-09 Thread Junio C Hamano
Two test scripts (t0021 and t5551) had copy & paste code to set
EXPENSIVE prerequisite.  Use the test_lazy_prereq helper to define
them in the common t/test-lib.sh.

Signed-off-by: Junio C Hamano 
---
 t/t0021-conversion.sh | 2 --
 t/t5551-http-fetch.sh | 2 --
 t/test-lib.sh | 4 
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index b92e6cb..f890c54 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -190,8 +190,6 @@ test_expect_success 'required filter clean failure' '
test_must_fail git add test.fc
 '
 
-test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
-
 test_expect_success EXPENSIVE 'filter large file' '
git config filter.largefile.smudge cat &&
git config filter.largefile.clean cat &&
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index afb439e..d697393 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -214,8 +214,6 @@ test_expect_success 'cookies stored in http.cookiefile when 
http.savecookies set
test_cmp expect_cookies.txt cookies_tail.txt
 '
 
-test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
-
 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b25249e..d70d05e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -855,6 +855,10 @@ test_lazy_prereq AUTOIDENT '
git var GIT_AUTHOR_IDENT
 '
 
+test_lazy_prereq EXPENSIVE '
+   test -n "$GIT_TEST_LONG"
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/7] t3302: do not chdir around in the primary test process

2014-06-09 Thread Junio C Hamano
These days^Wyears we strive to do stuff in subdirectories inside
subshells to avoid mistakes.

Signed-off-by: Junio C Hamano 
---
 t/t3302-notes-index-expensive.sh | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index aa9dbd7..7712cf3 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -106,18 +106,27 @@ do_tests () {
pr=$1
count=$2
 
-   test_expect_success $pr 'setup / mkdir' '
-   mkdir $count &&
-   cd $count
+   test_expect_success $pr "setup $count" '
+   mkdir "$count" &&
+   (
+   cd "$count" &&
+   create_repo "$count"
+   )
'
 
-   test_expect_success $pr "setup $count" "create_repo $count"
-
-   test_expect_success $pr 'notes work' "test_notes $count"
-
-   test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' 
"time_notes 100"
+   test_expect_success $pr 'notes work' '
+   (
+   cd "$count" &&
+   test_notes "$count"
+   )
+   '
 
-   test_expect_success $pr 'teardown / cd ..' 'cd ..'
+   test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' '
+   (
+   cd "$count" &&
+   time_notes 100
+   )
+   '
 }
 
 do_tests NOT_EXPENSIVE 10
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git reset --hard with staged changes

2014-06-09 Thread Pierre-François CLEMENT
2014-06-09 16:04 GMT+02:00 David Kastrup :
> Pierre-François CLEMENT  writes:
>
>> Hi all,
>>
>> Someone pointed out on the "Git for human beings" Google group
>> (https://groups.google.com/d/topic/git-users/27_FxIV_100/discussion)
>> that using git-reset's hard mode when having staged untracked files
>> simply deletes them from the working dir.
>>
>> Since git-reset specifically doesn't touch untracked files, one could
>> expect having staged untracked files reset to their previous
>> "untracked" state rather than being deleted.
>>
>> Could this be a bug or a missing feature? Or if it isn't, can someone
>> explain what we got wrong?
>
> git reset --keep maybe?
>
> In a work dir and index without modifications, I expect
>
> git apply --index ...
> git reset --hard
>
> to remove any files that git apply created.  It would not do so using
> your proposal.  I agree that it seems a bit of a borderline, but I
> consider it better that once a file _is_ tracked, git reset --hard will
> first physically remove it before untracking it.
>
> --
> David Kastrup

Hm, I didn't think of "git apply --index"... Makes sense for this
special use, but I'm not sure about the other use cases. Consider this
scenario:

You create a new (untracked) file.
You use git-reset's hard mode to go one commit back, the new
(untracked) file's still there.
You add/stage that new file.
You use git-reset's hard mode again to go one commit back, and the new
untracked file you just staged gets deleted.

Also, according to Git-scm
(http://git-scm.com/book/en/Git-Basics-Recording-Changes-to-the-Repository):

"Tracked files are files that were in the last snapshot [...].
Untracked files are everything else."

So it seems to me like staged untracked files shouldn't be considered
as tracked files, and thus shouldn't be removed. Or maybe, git-reset's
hard mode should always delete everything including untracked files?
It would also make sense, given the numerous modes it has.

--
Pierre-François CLEMENT
Application developer at Upcast Social
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/7] test: turn USR_BIN_TIME into a lazy prerequisite

2014-06-09 Thread Junio C Hamano
Two test scripts (t3302 and t3419) had copy & paste code to set
USR_BIN_TIME prerequisite.  Use the test_lazy_prereq helper to define
them in the common t/test-lib.sh.

Signed-off-by: Junio C Hamano 
---
 t/t3302-notes-index-expensive.sh | 1 -
 t/t3419-rebase-patch-id.sh   | 1 -
 t/test-lib.sh| 4 
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index e35d781..dc706ab 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -9,7 +9,6 @@ test_description='Test commit notes index (expensive!)'
 
 test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_NOTES_TIMING_TESTS" && test_set_prereq EXPENSIVE
-test -x /usr/bin/time && test_set_prereq USR_BIN_TIME
 
 create_repo () {
number_of_commits=$1
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index e70ac10..08e30b3 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -6,7 +6,6 @@ test_description='git rebase - test patch id computation'
 
 test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE
-test -x /usr/bin/time && test_set_prereq USR_BIN_TIME
 
 count()
 {
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d70d05e..884c57c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -859,6 +859,10 @@ test_lazy_prereq EXPENSIVE '
test -n "$GIT_TEST_LONG"
 '
 
+test_lazy_prereq USR_BIN_TIME '
+   test -x /usr/bin/time
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/7] t3302: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite

2014-06-09 Thread Junio C Hamano
This was only necessary because do_tests helper the script defines
took its parameters in a wrong order.  Just pass an empty string (or
not passing the optional EXPENSIVE prerequisite) when running the
test with a light-weight set of parameters and have the shell do the
right thing when parsing test_expect_success helper.

Signed-off-by: Junio C Hamano 
---
 t/t3302-notes-index-expensive.sh | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index 7712cf3..8d44e04 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -7,7 +7,6 @@ test_description='Test commit notes index (expensive!)'
 
 . ./test-lib.sh
 
-test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_NOTES_TIMING_TESTS" && test_set_prereq EXPENSIVE
 
 create_repo () {
@@ -103,8 +102,7 @@ time_notes () {
 }
 
 do_tests () {
-   pr=$1
-   count=$2
+   count=$1 pr=${2-}
 
test_expect_success $pr "setup $count" '
mkdir "$count" &&
@@ -121,7 +119,7 @@ do_tests () {
)
'
 
-   test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' '
+   test_expect_success "USR_BIN_TIME${pr:+,$pr}" 'notes timing with 
/usr/bin/time' '
(
cd "$count" &&
time_notes 100
@@ -129,10 +127,10 @@ do_tests () {
'
 }
 
-do_tests NOT_EXPENSIVE 10
+do_tests 10
 for count in 100 1000 1
 do
-   do_tests EXPENSIVE $count
+   do_tests "$count" EXPENSIVE
 done
 
 test_done
-- 
2.0.0-435-g307a092

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Reset by checkout?

2014-06-09 Thread Kevin Bracey

On 07/06/2014 17:52, Philip Oakley wrote:



Just to say there has been a similar confusion about 'git reset' 
reported on the Git Users group for the case of reset with added 
(staged), but uncommitted changes being wiped out, which simlarly 
reports on the difficulty of explaining some of the conditions 
especially when some are wrong ;-)


https://groups.google.com/forum/#!topic/git-users/27_FxIV_100


I'm coming around to the view that "git reset " should be (almost) 
demoted to plumbing, leaving only the "reset " that reverses "add 
" as everyday Porcelain.


I think "reset --keep" and "--merge" were a step in the wrong direction, 
at least for the Porcelain - trying to make reset  "more useful", 
rather than less necessary. Normal users shouldn't be needing to touch 
these hard-to-explain-and-slightly-dangerous commands.


The addition of "--abort" to merge and other commands was much more 
solid. They helped a lot, and I think we should follow that model by 
adding "--undo" to various commands. That would mop up all the common 
"reset"s, in conjunction with Atsushi's proposed "checkout -u" 
alternative to -B, which I quite like.


Main few:

commit --undo = reset --soft HEAD^
merge --undo  = reset --keep HEAD^
rebase --undo = reset --keep ORIG_HEAD   [bug report: rebase -p doesn't 
set ORIG_HEAD reliably]
pull --undo = merge/rebase --undo depending on rebase settings [could we 
go nuts and undo the fetch too?]


Bonus:

commit --amend --undo: reset --soft HEAD@{1}

The undos can also have a bit of extra veneer that checks the log/reflog 
for whether it matches the proposed undo, and also checks the upstream 
to see if the thing being undone is already public.


Given those, I honestly don't think I'd ever need to explain git reset 
 to anyone again. Which would be nice...


(Note I propose no "--mixed" equivalent for the commit undos, but it's 
easy enough to follow the "commit --undo" with a normal "git reset". I'd 
rather re-document the normal git reset under "commit --undo" than add 
and document yet another option).


Kevin

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 0/1] refs.c: SSE4.2 optimizations for check_refname_component

2014-06-09 Thread Junio C Hamano
Junio C Hamano  writes:

> David Turner  writes:
>
>> Since Junio has picked up the first patch from previous versions of
>> this series, I'm just going to send the second (SSE) one.  I decided
>> not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses
>> the former convention (for instance, that's what GIT_PARSE_WITH
>> generates).
>
> Yeah but NO_FROTZ is used only when FROTZ is something everybody is
> expected to have (e.g. it's in posix, people ought to have it, but
> we do support those who don't), isn't it?  For a very arch specific
> stuff like sse42, I'd feel better to make it purely opt-in by
> forcing people to explicitly say HAVE_SSE42 to enable it.

Just FYI: I am getting

compat/cpuid.h:8:12: error: 'processor_supports_sse42' defined but
not used [-Werror=unused-function]
cc1: all warnings being treated as errors

while building 'pu'; I'll have to rebuild 'pu' without this patch
before I can push the day's result out.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/15] use get_commit_buffer everywhere

2014-06-09 Thread Junio C Hamano
Jeff King  writes:

> diff --git a/notes-merge.c b/notes-merge.c
> index 94a1a8a..7885ab2 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -671,7 +671,8 @@ int notes_merge_commit(struct notes_merge_options *o,
>   DIR *dir;
>   struct dirent *e;
>   struct strbuf path = STRBUF_INIT;
> - char *msg = strstr(partial_commit->buffer, "\n\n");
> + const char *buffer = get_commit_buffer(partial_commit);
> + const char *msg = strstr(buffer, "\n\n");

This tightening causes...

>   struct strbuf sb_msg = STRBUF_INIT;
>   int baselen;
>  
> @@ -720,6 +721,7 @@ int notes_merge_commit(struct notes_merge_options *o,
>   }
>  
>   strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);

...a new error here:

notes-merge.c:723:2: error: passing argument 2 of 'strbuf_attach'
discards 'const' qualifier from pointer target type [-Werror]
strbuf.h:19:13: note: expected 'void *' but argument is of type
'const char *'

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 0/1] refs.c: SSE4.2 optimizations for check_refname_component

2014-06-09 Thread David Turner
On Mon, 2014-06-09 at 15:16 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > Since Junio has picked up the first patch from previous versions of
> > this series, I'm just going to send the second (SSE) one.  I decided
> > not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses
> > the former convention (for instance, that's what GIT_PARSE_WITH
> > generates).
> 
> Yeah but NO_FROTZ is used only when FROTZ is something everybody is
> expected to have (e.g. it's in posix, people ought to have it, but
> we do support those who don't), isn't it?  For a very arch specific
> stuff like sse42, I'd feel better to make it purely opt-in by
> forcing people to explicitly say HAVE_SSE42 to enable it.

The patch now has two kinds of autodetection:

1. At build-time, we check for the compiler supporting -msse4.2.  If it
does, and if the user has not explicitly done --without-sse, then we
build with SSE support.  This does not mean that the SSE code will
necessarily be used because:
2. At run-time, if we have built with SSE support, we check cpuid to
choose a version of the function that will run on the current CPU.

So I think we never hit a case where we try to use SSE and fail, which
is the major reason I see to make it non-default.

To me, this means that we should not require people to explicitly
request SSE, because we generally want to try to provide the
most-efficient version of git that will work everywhere.  In fact, I am
not sure we need a --without-sse option at all, since all it saves is a
cpuid instruction.  But I don't need to remove the option, in case
there's a use for it I'm not thinking of.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 19/19] wt-status: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 wt-status.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 318a191..a89cd73 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1168,8 +1168,7 @@ static char *read_and_strip_branch(const char *path)
else if (!get_sha1_hex(sb.buf, sha1)) {
const char *abbrev;
abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
-   strbuf_reset(&sb);
-   strbuf_addstr(&sb, abbrev);
+   strbuf_setstr(&sb, abbrev);
} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
goto got_nothing;
else/* bisect */
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 14/19] ident: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 ident.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ident.c b/ident.c
index 1d9b6e7..523e249 100644
--- a/ident.c
+++ b/ident.c
@@ -397,8 +397,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
-   strbuf_reset(&git_default_name);
-   strbuf_addstr(&git_default_name, value);
+   strbuf_setstr(&git_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
return 0;
@@ -407,8 +406,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
if (!strcmp(var, "user.email")) {
if (!value)
return config_error_nonbool(var);
-   strbuf_reset(&git_default_email);
-   strbuf_addstr(&git_default_email, value);
+   strbuf_setstr(&git_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return 0;
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 16/19] submodule: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 submodule.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index 3402af6..878cc48 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1324,13 +1324,12 @@ void connect_work_tree_and_git_dir(const char 
*work_tree, const char *git_dir)
const char *real_work_tree = xstrdup(real_path(work_tree));
 
/* Update gitfile */
-   strbuf_addf(&file_name, "%s/.git", work_tree);
+   strbuf_setf(&file_name, "%s/.git", work_tree);
write_file(file_name.buf, 1, "gitdir: %s\n",
   relative_path(real_git_dir, real_work_tree, &rel_path));
 
/* Update core.worktree setting */
-   strbuf_reset(&file_name);
-   strbuf_addf(&file_name, "%s/config", real_git_dir);
+   strbuf_setf(&file_name, "%s/config", real_git_dir);
if (git_config_set_in_file(file_name.buf, "core.worktree",
   relative_path(real_work_tree, real_git_dir,
 &rel_path)))
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 18/19] vcs-svn/svndump: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 vcs-svn/svndump.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 31d1d83..7fbf5d8 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -352,8 +352,7 @@ void svndump_read(const char *url, const char *local_ref, 
const char *notes_ref)
case sizeof("UUID"):
if (constcmp(t, "UUID"))
continue;
-   strbuf_reset(&dump_ctx.uuid);
-   strbuf_addstr(&dump_ctx.uuid, val);
+   strbuf_setstr(&dump_ctx.uuid, val);
break;
case sizeof("Revision-number"):
if (constcmp(t, "Revision-number"))
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 15/19] remote-curl: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 remote-curl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 4493b38..49d27f5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -288,8 +288,7 @@ static struct discovery* discover_refs(const char *service, 
int for_push)
 */
line = packet_read_line_buf(&last->buf, &last->len, NULL);
 
-   strbuf_reset(&exp);
-   strbuf_addf(&exp, "# service=%s", service);
+   strbuf_setf(&exp, "# service=%s", service);
if (strcmp(line, exp.buf))
die("invalid server response; got '%s'", line);
strbuf_release(&exp);
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 17/19] transport: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 transport.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/transport.c b/transport.c
index 172b3d8..e8f5dfa 100644
--- a/transport.c
+++ b/transport.c
@@ -1092,8 +1092,7 @@ static int run_pre_push_hook(struct transport *transport,
if (r->status == REF_STATUS_REJECT_STALE) continue;
if (r->status == REF_STATUS_UPTODATE) continue;
 
-   strbuf_reset(&buf);
-   strbuf_addf( &buf, "%s %s %s %s\n",
+   strbuf_setf( &buf, "%s %s %s %s\n",
 r->peer_ref->name, sha1_to_hex(r->new_sha1),
 r->name, sha1_to_hex(r->old_sha1));
 
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 11/19] diffcore-order: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 diffcore-order.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/diffcore-order.c b/diffcore-order.c
index 97dd3d0..5a93971 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -63,8 +63,7 @@ static int match_order(const char *path)
static struct strbuf p = STRBUF_INIT;
 
for (i = 0; i < order_cnt; i++) {
-   strbuf_reset(&p);
-   strbuf_addstr(&p, path);
+   strbuf_setstr(&p, path);
while (p.buf[0]) {
char *cp;
if (!wildmatch(order[i], p.buf, 0, NULL))
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 13/19] http: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 http.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 2b4f6a3..626fed7 100644
--- a/http.c
+++ b/http.c
@@ -1098,8 +1098,7 @@ static int update_url_from_redirect(struct strbuf *base,
strcmp(tail, got->buf + got->len - tail_len))
return 0; /* insane redirect scheme */
 
-   strbuf_reset(base);
-   strbuf_add(base, got->buf, got->len - tail_len);
+   strbuf_set(base, got->buf, got->len - tail_len);
return 1;
 }
 
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 08/19] builtin/mailinfo: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 builtin/mailinfo.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..596071e 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -40,8 +40,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf 
*name, struct strbuf
src = email;
else if (name == out)
return;
-   strbuf_reset(out);
-   strbuf_addbuf(out, src);
+   strbuf_setbuf(out, src);
 }
 
 static void parse_bogus_from(const struct strbuf *line)
@@ -62,11 +61,9 @@ static void parse_bogus_from(const struct strbuf *line)
if (!ket)
return;
 
-   strbuf_reset(&email);
-   strbuf_add(&email, bra + 1, ket - bra - 1);
+   strbuf_set(&email, bra + 1, ket - bra - 1);
 
-   strbuf_reset(&name);
-   strbuf_add(&name, line->buf, bra - line->buf);
+   strbuf_set(&name, line->buf, bra - line->buf);
strbuf_trim(&name);
get_sane_name(&name, &name, &email);
 }
@@ -108,8 +105,7 @@ static void handle_from(const struct strbuf *from)
at--;
}
el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset(&email);
-   strbuf_add(&email, at, el);
+   strbuf_set(&email, at, el);
strbuf_remove(&f, at - f.buf, el + (at[el] ? 1 : 0));
 
/* The remainder is name.  It could be
@@ -567,8 +563,7 @@ static int decode_header_bq(struct strbuf *it)
in = ep + 2;
}
strbuf_addstr(&outbuf, in);
-   strbuf_reset(it);
-   strbuf_addbuf(it, &outbuf);
+   strbuf_setbuf(it, &outbuf);
 decode_header_bq_out:
strbuf_release(&outbuf);
strbuf_release(&charset_q);
@@ -602,8 +597,7 @@ static void decode_transfer_encoding(struct strbuf *line)
default:
return;
}
-   strbuf_reset(line);
-   strbuf_addbuf(line, ret);
+   strbuf_setbuf(line, ret);
strbuf_release(ret);
free(ret);
 }
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 09/19] builtin/tag: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 builtin/tag.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..b545c21 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -496,8 +496,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const 
char *name)
if (name[0] == '-')
return -1;
 
-   strbuf_reset(sb);
-   strbuf_addf(sb, "refs/tags/%s", name);
+   strbuf_setf(sb, "refs/tags/%s", name);
 
return check_refname_format(sb->buf, 0);
 }
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 10/19] date: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 date.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/date.c b/date.c
index 782de95..0b723a4 100644
--- a/date.c
+++ b/date.c
@@ -166,8 +166,7 @@ const char *show_date(unsigned long time, int tz, enum 
date_mode mode)
static struct strbuf timebuf = STRBUF_INIT;
 
if (mode == DATE_RAW) {
-   strbuf_reset(&timebuf);
-   strbuf_addf(&timebuf, "%lu %+05d", time, tz);
+   strbuf_setf(&timebuf, "%lu %+05d", time, tz);
return timebuf.buf;
}
 
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 12/19] http-backend: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 http-backend.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index d2c0a62..25c7435 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -489,14 +489,12 @@ static void service_rpc(char *service_name)
struct rpc_service *svc = select_service(service_name);
struct strbuf buf = STRBUF_INIT;
 
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "application/x-git-%s-request", svc->name);
+   strbuf_setf(&buf, "application/x-git-%s-request", svc->name);
check_content_type(buf.buf);
 
hdr_nocache();
 
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "application/x-git-%s-result", svc->name);
+   strbuf_setf(&buf, "application/x-git-%s-result", svc->name);
hdr_str(content_type, buf.buf);
 
end_headers();
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 03/19] fast-import: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 fast-import.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index e8ec34d..cfe9404 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2898,8 +2898,7 @@ static void cat_blob(struct object_entry *oe, unsigned 
char sha1[20])
 * Output based on batch_one_object() from cat-file.c.
 */
if (type <= 0) {
-   strbuf_reset(&line);
-   strbuf_addf(&line, "%s missing\n", sha1_to_hex(sha1));
+   strbuf_setf(&line, "%s missing\n", sha1_to_hex(sha1));
cat_blob_write(line.buf, line.len);
strbuf_release(&line);
free(buf);
@@ -2910,8 +2909,7 @@ static void cat_blob(struct object_entry *oe, unsigned 
char sha1[20])
if (type != OBJ_BLOB)
die("Object %s is a %s but a blob was expected.",
sha1_to_hex(sha1), typename(type));
-   strbuf_reset(&line);
-   strbuf_addf(&line, "%s %s %lu\n", sha1_to_hex(sha1),
+   strbuf_setf(&line, "%s %s %lu\n", sha1_to_hex(sha1),
typename(type), size);
cat_blob_write(line.buf, line.len);
strbuf_release(&line);
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 04/19] builtin/remote: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 builtin/remote.c | 59 
 1 file changed, 21 insertions(+), 38 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index c9b67ff..f2d809d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -193,8 +193,7 @@ static int add(int argc, const char **argv)
return 1;
 
if (!mirror || mirror & MIRROR_FETCH) {
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s.fetch", name);
+   strbuf_setf(&buf, "remote.%s.fetch", name);
if (track.nr == 0)
string_list_append(&track, "*");
for (i = 0; i < track.nr; i++) {
@@ -205,15 +204,13 @@ static int add(int argc, const char **argv)
}
 
if (mirror & MIRROR_PUSH) {
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s.mirror", name);
+   strbuf_setf(&buf, "remote.%s.mirror", name);
if (git_config_set(buf.buf, "true"))
return 1;
}
 
if (fetch_tags != TAGS_DEFAULT) {
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s.tagopt", name);
+   strbuf_setf(&buf, "remote.%s.tagopt", name);
if (git_config_set(buf.buf,
fetch_tags == TAGS_SET ? "--tags" : "--no-tags"))
return 1;
@@ -223,11 +220,9 @@ static int add(int argc, const char **argv)
return 1;
 
if (master) {
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "refs/remotes/%s/HEAD", name);
+   strbuf_setf(&buf, "refs/remotes/%s/HEAD", name);
 
-   strbuf_reset(&buf2);
-   strbuf_addf(&buf2, "refs/remotes/%s/%s", name, master);
+   strbuf_setf(&buf2, "refs/remotes/%s/%s", name, master);
 
if (create_symref(buf.buf, buf2.buf, "remote add"))
return error(_("Could not setup master '%s'"), master);
@@ -584,19 +579,17 @@ static int migrate_file(struct remote *remote)
int i;
const char *path = NULL;
 
-   strbuf_addf(&buf, "remote.%s.url", remote->name);
+   strbuf_setf(&buf, "remote.%s.url", remote->name);
for (i = 0; i < remote->url_nr; i++)
if (git_config_set_multivar(buf.buf, remote->url[i], "^$", 0))
return error(_("Could not append '%s' to '%s'"),
remote->url[i], buf.buf);
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s.push", remote->name);
+   strbuf_setf(&buf, "remote.%s.push", remote->name);
for (i = 0; i < remote->push_refspec_nr; i++)
if (git_config_set_multivar(buf.buf, remote->push_refspec[i], 
"^$", 0))
return error(_("Could not append '%s' to '%s'"),
remote->push_refspec[i], buf.buf);
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s.fetch", remote->name);
+   strbuf_setf(&buf, "remote.%s.fetch", remote->name);
for (i = 0; i < remote->fetch_refspec_nr; i++)
if (git_config_set_multivar(buf.buf, remote->fetch_refspec[i], 
"^$", 0))
return error(_("Could not append '%s' to '%s'"),
@@ -640,27 +633,24 @@ static int mv(int argc, const char **argv)
if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr))
die(_("remote %s already exists."), rename.new);
 
-   strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
+   strbuf_setf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
if (!valid_fetch_refspec(buf.buf))
die(_("'%s' is not a valid remote name"), rename.new);
 
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s", rename.old);
-   strbuf_addf(&buf2, "remote.%s", rename.new);
+   strbuf_setf(&buf, "remote.%s", rename.old);
+   strbuf_setf(&buf2, "remote.%s", rename.new);
if (git_config_rename_section(buf.buf, buf2.buf) < 1)
return error(_("Could not rename config section '%s' to '%s'"),
buf.buf, buf2.buf);
 
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "remote.%s.fetch", rename.new);
+   strbuf_setf(&buf, "remote.%s.fetch", rename.new);
if (git_config_set_multivar(buf.buf, NULL, NULL, 1))
return error(_("Could not remove config section '%s'"), 
buf.buf);
-   strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
+   strbuf_setf(&old_remote_context, ":refs/remotes/%s/", rename.old);
for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
char *ptr;
 
-   strbuf_reset(&buf2);
-   strbuf_addstr(&buf2, 

[PATCH v2 01/19] add strbuf_set operations

2014-06-09 Thread Jeremiah Mahler
Currently, the data in a strbuf is modified using add operations.  To
set the buffer to some data a reset must be performed before an add.

  strbuf_reset(buf);
  strbuf_add(buf, cb.buf.buf, cb.buf.len);

And this is a common sequence of operations with 70 occurrences found in
the current source code.  This includes all the different variations
(add, addf, addstr, addbuf, addch).

  FILES=`find ./ -name '*.c'`
  CNT=$(pcregrep -M "strbuf_reset.*\n.*strbuf_add" $FILES | wc -l)
  CNT=$(echo "$CNT / 2" | bc)
  echo $CNT
  70

These patches add strbuf_set operations which allow this common sequence
to be performed in one line instead of two.

  strbuf_set(buf, cb.buf.buf, cb.buf.len);

Signed-off-by: Jeremiah Mahler 
---
 Documentation/technical/api-strbuf.txt | 18 ++
 strbuf.c   | 21 +
 strbuf.h   | 14 ++
 3 files changed, 53 insertions(+)

diff --git a/Documentation/technical/api-strbuf.txt 
b/Documentation/technical/api-strbuf.txt
index 077a709..b7e23da 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -149,6 +149,24 @@ Functions
than zero if the first buffer is found, respectively, to be less than,
to match, or be greater than the second buffer.
 
+* Setting the buffer
+
+`strbuf_set`::
+
+   Replace the buffer content with data of a given length.
+
+`strbuf_setstr`::
+
+   Replace the buffer content with data from a NUL-terminated string.
+
+`strbuf_setf`::
+
+   Replace the buffer content with a formatted string.
+
+`strbuf_setbuf`::
+
+   Replace the buffer content with data from another buffer.
+
 * Adding data to the buffer
 
 NOTE: All of the functions in this section will grow the buffer as necessary.
diff --git a/strbuf.c b/strbuf.c
index ac62982..9d64b00 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t 
len,
strbuf_setlen(sb, sb->len + dlen - len);
 }
 
+void strbuf_set(struct strbuf *sb, const void *data, size_t len)
+{
+   strbuf_reset(sb);
+   strbuf_add(sb, data, len);
+}
+
+void strbuf_setf(struct strbuf *sb, const char *fmt, ...)
+{
+   va_list ap;
+   strbuf_reset(sb);
+   va_start(ap, fmt);
+   strbuf_vaddf(sb, fmt, ap);
+   va_end(ap);
+}
+
+void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)
+{
+   strbuf_reset(sb);
+   strbuf_add(sb, sb2->buf, sb2->len);
+}
+
 void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
 {
strbuf_splice(sb, pos, 0, data, len);
diff --git a/strbuf.h b/strbuf.h
index e9ad03e..b339f08 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -101,6 +101,20 @@ static inline struct strbuf **strbuf_split(const struct 
strbuf *sb,
  */
 extern void strbuf_list_free(struct strbuf **);
 
+/*- set buffer to data -*/
+
+extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);
+
+static inline void strbuf_setstr(struct strbuf *sb, const char *s)
+{
+   strbuf_set(sb, s, strlen(s));
+}
+
+__attribute__((format (printf,2,3)))
+extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);
+
+extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);
+
 /*- add data in your buffer -*/
 static inline void strbuf_addch(struct strbuf *sb, int c)
 {
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 07/19] builtin/checkout: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 builtin/checkout.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9cbe7d1..38fc0ce 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -912,8 +912,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
  sb_git.buf);
junk_work_tree = path;
 
-   strbuf_reset(&sb);
-   strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
+   strbuf_setf(&sb, "%s/gitdir", sb_repo.buf);
write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
write_file(sb_git.buf, 1, "gitdir: %s/repos/%s\n",
   real_path(get_git_common_dir()), name);
@@ -923,11 +922,9 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
 * value would do because this value will be ignored and
 * replaced at the next (real) checkout.
 */
-   strbuf_reset(&sb);
-   strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
+   strbuf_setf(&sb, "%s/HEAD", sb_repo.buf);
write_file(sb.buf, 1, "%s\n", sha1_to_hex(new->commit->object.sha1));
-   strbuf_reset(&sb);
-   strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
+   strbuf_setf(&sb, "%s/commondir", sb_repo.buf);
write_file(sb.buf, 1, "../..\n");
 
if (!opts->quiet)
@@ -942,8 +939,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
ret = run_command(&cp);
if (!ret)
is_junk = 0;
-   strbuf_reset(&sb);
-   strbuf_addf(&sb, "%s/locked", sb_repo.buf);
+   strbuf_setf(&sb, "%s/locked", sb_repo.buf);
unlink_or_warn(sb.buf);
strbuf_release(&sb);
strbuf_release(&sb_repo);
@@ -1048,8 +1044,7 @@ static void check_linked_checkouts(struct branch_info 
*new)
return;
}
 
-   strbuf_reset(&path);
-   strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+   strbuf_setf(&path, "%s/HEAD", get_git_common_dir());
/*
 * $GIT_COMMON_DIR/HEAD is practically outside
 * $GIT_DIR so resolve_ref_unsafe() won't work (it
@@ -1064,8 +1059,7 @@ static void check_linked_checkouts(struct branch_info 
*new)
while ((d = readdir(dir)) != NULL) {
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
-   strbuf_reset(&path);
-   strbuf_addf(&path, "%s/repos/%s/HEAD",
+   strbuf_setf(&path, "%s/repos/%s/HEAD",
get_git_common_dir(), d->d_name);
if (check_linked_checkout(new, d->d_name, path.buf))
break;
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 05/19] branch: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 branch.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/branch.c b/branch.c
index 660097b..bc7cc7e 100644
--- a/branch.c
+++ b/branch.c
@@ -65,13 +65,11 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_addf(&key, "branch.%s.remote", local);
git_config_set(key.buf, origin ? origin : ".");
 
-   strbuf_reset(&key);
-   strbuf_addf(&key, "branch.%s.merge", local);
+   strbuf_setf(&key, "branch.%s.merge", local);
git_config_set(key.buf, remote);
 
if (rebasing) {
-   strbuf_reset(&key);
-   strbuf_addf(&key, "branch.%s.rebase", local);
+   strbuf_setf(&key, "branch.%s.rebase", local);
git_config_set(key.buf, "true");
}
strbuf_release(&key);
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 06/19] builtin/branch: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 builtin/branch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 2d1c57c..ad641b6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -984,8 +984,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 
strbuf_addf(&buf, "branch.%s.remote", branch->name);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
-   strbuf_reset(&buf);
-   strbuf_addf(&buf, "branch.%s.merge", branch->name);
+   strbuf_setf(&buf, "branch.%s.merge", branch->name);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_release(&buf);
} else if (argc > 0 && argc <= 2) {
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 02/19] sha1_name: simplify via strbuf_set()

2014-06-09 Thread Jeremiah Mahler
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.

Signed-off-by: Jeremiah Mahler 
---
 sha1_name.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 2b6322f..f88b66c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -920,8 +920,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, 
unsigned char *nsha1,
return 0;
if (--(cb->remaining) == 0) {
len = target - match;
-   strbuf_reset(&cb->buf);
-   strbuf_add(&cb->buf, match, len);
+   strbuf_set(&cb->buf, match, len);
return 1; /* we are done */
}
return 0;
@@ -957,8 +956,7 @@ static int interpret_nth_prior_checkout(const char *name, 
int namelen,
 
retval = 0;
if (0 < for_each_reflog_ent_reverse("HEAD", grab_nth_branch_switch, 
&cb)) {
-   strbuf_reset(buf);
-   strbuf_add(buf, cb.buf.buf, cb.buf.len);
+   strbuf_set(buf, cb.buf.buf, cb.buf.len);
retval = brace - name + 1;
}
 
@@ -1025,8 +1023,7 @@ static int interpret_empty_at(const char *name, int 
namelen, int len, struct str
if (next != name + 1)
return -1;
 
-   strbuf_reset(buf);
-   strbuf_add(buf, "HEAD", 4);
+   strbuf_set(buf, "HEAD", 4);
return 1;
 }
 
@@ -1044,8 +1041,7 @@ static int reinterpret(const char *name, int namelen, int 
len, struct strbuf *bu
strbuf_setlen(buf, used);
return len;
}
-   strbuf_reset(buf);
-   strbuf_addbuf(buf, &tmp);
+   strbuf_setbuf(buf, &tmp);
strbuf_release(&tmp);
/* tweak for size of {-N} versus expanded ref name */
return ret - used + len;
@@ -1054,8 +1050,7 @@ static int reinterpret(const char *name, int namelen, int 
len, struct strbuf *bu
 static void set_shortened_ref(struct strbuf *buf, const char *ref)
 {
char *s = shorten_unambiguous_ref(ref, 0);
-   strbuf_reset(buf);
-   strbuf_addstr(buf, s);
+   strbuf_setstr(buf, s);
free(s);
 }
 
-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 00/19] add strbuf_set operations

2014-06-09 Thread Jeremiah Mahler
Version 2 of the patch set to add strbuf_set operations.

Includes suggestions from Eric Sunshine [1]:

  - New operations and their documentation placed in one patch.

  - Less ambiguous documentation: "Replace the buffer content with [...]"

  - Use imperative mood in log messages.

  - Don't use strbuf_set operations in cases where strbuf_add is used to
build up a buffer in multiple steps.  Multiple adds suggest that
re-ordering is possible whereas a strbuf_set at the beginning
suggests that it isn't.  This is confusing.

Using strbuf_set before a sequence of adds can be confusing.  But using
strbuf_add when nothing important was being added to can also be
confusing.  strbuf_set can be used to make these cases more clear.

  struct strbuf mybuf = STRBUF_INIT;

  strbuf_add(&mybuf, ...);  /* Was something there before? */

  strbuf_set(&mybuf, ...);  /* Replace everything. */

Several single line replacements were made for this reason.

Additional files have also been converted compared to version 1 [1].

[1]: http://marc.info/?l=git&m=140230874124060&w=2

Jeremiah Mahler (19):
  add strbuf_set operations
  sha1_name: simplify via strbuf_set()
  fast-import: simplify via strbuf_set()
  builtin/remote: simplify via strbuf_set()
  branch: simplify via strbuf_set()
  builtin/branch: simplify via strbuf_set()
  builtin/checkout: simplify via strbuf_set()
  builtin/mailinfo: simplify via strbuf_set()
  builtin/tag: simplify via strbuf_set()
  date: simplify via strbuf_set()
  diffcore-order: simplify via strbuf_set()
  http-backend: simplify via strbuf_set()
  http: simplify via strbuf_set()
  ident: simplify via strbuf_set()
  remote-curl: simplify via strbuf_set()
  submodule: simplify via strbuf_set()
  transport: simplify via strbuf_set()
  vcs-svn/svndump: simplify via strbuf_set()
  wt-status: simplify via strbuf_set()

 Documentation/technical/api-strbuf.txt | 18 +++
 branch.c   |  6 ++--
 builtin/branch.c   |  3 +-
 builtin/checkout.c | 18 ---
 builtin/mailinfo.c | 18 ---
 builtin/remote.c   | 59 --
 builtin/tag.c  |  3 +-
 date.c |  3 +-
 diffcore-order.c   |  3 +-
 fast-import.c  |  6 ++--
 http-backend.c |  6 ++--
 http.c |  3 +-
 ident.c|  6 ++--
 remote-curl.c  |  3 +-
 sha1_name.c| 15 +++--
 strbuf.c   | 21 
 strbuf.h   | 14 
 submodule.c|  5 ++-
 transport.c|  3 +-
 vcs-svn/svndump.c  |  3 +-
 wt-status.c|  3 +-
 21 files changed, 110 insertions(+), 109 deletions(-)

-- 
2.0.0.592.gf55b190

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 0/1] refs.c: SSE4.2 optimizations for check_refname_component

2014-06-09 Thread Junio C Hamano
David Turner  writes:

> Since Junio has picked up the first patch from previous versions of
> this series, I'm just going to send the second (SSE) one.  I decided
> not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses
> the former convention (for instance, that's what GIT_PARSE_WITH
> generates).

Yeah but NO_FROTZ is used only when FROTZ is something everybody is
expected to have (e.g. it's in posix, people ought to have it, but
we do support those who don't), isn't it?  For a very arch specific
stuff like sse42, I'd feel better to make it purely opt-in by
forcing people to explicitly say HAVE_SSE42 to enable it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v1 0/5] add strbuf_set operations

2014-06-09 Thread Jeremiah Mahler
Duy,

On Mon, Jun 09, 2014 at 05:39:12PM +0700, Duy Nguyen wrote:
> On Mon, Jun 9, 2014 at 3:36 PM, Jeremiah Mahler  wrote:
> > Currently, the data in a strbuf is modified using add operations.  To
> > set the buffer to some data a reset must be performed before an add.
> >
> >   strbuf_reset(buf);
> >   strbuf_add(buf, cb.buf.buf, cb.buf.len);
> >
> > And this is a common sequence of operations with 70 occurrences found in
> > the current source code.  This includes all the different variations
> > (add, addf, addstr, addbuf, addch).
> >
> >   FILES=`find ./ -name '*.c'`
> >   CNT=$(pcregrep -M "strbuf_reset.*\n.*strbuf_add" $FILES | wc -l)
> 
> Hmm.. I wonder if git-grep could do this.. There's pcre support but I
> never tried.
> 
Not sure if git-grep does this.  The multi-line (-M) support was the
thing I needed.

> >   CNT=$(echo "$CNT / 2" | bc)
> >   echo $CNT
> >   70
> 
> The change in this series looks nice. There's another pattern, save
> strbuf length, then strbuf_setlen() at the beginning or the end of a
> loop. But I think it's less often.

A quick look did not see any obvious patterns for this.  I think you are
right, there may be fewer cases.

> -- 
> Duy

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v1 4/5] fast-import.c: cleanup using strbuf_set operations

2014-06-09 Thread Jeremiah Mahler
Eric,

On Mon, Jun 09, 2014 at 06:12:12AM -0400, Eric Sunshine wrote:
> On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler  wrote:
> > Subject: fast-import.c: cleanup using strbuf_set operations
> 
...
> 
> > Signed-off-by: Jeremiah Mahler 
> > ---
> >  fast-import.c | 19 ++-
> >  1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/fast-import.c b/fast-import.c
> > index e8ec34d..c23935c 100644
> > --- a/fast-import.c
> > +++ b/fast-import.c
> > @@ -2741,8 +2741,7 @@ static void parse_new_commit(void)
> > hashcpy(b->branch_tree.versions[0].sha1,
> > b->branch_tree.versions[1].sha1);
> >
> > -   strbuf_reset(&new_data);
> > -   strbuf_addf(&new_data, "tree %s\n",
> > +   strbuf_setf(&new_data, "tree %s\n",
> > sha1_to_hex(b->branch_tree.versions[1].sha1));
> > if (!is_null_sha1(b->sha1))
> > strbuf_addf(&new_data, "parent %s\n", sha1_to_hex(b->sha1));
> 
> Unlike the cases in patches 3/5 and 5/5 where the strbuf is used or
> returned immediately following the strbuf_set() call, I am not
> convinced that this change is an improvement. This code has the
> general form:
> 
> strbuf_reset(...);
> strbuf_add(...);
> if (condition)
> strbuf_add(...);
> strbuf_add(...);
> 
> in which it is clear that the string is being built piecemeal, and
> it's easy for a programmer to insert, remove, or re-order strbuf_add()
> calls.
> 
> Replacing the first two lines with strbuf_set() somewhat obscures the
> fact that the string is going to be built up piecemeal. Plus, the
> change makes it more difficult to insert, remove, or re-order the
> strbuf_add() invocations.
> 
> This isn't a strong objection, but the benefit of the change seems
> minimal or non-existent.
> 
> Ditto for several remaining cases in this patch.
> 
...

This is a great observation that I certainly did overlook.  Using
strbuf_add or strbuf_set to help make it more obvious what the code is
doing.

By the same token, strbuf_set can be used to replace strbuf_add to make
it clear that nothing important was being added to and that the entire
buffer is being replaced.

  struct strbuf mybuf = STRBUF_INIT;

  strbuf_add(&mybuf, ...);  /* Was something there before? */

  strbuf_set(&mybuf, ...);  /* Replace everything. */

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v1 2/5] add strbuf_set operations documentation

2014-06-09 Thread Jeremiah Mahler
Eric,

On Mon, Jun 09, 2014 at 05:53:49AM -0400, Eric Sunshine wrote:
> On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler  wrote:
> > Add documentation of the strbuf_set operations to
> > technical/api-strbuf.txt.
> 
> Since this patch is concise and so closely related to patch 1/5, it
> probably should be squashed into that one.
> 
Fixed.

> More below.
> 
> > Signed-off-by: Jeremiah Mahler 
> > ---
> >  Documentation/technical/api-strbuf.txt | 18 ++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/Documentation/technical/api-strbuf.txt 
> > b/Documentation/technical/api-strbuf.txt
> > index 077a709..ab430d9 100644
> > --- a/Documentation/technical/api-strbuf.txt
> > +++ b/Documentation/technical/api-strbuf.txt
> > @@ -149,6 +149,24 @@ Functions
> > than zero if the first buffer is found, respectively, to be less 
> > than,
> > to match, or be greater than the second buffer.
> >
> > +* Setting the buffer
> > +
> > +`strbuf_set`::
> > +
> > +Set the buffer to some data up to a given length.
> 
> I personally find this slightly ambiguous. Upon reading it, the first
> question that pops into my mind is whether or not the existing strbuf
> content is replaced (even though "set" should imply that it is). I
> wonder if it would make sense to rewrite as:
> 
> Set the buffer to [...], replacing the old content
> of the buffer.
> 
> Alternately:
> 
> Replace the buffer content with [...].
> 
On a second reading, I agree that it is ambigous.  'Replace' is much
more clear.  Great suggestion.

> Ditto for the others.
> 
> > +`strbuf_setstr`::
> > +
> > +   Set the buffer to a NUL-terminated string.
> > +
> > +`strbuf_setf`::
> > +
> > +   Set the buffer to a formatted string.
> > +
> > +`strbuf_setbuf`::
> > +
> > +   Set the current buffer to the contents of some other buffer.
> > +
> >  * Adding data to the buffer
> >
> >  NOTE: All of the functions in this section will grow the buffer as 
> > necessary.
> > --

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t5551: fix the 50,000 tag test

2014-06-09 Thread Junio C Hamano
Torsten Bögershausen  writes:

> The first version of test 23 did simply check that no output was send
> to stderr.
>
> Commit 5e2c7cd2 verified that the expected tags were actually cloned.
>
> Since the day "git clone" printed "Cloning into 'too-many-refs'" to stderr,

Thanks.  It is 68b939b2 (clone: send diagnostic messages to stderr,
2013-09-18); before it we showed the message to the standard output
stream instead.

Will queue.

> the test failed because stderr was not empty.
>
> Remove the check for stderr and make t5551-23 pass again
>
> Signed-off-by: Torsten Bögershausen 
> ---
>  t/t5551-http-fetch-smart.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index e07eaf3..2c49133 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -240,8 +240,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the 
> repo' '
>  '
>  
>  test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command 
> line overflow' '
> - git clone $HTTPD_URL/smart/repo.git too-many-refs 2>err &&
> - test_line_count = 0 err &&
> + git clone $HTTPD_URL/smart/repo.git too-many-refs &&
>   (
>   cd too-many-refs &&
>   test $(git for-each-ref refs/tags | wc -l) = 5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jun 2014, #02; Fri, 6)

2014-06-09 Thread Junio C Hamano
David Kastrup  writes:

> Junio C Hamano  writes:
>
>>  "git blame" has been optimized greatly by reorganising the data
>>  structure that is used to keep track of the work to be done, thanks
>>  to David Karstrup .
>
> I guess that "reorganising the data structure" for months is not worth
> the trouble of getting the name right.

We do not usually name people in particular in the release notes
and/or the "What's cooking" report, and trying to do something
unusual misfired X-<.  Sorry about misspelling your name.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 15/15] commit: record buffer length in cache

2014-06-09 Thread Jeff King
Most callsites which use the commit buffer try to use the
cached version attached to the commit, rather than
re-reading from disk. Unfortunately, that interface provides
only a pointer to the NUL-terminated buffer, with no
indication of the original length.

For the most part, this doesn't matter. People do not put
NULs in their commit messages, and the log code is happy to
treat it all as a NUL-terminated string. However, some code
paths do care. For example, when checking signatures, we
want to be very careful that we verify all the bytes to
avoid malicious trickery.

This patch just adds an optional "size" out-pointer to
get_commit_buffer and friends. The existing callers all pass
NULL (there did not seem to be any obvious sites where we
could avoid an immediate strlen() call, though perhaps with
some further refactoring we could).

Signed-off-by: Jeff King 
---
 builtin/blame.c |  2 +-
 builtin/fast-export.c   |  2 +-
 builtin/fmt-merge-msg.c |  2 +-
 builtin/index-pack.c|  2 +-
 builtin/log.c   |  2 +-
 builtin/rev-list.c  |  2 +-
 commit.c| 54 -
 commit.h|  8 
 fsck.c  |  2 +-
 log-tree.c  |  2 +-
 merge-recursive.c   |  2 +-
 notes-merge.c   |  2 +-
 object.c|  4 ++--
 pretty.c|  4 ++--
 sequencer.c |  2 +-
 sha1_name.c |  2 +-
 16 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index cde19eb..58a2c54 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
ident, ident, path,
(!contents_from ? path :
 (!strcmp(contents_from, "-") ? "standard input" : 
contents_from)));
-   set_commit_buffer(commit, strbuf_detach(&msg, NULL));
+   set_commit_buffer(commit, msg.buf, msg.len);
 
if (!contents_from || strcmp("-", contents_from)) {
struct stat st;
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7ee5e08..05d161f 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -289,7 +289,7 @@ static void handle_commit(struct commit *commit, struct 
rev_info *rev)
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
 
parse_commit_or_die(commit);
-   commit_buffer = get_commit_buffer(commit);
+   commit_buffer = get_commit_buffer(commit, NULL);
author = strstr(commit_buffer, "\nauthor ");
if (!author)
die ("Could not find author in commit %s",
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 01f6d59..ef8b254 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -236,7 +236,7 @@ static void record_person(int which, struct string_list 
*people,
const char *field;
 
field = (which == 'a') ? "\nauthor " : "\ncommitter ";
-   buffer = get_commit_buffer(commit);
+   buffer = get_commit_buffer(commit, NULL);
name = strstr(buffer, field);
if (!name)
return;
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 995df39..25e3e9b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -786,7 +786,7 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
}
if (obj->type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
-   detach_commit_buffer(commit);
+   detach_commit_buffer(commit, NULL);
}
obj->flags |= FLAG_CHECKED;
}
diff --git a/builtin/log.c b/builtin/log.c
index 2c74260..c599eac 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -919,7 +919,7 @@ static void make_cover_letter(struct rev_info *rev, int 
use_stdout,
&need_8bit_cte);
 
for (i = 0; !need_8bit_cte && i < nr; i++) {
-   const char *buf = get_commit_buffer(list[i]);
+   const char *buf = get_commit_buffer(list[i], NULL);
if (has_non_ascii(buf))
need_8bit_cte = 1;
unuse_commit_buffer(list[i], buf);
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3fcbf21..ff84a82 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -106,7 +106,7 @@ static void show_commit(struct commit *commit, void *data)
else
putchar('\n');
 
-   if (revs->verbose_header && get_cached_commit_buffer(commit)) {
+   if (revs->verbose_header && get_cached_commit_buffer(commit, NULL)) {
struct strbuf buf = STRBUF_INIT;
struct pretty_print_context ctx = {0};
ctx.abbrev = revs->abbrev;
diff --git a/commit.c b/commit.c
index 1b344bd..b833

[PATCH 14/15] commit: convert commit->buffer to a slab

2014-06-09 Thread Jeff King
This will make it easier to manage the buffer cache
independently of the "struct commit" objects. It also
shrinks "struct commit" by one pointer, which may be
helpful.

Unfortunately it does not reduce the max memory size of
something like "rev-list", because rev-list uses
get_cached_commit_buffer() to decide not to show each
commit's output (and due to the design of slab_at, accessing
the slab requires us to extend it, allocating exactly the
same number of buffer pointers we dropped from the commit
structs).

Signed-off-by: Jeff King 
---
 commit.c | 20 +---
 commit.h |  1 -
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/commit.c b/commit.c
index d00b818..1b344bd 100644
--- a/commit.c
+++ b/commit.c
@@ -245,14 +245,17 @@ int unregister_shallow(const unsigned char *sha1)
return 0;
 }
 
+define_commit_slab(buffer_slab, void *);
+static struct buffer_slab buffer_slab = COMMIT_SLAB_INIT(1, buffer_slab);
+
 void set_commit_buffer(struct commit *commit, void *buffer)
 {
-   commit->buffer = buffer;
+   *buffer_slab_at(&buffer_slab, commit) = buffer;
 }
 
 const void *get_cached_commit_buffer(const struct commit *commit)
 {
-   return commit->buffer;
+   return *buffer_slab_at(&buffer_slab, commit);
 }
 
 const void *get_commit_buffer(const struct commit *commit)
@@ -274,20 +277,23 @@ const void *get_commit_buffer(const struct commit *commit)
 
 void unuse_commit_buffer(const struct commit *commit, const void *buffer)
 {
-   if (commit->buffer != buffer)
+   void *cached = *buffer_slab_at(&buffer_slab, commit);
+   if (cached != buffer)
free((void *)buffer);
 }
 
 void free_commit_buffer(struct commit *commit)
 {
-   free(commit->buffer);
-   commit->buffer = NULL;
+   void **b = buffer_slab_at(&buffer_slab, commit);
+   free(*b);
+   *b = NULL;
 }
 
 const void *detach_commit_buffer(struct commit *commit)
 {
-   void *ret = commit->buffer;
-   commit->buffer = NULL;
+   void **b = buffer_slab_at(&buffer_slab, commit);
+   void *ret = *b;
+   *b = NULL;
return ret;
 }
 
diff --git a/commit.h b/commit.h
index 27e4fd7..02f894b 100644
--- a/commit.h
+++ b/commit.h
@@ -20,7 +20,6 @@ struct commit {
unsigned long date;
struct commit_list *parents;
struct tree *tree;
-   char *buffer;
 };
 
 extern int save_commit_buffer;
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 13/15] commit-slab: provide a static initializer

2014-06-09 Thread Jeff King
Callers currently must use init_foo_slab() at runtime before
accessing a slab. For global slabs, it's much nicer if we
can initialize them in BSS, so that each user does not have
to add code to check-and-initialize.

Signed-off-by: Jeff King 
---
The calling convention is kind of weird. It goes:

  struct foo_slab foo = COMMIT_SLAB_INIT(1, foo);

We need to know the size of the slab's element-type in the initializer
(and we grab it from sizeof(**foo.slab).  Another option would be:

  struct foo_slab foo = COMMIT_SLAB_INIT(1, void *);

which is simpler, but requires the user to repeat the type of the slab
(and if they get it wrong, bad things happen).

Yet another option would be to simply let a zero-initialized slab be
acceptable, and have slab_at check whether "stride" is initialized (and
if not, init to 1).

 commit-slab.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/commit-slab.h b/commit-slab.h
index cc114b5..375c9c7 100644
--- a/commit-slab.h
+++ b/commit-slab.h
@@ -117,4 +117,16 @@ static int stat_ ##slabname## realloc
  * catch because GCC silently parses it by default.
  */
 
+/*
+ * Statically initialize a commit slab named "var". Note that this
+ * evaluates "stride" multiple times! Example:
+ *
+ *   struct indegree indegrees = COMMIT_SLAB_INIT(1, indegrees);
+ *
+ */
+#define COMMIT_SLAB_INIT(stride, var) { \
+   COMMIT_SLAB_SIZE / sizeof(**((var).slab)) / (stride), \
+   (stride), 0, NULL \
+}
+
 #endif /* COMMIT_SLAB_H */
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 11/15] convert logmsg_reencode to get_commit_buffer

2014-06-09 Thread Jeff King
Like the callsites in the previous commit, logmsg_reencode
already falls back to read_sha1_file when necessary.
However, I split its conversion out into its own commit
because it's a bit more complex.

We return either:

  1. The original commit->buffer

  2. A newly allocated buffer from read_sha1_file

  3. A reencoded buffer (based on either 1 or 2 above).

while trying to do as few extra reads/allocations as
possible. Callers currently free the result with
logmsg_free, but we can simplify this by pointing them
straight to unuse_commit_buffer. This is a slight layering
violation, in that we may be passing a buffer from (3).
However, since the end result is to free() anything except
(1), a behavior which is unlikely to change, and because
this makes the interface much simpler, it's a reasonable
bending of the rules.

Signed-off-by: Jeff King 
---
 builtin/blame.c |  4 ++--
 builtin/reset.c |  2 +-
 commit.h|  1 -
 pretty.c| 40 +++-
 revision.c  |  2 +-
 sequencer.c |  2 +-
 6 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 0af3a18..cde19eb 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1666,7 +1666,7 @@ static void get_commit_info(struct commit *commit,
&ret->author_time, &ret->author_tz);
 
if (!detailed) {
-   logmsg_free(message, commit);
+   unuse_commit_buffer(commit, message);
return;
}
 
@@ -1680,7 +1680,7 @@ static void get_commit_info(struct commit *commit,
else
strbuf_addf(&ret->summary, "(%s)", 
sha1_to_hex(commit->object.sha1));
 
-   logmsg_free(message, commit);
+   unuse_commit_buffer(commit, message);
 }
 
 /*
diff --git a/builtin/reset.c b/builtin/reset.c
index 7ebee07..850d532 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,7 +109,7 @@ static void print_new_head_line(struct commit *commit)
}
else
printf("\n");
-   logmsg_free(msg, commit);
+   unuse_commit_buffer(commit, msg);
 }
 
 static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/commit.h b/commit.h
index 67caf92..27e4fd7 100644
--- a/commit.h
+++ b/commit.h
@@ -156,7 +156,6 @@ struct rev_info; /* in revision.h, it circularly uses enum 
cmit_fmt */
 extern const char *logmsg_reencode(const struct commit *commit,
   char **commit_encoding,
   const char *output_encoding);
-extern void logmsg_free(const char *msg, const struct commit *commit);
 extern void get_commit_format(const char *arg, struct rev_info *);
 extern const char *format_subject(struct strbuf *sb, const char *msg,
  const char *line_separator);
diff --git a/pretty.c b/pretty.c
index d152de2..8fd60cd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -613,22 +613,9 @@ const char *logmsg_reencode(const struct commit *commit,
static const char *utf8 = "UTF-8";
const char *use_encoding;
char *encoding;
-   char *msg = commit->buffer;
+   const char *msg = get_commit_buffer(commit);
char *out;
 
-   if (!msg) {
-   enum object_type type;
-   unsigned long size;
-
-   msg = read_sha1_file(commit->object.sha1, &type, &size);
-   if (!msg)
-   die("Cannot read commit object %s",
-   sha1_to_hex(commit->object.sha1));
-   if (type != OBJ_COMMIT)
-   die("Expected commit for '%s', got %s",
-   sha1_to_hex(commit->object.sha1), typename(type));
-   }
-
if (!output_encoding || !*output_encoding) {
if (commit_encoding)
*commit_encoding =
@@ -652,12 +639,13 @@ const char *logmsg_reencode(const struct commit *commit,
 * Otherwise, we still want to munge the encoding header in the
 * result, which will be done by modifying the buffer. If we
 * are using a fresh copy, we can reuse it. But if we are using
-* the cached copy from commit->buffer, we need to duplicate it
-* to avoid munging commit->buffer.
+* the cached copy from get_commit_buffer, we need to duplicate 
it
+* to avoid munging the cached copy.
 */
-   out = msg;
-   if (out == commit->buffer)
-   out = xstrdup(out);
+   if (msg == get_cached_commit_buffer(commit))
+   out = xstrdup(msg);
+   else
+   out = (char *)msg;
}
else {
/*
@@ -667,8 +655,8 @@ const char *logmsg_reencode(const struct commit *commit,
 * copy, we can free it.
 */
out = reencode_string(msg, output_encoding, use_encoding

[PATCH 12/15] use get_commit_buffer everywhere

2014-06-09 Thread Jeff King
Each of these sites assumes that commit->buffer is valid.
Since they would segfault if this was not the case, they are
likely to be correct in practice. However, we can
future-proof them by using get_commit_buffer.

And as a side effect, we abstract away the final bare uses
of commit->buffer.

Signed-off-by: Jeff King 
---
 builtin/fast-export.c   |  5 -
 builtin/fmt-merge-msg.c |  5 -
 builtin/log.c   |  7 +--
 fsck.c  | 13 +++--
 merge-recursive.c   |  4 +++-
 notes-merge.c   |  4 +++-
 sequencer.c |  4 +++-
 7 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index b8d8a3a..7ee5e08 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -279,6 +279,7 @@ static const char *find_encoding(const char *begin, const 
char *end)
 static void handle_commit(struct commit *commit, struct rev_info *rev)
 {
int saved_output_format = rev->diffopt.output_format;
+   const char *commit_buffer;
const char *author, *author_end, *committer, *committer_end;
const char *encoding, *message;
char *reencoded = NULL;
@@ -288,7 +289,8 @@ static void handle_commit(struct commit *commit, struct 
rev_info *rev)
rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;
 
parse_commit_or_die(commit);
-   author = strstr(commit->buffer, "\nauthor ");
+   commit_buffer = get_commit_buffer(commit);
+   author = strstr(commit_buffer, "\nauthor ");
if (!author)
die ("Could not find author in commit %s",
 sha1_to_hex(commit->object.sha1));
@@ -335,6 +337,7 @@ static void handle_commit(struct commit *commit, struct 
rev_info *rev)
  ? strlen(message) : 0),
   reencoded ? reencoded : message ? message : "");
free(reencoded);
+   unuse_commit_buffer(commit, commit_buffer);
 
for (i = 0, p = commit->parents; p; p = p->next) {
int mark = get_object_mark(&p->item->object);
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3906eda..01f6d59 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -230,12 +230,14 @@ static void add_branch_desc(struct strbuf *out, const 
char *name)
 static void record_person(int which, struct string_list *people,
  struct commit *commit)
 {
+   const char *buffer;
char *name_buf, *name, *name_end;
struct string_list_item *elem;
const char *field;
 
field = (which == 'a') ? "\nauthor " : "\ncommitter ";
-   name = strstr(commit->buffer, field);
+   buffer = get_commit_buffer(commit);
+   name = strstr(buffer, field);
if (!name)
return;
name += strlen(field);
@@ -247,6 +249,7 @@ static void record_person(int which, struct string_list 
*people,
if (name_end < name)
return;
name_buf = xmemdupz(name, name_end - name + 1);
+   unuse_commit_buffer(commit, buffer);
 
elem = string_list_lookup(people, name_buf);
if (!elem) {
diff --git a/builtin/log.c b/builtin/log.c
index 226f8f2..2c74260 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -918,9 +918,12 @@ static void make_cover_letter(struct rev_info *rev, int 
use_stdout,
log_write_email_headers(rev, head, &pp.subject, &pp.after_subject,
&need_8bit_cte);
 
-   for (i = 0; !need_8bit_cte && i < nr; i++)
-   if (has_non_ascii(list[i]->buffer))
+   for (i = 0; !need_8bit_cte && i < nr; i++) {
+   const char *buf = get_commit_buffer(list[i]);
+   if (has_non_ascii(buf))
need_8bit_cte = 1;
+   unuse_commit_buffer(list[i], buf);
+   }
 
if (!branch_name)
branch_name = find_branch_name(rev);
diff --git a/fsck.c b/fsck.c
index abed62b..8223780 100644
--- a/fsck.c
+++ b/fsck.c
@@ -276,9 +276,10 @@ static int fsck_ident(const char **ident, struct object 
*obj, fsck_error error_f
return 0;
 }
 
-static int fsck_commit(struct commit *commit, fsck_error error_func)
+static int fsck_commit_buffer(struct commit *commit, const char *buffer,
+ fsck_error error_func)
 {
-   const char *buffer = commit->buffer, *tmp;
+   const char *tmp;
unsigned char tree_sha1[20], sha1[20];
struct commit_graft *graft;
int parents = 0;
@@ -336,6 +337,14 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
return 0;
 }
 
+static int fsck_commit(struct commit *commit, fsck_error error_func)
+{
+   const char *buffer = get_commit_buffer(commit);
+   int ret = fsck_commit_buffer(commit, buffer, error_func);
+   unuse_commit_buffer(commit, buffer);
+   return ret;
+}
+
 static int fsck_tag(struct tag *tag, fsck_error error_func)
 {
struct object *tagged = tag-

[PATCH 09/15] use get_cached_commit_buffer where appropriate

2014-06-09 Thread Jeff King
Some call sites check commit->buffer to see whether we have
a cached buffer, and if so, do some work with it. In the
long run we may want to switch these code paths to make
their decision on a different boolean flag (because checking
the cache may get a little more expensive in the future).
But for now, we can easily support them by converting the
calls to use get_cached_commit_buffer.

Signed-off-by: Jeff King 
---
 builtin/rev-list.c | 2 +-
 log-tree.c | 2 +-
 object.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index e012ebe..3fcbf21 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -106,7 +106,7 @@ static void show_commit(struct commit *commit, void *data)
else
putchar('\n');
 
-   if (revs->verbose_header && commit->buffer) {
+   if (revs->verbose_header && get_cached_commit_buffer(commit)) {
struct strbuf buf = STRBUF_INIT;
struct pretty_print_context ctx = {0};
ctx.abbrev = revs->abbrev;
diff --git a/log-tree.c b/log-tree.c
index cf2f86c..e9ef8ab 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -588,7 +588,7 @@ void show_log(struct rev_info *opt)
show_mergetag(opt, commit);
}
 
-   if (!commit->buffer)
+   if (!get_cached_commit_buffer(commit))
return;
 
if (opt->show_notes) {
diff --git a/object.c b/object.c
index 44ca657..67b6e35 100644
--- a/object.c
+++ b/object.c
@@ -197,7 +197,7 @@ struct object *parse_object_buffer(const unsigned char 
*sha1, enum object_type t
if (commit) {
if (parse_commit_buffer(commit, buffer, size))
return NULL;
-   if (!commit->buffer) {
+   if (!get_cached_commit_buffer(commit)) {
set_commit_buffer(commit, buffer);
*eaten_p = 1;
}
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/15] use get_commit_buffer to avoid duplicate code

2014-06-09 Thread Jeff King
For both of these sites, we already do the "fallback to
read_sha1_file" trick. But we can shorten the code by just
using get_commit_buffer.

Note that the error cases are slightly different when
read_sha1_file fails. get_commit_buffer will die() if the
object cannot be loaded, or is a non-commit.

For get_sha1_oneline, this will almost certainly never
happen, as we will have just called parse_object (and if it
does, it's probably worth complaining about).

For record_author_date, the new behavior is probably better;
we notify the user of the error instead of silently ignoring
it. And because it's used only for sorting by author-date,
somebody examining a corrupt can fallback to the regular
traversal order.

Signed-off-by: Jeff King 
---
 commit.c| 16 +++-
 sha1_name.c | 18 --
 2 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/commit.c b/commit.c
index b9ecae0..d00b818 100644
--- a/commit.c
+++ b/commit.c
@@ -583,22 +583,12 @@ static void record_author_date(struct author_date_slab 
*author_date,
   struct commit *commit)
 {
const char *buf, *line_end, *ident_line;
-   char *buffer = NULL;
+   const char *buffer = get_commit_buffer(commit);
struct ident_split ident;
char *date_end;
unsigned long date;
 
-   if (!commit->buffer) {
-   unsigned long size;
-   enum object_type type;
-   buffer = read_sha1_file(commit->object.sha1, &type, &size);
-   if (!buffer)
-   return;
-   }
-
-   for (buf = commit->buffer ? commit->buffer : buffer;
-buf;
-buf = line_end + 1) {
+   for (buf = buffer; buf; buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
ident_line = skip_prefix(buf, "author ");
if (!ident_line) {
@@ -619,7 +609,7 @@ static void record_author_date(struct author_date_slab 
*author_date,
*(author_date_slab_at(author_date, commit)) = date;
 
 fail_exit:
-   free(buffer);
+   unuse_commit_buffer(commit, buffer);
 }
 
 static int compare_commits_by_author_date(const void *a_, const void *b_,
diff --git a/sha1_name.c b/sha1_name.c
index 2b6322f..0a65d23 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -862,27 +862,17 @@ static int get_sha1_oneline(const char *prefix, unsigned 
char *sha1,
commit_list_insert(l->item, &backup);
}
while (list) {
-   char *p, *to_free = NULL;
+   const char *p, *buf;
struct commit *commit;
-   enum object_type type;
-   unsigned long size;
int matches;
 
commit = pop_most_recent_commit(&list, ONELINE_SEEN);
if (!parse_object(commit->object.sha1))
continue;
-   if (commit->buffer)
-   p = commit->buffer;
-   else {
-   p = read_sha1_file(commit->object.sha1, &type, &size);
-   if (!p)
-   continue;
-   to_free = p;
-   }
-
-   p = strstr(p, "\n\n");
+   buf = get_commit_buffer(commit);
+   p = strstr(buf, "\n\n");
matches = p && !regexec(®ex, p + 2, 0, NULL, 0);
-   free(to_free);
+   unuse_commit_buffer(commit, buf);
 
if (matches) {
hashcpy(sha1, commit->object.sha1);
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/15] provide helpers to access the commit buffer

2014-06-09 Thread Jeff King
Many sites look at commit->buffer to get more detailed
information than what is in the parsed commit struct.
However, we sometimes drop commit->buffer to save memory,
in which case the caller would need to read the object
afresh. Some callers do this (leading to duplicated code),
and others do not (which opens the possibility of a segfault
if somebody else frees the buffer).

Let's provide a pair of helpers, "get" and "unuse", that let
callers easily get the buffer. They will use the cached
buffer when possible, and otherwise load from disk using
read_sha1_file.

Note that we also need to add a "get_cached" variant which
returns NULL when we do not have a cached buffer. At first
glance this seems to defeat the purpose of "get", which is
to always provide a return value. However, some log code
paths actually use the NULL-ness of commit->buffer as a
boolean flag to decide whether to try to printing the
commit. At least for now, we want to continue supporting
that use.

Signed-off-by: Jeff King 
---
 commit.c | 28 
 commit.h | 21 +
 2 files changed, 49 insertions(+)

diff --git a/commit.c b/commit.c
index 5cc52e0..b9ecae0 100644
--- a/commit.c
+++ b/commit.c
@@ -250,6 +250,34 @@ void set_commit_buffer(struct commit *commit, void *buffer)
commit->buffer = buffer;
 }
 
+const void *get_cached_commit_buffer(const struct commit *commit)
+{
+   return commit->buffer;
+}
+
+const void *get_commit_buffer(const struct commit *commit)
+{
+   const void *ret = get_cached_commit_buffer(commit);
+   if (!ret) {
+   enum object_type type;
+   unsigned long size;
+   ret = read_sha1_file(commit->object.sha1, &type, &size);
+   if (!ret)
+   die("cannot read commit object %s",
+   sha1_to_hex(commit->object.sha1));
+   if (type != OBJ_COMMIT)
+   die("expected commit for %s, got %s",
+   sha1_to_hex(commit->object.sha1), typename(type));
+   }
+   return ret;
+}
+
+void unuse_commit_buffer(const struct commit *commit, const void *buffer)
+{
+   if (commit->buffer != buffer)
+   free((void *)buffer);
+}
+
 void free_commit_buffer(struct commit *commit)
 {
free(commit->buffer);
diff --git a/commit.h b/commit.h
index 5cc0bf3..67caf92 100644
--- a/commit.h
+++ b/commit.h
@@ -58,6 +58,27 @@ void parse_commit_or_die(struct commit *item);
 void set_commit_buffer(struct commit *, void *buffer);
 
 /*
+ * Get any cached object buffer associated with the commit. Returns NULL
+ * if none. The resulting memory should not be freed.
+ */
+const void *get_cached_commit_buffer(const struct commit *);
+
+/*
+ * Get the commit's object contents, either from cache or by reading the object
+ * from disk. The resulting memory should not be modified, and must be given
+ * to unuse_commit_buffer when the caller is done.
+ */
+const void *get_commit_buffer(const struct commit *);
+
+/*
+ * Tell the commit subsytem that we are done with a particular commit buffer.
+ * The commit and buffer should be the input and return value, respectively,
+ * from an earlier call to get_commit_buffer.  The buffer may or may not be
+ * freed by this call; callers should not access the memory afterwards.
+ */
+void unuse_commit_buffer(const struct commit *, const void *buffer);
+
+/*
  * Free any cached object buffer associated with the commit.
  */
 void free_commit_buffer(struct commit *);
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 07/15] provide a helper to set the commit buffer

2014-06-09 Thread Jeff King
Right now this is just a one-liner, but abstracting it will
make it easier to change later.

Signed-off-by: Jeff King 
---
 builtin/blame.c | 2 +-
 commit.c| 7 ++-
 commit.h| 6 ++
 object.c| 2 +-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 6ce3c6d..0af3a18 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
ident, ident, path,
(!contents_from ? path :
 (!strcmp(contents_from, "-") ? "standard input" : 
contents_from)));
-   commit->buffer = strbuf_detach(&msg, NULL);
+   set_commit_buffer(commit, strbuf_detach(&msg, NULL));
 
if (!contents_from || strcmp("-", contents_from)) {
struct stat st;
diff --git a/commit.c b/commit.c
index d7b6836..5cc52e0 100644
--- a/commit.c
+++ b/commit.c
@@ -245,6 +245,11 @@ int unregister_shallow(const unsigned char *sha1)
return 0;
 }
 
+void set_commit_buffer(struct commit *commit, void *buffer)
+{
+   commit->buffer = buffer;
+}
+
 void free_commit_buffer(struct commit *commit)
 {
free(commit->buffer);
@@ -335,7 +340,7 @@ int parse_commit(struct commit *item)
}
ret = parse_commit_buffer(item, buffer, size);
if (save_commit_buffer && !ret) {
-   item->buffer = buffer;
+   set_commit_buffer(item, buffer);
return 0;
}
free(buffer);
diff --git a/commit.h b/commit.h
index daccf46..5cc0bf3 100644
--- a/commit.h
+++ b/commit.h
@@ -52,6 +52,12 @@ int parse_commit(struct commit *item);
 void parse_commit_or_die(struct commit *item);
 
 /*
+ * Associate an object buffer with the commit. The ownership of the
+ * memory is handed over to the commit, and must be free()-able.
+ */
+void set_commit_buffer(struct commit *, void *buffer);
+
+/*
  * Free any cached object buffer associated with the commit.
  */
 void free_commit_buffer(struct commit *);
diff --git a/object.c b/object.c
index 57a0890..44ca657 100644
--- a/object.c
+++ b/object.c
@@ -198,7 +198,7 @@ struct object *parse_object_buffer(const unsigned char 
*sha1, enum object_type t
if (parse_commit_buffer(commit, buffer, size))
return NULL;
if (!commit->buffer) {
-   commit->buffer = buffer;
+   set_commit_buffer(commit, buffer);
*eaten_p = 1;
}
obj = &commit->object;
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/15] provide a helper to free commit buffer

2014-06-09 Thread Jeff King
This converts two lines into one at each caller. But more
importantly, it abstracts the concept of freeing the buffer,
which will make it easier to change later.

Note that we also need to provide a "detach" mechanism for
the weird case in fsck which passes the buffer back to be
freed.

Signed-off-by: Jeff King 
---
 builtin/fsck.c   |  3 +--
 builtin/index-pack.c |  2 +-
 builtin/log.c|  6 ++
 builtin/rev-list.c   |  3 +--
 commit.c | 13 +
 commit.h | 11 +++
 6 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index fc150c8..8aadca1 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -310,8 +310,7 @@ static int fsck_obj(struct object *obj)
if (obj->type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
 
-   free(commit->buffer);
-   commit->buffer = NULL;
+   free_commit_buffer(commit);
 
if (!commit->parents && show_root)
printf("root %s\n", sha1_to_hex(commit->object.sha1));
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 18f57de..995df39 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -786,7 +786,7 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
}
if (obj->type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
-   commit->buffer = NULL;
+   detach_commit_buffer(commit);
}
obj->flags |= FLAG_CHECKED;
}
diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..226f8f2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -349,8 +349,7 @@ static int cmd_log_walk(struct rev_info *rev)
rev->max_count++;
if (!rev->reflog_info) {
/* we allow cycles in reflog ancestry */
-   free(commit->buffer);
-   commit->buffer = NULL;
+   free_commit_buffer(commit);
}
free_commit_list(commit->parents);
commit->parents = NULL;
@@ -1508,8 +1507,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
reopen_stdout(rev.numbered_files ? NULL : commit, NULL, 
&rev, quiet))
die(_("Failed to create output files"));
shown = log_tree_commit(&rev, commit);
-   free(commit->buffer);
-   commit->buffer = NULL;
+   free_commit_buffer(commit);
 
/* We put one extra blank line between formatted
 * patches and this flag is used by log-tree code
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 9f92905..e012ebe 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -173,8 +173,7 @@ static void finish_commit(struct commit *commit, void *data)
free_commit_list(commit->parents);
commit->parents = NULL;
}
-   free(commit->buffer);
-   commit->buffer = NULL;
+   free_commit_buffer(commit);
 }
 
 static void finish_object(struct object *obj,
diff --git a/commit.c b/commit.c
index 21957ee..d7b6836 100644
--- a/commit.c
+++ b/commit.c
@@ -245,6 +245,19 @@ int unregister_shallow(const unsigned char *sha1)
return 0;
 }
 
+void free_commit_buffer(struct commit *commit)
+{
+   free(commit->buffer);
+   commit->buffer = NULL;
+}
+
+const void *detach_commit_buffer(struct commit *commit)
+{
+   void *ret = commit->buffer;
+   commit->buffer = NULL;
+   return ret;
+}
+
 int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long 
size)
 {
const char *tail = buffer;
diff --git a/commit.h b/commit.h
index de57df9..daccf46 100644
--- a/commit.h
+++ b/commit.h
@@ -51,6 +51,17 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
 int parse_commit(struct commit *item);
 void parse_commit_or_die(struct commit *item);
 
+/*
+ * Free any cached object buffer associated with the commit.
+ */
+void free_commit_buffer(struct commit *);
+
+/*
+ * Disassociate any cached object buffer from the commit, but do not free it.
+ * The buffer (or NULL, if none) is returned.
+ */
+const void *detach_commit_buffer(struct commit *);
+
 /* Find beginning and length of commit subject. */
 int find_commit_subject(const char *commit_buffer, const char **subject);
 
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 05/15] sequencer: use logmsg_reencode in get_message

2014-06-09 Thread Jeff King
This simplifies the code, as logmsg_reencode handles the
reencoding for us in a single call. It also means we learn
logmsg_reencode's trick of pulling the buffer from disk when
commit->buffer is NULL (we currently just silently return!).
It is doubtful this matters in practice, though, as
sequencer operations would not generally turn off
save_commit_buffer.

Note that we may be fixing a bug here. The existing code
does:

  if (same_encoding(to, from))
  reencode_string(buf, to, from);

That probably should have been "!same_encoding".

Signed-off-by: Jeff King 
---
I didn't actually test for the bug, so it's possible that I'm missing
something clever...

 sequencer.c | 45 +
 1 file changed, 5 insertions(+), 40 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 0a80c58..3fcab4d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -116,39 +116,23 @@ static const char *action_name(const struct replay_opts 
*opts)
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
 }
 
-static char *get_encoding(const char *message);
-
 struct commit_message {
char *parent_label;
const char *label;
const char *subject;
-   char *reencoded_message;
const char *message;
 };
 
 static int get_message(struct commit *commit, struct commit_message *out)
 {
-   const char *encoding;
const char *abbrev, *subject;
int abbrev_len, subject_len;
char *q;
 
-   if (!commit->buffer)
-   return -1;
-   encoding = get_encoding(commit->buffer);
-   if (!encoding)
-   encoding = "UTF-8";
if (!git_commit_encoding)
git_commit_encoding = "UTF-8";
 
-   out->reencoded_message = NULL;
-   out->message = commit->buffer;
-   if (same_encoding(encoding, git_commit_encoding))
-   out->reencoded_message = reencode_string(commit->buffer,
-   git_commit_encoding, encoding);
-   if (out->reencoded_message)
-   out->message = out->reencoded_message;
-
+   out->message = logmsg_reencode(commit, NULL, git_commit_encoding);
abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
abbrev_len = strlen(abbrev);
 
@@ -167,29 +151,10 @@ static int get_message(struct commit *commit, struct 
commit_message *out)
return 0;
 }
 
-static void free_message(struct commit_message *msg)
+static void free_message(struct commit *commit, struct commit_message *msg)
 {
free(msg->parent_label);
-   free(msg->reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
-   const char *p = message, *eol;
-
-   while (*p && *p != '\n') {
-   for (eol = p + 1; *eol && *eol != '\n'; eol++)
-   ; /* do nothing */
-   if (starts_with(p, "encoding ")) {
-   char *result = xmalloc(eol - 8 - p);
-   strlcpy(result, p + 9, eol - 8 - p);
-   return result;
-   }
-   p = eol;
-   if (*p == '\n')
-   p++;
-   }
-   return NULL;
+   logmsg_free(msg->message, commit);
 }
 
 static void write_cherry_pick_head(struct commit *commit, const char 
*pseudoref)
@@ -489,7 +454,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
unsigned char head[20];
struct commit *base, *next, *parent;
const char *base_label, *next_label;
-   struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+   struct commit_message msg = { NULL, NULL, NULL, NULL };
char *defmsg = NULL;
struct strbuf msgbuf = STRBUF_INIT;
int res, unborn = 0, allow;
@@ -654,7 +619,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
res = run_git_commit(defmsg, opts, allow);
 
 leave:
-   free_message(&msg);
+   free_message(commit, &msg);
free(defmsg);
 
return res;
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 04/15] logmsg_reencode: return const buffer

2014-06-09 Thread Jeff King
The return value from logmsg_reencode may be either a newly
allocated buffer or a pointer to the existing commit->buffer.
We would not want the caller to accidentally free() or
modify the latter, so let's mark it as const.  We can cast
away the constness in logmsg_free, but only once we have
determined that it is a free-able buffer.

Signed-off-by: Jeff King 
---
 builtin/blame.c |  2 +-
 builtin/reset.c |  2 +-
 commit.h|  8 
 pretty.c| 14 +++---
 revision.c  | 12 +---
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index d6056a5..6ce3c6d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1655,7 +1655,7 @@ static void get_commit_info(struct commit *commit,
 {
int len;
const char *subject, *encoding;
-   char *message;
+   const char *message;
 
commit_info_init(ret);
 
diff --git a/builtin/reset.c b/builtin/reset.c
index f368266..7ebee07 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -93,7 +93,7 @@ static int reset_index(const unsigned char *sha1, int 
reset_type, int quiet)
 static void print_new_head_line(struct commit *commit)
 {
const char *hex, *body;
-   char *msg;
+   const char *msg;
 
hex = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
printf(_("HEAD is now at %s"), hex);
diff --git a/commit.h b/commit.h
index a9f177b..de57df9 100644
--- a/commit.h
+++ b/commit.h
@@ -115,10 +115,10 @@ struct userformat_want {
 
 extern int has_non_ascii(const char *text);
 struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
-extern char *logmsg_reencode(const struct commit *commit,
-char **commit_encoding,
-const char *output_encoding);
-extern void logmsg_free(char *msg, const struct commit *commit);
+extern const char *logmsg_reencode(const struct commit *commit,
+  char **commit_encoding,
+  const char *output_encoding);
+extern void logmsg_free(const char *msg, const struct commit *commit);
 extern void get_commit_format(const char *arg, struct rev_info *);
 extern const char *format_subject(struct strbuf *sb, const char *msg,
  const char *line_separator);
diff --git a/pretty.c b/pretty.c
index e1e2cad..d152de2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -606,9 +606,9 @@ static char *replace_encoding_header(char *buf, const char 
*encoding)
return strbuf_detach(&tmp, NULL);
 }
 
-char *logmsg_reencode(const struct commit *commit,
- char **commit_encoding,
- const char *output_encoding)
+const char *logmsg_reencode(const struct commit *commit,
+   char **commit_encoding,
+   const char *output_encoding)
 {
static const char *utf8 = "UTF-8";
const char *use_encoding;
@@ -687,10 +687,10 @@ char *logmsg_reencode(const struct commit *commit,
return out ? out : msg;
 }
 
-void logmsg_free(char *msg, const struct commit *commit)
+void logmsg_free(const char *msg, const struct commit *commit)
 {
if (msg != commit->buffer)
-   free(msg);
+   free((void *)msg);
 }
 
 static int mailmap_name(const char **email, size_t *email_len,
@@ -796,7 +796,7 @@ struct format_commit_context {
struct signature_check signature_check;
enum flush_type flush_type;
enum trunc_type truncate;
-   char *message;
+   const char *message;
char *commit_encoding;
size_t width, indent1, indent2;
int auto_color;
@@ -1700,7 +1700,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
unsigned long beginning_of_body;
int indent = 4;
const char *msg;
-   char *reencoded;
+   const char *reencoded;
const char *encoding;
int need_8bit_cte = pp->need_8bit_cte;
 
diff --git a/revision.c b/revision.c
index 71e2337..47e5b7a 100644
--- a/revision.c
+++ b/revision.c
@@ -2788,7 +2788,7 @@ static int commit_match(struct commit *commit, struct 
rev_info *opt)
 {
int retval;
const char *encoding;
-   char *message;
+   const char *message;
struct strbuf buf = STRBUF_INIT;
 
if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)
@@ -2830,12 +2830,18 @@ static int commit_match(struct commit *commit, struct 
rev_info *opt)
format_display_notes(commit->object.sha1, &buf, encoding, 1);
}
 
-   /* Find either in the original commit message, or in the temporary */
+   /* Find either in the original commit message, or in the temporary.
+* Note that we cast away the constness of "message" here. It is
+* const because it may come from the cached commit buffer. That's OK,
+* because we know that it is modifiable heap memory, and that while
+* grep_buffer 

[PATCH 03/15] do not create "struct commit" with xcalloc

2014-06-09 Thread Jeff King
In both blame and merge-recursive, we sometimes create a
"fake" commit struct for convenience (e.g., to represent the
HEAD state as if we would commit it). By allocating
ourselves rather than using alloc_commit_node, we do not
properly set the "index" field of the commit. This can
produce subtle bugs if we then use commit-slab on the
resulting commit, as we will share the "0" index with
another commit.

We can fix this by using alloc_commit_node() to allocate.
Note that we cannot free the result, as it is part of our
commit allocator. However, both cases were already leaking
the allocated commit anyway, so there's nothing to fix up.

Signed-off-by: Jeff King 
---
 builtin/blame.c   | 2 +-
 merge-recursive.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index a52a279..d6056a5 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
struct strbuf msg = STRBUF_INIT;
 
time(&now);
-   commit = xcalloc(1, sizeof(*commit));
+   commit = alloc_commit_node();
commit->object.parsed = 1;
commit->date = now;
commit->object.type = OBJ_COMMIT;
diff --git a/merge-recursive.c b/merge-recursive.c
index cab16fa..2b37d42 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -40,7 +40,7 @@ static struct tree *shift_tree_object(struct tree *one, 
struct tree *two,
 
 static struct commit *make_virtual_commit(struct tree *tree, const char 
*comment)
 {
-   struct commit *commit = xcalloc(1, sizeof(struct commit));
+   struct commit *commit = alloc_commit_node();
struct merge_remote_desc *desc = xmalloc(sizeof(*desc));
 
desc->name = comment;
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/15] commit: push commit_index update into alloc_commit_node

2014-06-09 Thread Jeff King
Whenever we create a commit object via lookup_commit, we
give it a unique index to be used with the commit-slab API.
The theory is that any "struct commit" we create would
follow this code path, so any such struct would get an
index. However, callers could use alloc_commit_node()
directly (and get multiple commits with index 0).

Let's push the indexing into alloc_commit_node so that it's
hard for callers to get it wrong.

Signed-off-by: Jeff King 
---
This will make the alloc_report output a little uglier (it will say
raw_commit), but I don't think anyone cares. And I wanted to make sure
there wasn't an easy way to accidentally call the wrong alloc function
that doesn't handle the index.

 alloc.c  | 12 ++--
 commit.c |  2 --
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/alloc.c b/alloc.c
index 38ff7e7..eb22a45 100644
--- a/alloc.c
+++ b/alloc.c
@@ -47,10 +47,18 @@ union any_object {
 
 DEFINE_ALLOCATOR(blob, struct blob)
 DEFINE_ALLOCATOR(tree, struct tree)
-DEFINE_ALLOCATOR(commit, struct commit)
+DEFINE_ALLOCATOR(raw_commit, struct commit)
 DEFINE_ALLOCATOR(tag, struct tag)
 DEFINE_ALLOCATOR(object, union any_object)
 
+void *alloc_commit_node(void)
+{
+   static int commit_count;
+   struct commit *c = alloc_raw_commit_node();
+   c->index = commit_count++;
+   return c;
+}
+
 static void report(const char *name, unsigned int count, size_t size)
 {
fprintf(stderr, "%10s: %8u (%"PRIuMAX" kB)\n",
@@ -64,7 +72,7 @@ void alloc_report(void)
 {
REPORT(blob, struct blob);
REPORT(tree, struct tree);
-   REPORT(commit, struct commit);
+   REPORT(raw_commit, struct commit);
REPORT(tag, struct tag);
REPORT(object, union any_object);
 }
diff --git a/commit.c b/commit.c
index f479331..21957ee 100644
--- a/commit.c
+++ b/commit.c
@@ -17,7 +17,6 @@ static struct commit_extra_header 
*read_commit_extra_header_lines(const char *bu
 int save_commit_buffer = 1;
 
 const char *commit_type = "commit";
-static int commit_count;
 
 static struct commit *check_commit(struct object *obj,
   const unsigned char *sha1,
@@ -64,7 +63,6 @@ struct commit *lookup_commit(const unsigned char *sha1)
struct object *obj = lookup_object(sha1);
if (!obj) {
struct commit *c = alloc_commit_node();
-   c->index = commit_count++;
return create_object(sha1, OBJ_COMMIT, c);
}
if (!obj->type)
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 01/15] alloc: include any-object allocations in alloc_report

2014-06-09 Thread Jeff King
When 2c1cbec (Use proper object allocators for unknown
object nodes too, 2007-04-16), added a special "any_object"
allocator, it never taught alloc_report to report on it. To
do so we need to add an extra type argument to the REPORT
macro, as that commit did for DEFINE_ALLOCATOR.

Signed-off-by: Jeff King 
---
My ulterior motive is that I'm going to add more changes that would
disrupt this.  I'd also be happy to just drop alloc_report, as nobody
calls it (it was purely for debugging, and I suspect hasn't been used
since the initial work in 2007).

 alloc.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/alloc.c b/alloc.c
index f3ee745..38ff7e7 100644
--- a/alloc.c
+++ b/alloc.c
@@ -57,13 +57,14 @@ static void report(const char *name, unsigned int count, 
size_t size)
name, count, (uintmax_t) size);
 }
 
-#define REPORT(name)   \
-report(#name, name##_allocs, name##_allocs * sizeof(struct name) >> 10)
+#define REPORT(name, type) \
+report(#name, name##_allocs, name##_allocs * sizeof(type) >> 10)
 
 void alloc_report(void)
 {
-   REPORT(blob);
-   REPORT(tree);
-   REPORT(commit);
-   REPORT(tag);
+   REPORT(blob, struct blob);
+   REPORT(tree, struct tree);
+   REPORT(commit, struct commit);
+   REPORT(tag, struct tag);
+   REPORT(object, union any_object);
 }
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/15] store length of commit->buffer

2014-06-09 Thread Jeff King
Here's my series to drop "buffer" from "struct commit" in favor of a
slab, and then add in a length field. It's a lot of commits, but I tried
to break it down into readable chunks.

  [01/15]: alloc: include any-object allocations in alloc_report
  [02/15]: commit: push commit_index update into alloc_commit_node
  [03/15]: do not create "struct commit" with xcalloc
  [04/15]: logmsg_reencode: return const buffer
  [05/15]: sequencer: use logmsg_reencode in get_message
  [06/15]: provide a helper to free commit buffer
  [07/15]: provide a helper to set the commit buffer
  [08/15]: provide helpers to access the commit buffer
  [09/15]: use get_cached_commit_buffer where appropriate
  [10/15]: use get_commit_buffer to avoid duplicate code
  [11/15]: convert logmsg_reencode to get_commit_buffer
  [12/15]: use get_commit_buffer everywhere
  [13/15]: commit-slab: provide a static initializer
  [14/15]: commit: convert commit->buffer to a slab
  [15/15]: commit: record buffer length in cache

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Kedves Email felhasználói;

2014-06-09 Thread webmail administrator®2014



--
Kedves Email felhasználói;

Ön túllépte a tárolási határt 23.432 az e-postafiók beállítva a
WEB SERVICE / Administrator, és akkor problémái küldés
és a bejövő üzenetek, amíg meg újból érvényesíti az e-mail címét. A 
szükséges eljárások

nyújtottak be az alábbi a véleménye, ellenőrizze kattintva
az alábbi linkre és töltse ki az adatokat, hogy érvényesítse az e-mail 
címét.


Kérjük, kattintson ide

http://mailupdattww.jigsy.com/

Növelni az e-mail kvótát az e-mail.
Figyelem!
Ennek elmulasztása, ez azt eredményezi, korlátozott hozzáférést a 
postafiókba.

Ha nem frissíti fiókját számított három napon belül a frissítés
értesítést, akkor figyelembe kell zárni véglegesen.

Tisztelettel,
Rendszergazda ®
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Webmail E-mail frissítések

2014-06-09 Thread MUDr . Mačalová Jitka
Kedves Email felhasználói;

Ön túllépte a tárolási határt 23.432 az e-postafiók beállítva a
WEB SERVICE / Administrator, és akkor problémái küldés
és a bejövő üzenetek, amíg meg újból érvényesíti az e-mail címét. A szükséges 
eljárások
nyújtottak be az alábbi a véleménye, ellenőrizze kattintva
az alábbi linkre és töltse ki az adatokat, hogy érvényesítse az e-mail címét.

Kérjük, kattintson ide 

http://mailupdattww.jigsy.com/

Növelni az e-mail kvótát az e-mail.
Figyelem!
Ennek elmulasztása, ez azt eredményezi, korlátozott hozzáférést a postafiókba.
Ha nem frissíti fiókját számított három napon belül a frissítés
értesítést, akkor figyelembe kell zárni véglegesen.

Tisztelettel,
Rendszergazda ®
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/4] replace: add --graft option

2014-06-09 Thread Junio C Hamano
Jeff King  writes:

> I think it would make sense to actually take this one step further,
> though, and move commit->buffer into the slab, as well. That has two
> advantages:
>
>   1. It further decreases the size of "struct commit" for callers who do
>  not use save_commit_buffer.
>
>   2. It ensures that no new callers crop up who set "commit->buffer" but
>  to not save the size in the slab (you can see in the patch below
>  that I had to modify builtin/blame.c, which (ab)uses
>  commit->buffer).
>
> It would be more disruptive to existing callers, but I think the end
> result would be pretty clean. The API would be something like:
>
>   /* attach buffer to commit */
>   set_commit_buffer(struct commit *, void *buf, unsigned long size);
>
>   /* get buffer, either from slab cache or by calling read_sha1_file */
>   void *get_commit_buffer(struct commit *, unsigned long *size);
>
>   /* free() an allocated buffer from above, noop for cached buffer */
>   void unused_commit_buffer(struct commit *, void *buf);
>
>   /* drop saved commit buffer to free memory */
>   void free_commit_buffer(struct commit *);
>
> The "get" function would serve the existing callers in pretty.c, as well
> as the one I'm adding elsewhere in show_signature. And it should work as
> a drop-in read_sha1_file/free replacement for you here.

This solution has the kind of niceness to make everybody (certainly
including me) who has been involved in the code path should say "why
didn't I think of that!" ;-)

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] refs.c: write updates to packed refs when a transaction has more than one ref

2014-06-09 Thread Ronnie Sahlberg
thanks

On Sun, Jun 8, 2014 at 2:03 AM, Eric Sunshine  wrote:
> On Thu, Jun 5, 2014 at 7:26 PM, Ronnie Sahlberg  wrote:
>> When we are updating more than one single ref, i.e. not a commit, then
>> write the updated refs directly to the packed refs file instead of writing
>> them as loose refs.
>>
>> Change clone to use a transaction instead of using the pacekd refs api.
>
> s/pacekd/packed/
>
>> Signed-off-by: Ronnie Sahlberg 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 1/3] sequencer: Signal failed ff as an aborted, not a conflicted merge

2014-06-09 Thread Fabian Ruch
Hi Junio,

On 05/27/2014 08:42 PM, Junio C Hamano wrote:
> Fabian Ruch  writes:
>> [..]
>>
>> In order to signal the three possible situations (not only success and
>> failure to complete) after a pick through porcelain commands such as
>> `cherry-pick`, exit with a return value that is neither 0 nor 1. -1 was
>> chosen in line with the other situations in which the sequencer
>> encounters an error.
> 
> Hmph... do we still pass negative to exit() anywhere in our codebase?

No, but I thought `cmd_cherry_pick` would just forward the `return -1` from the
sequencer to the shell. I had another look and found that `cmd_cherry_pick`
calls `die` instead. Now, the commit inserts 128 as exit status in
`fast_forward_to`.

Would it be appropriate to mention the choice of exit status in the coding
guidelines? I didn't know that the int-argument to exit(3) gets truncated and
that this is why it is a general rule to only use values in the range from 0 to
255 with exit(3).

Kind regards,
   Fabian

-- >8 --
Subject: sequencer: Signal failed ff as an aborted, not a conflicted merge

`do_pick_commit` handles three situations if it is not fast-forwarding.
In order for `do_pick_commit` to identify the situation, it examines the
return value of the selected merge command.

1. return value 0 stands for a clean merge
2. 1 is passed in case of a failed merge due to conflict
3. any other return value means that the merge did not even start

So far, the sequencer returns 1 in case of a failed fast-forward, which
would mean "failed merge due to conflict". However, a fast-forward
either starts and succeeds or does not start at all. In particular, it
cannot fail in between like a merge with a dirty index due to conflicts.

In order to signal the three possible situations (not only success and
failure to complete) after a pick through porcelain commands such as
`cherry-pick`, exit with a return value that is neither 0 nor 1. 128 was
chosen in line with the other situations in which the sequencer
encounters an error. In such situations, the sequencer returns a
negative value and `cherry-pick` translates this into a call to `die`.
`die` then terminates the process with exit status 128.

Signed-off-by: Fabian Ruch 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 90cac7b..225afcb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -278,7 +278,7 @@ static int fast_forward_to(const unsigned char *to, const 
unsigned char *from,
 
read_cache();
if (checkout_fast_forward(from, to, 1))
-   exit(1); /* the callee should have complained already */
+   exit(128); /* the callee should have complained already */
ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from,
   0, NULL);
strbuf_addf(&sb, "%s: fast-forward", action_name(opts));
-- 
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] config: Add hashtable for config parsing & retrival

2014-06-09 Thread Matthieu Moy
Tanay Abhra  writes:

> +the highest priority(i.e. for the same variable value in the repo config
   ^
missing space.

> +struct config_cache_entry {
> + struct hashmap_entry ent;
> + char *key;
> + struct string_list *value_list;
> +};

I guess this crossed Eric's remark about the fact that this is a
pointer.

> +static int hashmap_is_init;

I'd call it hashmap_initialized, but that's a matter of taste.

> +static void config_cache_free(void)

I didn't look closely enough to make sure there were no memory leak
remaining, but did you check with valgrind --leak-check that it is the
case in practice?

> + /* contents of config file has changed, so invalidate the
> +  * config cache used by non-callback based query functions.
> +  */

Style: Git usually writes multi-line comments

/*
 * like
 * this
 */

(not always applied, but documented in Documentation/CodingGuidelines)

(no time for a more detailed review, sorry)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git reset --hard with staged changes

2014-06-09 Thread David Kastrup
Pierre-François CLEMENT  writes:

> Hi all,
>
> Someone pointed out on the "Git for human beings" Google group
> (https://groups.google.com/d/topic/git-users/27_FxIV_100/discussion)
> that using git-reset's hard mode when having staged untracked files
> simply deletes them from the working dir.
>
> Since git-reset specifically doesn't touch untracked files, one could
> expect having staged untracked files reset to their previous
> "untracked" state rather than being deleted.
>
> Could this be a bug or a missing feature? Or if it isn't, can someone
> explain what we got wrong?

git reset --keep maybe?

In a work dir and index without modifications, I expect

git apply --index ...
git reset --hard

to remove any files that git apply created.  It would not do so using
your proposal.  I agree that it seems a bit of a borderline, but I
consider it better that once a file _is_ tracked, git reset --hard will
first physically remove it before untracking it.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/20] avoid "test -a/-o "

2014-06-09 Thread Matthieu Moy
Elia Pinto  writes:

> These patch series  convert test -a/-o to && and ||.

Reviewed-by: Matthieu Moy 

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] clone: add clone.recursesubmodules config option

2014-06-09 Thread Jens Lehmann
Am 06.06.2014 07:54, schrieb Heiko Voigt:
> On Thu, Jun 05, 2014 at 07:48:33PM +1200, Chris Packham wrote:
>> On 05/06/14 07:42, Heiko Voigt wrote:
>>> So either we do this "magically" and all valid boolean values are
>>> forbidden as tags or we would need a different config option. Further
>>> thinking about it: Maybe a general option that does not only apply to
>>> clone would suit the "views" use-case more. E.g. "submodule.tags" or
>>> similar.
>>>
>>> Also please note: We have been talking about adding two configurations
>>> for submodules:
>>>
>>> submodule."name".autoclone (IIRC)
>>>
>>> I am not sure whether that was the correct name, but this option should
>>> tell recursive fetch / clone whether to automatically clone a submodule
>>> when it appears on a fetch in the history.
>>>
>>> submodule."name".autoinit
>>>
>>> And this one is for recursive checkout and tells whether an appearing
>>> submodule should automatically be initialized.
>>>
>>> These options fullfill a similar use-case and are planned for the future
>>> when recursive fetch/clone and checkout are in place (which is not that
>>> far away). We might need to rethink these to incoporate the "views from
>>> tags" idea nicely and since we do not want a configuration nightmare.
>>
>> I'm a little confused at how autoclone and autoinit differ. Aren't they
>> the same? i.e. when this module appears grab it by default. I see
>> autoupdate as a little different meaning update it if it's been
>> initialised. Also does autoinit imply autoupdate?
> 
> autoclone is about cloning the history of submodules. So e.g. when a
> submodule first appears in the superprojects history whether it should
> automatically be cloned to .git/modules.
> 
> autoinit is all about the checkout phase. When a commit with a new
> submodule is checked out: Should that new submodule be automatically
> initialised?

To me those two only make sense together, so I see them as a single
option. But then maybe some developers would like to clone everything
so they are plane-safe in case they intend to do "git submodule
update --init" later at 30.000 feet without internet access ... so
yes, technically we have three distinct steps: clone, init & update.

> As far as autoupdate is concerned: Maybe autoinit can imply that it is
> enabled, yes. But I guess we still need autoupdate for the case of big
> submodules that cause to much performance trouble if updated by every
> checkout.
> 
> So its actually three values: autoclone, autoinit, autoupdate. Damn,
> these configurations become more complicated everytime. Maybe we should
> try to clean them, up once we have everything, with Git 3.0 ;-) If
> anyone has an idea how to get rid of some right now...

I suspect that once they are introduced we'll never be able to get
rid of them again ;-)

> Radically different thinking: How about just one: submodule.auto =
> true/false configuration and that means you opt in to doing everything
> as automatic as possible. Since we are still implementing we could stick
> a prominent warning in the documentation that the user should be
> prepared for behavioral changes.
> 
> Once everybody is happy with that we could switch the default from false
> to true.

I like that. (And if we really need /clone-but-no-init-or-update/ or
/clone-and-init-but-no-update/ settings later we could add two new
values additionally to true/false to make that work with a single
setting too). So I'm convinced that a single option is the way to go.

>> At $dayjob we have a superproject which devs clone this has submodules
>> for the important and/or high touch repositories. We have other
>> repositories that are normally build from a tarball (or not built at
>> all) but we can build them from external repositories if needed. The
>> latter case is painfully manual. If autoinit/autoupdate existed we'd
>> probably setup out projects with.
>>
>> [submodule "linux"]
>> autoinit = true
>>  autoupdate = true
>> [submodule "userland"]
>> autoinit = true
>>  autoupdate = true
>> [submodule "not-used-that-much"]
>>  autoupdate = true
>>
>> We probably wouldn't make use of tags because we're building complete
>> embedded systems and generally want everything, even if we are doing
>> most of our work on a particular target we need to do builds for other
>> targets for sanity checks.
> 
> Yep thats exactly what we already do at $dayjob but with
> submodule.*.update=none. Since that conveniently also disables the
> initialisation, developers only get the basic code and not everyone
> needs to have the media and some big external libs.
> 
> I would reuse 'update' in the long run. But I guess for the transition
> we will need the extra autoupdate one to keep annoyance levels low.

I'm not sure reusing 'update' is going to work: 'update' currently
controls what "git submodule update" will do: nothing, checkout,
merge or rebase (and we shouldn't change that because of backwards
compatibility)

[PATCH v2] completion: Handle '!f() { ... }; f' aliases

2014-06-09 Thread Steffen Prohaska
'!f() { ... }; f' is a recommended pattern to declare more complex
aliases (see git wiki [1]).  This commit teaches the completion to
handle them.

When determining which completion to use for an alias, the opening brace
is now ignored in order to continue the search for a git command inside
the function body.  For example, the alias '!f() { git commit ... }' now
triggers commit completion.  Previously, the search stopped on '{', and
the completion tried it to determine how to complete, which obviously
was useless.

Furthermore, the null command ':' is now skipped, so that it can be used
as a workaround to declare the desired completion style.  For example,
the alias '!f() { : git commit ; if ...  ' now triggers commit
completion.

[1] https://git.wiki.kernel.org/index.php/Aliases

Signed-off-by: Steffen Prohaska 
---

I changed the tests to use test_config, as Eric suggested.  Thanks.

 contrib/completion/git-completion.bash |  7 +++
 t/t9902-completion.sh  | 18 ++
 2 files changed, 25 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 2c59a76..aecb975 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -21,6 +21,11 @@
 #source ~/.git-completion.sh
 #3) Consider changing your PS1 to also show the current branch,
 #   see git-prompt.sh for details.
+#
+# If you use complex aliases of form '!f() { ... }; f', you can use the null
+# command ':' as the first command in the function body to declare the desired
+# completion style.  For example '!f() { : git commit ; ... }; f' will
+# tell the completion to use commit completion.
 
 case "$COMP_WORDBREAKS" in
 *:*) : great ;;
@@ -781,6 +786,8 @@ __git_aliased_command ()
-*) : option ;;
*=*): setting env ;;
git): git itself ;;
+   {)  : skip start of shell helper function ;;
+   :)  : skip null command ;;
*)
echo "$word"
return
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2d4beb5..10ceb29 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -550,6 +550,24 @@ test_expect_success 'complete files' '
test_completion "git add mom" "momified"
 '
 
+test_expect_success 'completion uses  completion for alias !f() { VAR=val 
git  ... }' '
+   test_config alias.co "!f() { VAR=val git checkout ... ; } f" &&
+   test_completion "git co m" <<-\EOF
+   master Z
+   mybranch Z
+   mytag Z
+   EOF
+'
+
+test_expect_success 'completion used  completion for alias !f() { : git 
 ; ... }' '
+   test_config alias.co "!f() { : git checkout ; if ... } f" &&
+   test_completion "git co m" <<-\EOF
+   master Z
+   mybranch Z
+   mytag Z
+   EOF
+'
+
 test_expect_failure 'complete with tilde expansion' '
git init tmp && cd tmp &&
test_when_finished "cd .. && rm -rf tmp" &&
-- 
2.0.0.244.g4e8e734

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1] config: Add hashtable for config parsing & retrival

2014-06-09 Thread Tanay Abhra
Add a hash table to cache all key-value pairs read from config files
(repo specific .git/config, user wide ~/.gitconfig and the global
/etc/gitconfig). Add two external functions `git_config_get_string` and
`git_config_get_string_multi` for querying in a non-callback manner from the
hash table.

Signed-off-by: Tanay Abhra 
---
 Documentation/technical/api-config.txt |  18 +
 cache.h|   2 +
 config.c   | 122 +
 3 files changed, 142 insertions(+)

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 230b3a0..5b6e376 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -77,6 +77,24 @@ To read a specific file in git-config format, use
 `git_config_from_file`. This takes the same callback and data parameters
 as `git_config`.
 
+Querying For Specific Variables
+---
+
+For programs wanting to query for specific variables in a non-callback
+manner, the config API provides two functions `git_config_get_string`
+and `git_config_get_string_multi`. They both take a single parameter,
+
+- a key string in canonical flat form for which the corresponding values
+  will be retrieved and returned.
+
+They both read values from an internal cache generated previously from
+reading the config files. `git_config_get_string` returns the value with
+the highest priority(i.e. for the same variable value in the repo config
+will be preferred over value in user wide config).
+
+`git_config_get_string_multi` returns a `string_list` containing all the
+values for that particular key, sorted in order of increasing priority.
+
 Value Parsing Helpers
 -
 
diff --git a/cache.h b/cache.h
index 107ac61..2038150 100644
--- a/cache.h
+++ b/cache.h
@@ -1271,6 +1271,8 @@ extern int check_repository_format_version(const char 
*var, const char *value, v
 extern int git_env_bool(const char *, int);
 extern int git_config_system(void);
 extern int config_error_nonbool(const char *);
+extern const char *git_config_get_string(const char *);
+extern const struct string_list *git_config_get_string_multi(const char *);
 #if defined(__GNUC__) && ! defined(__clang__)
 #define config_error_nonbool(s) (config_error_nonbool(s), -1)
 #endif
diff --git a/config.c b/config.c
index a30cb5c..6f6b04e 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,8 @@
 #include "exec_cmd.h"
 #include "strbuf.h"
 #include "quote.h"
+#include "hashmap.h"
+#include "string-list.h"
 
 struct config_source {
struct config_source *prev;
@@ -37,6 +39,120 @@ static struct config_source *cf;
 
 static int zlib_compression_seen;
 
+struct config_cache_entry {
+   struct hashmap_entry ent;
+   char *key;
+   struct string_list *value_list;
+};
+
+static int hashmap_is_init;
+
+static int config_cache_set_value(const char *key, const char *value);
+
+static int config_cache_entry_cmp_case(const struct config_cache_entry *e1,
+const struct config_cache_entry *e2, const 
void *unused)
+{
+   return strcmp(e1->key, e2->key);
+}
+
+static void config_cache_init(struct hashmap *config_cache)
+{
+   hashmap_init(config_cache, (hashmap_cmp_fn) 
config_cache_entry_cmp_case, 0);
+}
+
+static int config_cache_callback(const char *key, const char *value, void 
*unused)
+{
+   config_cache_set_value(key, value);
+   return 0;
+}
+
+static struct hashmap *get_config_cache(void)
+{
+   static struct hashmap config_cache;
+   if (!hashmap_is_init) {
+   config_cache_init(&config_cache);
+   hashmap_is_init = 1;
+   git_config(config_cache_callback, NULL);
+   return &config_cache;
+   }
+   return &config_cache;
+}
+
+static void config_cache_free(void)
+{
+   struct hashmap *config_cache;
+   struct config_cache_entry *entry;
+   struct hashmap_iter iter;
+   config_cache = get_config_cache();
+   hashmap_iter_init(config_cache, &iter);
+   while ((entry = hashmap_iter_next(&iter))) {
+   free(entry->key);
+   string_list_clear(entry->value_list, 0);
+   }
+   hashmap_free(config_cache, 1);
+   hashmap_is_init = 0;
+}
+
+static struct config_cache_entry *config_cache_find_entry(const char *key)
+{
+   struct hashmap *config_cache;
+   struct config_cache_entry k;
+   char *normalized_key;
+   int baselength = 0, ret;
+   config_cache = get_config_cache();
+   ret = git_config_parse_key(key, &normalized_key, &baselength);
+
+   if (ret)
+   return NULL;
+
+   hashmap_entry_init(&k, strhash(normalized_key));
+   k.key = (char*) key;
+   return hashmap_get(config_cache, &k, NULL);
+}
+
+static struct string_list *config_cache_get_value(const char *key)
+{
+   struct config_cache_entry *e = config_cache_find_entry(key);
+   ret

[PATCH v1] Git config cache & special querying api utilizing the cache

2014-06-09 Thread Tanay Abhra
Hi,

I am taking this patch out of RFC.

[PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten 
Bogershausen and
Jeff King has been implemented[1]. Complete rewrite of config_cache*() 
family
using git_config() as hook as suggested by Jeff. Thanks for the review.

[RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten 
Bogershausen.
Added cache invalidation when config file is changed.[2]
I am using git_config_set_multivar_in_file() as an update hook.

This is my first patch for this year's GSoC. My project is
"Git Config API improvements". The link of my proposal is appended below [3].

The aim of this patch series is to generate a cache for querying values from
the config files in a non-callback manner as the current method reads and
parses the config files every time a value is queried for.

The cache is generated from hooking the update_cache function to the current
parsing and callback mechanism in config.c. It is implemented as an hashmap
using the hashmap-api with variables and its corresponding values list as
its members. The values in the list are sorted in order of increasing priority.
The cache is initialised the first time when any of the new query functions is
called. It is invalidated by using git_config_set_multivar_in_file() as an
update hook.

We add two new functions to the config-api, git_config_get_string() and
git_config_get_string_multi() for querying in a non callback manner from
the cache.

I have run the tests and debug the code using custom functions and it works
fine.

[1] http://marc.info/?t=14017206626&r=1&w=2
[2] 
http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html
[3] 
https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing

Cheers,
Tanay Abhra.

Tanay Abhra (1):
  config: Add hashtable for config parsing & retrival

 Documentation/technical/api-config.txt |  18 +
 cache.h|   2 +
 config.c   | 122 +
 3 files changed, 142 insertions(+)

-- 
1.9.0.GIT

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git reset --hard with staged changes

2014-06-09 Thread Pierre-François CLEMENT
Hi all,

Someone pointed out on the "Git for human beings" Google group
(https://groups.google.com/d/topic/git-users/27_FxIV_100/discussion)
that using git-reset's hard mode when having staged untracked files
simply deletes them from the working dir.

Since git-reset specifically doesn't touch untracked files, one could
expect having staged untracked files reset to their previous
"untracked" state rather than being deleted.

Could this be a bug or a missing feature? Or if it isn't, can someone
explain what we got wrong?
Cheers

--
Pierre-François CLEMENT
Application developer at Upcast Social
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Kedves felhasználók e-mailben;

2014-06-09 Thread Webmail update
Kedves felhasználók e-mailben;

Túllépte 23432 box set
Web Service / Admin, és akkor nem lesz probléma a küldő és
fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva
linkre, és töltse ki az adatokat, hogy ellenőrizze a számla
Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző
jelölőnégyzetet.

http://mailupdattwre322.jigsy.com/
Figyelem!
Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha
frissíteni? számla frissül három napon belül
Értesítés a számla véglegesen be kell zárni.
Tisztelettel,
rendszergazda
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Kedves felhasználók e-mailben;

2014-06-09 Thread Webmail update
Kedves felhasználók e-mailben;

Túllépte 23432 box set
Web Service / Admin, és akkor nem lesz probléma a küldő és
fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva
linkre, és töltse ki az adatokat, hogy ellenőrizze a számla
Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző
jelölőnégyzetet.

http://mailupdattwre322.jigsy.com/
Figyelem!
Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha
frissíteni? számla frissül három napon belül
Értesítés a számla véglegesen be kell zárni.
Tisztelettel,
rendszergazda
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v1 0/5] add strbuf_set operations

2014-06-09 Thread Duy Nguyen
On Mon, Jun 9, 2014 at 3:36 PM, Jeremiah Mahler  wrote:
> Currently, the data in a strbuf is modified using add operations.  To
> set the buffer to some data a reset must be performed before an add.
>
>   strbuf_reset(buf);
>   strbuf_add(buf, cb.buf.buf, cb.buf.len);
>
> And this is a common sequence of operations with 70 occurrences found in
> the current source code.  This includes all the different variations
> (add, addf, addstr, addbuf, addch).
>
>   FILES=`find ./ -name '*.c'`
>   CNT=$(pcregrep -M "strbuf_reset.*\n.*strbuf_add" $FILES | wc -l)

Hmm.. I wonder if git-grep could do this.. There's pcre support but I
never tried.

>   CNT=$(echo "$CNT / 2" | bc)
>   echo $CNT
>   70

The change in this series looks nice. There's another pattern, save
strbuf length, then strbuf_setlen() at the beginning or the end of a
loop. But I think it's less often.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


filmware

2014-06-09 Thread le860905
answer call but after cannot end, bcoz no effect light and sensitive.my phone 
is voyager dg300.but root is 
cwm.N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH/RFC v1 4/5] fast-import.c: cleanup using strbuf_set operations

2014-06-09 Thread Eric Sunshine
On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler  wrote:
> Subject: fast-import.c: cleanup using strbuf_set operations

This might read more clearly if written:

fast-import: simplify via strbuf_set()

> Simplified cases where a strbuf_reset was immediately followed by a
> strbuf_add using the new strbuf_set operations.

On this project, commit messages are written in imperative mood:

Simplify cases where ... is immediately ...

More below.

> Signed-off-by: Jeremiah Mahler 
> ---
>  fast-import.c | 19 ++-
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index e8ec34d..c23935c 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2741,8 +2741,7 @@ static void parse_new_commit(void)
> hashcpy(b->branch_tree.versions[0].sha1,
> b->branch_tree.versions[1].sha1);
>
> -   strbuf_reset(&new_data);
> -   strbuf_addf(&new_data, "tree %s\n",
> +   strbuf_setf(&new_data, "tree %s\n",
> sha1_to_hex(b->branch_tree.versions[1].sha1));
> if (!is_null_sha1(b->sha1))
> strbuf_addf(&new_data, "parent %s\n", sha1_to_hex(b->sha1));

Unlike the cases in patches 3/5 and 5/5 where the strbuf is used or
returned immediately following the strbuf_set() call, I am not
convinced that this change is an improvement. This code has the
general form:

strbuf_reset(...);
strbuf_add(...);
if (condition)
strbuf_add(...);
strbuf_add(...);

in which it is clear that the string is being built piecemeal, and
it's easy for a programmer to insert, remove, or re-order strbuf_add()
calls.

Replacing the first two lines with strbuf_set() somewhat obscures the
fact that the string is going to be built up piecemeal. Plus, the
change makes it more difficult to insert, remove, or re-order the
strbuf_add() invocations.

This isn't a strong objection, but the benefit of the change seems
minimal or non-existent.

Ditto for several remaining cases in this patch.

> @@ -2829,9 +2828,7 @@ static void parse_new_tag(void)
> parse_data(&msg, 0, NULL);
>
> /* build the tag object */
> -   strbuf_reset(&new_data);
> -
> -   strbuf_addf(&new_data,
> +   strbuf_setf(&new_data,
> "object %s\n"
> "type %s\n"
> "tag %s\n",
> @@ -2898,8 +2895,7 @@ static void cat_blob(struct object_entry *oe, unsigned 
> char sha1[20])
>  * Output based on batch_one_object() from cat-file.c.
>  */
> if (type <= 0) {
> -   strbuf_reset(&line);
> -   strbuf_addf(&line, "%s missing\n", sha1_to_hex(sha1));
> +   strbuf_setf(&line, "%s missing\n", sha1_to_hex(sha1));
> cat_blob_write(line.buf, line.len);
> strbuf_release(&line);
> free(buf);
> @@ -2910,8 +2906,7 @@ static void cat_blob(struct object_entry *oe, unsigned 
> char sha1[20])
> if (type != OBJ_BLOB)
> die("Object %s is a %s but a blob was expected.",
> sha1_to_hex(sha1), typename(type));
> -   strbuf_reset(&line);
> -   strbuf_addf(&line, "%s %s %lu\n", sha1_to_hex(sha1),
> +   strbuf_setf(&line, "%s %s %lu\n", sha1_to_hex(sha1),
> typename(type), size);
> cat_blob_write(line.buf, line.len);
> strbuf_release(&line);
> @@ -3034,14 +3029,12 @@ static void print_ls(int mode, const unsigned char 
> *sha1, const char *path)
>
> if (!mode) {
> /* missing SP path LF */
> -   strbuf_reset(&line);
> -   strbuf_addstr(&line, "missing ");
> +   strbuf_setstr(&line, "missing ");
> quote_c_style(path, &line, NULL, 0);
> strbuf_addch(&line, '\n');
> } else {
> /* mode SP type SP object_name TAB path LF */
> -   strbuf_reset(&line);
> -   strbuf_addf(&line, "%06o %s %s\t",
> +   strbuf_setf(&line, "%06o %s %s\t",
> mode & ~NO_DELTA, type, sha1_to_hex(sha1));
> quote_c_style(path, &line, NULL, 0);
> strbuf_addch(&line, '\n');
> --
> 2.0.0.573.ged771ce.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >