{bug} warning: unable to access 'RelNotes/.gitattributes'

2012-09-12 Thread Junio C Hamano
"git repack" started giving the above warning, and I am guessing
that the recent 11e50b2 (attr: warn on inaccessible attribute files,
2012-08-21) exposed a bug where we ask stat(2) not lstat(2) by
mistake before deciding to append .gitattributes to see if that
directory has a per-directory attributes file.  We simply used to
notice and ignore any failure from open() and moved on, but we
started distinguishing between ENOENT and others (in this case, we
get ENOTDIR), and added a warning for non-ENOENT cases and I think
that is what I am seeing.

It is getting late so I won't dig it further for now, but just a
heads up.
--
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 3/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Yann Droneaud
Le mercredi 12 septembre 2012 à 17:04 -0400, Jeff King a écrit :

> > > Wouldn't this break all of the code that is planning to index "W" by
> > > 32-bit words (see the definitions of setW in block-sha1/sha1.c)?
> > > 
> > That's not the same "W" ... This part of the code is indeed unclear.
> 
> Sorry, you're right, that's a different work array (though it has the
> identical issue, no?).

No, this one is really accessed as int. But would probably benefit being
declared as uint32_t.

> But the point still stands.  Did you audit the
> block-sha1 code to make sure nobody is ever indexing the W array? 

Yes. It was the first thing to do before changing its definition
(for alignment purpose especially).

> If you didn't, then your change is not safe. If you did, then you should 
> really
> mention that in the commit message.
> 

Sorry about this.
I thought having the test suite OK was enough to prove this.

> > > If that is indeed the problem, wouldn't the simplest fix be using
> > > uint32_t instead of "unsigned int"?
> > 
> > It's another way to fix this oddity, but not simpler.
> 
> It is simpler in the sense that it does not have any side effects (like
> changing how every user of the data structure needs to index it).
> 

There's no other user than blk_SHA1_Update()

> > > Moreover, would that be sufficient to run on such a platform? At the
> > > very least, "H" above would want the same treatment. And I would not be
> > > surprised if some of the actual code in block-sha1/sha1.c needed
> > > updating, as well.
> > 
> > ctx->H is actually used as an array of integer, so it would benefits of
> > being declared uint32_t for an ILP64 system. This fix would also be
> > required for blk_SHA1_Block() function.
> 
> So...if we are not ready to run on an ILP system after this change, then
> what is the purpose?
> 

Readility: in blk_SHA1_Block(), the ctx->W array is used a 64 bytes len
array, so, AFAIK, there's no point of having it defined as a 16 int len.
It's disturbing while reading the code.

This could allows us to change the memcpy() call further:

@@ -246,7 +246,7 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void
*data, unsigned long len)
unsigned int left = 64 - lenW;
if (len < left)
left = len;
-   memcpy((char *)ctx->W + lenW, data, left);
+   memcpy(ctx->W + lenW, data, left);
lenW = (lenW + left) & 63;
if (lenW)

Regards.

-- 
Yann Droneaud
OPTEYA


--
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: Suggestions for "What's cooking"

2012-09-12 Thread Andrew Ardill
(sorry about double replying - html sub-part creeped in!)
On 13 September 2012 08:49, Philip Oakley  wrote:
>
> From: "Jens Lehmann" 
> Sent: Wednesday, September 12, 2012 8:21 PM
>
>> Am 11.09.2012 21:41, schrieb Junio C Hamano:
>>>
>>> Thanks.  I wish all others paid attention to "What's cooking" like
>>> you did here.
>>>
>>> And if it is hard to do so for whatever reason, suggest a better way
>>> for me to publish "What's cooking" or an equivalent (I am interested
>>> in finding the least bureaucratic way to help people and keep the
>>> balls rolling).
>>
>>
>> I think "What's cooking" makes lots of sense in its current form
>> as one gets a very good overview over current development tracks.
>>
>> Maybe in addition it would be nice to email the author(s) of a
>> series when the state changes or new comments are added (and to
>> only include the relevant part from "What's cooking" there). For
>> me it's not a big problem as I just have to grep for "submodule"
>> to get the bits I care about, but I suspect others might have to
>> invest much more time to check the current state of their series
>> and may appreciate being mailed directly when something happens.
>> Opinions?
>
>
> My comment, as a simple reader, is that I misread the order of the items, in 
> that I miss-associate the description paragraph with the * title _below_. 
> That is, I see the description first and then read on...
>
> Thinking about it, if the description paragraph was indented by one space 
> then the * title  would create that obvious content indent that (I am) would 
> be expected.
>
> Obviously only a useful suggestion if it's easy to implement...


I can attest to the fact that the format can be at times difficult to
parse, and I often find myself rereading sections to make sure I
understood what each was referring to.

As a casual reader, interested in the development that is going on,
the things I am interested in for each branch/topic are like:
 - Branch/Topic description
 - Current integration status
 - Next steps required
 - Notes and memoranda

I understand that references to where the branch is found (it's name)
and what it includes (commit list) are important too, but these are
less important for me.

Currently, the output for each branch looks something like:
*  () 
  ()
 [list-of-commits]
  ()




and these are grouped by current integration status (new, graduated,
stalled etc)

A format that would make this information easier for me to parse would
be something like:


  
  
  
  *  () 
()
   [list-of-commits]
()

Essentially, shifting the details of the branch to the bottom, and
adding a short description for the entire branch. Indent everything
after the short description to make it clear that they belong
together.

The only real 'new' information required is the short description, but
that could be replaced with the topic name if short description is not
available (or the topic name is self explanatory).

Most of the parsing benefit would come from the indentation, but
having the 'summary' information near the top would let me skip things
I am not interested in without having to scan the list of commits and
other details.

Regards,

Andrew Ardill
--
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: Please pull git-l10n updates for git v1.7.12-146-g16d26

2012-09-12 Thread Junio C Hamano
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: [PATCH] git-am: Handle "git show" output correctly

2012-09-12 Thread Junio C Hamano
Dan Johnson  writes:

> I was assuming Peter would accept the patch, and reply with a "in the
> future, please submit the output of format-patch", thus correcting the
> submitter's behavior. This warning would serve someone who did not
> know that they wanted the output of format-patch, and hopefully teach
> them to send such a reply message.

"Next time, please do this" rarely has worked in practice.  This is
because the moment you accepted the current patch, you have already
lost the "carrot" ;-)

--
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: Suggestions for "What's cooking"

2012-09-12 Thread Philip Oakley

From: "Jens Lehmann" 
Sent: Wednesday, September 12, 2012 8:21 PM

Am 11.09.2012 21:41, schrieb Junio C Hamano:

Thanks.  I wish all others paid attention to "What's cooking" like
you did here.

And if it is hard to do so for whatever reason, suggest a better way
for me to publish "What's cooking" or an equivalent (I am interested
in finding the least bureaucratic way to help people and keep the
balls rolling).


I think "What's cooking" makes lots of sense in its current form
as one gets a very good overview over current development tracks.

Maybe in addition it would be nice to email the author(s) of a
series when the state changes or new comments are added (and to
only include the relevant part from "What's cooking" there). For
me it's not a big problem as I just have to grep for "submodule"
to get the bits I care about, but I suspect others might have to
invest much more time to check the current state of their series
and may appreciate being mailed directly when something happens.
Opinions?


My comment, as a simple reader, is that I misread the order of the 
items, in that I miss-associate the description paragraph with the * 
title _below_. That is, I see the description first and then read on...


Thinking about it, if the description paragraph was indented by one 
space then the * title  would create that obvious content indent that (I 
am) would be expected.


Obviously only a useful suggestion if it's easy to implement...

Philip 


--
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] cherry-pick: don't forget -s on failure

2012-09-12 Thread Junio C Hamano
Junio C Hamano  writes:

> I think we had a separate topic around cherry-pick that needs the
> footer thing accessible from cherry-pick recently ($gmane/204755).
>
> I think the code movement in this patch is a good one.
>
> Thanks.

Having said that, the behaviour after this patch is applied is not
quite right.

A typical .git/MERGE_MSG that is left after "cherry-pick" gives the
control back to you asking for help, with your patch that adds the
sign-off at the end, would look like this:


cherry-pick: don't forget -s on failure

In case 'git cherry-pick -s ' failed, the user had to use 'git
commit -s' (i.e. state the -s option again), which is easy to forget
about.  Instead, write the signed-off-by line early, so plain 'git
commit' will have the same result.

Signed-off-by: Miklos Vajna 
Signed-off-by: Junio C Hamano 

Conflicts:
builtin/commit.c

Signed-off-by: Junio C Hamano 


Notice two issues?

 - The additional S-o-b should come immediately after the existing
   block of footers.

 - And the last entry in the existing footer block is already mine,
   so there shouldn't have been a new and duplicated one added.


I am not sure how reusable the moved function is without
enhancements for your purpose.  The logic to identify the footer
needs to be enhanced so that an "end" pointer to point at the byte
before the caller added "Conflicts: " can be given, and pretend as
if it is the end of the buffer, unlike in the fresh commit case
where it can consider the real end of the buffer as such.

Or something like 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] git-am: Handle "git show" output correctly

2012-09-12 Thread Andreas Ericsson
On 09/13/2012 12:19 AM, Junio C Hamano wrote:
> Dan Johnson  writes:
> 
>>> Not really.  If we start encouraging people to use "git show" output
>>> as a kosher input to "am", we would have to support such use
>>> forever, and we end up painting ourselves in a corner we cannot get
>>> out of easily.
>>
>> If git am emitted a warning when accepting "git show" output, it seems
>> like it would support Peter's use-case without encouraging bad
>> behavior?
> 
> Are you seriously suggesting me to sell to our users a new feature
> saying "this does not work reliably, we would not recommend using
> it, no, really, don't trust it." from the day the feature is
> introduced, especially when we know it will not be "the feature does
> not work well yet, but it will, we promise" but is "and it may become
> worse in the future"?
> 
> I do not see much point in doing that.
> 
> Besides, what bad behaviour do we avoid from encouraging with such
> an approach?  As Peter said, the problem is not on the part of the
> user who ended up with an output from "git show", when he really
> wants output from "git format-patch".  Giving the warning to the
> user of "git am" is too late.
> 

It might be enough to either enable format-patch output or print a
warning to stderr when stdout is not a tty. I believe that would at
least mitigate the problem, and it might educate the user as well.
We already modify output format when stdout is not a tty (removing
colors), so we're not giving guarantees about its format when it's
piped somewhere. I believe that would provide almost every scenario
with the expected outcome (including 'git show | grep'), but there
will be a handful of very surprised people as well.

-- 
Andreas Ericsson   andreas.erics...@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
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


Please pull git-l10n updates for git v1.7.12-146-g16d26

2012-09-12 Thread Jiang Xin
Hi Junio,

The following changes since commit 16d26b168b371b2f4f86b1adb61470c6b08b27b9:

  Latter half of the second batch for 1.8.0 (2012-08-29 15:00:30 -0700)

are available in the git repository at:

  git://github.com/git-l10n/git-po master

for you to fetch changes up to 9a4f34bb6d11ab47d532f7798ecc2b051f6d8893:

  l10n: Update Swedish translation (1166t0f0u) (2012-09-13 06:33:25 +0800)


Jiang Xin (3):
  l10n: Update git.pot (2 new, 4 removed messages)
  l10n: zh_CN.po: translate 2 new messages
  Merge git://github.com/vnwildman/git

Peter Krefting (1):
  l10n: Update Swedish translation (1166t0f0u)

Ralf Thielow (1):
  l10n: de.po: translate 2 new messages

Tran Ngoc Quan (2):
  l10n: vi.po & TEAMS: review Vietnamese translation
  l10n: vi.po: update to v1.7.12-146-g16d26

 po/TEAMS|3 +-
 po/de.po|  712 
 po/git.pot  |  684 +++
 po/sv.po|  715 
 po/vi.po| 1767 +++
 po/zh_CN.po |  712 
 6 files changed, 2394 insertions(+), 2199 deletions(-)


-- 
Jiang Xin
--
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] cherry-pick: don't forget -s on failure

2012-09-12 Thread Junio C Hamano
Miklos Vajna  writes:

> In case 'git cherry-pick -s ' failed, the user had to use 'git
> commit -s' (i.e. state the -s option again), which is easy to forget
> about.  Instead, write the signed-off-by line early, so plain 'git
> commit' will have the same result.
>
> Signed-off-by: Miklos Vajna 
> ---
>
> Hi,
>
> See
> http://article.gmane.org/gmane.comp.documentfoundation.libreoffice.devel/36103
> for motivation. :-)
>
> Given that I needed sign_off_header / ends_rfc2822_footer and some code
> from builtin/commit.c in sequencer.c, I moved them there, let me know if
> there is a place better than sequencer.c for them.

I think we had a separate topic around cherry-pick that needs the
footer thing accessible from cherry-pick recently ($gmane/204755).

I think the code movement in this patch is a good one.

Thanks.

>
> Thanks,
>
> Miklos
>
>  builtin/commit.c|   63 ++
>  sequencer.c |   65 
> +++
>  sequencer.h |4 ++
>  t/t3507-cherry-pick-conflict.sh |6 +++
>  4 files changed, 78 insertions(+), 60 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 778cf16..7d0df9a 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -28,6 +28,7 @@
>  #include "submodule.h"
>  #include "gpg-interface.h"
>  #include "column.h"
> +#include "sequencer.h"
>  
>  static const char * const builtin_commit_usage[] = {
>   N_("git commit [options] [--] ..."),
> @@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)
>   return !!(current_head->parents && current_head->parents->next);
>  }
>  
> -static const char sign_off_header[] = "Signed-off-by: ";
> -
>  static void export_one(const char *var, const char *s, const char *e, int 
> hack)
>  {
>   struct strbuf buf = STRBUF_INIT;
> @@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf 
> *author_ident)
>   }
>  }
>  
> -static int ends_rfc2822_footer(struct strbuf *sb)
> -{
> - int ch;
> - int hit = 0;
> - int i, j, k;
> - int len = sb->len;
> - int first = 1;
> - const char *buf = sb->buf;
> -
> - for (i = len - 1; i > 0; i--) {
> - if (hit && buf[i] == '\n')
> - break;
> - hit = (buf[i] == '\n');
> - }
> -
> - while (i < len - 1 && buf[i] == '\n')
> - i++;
> -
> - for (; i < len; i = k) {
> - for (k = i; k < len && buf[k] != '\n'; k++)
> - ; /* do nothing */
> - k++;
> -
> - if ((buf[k] == ' ' || buf[k] == '\t') && !first)
> - continue;
> -
> - first = 0;
> -
> - for (j = 0; i + j < len; j++) {
> - ch = buf[i + j];
> - if (ch == ':')
> - break;
> - if (isalnum(ch) ||
> - (ch == '-'))
> - continue;
> - return 0;
> - }
> - }
> - return 1;
> -}
> -
>  static char *cut_ident_timestamp_part(char *string)
>  {
>   char *ket = strrchr(string, '>');
> @@ -716,23 +674,8 @@ static int prepare_to_commit(const char *index_file, 
> const char *prefix,
>   if (clean_message_contents)
>   stripspace(&sb, 0);
>  
> - if (signoff) {
> - struct strbuf sob = STRBUF_INIT;
> - int i;
> -
> - strbuf_addstr(&sob, sign_off_header);
> - strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
> -  getenv("GIT_COMMITTER_EMAIL")));
> - strbuf_addch(&sob, '\n');
> - for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
> - ; /* do nothing */
> - if (prefixcmp(sb.buf + i, sob.buf)) {
> - if (!i || !ends_rfc2822_footer(&sb))
> - strbuf_addch(&sb, '\n');
> - strbuf_addbuf(&sb, &sob);
> - }
> - strbuf_release(&sob);
> - }
> + if (signoff)
> + append_signoff(&sb);
>  
>   if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
>   die_errno(_("could not write commit template"));
> diff --git a/sequencer.c b/sequencer.c
> index f86f116..402dcd6 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -17,6 +17,8 @@
>  
>  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>  
> +const char sign_off_header[] = "Signed-off-by: ";
> +
>  void remove_sequencer_state(void)
>  {
>   struct strbuf seq_dir = STRBUF_INIT;
> @@ -249,6 +251,9 @@ static int do_recursive_merge(struct commit *base, struct 
> commit *next,
>   }
>   }
>  
> + if (opts->signoff)
> + append_signoff(msgbuf);
> +
>   return !clean;
>  }
>  
> @@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)
>   save_opts(opts);
> 

Re: [PATCH] git-am: Handle "git show" output correctly

2012-09-12 Thread Dan Johnson
On Wed, Sep 12, 2012 at 6:19 PM, Junio C Hamano  wrote:
> Dan Johnson  writes:
>
>>> Not really.  If we start encouraging people to use "git show" output
>>> as a kosher input to "am", we would have to support such use
>>> forever, and we end up painting ourselves in a corner we cannot get
>>> out of easily.
>>
>> If git am emitted a warning when accepting "git show" output, it seems
>> like it would support Peter's use-case without encouraging bad
>> behavior?
>
> Are you seriously suggesting me to sell to our users a new feature
> saying "this does not work reliably, we would not recommend using
> it, no, really, don't trust it." from the day the feature is
> introduced, especially when we know it will not be "the feature does
> not work well yet, but it will, we promise" but is "and it may become
> worse in the future"?
>
> I do not see much point in doing that.
Fair enough.

> Besides, what bad behaviour do we avoid from encouraging with such
> an approach?  As Peter said, the problem is not on the part of the
> user who ended up with an output from "git show", when he really
> wants output from "git format-patch".  Giving the warning to the
> user of "git am" is too late.
I was assuming Peter would accept the patch, and reply with a "in the
future, please submit the output of format-patch", thus correcting the
submitter's behavior. This warning would serve someone who did not
know that they wanted the output of format-patch, and hopefully teach
them to send such a reply message.

> I may be able to be pursuaded to swallow a new script somewhere in
> the contrib/ hierarchy that takes a "git show" output and formats it
> to look like "format-patch" output to be fed to "git am".  That way,
> when a user has trouble with its parsing of "git show" output, at
> least we can ask for the output of the format massaging step to help
> us diagnose where the problem lies.

That sounds like a better approach to me as well.

-- 
-Dan
--
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] git-am: Handle "git show" output correctly

2012-09-12 Thread Junio C Hamano
Dan Johnson  writes:

>> Not really.  If we start encouraging people to use "git show" output
>> as a kosher input to "am", we would have to support such use
>> forever, and we end up painting ourselves in a corner we cannot get
>> out of easily.
>
> If git am emitted a warning when accepting "git show" output, it seems
> like it would support Peter's use-case without encouraging bad
> behavior?

Are you seriously suggesting me to sell to our users a new feature
saying "this does not work reliably, we would not recommend using
it, no, really, don't trust it." from the day the feature is
introduced, especially when we know it will not be "the feature does
not work well yet, but it will, we promise" but is "and it may become
worse in the future"?

I do not see much point in doing that.

Besides, what bad behaviour do we avoid from encouraging with such
an approach?  As Peter said, the problem is not on the part of the
user who ended up with an output from "git show", when he really
wants output from "git format-patch".  Giving the warning to the
user of "git am" is too late.

I may be able to be pursuaded to swallow a new script somewhere in
the contrib/ hierarchy that takes a "git show" output and formats it
to look like "format-patch" output to be fed to "git am".  That way,
when a user has trouble with its parsing of "git show" output, at
least we can ask for the output of the format massaging step to help
us diagnose where the problem lies.
--
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: Suggestions for "What's cooking"

2012-09-12 Thread Junio C Hamano
Dan Johnson  writes:

> I was thinking about this earlier. I wondered if it might even be
> worth it just to CC the authors of all topics whose status has changed
> since the last what's cooking, to make sure that they see updates
> pertinent to them. I know that I at least have filters which catch
> emails which CC me and promote them to my inbox, so I would see them
> more readily.

I've done that a few times per release cycle, usually before we go
into the pre-release freeze, but doing so manually is very time
consuming.  It's the kind of bureaucratic overhead I'd rather avoid.
If somebody volunteers to write a script that takes something like

git diff whats-cooking.txt

in a checkout of the 'todo' branch and figure out whom to Cc, and do
so reliably, it may be an option.

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: [PATCH] git-am: Handle "git show" output correctly

2012-09-12 Thread Dan Johnson
On Wed, Sep 12, 2012 at 5:18 PM, Junio C Hamano  wrote:
> Peter Jones  writes:
>
>> Well, if that happens, maybe we could regexp match on
>> "[[:alnum:]_-]+: /someexprthatlookslikeanemailaddress/" ?
>
> I doubt that would be even reliably done.
>
>> But we could
>> also just wait to cross that bridge until we get to it?
>
> Not really.  If we start encouraging people to use "git show" output
> as a kosher input to "am", we would have to support such use
> forever, and we end up painting ourselves in a corner we cannot get
> out of easily.

If git am emitted a warning when accepting "git show" output, it seems
like it would support Peter's use-case without encouraging bad
behavior?

-- 
-Dan
--
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 2/3] sha1: clean pointer arithmetic

2012-09-12 Thread Bruce Korb
On Wed, Sep 12, 2012 at 1:50 PM, Yann Droneaud  wrote:
>> Both are correct.  Aren't ctx->w[lenW] and lenW[ctx-w] both correct,
>> even?
>>
>
> "correct" in my commit log message should be read as "the way it's used
> by most C developer".
>
> It's again a cosmetic fix.

It's a maintenance fix.  The fewer distractions, the easier it is to understand.
"lenW[ctx-w]" is distracting.
--
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] git-am: Handle "git show" output correctly

2012-09-12 Thread Junio C Hamano
Peter Jones  writes:

> Well, if that happens, maybe we could regexp match on
> "[[:alnum:]_-]+: /someexprthatlookslikeanemailaddress/" ?

I doubt that would be even reliably done.

> But we could
> also just wait to cross that bridge until we get to it?

Not really.  If we start encouraging people to use "git show" output
as a kosher input to "am", we would have to support such use
forever, and we end up painting ourselves in a corner we cannot get
out of easily.
--
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 3/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Jeff King
On Wed, Sep 12, 2012 at 10:37:10PM +0200, Yann Droneaud wrote:

> > > diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> > > index b864df6..d29ff6a 100644
> > > --- a/block-sha1/sha1.h
> > > +++ b/block-sha1/sha1.h
> > > @@ -9,7 +9,7 @@
> > >  typedef struct {
> > >   unsigned long long size;
> > >   unsigned int H[5];
> > > - unsigned int W[16];
> > > + unsigned char W[64];
> > >  } blk_SHA_CTX;
> > 
> > Wouldn't this break all of the code that is planning to index "W" by
> > 32-bit words (see the definitions of setW in block-sha1/sha1.c)?
> > 
> That's not the same "W" ... This part of the code is indeed unclear.

Sorry, you're right, that's a different work array (though it has the
identical issue, no?). But the point still stands.  Did you audit the
block-sha1 code to make sure nobody is ever indexing the W array? If you
didn't, then your change is not safe. If you did, then you should really
mention that in the commit message.

> > If that is indeed the problem, wouldn't the simplest fix be using
> > uint32_t instead of "unsigned int"?
> 
> It's another way to fix this oddity, but not simpler.

It is simpler in the sense that it does not have any side effects (like
changing how every user of the data structure needs to index it).

> > Moreover, would that be sufficient to run on such a platform? At the
> > very least, "H" above would want the same treatment. And I would not be
> > surprised if some of the actual code in block-sha1/sha1.c needed
> > updating, as well.
> 
> ctx->H is actually used as an array of integer, so it would benefits of
> being declared uint32_t for an ILP64 system. This fix would also be
> required for blk_SHA1_Block() function.

So...if we are not ready to run on an ILP system after this change, then
what is the purpose?

-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


[PATCH] git-am: Handle "git show" output correctly

2012-09-12 Thread Peter Jones
This patch adds the ability for "git am" to accept patches in the format
generated by "git show".  Some people erroneously use "git show" instead
of "git format-patch", and it's nice as a maintainer to be able to
easily take their patch rather than going back and forth with them to
get a "correctly" formatted patch containing exactly the same actual
information.

Signed-off-by: Peter Jones 
---
 git-am.sh | 57 +
 1 file changed, 57 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index c682d34..d20f249 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,18 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+   "commit "*)
+   case "$l2,,$l3" in
+   "Author: "*,,"Date: "*)
+   if expr "$l1" : 'commit [0-9a-f]\{40\}$' \
+   >/dev/null ; then
+   patch_format=gitshow
+   fi
+   ;;
+   *)
+   ;;
+   esac
+   ;;
"From "* | "From: "*)
patch_format=mbox
;;
@@ -321,6 +333,51 @@ split_patches () {
this=
msgnum=
;;
+   gitshow)
+   this=0
+   for patch in "$@"
+   do
+   this=`expr "$this" + 1`
+   msgnum=`printf "%0${prec}d" $this`
+   # The first nonemptyline after an empty line is the
+   # subject, and the body starts with the next nonempty
+   # line.
+   perl -ne 'BEGIN {
+   $diff = 0; $subject = 0; $subjtext="";
+   }
+   if ($diff == 1 || /^diff/ || /^---$/) {
+   $diff = 1 ;
+   print ;
+   } elsif ($subject > 1) {
+   s/^// ;
+   print ;
+   } elsif ($subject == 1 && !/^\s+$/) {
+   s/^// ;
+   $subjtext = "$subjtext $_";
+   } elsif ($subject == 1) {
+   $subject = 2 ;
+   print "Subject: ", $subjtext ;
+   s/^// ;
+   print ;
+   } elsif ($subject) {
+   print "\n" ;
+   s/^// ;
+   print ;
+   } elsif (/^\s+$/) { next ; }
+   elsif (/^Author:/) { s/Author/From/ ; print ;}
+   elsif (/^Date:/) { print ; }
+   elsif (/^commit/) { next ; }
+   else {
+   s/^// ;
+   $subjtext = $_;
+   $subject = 1;
+   }
+   ' < "$patch" > "$dotest/$msgnum" || clean_abort
+   done
+   echo "$this" > "$dotest/last"
+   this=
+   msgnum=
+   ;;
hg)
this=0
for hg in "$@"
-- 
1.7.11.4

--
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 3/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Yann Droneaud
Le mercredi 12 septembre 2012 à 14:38 -0400, Jeff King a écrit :
> On Wed, Sep 12, 2012 at 12:30:45PM +0200, Yann Droneaud wrote:
> 
> > The SHA context is holding a temporary buffer for partial block.
> > 
> > This block must 64 bytes long. It is currently described as
> > an array of 16 integers.
> > 
> > Signed-off-by: Yann Droneaud 
> > ---
> >  block-sha1/sha1.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> > index b864df6..d29ff6a 100644
> > --- a/block-sha1/sha1.h
> > +++ b/block-sha1/sha1.h
> > @@ -9,7 +9,7 @@
> >  typedef struct {
> > unsigned long long size;
> > unsigned int H[5];
> > -   unsigned int W[16];
> > +   unsigned char W[64];
> >  } blk_SHA_CTX;
> 
> Wouldn't this break all of the code that is planning to index "W" by
> 32-bit words (see the definitions of setW in block-sha1/sha1.c)?
> 

That's not the same "W" ... This part of the code is indeed unclear.

> You do not describe an actual problem in the commit message, but reading
> between the lines it would be "system X would like to use block-sha1,
> but has an "unsigned int" that is not 32 bits". IOW, an ILP64 type of
> architecture. Do you have some specific platform in mind?
> 
> If that is indeed the problem, wouldn't the simplest fix be using
> uint32_t instead of "unsigned int"?
> 

It's another way to fix this oddity, but not simpler.


> Moreover, would that be sufficient to run on such a platform? At the
> very least, "H" above would want the same treatment. And I would not be
> surprised if some of the actual code in block-sha1/sha1.c needed
> updating, as well.
> 

ctx->H is actually used as an array of integer, so it would benefits of
being declared uint32_t for an ILP64 system. This fix would also be
required for blk_SHA1_Block() function.

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 2/3] sha1: clean pointer arithmetic

2012-09-12 Thread Yann Droneaud
Le mercredi 12 septembre 2012 à 11:37 -0700, Junio C Hamano a écrit :
> Yann Droneaud  writes:
> 
> > One memcpy() argument is computed from a pointer added to an integer:
> > eg. int + pointer. It's unusal.
> > Let's write it in the correct order: pointer + offset.
> 
> Meh.
> 
> Both are correct.  Aren't ctx->w[lenW] and lenW[ctx-w] both correct,
> even?
> 

"correct" in my commit log message should be read as "the way it's used
by most C developer".

It's again a cosmetic fix.

-- 
Yann Droneaud


--
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] git-am: Handle "git show" output correctly

2012-09-12 Thread Peter Jones
On Wed, 2012-09-12 at 13:06 -0700, Junio C Hamano wrote:
> Peter Jones  writes:
> 
> > This patch adds the ability for "git am" to accept patches in the format
> > generated by "git show".  Some people erroneously use "git show" instead
> > of "git format-patch", and it's nice as a maintainer to be able to
> > easily take their patch rather than going back and forth with them to
> > get a "correctly" formatted patch containing exactly the same actual
> > information.
> >
> > Signed-off-by: Peter Jones 
> > ---
> >  git-am.sh | 60 
> >  1 file changed, 60 insertions(+)
> >
> > diff --git a/git-am.sh b/git-am.sh
> > index c682d34..210e9fe 100755
> > --- a/git-am.sh
> > +++ b/git-am.sh
> > @@ -216,6 +216,21 @@ check_patch_format () {
> > read l2
> > read l3
> > case "$l1" in
> > +   "commit "*)
> > +   case "$l2" in
> > +   "Author: "*)
> > +   case "$l3" in
> > +   "Date: "*)
> > +   patch_format=gitshow
> > +   ;;
> > +   *)
> > +   ;;
> > +   esac
> > +   ;;
> > +   *)
> > +   ;;
> > +   esac
> > +   ;;
> 
> At least the inner one could become easier to read by losing one
> level of nesting, e.g.
> 
>   case "$l2,,$l3" in
> "Author: *",,"Date: ")
>   found it
> ;;
>   esac

Yeah, I can do that.

> I wonder what the severity of the damage if we misidentify the patch
> format in this function would be?  If it is severe enough, the check
> for the first line may want to become a bit more strict to avoid
> misidentification (e.g. expr "$l1" : 'commit [0-9a-f]\{40\}$').
> Perhaps we don't care.  I dunno.

I hadn't really even considered it - are there other formats that use
commit and author which git-am.sh is supposed to support? It seems as
though if we get something wrong you'll wind up in clean_abort at some
point anyway.  At worst your patch will still be there and you'll need
to do "git am --abort".

> > @@ -321,6 +336,51 @@ split_patches () {
> > this=
> > msgnum=
> > ;;
> > +   gitshow)
> > +   this=0
> > +   for patch in "$@"
> > +   do
> 
> So each input file is expected to be nothing but an output from "git
> show" for a single commit; in other words, not concatenation of
> them, nor just an e-mail message that has "git show" output
> copy&pasted in the body with some other cruft, but plausibly was
> delibered as a separate attachment file.
> 
> I somehow was visualizing that you were trying to accept mails I
> sometimes see here like:
> 
>   From: somebody
> Date: someday
> 
> Hi, a long winded discussion that talks about the motivation
> behind the patch comes here.
> 
>   commit 4d8c4db13c8c4c79b6fc0a38ff52d85d3543aa7a
> Author: A U Thor 
> Date: Tue Sep 11 12:34:56 2012 +0900
> 
>   a one liner that just says "bugfix" and nothing else
> 
>   diff --git 
> 
> and that was one of the reasons I thought (but didn't say in my
> responses) "Why bother?  When running 'am' on such a message you
> will have to edit the message to move things around anyway".

Yeah, that sounds like madness.

> If the target is a stand-alone "git show" output, at least we do not
> have to worry about such a case.

Right.

> 
> > +   this=`expr "$this" + 1`
> > +   msgnum=`printf "%0${prec}d" $this`
> > +   # The first nonemptyline after an empty line is the
> > +   # subject, and the body starts with the next nonempty
> > +   # line.
> > +   perl -ne 'BEGIN {
> > +   $diff = 0; $subject = 0; $subjtext="";
> > +   }
> > +   if ($diff == 1 || /^diff/ || /^---$/) {
> > +   $diff = 1 ;
> > +   print ;
> > +   } elsif ($subject > 1) {
> > +   s/^// ;
> > +   print ;
> > +   } elsif ($subject == 1 && !/^\s+$/) {
> > +   s/^// ;
> > +   $subjtext = "$subjtext $_";
> > +   } elsif ($subject == 1) {
> > +   $subject = 2 ;
> > +   print "Subject: ", $subjtext ;
> > +   s/^// ;
> > +   print ;
> > +   } elsif ($subject) {
> > +   

Re: [PATCH 3/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Yann Droneaud
Le mercredi 12 septembre 2012 à 11:42 -0700, Junio C Hamano a écrit :
> Yann Droneaud  writes:
> 
> > The SHA context is holding a temporary buffer for partial block.
> >
> > This block must 64 bytes long. It is currently described as
> > an array of 16 integers.
> >
> > Signed-off-by: Yann Droneaud 
> > ---
> 
> As we do not work with 16-bit integers anyway, 16 integers occupy 64
> bytes anyway.
> 

It's unclear why this array is declared as 'int' but used as 'char'.

> What problem does this series fix?
> 

The question I was hoping to not see asked :)
This is mostly some cosmetic fixes to improve readability and
portability.

Regards

-- 
Yann Droneaud
OPTEYA


--
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] fetch: align new ref summary printout in UTF-8 locales

2012-09-12 Thread Torsten Bögershausen
On 12.09.12 20:02, Junio C Hamano wrote:
> Nguyen Thai Ngoc Duy  writes:
> 
>> Ping.. what happens to this patch? Do you want to support other
>> encodings as well via iconv()? I can't test that though.
> 
> I thought I refuted a potential blocker, which was an implied
> objection from Torsten, in $gmane/204912 already.  As long as we
> make it clear that your patch helps only "ASCII/UTF-8 only" audience
> but we still "try to be nicer to 'others'" by doing two things I
> said in the message, I think we should proceed without iconv() to
> keep things simple.

Please unblock and proceed (as I can't test other encodings either)

And even special thanks for the excellent lines from Junio,
which explained the update philosophy in git so well, 
that I take the freedom to re-post them here:

>> I try to re-phrase my question:
>>
>> Do installations still exist which use e.g. BIG5 or any other
>> multi byte encoding which is not UTF-8?
>
>Yes.
>
>> Do we want to support other encodings than ASCII or UTF-8?
>> (Because then the screen width needs to be calculate different, I think)
>
>That depends on who "we" are and what timeframe you have in mind.
>
>Do our developers care about these encodings so much that we would
>reject "ASCCI/UTF-8 only" patch and wait until we enhance it to do
>the right thing for other encodings that we do not use ourselves?
>No.  That does not make any sense, especially when we know we will
>not have a good test coverage on the additional parts that we will
>not be using ourselves.
>
>"This change only helps people with ASCII or UTF-8 and does not help
>others" alone is never a valid reason to reject a change, but we
>still try to be nicer to "others" that may come after we leave this
>topic behind by doing a few things:
>
> - If the change will make things worse than it currently is for
>   "others", we would try to minimize the regression for them.
>
> - If the change will make the code harder to update later to
>   enhance with additional change to support "others", we would try
>   to anticipate what kind of changes are needed on top, and
>   structure the code in such a way that future changes can be made
>   cleanly.
>
>For the first point, for multi-byte encodings (e.g. ISO-2022), the
>display columns and byte length do not match and in general byte
>length is longer than the display columns in the current code.  With
>the current code that measures the required columns across elements
>by taking the maximum of byte length, they will see wrong number of
>filler, so they are already getting a wrong alignment.  With the
>"UTF-8 only" change, the required columns and the filler will be
>calculated by assuming that the string is in UTF-8, which may make
>the computation off in a different way, and if we underestimate the
>display columns for a string, they may see the strings truncated,
>which is bad.
>
>So as long as gettext_width() punts and returns strlen() for a
>malformed UTF-8, it would be OK [*1*].
>
>For the second point, I think the API "here is a string, give me the
>number of display columns it will occupy, as I am interested in
>aligning them" is a good abstraction that can be later enhanced to
>other encodings fairly easily, so I do not see a problem in the
>patch that goes in that direction.
>
>
>
>[Footnote]
>
>*1* If the patch used utf_strwidth() (which I didn't bother to go
>back and check, though), it should be OK.  The underlying
>utf8_width() will reject a malformed UTF-8 sequence and the code
>falls back to strlen().

 










--
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: Suggestions for "What's cooking"

2012-09-12 Thread Dan Johnson
On Wed, Sep 12, 2012 at 3:21 PM, Jens Lehmann  wrote:
> Am 11.09.2012 21:41, schrieb Junio C Hamano:
>> Thanks.  I wish all others paid attention to "What's cooking" like
>> you did here.
>>
>> And if it is hard to do so for whatever reason, suggest a better way
>> for me to publish "What's cooking" or an equivalent (I am interested
>> in finding the least bureaucratic way to help people and keep the
>> balls rolling).
>
> I think "What's cooking" makes lots of sense in its current form
> as one gets a very good overview over current development tracks.
>
> Maybe in addition it would be nice to email the author(s) of a
> series when the state changes or new comments are added (and to
> only include the relevant part from "What's cooking" there). For
> me it's not a big problem as I just have to grep for "submodule"
> to get the bits I care about, but I suspect others might have to
> invest much more time to check the current state of their series
> and may appreciate being mailed directly when something happens.
> Opinions?

I was thinking about this earlier. I wondered if it might even be
worth it just to CC the authors of all topics whose status has changed
since the last what's cooking, to make sure that they see updates
pertinent to them. I know that I at least have filters which catch
emails which CC me and promote them to my inbox, so I would see them
more readily.

My normal mode of operation is that when I have a patch in I check all
the "What's cooking" messages as if I was F5-ing a webpage, to follow
its status. Were I CCd on the message, I would be updated whenever the
mail was sent, which I would appreciate. This also has the nice side
effect of updating patch authors who are not subscribed to the list.

On the other hand, its possible some people would find that this
generated lots of noise, and it might also cause unrelated replies to
the "What's Cooking" message to CC all authors.
-- 
-Dan
--
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] git-am: Handle "git show" output correctly

2012-09-12 Thread Junio C Hamano
Peter Jones  writes:

> This patch adds the ability for "git am" to accept patches in the format
> generated by "git show".  Some people erroneously use "git show" instead
> of "git format-patch", and it's nice as a maintainer to be able to
> easily take their patch rather than going back and forth with them to
> get a "correctly" formatted patch containing exactly the same actual
> information.
>
> Signed-off-by: Peter Jones 
> ---
>  git-am.sh | 60 
>  1 file changed, 60 insertions(+)
>
> diff --git a/git-am.sh b/git-am.sh
> index c682d34..210e9fe 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -216,6 +216,21 @@ check_patch_format () {
>   read l2
>   read l3
>   case "$l1" in
> + "commit "*)
> + case "$l2" in
> + "Author: "*)
> + case "$l3" in
> + "Date: "*)
> + patch_format=gitshow
> + ;;
> + *)
> + ;;
> + esac
> + ;;
> + *)
> + ;;
> + esac
> + ;;

At least the inner one could become easier to read by losing one
level of nesting, e.g.

case "$l2,,$l3" in
"Author: *",,"Date: ")
found it
;;
esac

I wonder what the severity of the damage if we misidentify the patch
format in this function would be?  If it is severe enough, the check
for the first line may want to become a bit more strict to avoid
misidentification (e.g. expr "$l1" : 'commit [0-9a-f]\{40\}$').
Perhaps we don't care.  I dunno.

> @@ -321,6 +336,51 @@ split_patches () {
>   this=
>   msgnum=
>   ;;
> + gitshow)
> + this=0
> + for patch in "$@"
> + do

So each input file is expected to be nothing but an output from "git
show" for a single commit; in other words, not concatenation of
them, nor just an e-mail message that has "git show" output
copy&pasted in the body with some other cruft, but plausibly was
delibered as a separate attachment file.

I somehow was visualizing that you were trying to accept mails I
sometimes see here like:

From: somebody
Date: someday

Hi, a long winded discussion that talks about the motivation
behind the patch comes here.

commit 4d8c4db13c8c4c79b6fc0a38ff52d85d3543aa7a
Author: A U Thor 
Date: Tue Sep 11 12:34:56 2012 +0900

a one liner that just says "bugfix" and nothing else

diff --git 

and that was one of the reasons I thought (but didn't say in my
responses) "Why bother?  When running 'am' on such a message you
will have to edit the message to move things around anyway".

If the target is a stand-alone "git show" output, at least we do not
have to worry about such a case.

> + this=`expr "$this" + 1`
> + msgnum=`printf "%0${prec}d" $this`
> + # The first nonemptyline after an empty line is the
> + # subject, and the body starts with the next nonempty
> + # line.
> + perl -ne 'BEGIN {
> + $diff = 0; $subject = 0; $subjtext="";
> + }
> + if ($diff == 1 || /^diff/ || /^---$/) {
> + $diff = 1 ;
> + print ;
> + } elsif ($subject > 1) {
> + s/^// ;
> + print ;
> + } elsif ($subject == 1 && !/^\s+$/) {
> + s/^// ;
> + $subjtext = "$subjtext $_";
> + } elsif ($subject == 1) {
> + $subject = 2 ;
> + print "Subject: ", $subjtext ;
> + s/^// ;
> + print ;
> + } elsif ($subject) {
> + print "\n" ;
> + s/^// ;
> + print ;
> + } elsif (/^\s+$/) { next ; }
> + elsif (/^Author:/) { s/Author/From/ ; print ;}
> + elsif (/^(From|Date)/) { print ; }

Where does "^From" come from? Should this be /^Date: / instead?

> + elsif (/^commit/) { next ; }
> + else {
> + s/^// ;
> + 

[PATCH] cherry-pick: don't forget -s on failure

2012-09-12 Thread Miklos Vajna
In case 'git cherry-pick -s ' failed, the user had to use 'git
commit -s' (i.e. state the -s option again), which is easy to forget
about.  Instead, write the signed-off-by line early, so plain 'git
commit' will have the same result.

Signed-off-by: Miklos Vajna 
---

Hi,

See
http://article.gmane.org/gmane.comp.documentfoundation.libreoffice.devel/36103
for motivation. :-)

Given that I needed sign_off_header / ends_rfc2822_footer and some code
from builtin/commit.c in sequencer.c, I moved them there, let me know if
there is a place better than sequencer.c for them.

Thanks,

Miklos

 builtin/commit.c|   63 ++
 sequencer.c |   65 +++
 sequencer.h |4 ++
 t/t3507-cherry-pick-conflict.sh |6 +++
 4 files changed, 78 insertions(+), 60 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 778cf16..7d0df9a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -28,6 +28,7 @@
 #include "submodule.h"
 #include "gpg-interface.h"
 #include "column.h"
+#include "sequencer.h"
 
 static const char * const builtin_commit_usage[] = {
N_("git commit [options] [--] ..."),
@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)
return !!(current_head->parents && current_head->parents->next);
 }
 
-static const char sign_off_header[] = "Signed-off-by: ";
-
 static void export_one(const char *var, const char *s, const char *e, int hack)
 {
struct strbuf buf = STRBUF_INIT;
@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf 
*author_ident)
}
 }
 
-static int ends_rfc2822_footer(struct strbuf *sb)
-{
-   int ch;
-   int hit = 0;
-   int i, j, k;
-   int len = sb->len;
-   int first = 1;
-   const char *buf = sb->buf;
-
-   for (i = len - 1; i > 0; i--) {
-   if (hit && buf[i] == '\n')
-   break;
-   hit = (buf[i] == '\n');
-   }
-
-   while (i < len - 1 && buf[i] == '\n')
-   i++;
-
-   for (; i < len; i = k) {
-   for (k = i; k < len && buf[k] != '\n'; k++)
-   ; /* do nothing */
-   k++;
-
-   if ((buf[k] == ' ' || buf[k] == '\t') && !first)
-   continue;
-
-   first = 0;
-
-   for (j = 0; i + j < len; j++) {
-   ch = buf[i + j];
-   if (ch == ':')
-   break;
-   if (isalnum(ch) ||
-   (ch == '-'))
-   continue;
-   return 0;
-   }
-   }
-   return 1;
-}
-
 static char *cut_ident_timestamp_part(char *string)
 {
char *ket = strrchr(string, '>');
@@ -716,23 +674,8 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
if (clean_message_contents)
stripspace(&sb, 0);
 
-   if (signoff) {
-   struct strbuf sob = STRBUF_INIT;
-   int i;
-
-   strbuf_addstr(&sob, sign_off_header);
-   strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
-getenv("GIT_COMMITTER_EMAIL")));
-   strbuf_addch(&sob, '\n');
-   for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
-   ; /* do nothing */
-   if (prefixcmp(sb.buf + i, sob.buf)) {
-   if (!i || !ends_rfc2822_footer(&sb))
-   strbuf_addch(&sb, '\n');
-   strbuf_addbuf(&sb, &sob);
-   }
-   strbuf_release(&sob);
-   }
+   if (signoff)
+   append_signoff(&sb);
 
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
die_errno(_("could not write commit template"));
diff --git a/sequencer.c b/sequencer.c
index f86f116..402dcd6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -17,6 +17,8 @@
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
+const char sign_off_header[] = "Signed-off-by: ";
+
 void remove_sequencer_state(void)
 {
struct strbuf seq_dir = STRBUF_INIT;
@@ -249,6 +251,9 @@ static int do_recursive_merge(struct commit *base, struct 
commit *next,
}
}
 
+   if (opts->signoff)
+   append_signoff(msgbuf);
+
return !clean;
 }
 
@@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)
save_opts(opts);
return pick_commits(todo_list, opts);
 }
+
+static int ends_rfc2822_footer(struct strbuf *sb)
+{
+   int ch;
+   int hit = 0;
+   int i, j, k;
+   int len = sb->len;
+   int first = 1;
+   const char *buf = sb->buf;
+
+   for (i = len - 1; i > 0; i--) {
+   if (hit && buf[i] == '\n')
+   break;
+   hit = (buf[i] =

Re: Suggestions for "What's cooking"

2012-09-12 Thread Jeff King
On Wed, Sep 12, 2012 at 09:21:46PM +0200, Jens Lehmann wrote:

> Am 11.09.2012 21:41, schrieb Junio C Hamano:
> > Thanks.  I wish all others paid attention to "What's cooking" like
> > you did here.
> > 
> > And if it is hard to do so for whatever reason, suggest a better way
> > for me to publish "What's cooking" or an equivalent (I am interested
> > in finding the least bureaucratic way to help people and keep the
> > balls rolling).
> 
> I think "What's cooking" makes lots of sense in its current form
> as one gets a very good overview over current development tracks.
>
> Maybe in addition it would be nice to email the author(s) of a
> series when the state changes or new comments are added (and to
> only include the relevant part from "What's cooking" there).

Yeah, in general I think the current system is fine. It might be
slightly more convenient to send out the update email, but finding the
right thread is more work for Junio (I guess you could just ignore that
and start a new thread, but for readers it is nice if it is connected to
the original series thread).

And I personally think it is nice to read through the whole list of
topics and their current status occasionally. I sometimes end up
commenting on a topic that I probably would not have otherwise seen that
way.  Of course, I likely have a lot more git time than most people, so
the effort of skimming "what's cooking" is not too high for me.

-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 v3] Teach rm to remove submodules unless they contain a git directory

2012-09-12 Thread Jens Lehmann
Am 11.09.2012 21:41, schrieb Junio C Hamano:
> Lilewise.  It may make sense to introduce a helper function to tell
> if it is a submodule on our side by checking only the stage #2 entry
> when you see a nagetive pos returned from cache_name_pos() and call
> it "is_ours_submodule?()" or something.

Thanks, will do so.
--
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


Suggestions for "What's cooking"

2012-09-12 Thread Jens Lehmann
Am 11.09.2012 21:41, schrieb Junio C Hamano:
> Thanks.  I wish all others paid attention to "What's cooking" like
> you did here.
> 
> And if it is hard to do so for whatever reason, suggest a better way
> for me to publish "What's cooking" or an equivalent (I am interested
> in finding the least bureaucratic way to help people and keep the
> balls rolling).

I think "What's cooking" makes lots of sense in its current form
as one gets a very good overview over current development tracks.

Maybe in addition it would be nice to email the author(s) of a
series when the state changes or new comments are added (and to
only include the relevant part from "What's cooking" there). For
me it's not a big problem as I just have to grep for "submodule"
to get the bits I care about, but I suspect others might have to
invest much more time to check the current state of their series
and may appreciate being mailed directly when something happens.
Opinions?
--
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] [git-am] Handle "git show" output correctly

2012-09-12 Thread Junio C Hamano
Peter Jones  writes:

> Let me put it a different way - if you won't accept git-am handling "git
> show" output because "git show" has output that wasn't designed to be
> parsed ever, would you be opposed to a patch that switches the "git
> show" output to be something usable?

The output from the command is optimized for humans, but you could
invoke "git show --pretty=email" if you want to, so I do not think
you need any patch to do 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 3/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Junio C Hamano
Yann Droneaud  writes:

> The SHA context is holding a temporary buffer for partial block.
>
> This block must 64 bytes long. It is currently described as
> an array of 16 integers.
>
> Signed-off-by: Yann Droneaud 
> ---

As we do not work with 16-bit integers anyway, 16 integers occupy 64
bytes anyway.

What problem does this series fix?

>  block-sha1/sha1.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> index b864df6..d29ff6a 100644
> --- a/block-sha1/sha1.h
> +++ b/block-sha1/sha1.h
> @@ -9,7 +9,7 @@
>  typedef struct {
>   unsigned long long size;
>   unsigned int H[5];
> - unsigned int W[16];
> + unsigned char W[64];
>  } blk_SHA_CTX;
>  
>  void blk_SHA1_Init(blk_SHA_CTX *ctx);
--
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 3/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Jeff King
On Wed, Sep 12, 2012 at 12:30:45PM +0200, Yann Droneaud wrote:

> The SHA context is holding a temporary buffer for partial block.
> 
> This block must 64 bytes long. It is currently described as
> an array of 16 integers.
> 
> Signed-off-by: Yann Droneaud 
> ---
>  block-sha1/sha1.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> index b864df6..d29ff6a 100644
> --- a/block-sha1/sha1.h
> +++ b/block-sha1/sha1.h
> @@ -9,7 +9,7 @@
>  typedef struct {
>   unsigned long long size;
>   unsigned int H[5];
> - unsigned int W[16];
> + unsigned char W[64];
>  } blk_SHA_CTX;

Wouldn't this break all of the code that is planning to index "W" by
32-bit words (see the definitions of setW in block-sha1/sha1.c)?

You do not describe an actual problem in the commit message, but reading
between the lines it would be "system X would like to use block-sha1,
but has an "unsigned int" that is not 32 bits". IOW, an ILP64 type of
architecture. Do you have some specific platform in mind?

If that is indeed the problem, wouldn't the simplest fix be using
uint32_t instead of "unsigned int"?

Moreover, would that be sufficient to run on such a platform? At the
very least, "H" above would want the same treatment. And I would not be
surprised if some of the actual code in block-sha1/sha1.c needed
updating, as well.

-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 2/3] sha1: clean pointer arithmetic

2012-09-12 Thread Junio C Hamano
Yann Droneaud  writes:

> One memcpy() argument is computed from a pointer added to an integer:
> eg. int + pointer. It's unusal.
> Let's write it in the correct order: pointer + offset.

Meh.

Both are correct.  Aren't ctx->w[lenW] and lenW[ctx-w] both correct,
even?


>
> Signed-off-by: Yann Droneaud 
> ---
>  block-sha1/sha1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
> index c1af112..a7c4470 100644
> --- a/block-sha1/sha1.c
> +++ b/block-sha1/sha1.c
> @@ -246,7 +246,7 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, 
> unsigned long len)
>   unsigned int left = 64 - lenW;
>   if (len < left)
>   left = len;
> - memcpy(lenW + (char *)ctx->W, data, left);
> + memcpy((char *)ctx->W + lenW, data, left);
>   lenW = (lenW + left) & 63;
>   if (lenW)
>   return;
--
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/3] sha1: update pointer and remaining length after subfunction call

2012-09-12 Thread Junio C Hamano
Yann Droneaud  writes:

> There's no need to update the pointer and remaining length before
> leaving or calling the SHA1 sub function.
>
> Additionnaly, the partial block code could be looking more like
> the full block handling branch.
>
> Signed-off-by: Yann Droneaud 
> ---
>  block-sha1/sha1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
> index a8d4bf9..c1af112 100644
> --- a/block-sha1/sha1.c
> +++ b/block-sha1/sha1.c
> @@ -248,11 +248,11 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void 
> *data, unsigned long len)
>   left = len;
>   memcpy(lenW + (char *)ctx->W, data, left);
>   lenW = (lenW + left) & 63;
> - len -= left;
> - data = ((const char *)data + left);
>   if (lenW)
>   return;
>   blk_SHA1_Block(ctx, ctx->W);
> + data = ((const char *)data + left);
> + len -= left;
>   }
>   while (len >= 64) {
>   blk_SHA1_Block(ctx, data);

It is not wrong per-se, but doesn't the compiler optimize it out if
this is worth doing?  Just being curious.

--
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 0/2] Mixing English and a local language

2012-09-12 Thread Junio C Hamano
Nguyen Thai Ngoc Duy  writes:

> Should I interpret the silence as "I don't care, if you want it, go
> for it" or "not acceptable, but no reasons given"?

I do not speak for the others, but the reason I didn't respond is
none of the above. It is somewhere between "Meh" and "Anything that
says 'local language' and 'English' cannot be worth looking at."

I _think_ the patch was inspired by $gmane/204979, where I said:

Or "LC_ALL=C LANG=C git format-patch ...".

It does not bother me (even though I do not read Vietnamese), but
this has been brought up a few times, and we may want to revert the
i18n of the diffstat summary.  It does not seem to add much value to
the system but annoys people.  After all, the "upstream" diffstat
does not localizes this string (I just checked diffstat-1.55 with
Jan 2012 timestamp).

and I have been waiting to see what others think.  I am so far
taking the silence in the thread to mean they do not mind seeing the
diffstat summary untranslated and they do not mind seeing it in
Klingon, as long as the three numbers are there with (+) and (-)
markings.

It is bad enough having to decide where the boundary between 'local
language' and 'C locale' should be drawn in the mixture.  I am not
enthused by an attempt to make the boundary tweakable, and worse
yet, to do so per command.

IMHO, we should just decide where to draw the line and be done with
it.  The users already know or can be trained to know to choose the
greatest common denominator when interacting with others.
--
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] git-am: Handle "git show" output correctly

2012-09-12 Thread Peter Jones
This patch adds the ability for "git am" to accept patches in the format
generated by "git show".  Some people erroneously use "git show" instead
of "git format-patch", and it's nice as a maintainer to be able to
easily take their patch rather than going back and forth with them to
get a "correctly" formatted patch containing exactly the same actual
information.

Signed-off-by: Peter Jones 
---
 git-am.sh | 60 
 1 file changed, 60 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index c682d34..210e9fe 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+   "commit "*)
+   case "$l2" in
+   "Author: "*)
+   case "$l3" in
+   "Date: "*)
+   patch_format=gitshow
+   ;;
+   *)
+   ;;
+   esac
+   ;;
+   *)
+   ;;
+   esac
+   ;;
"From "* | "From: "*)
patch_format=mbox
;;
@@ -321,6 +336,51 @@ split_patches () {
this=
msgnum=
;;
+   gitshow)
+   this=0
+   for patch in "$@"
+   do
+   this=`expr "$this" + 1`
+   msgnum=`printf "%0${prec}d" $this`
+   # The first nonemptyline after an empty line is the
+   # subject, and the body starts with the next nonempty
+   # line.
+   perl -ne 'BEGIN {
+   $diff = 0; $subject = 0; $subjtext="";
+   }
+   if ($diff == 1 || /^diff/ || /^---$/) {
+   $diff = 1 ;
+   print ;
+   } elsif ($subject > 1) {
+   s/^// ;
+   print ;
+   } elsif ($subject == 1 && !/^\s+$/) {
+   s/^// ;
+   $subjtext = "$subjtext $_";
+   } elsif ($subject == 1) {
+   $subject = 2 ;
+   print "Subject: ", $subjtext ;
+   s/^// ;
+   print ;
+   } elsif ($subject) {
+   print "\n" ;
+   s/^// ;
+   print ;
+   } elsif (/^\s+$/) { next ; }
+   elsif (/^Author:/) { s/Author/From/ ; print ;}
+   elsif (/^(From|Date)/) { print ; }
+   elsif (/^commit/) { next ; }
+   else {
+   s/^// ;
+   $subjtext = $_;
+   $subject = 1;
+   }
+   ' < "$patch" > "$dotest/$msgnum" || clean_abort
+   done
+   echo "$this" > "$dotest/last"
+   this=
+   msgnum=
+   ;;
hg)
this=0
for hg in "$@"
-- 
1.7.11.4

--
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] [git-am] Handle "git show" output correctly

2012-09-12 Thread Peter Jones
On Wed, 2012-09-12 at 10:32 -0700, Junio C Hamano wrote:
> We do not want to apply "git show" output that munges the log
> message, period.
> 
> If you want to give patches to somebody (or to yourself) via e-mail
> or via sneaker-net, "git format-patch" is there for you.  Do not
> butcher "am" to accept a format that is not meant for patch
> transport in the first place.
> 
> If you want to screw something in to your shelf, you would use a
> screw and a screwdriver.  You do not try to hammer a nail using your
> screwdriver, find that the screwdriver is not very useful as a
> hammer and modify the screwdriver to hit your nail.

That seems to be completely missing the point - people /send/ them
without knowing, and as a maintainer of several projects, it's /hostile/
to people who are trying to help by sending patches to go around in
circles with them about the fact that they typed the wrong command. I'd
rather just take the patch, but right now the tools won't let me, and
for completely arbitrary reasons.

Let me put it a different way - if you won't accept git-am handling "git
show" output because "git show" has output that wasn't designed to be
parsed ever, would you be opposed to a patch that switches the "git
show" output to be something usable?

-- 
  Peter

--
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] fetch: align new ref summary printout in UTF-8 locales

2012-09-12 Thread Junio C Hamano
Nguyen Thai Ngoc Duy  writes:

> Ping.. what happens to this patch? Do you want to support other
> encodings as well via iconv()? I can't test that though.

I thought I refuted a potential blocker, which was an implied
objection from Torsten, in $gmane/204912 already.  As long as we
make it clear that your patch helps only "ASCII/UTF-8 only" audience
but we still "try to be nicer to 'others'" by doing two things I
said in the message, I think we should proceed without iconv() to
keep things simple.
--
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] Handle "git show" output correctly.

2012-09-12 Thread Peter Jones
On Wed, 2012-09-12 at 17:40 +0200, Matthieu Moy wrote:
> 
> How does this react to multi-line subject, e.g
> 
> This should be the
> subject line.
> 
> And this is the body.
> 
> ?
> 
> git format-patch will merge the lines in a single Subject: header, and
> your version seems to take only the first line.
> 
> A test showing this would be welcome. 

An updated patch to fix this will be my next mail.  It's not as succinct
as it once was, but such is life.

The two attached commits to this message can be used as a test case.
Basically, do (in any repo)

git am 0001* 0002*
git show > foo.patch
git reset HEAD^ --hard
git am foo.patch
git show # check the output here
git format-patch -1
# check 0001-bar-bar-bar-this-is-a-very-very-long-line-I-am-tired.patch
# here.

It winds up merging the subject lines before the rest of git-am does - I
couldn't get it to work if I preserved the newline; for some reason I
always get a second newline and that's /more/ wrong.

-- 
  Peter
>From f7521f88731f9fc696dcd8e32de58cc9d98ed892 Mon Sep 17 00:00:00 2001
From: Peter Jones 
Date: Wed, 12 Sep 2012 13:17:11 -0400
Subject: [PATCH 1/2] this is an example with a very long subject line which
 is completely unreasonable and nevertheless a thing.

It also has other stuff here.
---
 foo | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 foo

diff --git a/foo b/foo
new file mode 100644
index 000..fa5ef85
--- /dev/null
+++ b/foo
@@ -0,0 +1 @@
+za za za
-- 
1.7.11.4

>From bc471f2b89ada6e6ddf35b5ec2538242b5639836 Mon Sep 17 00:00:00 2001
From: Peter Jones 
Date: Wed, 12 Sep 2012 13:39:56 -0400
Subject: [PATCH 2/2] bar bar bar this is a very very long line I am tired of
 this game and it is quite annoying.

this is really annoying.  I hate perl.
zonk.

yes.
no.
---
 foo | 1 +
 1 file changed, 1 insertion(+)

diff --git a/foo b/foo
index fa5ef85..3b572f4 100644
--- a/foo
+++ b/foo
@@ -1 +1,2 @@
 za za za
+bar bar bar
-- 
1.7.11.4



Re: [PATCH v2 2/6] string_list: add two new functions for splitting strings

2012-09-12 Thread Junio C Hamano
Michael Haggerty  writes:

> On 09/11/2012 12:33 AM, Junio C Hamano wrote:
>> Michael Haggerty  writes:
>> 
>>> +`string_list_split`, `string_list_split_in_place`::
>>> +
>>> +   Split a string into substrings on a delimiter character and
>>> +   append the substrings to a `string_list`.  If `maxsplit` is
>>> +   non-negative, then split at most `maxsplit` times.  Return the
>>> +   number of substrings appended to the list.
>> 
>> 
>> I recall that we favor
>> 
>> `A`::
>> `B`::
>> 
>>  Description for A and B
>> 
>> for some reason but do not remember exactly why.
>
> Will change.  Thanks.

Thanks.  It comes from this one:

commit bf474e2402e51843e8230c064da6ccfdf3a8ff54
Author: Markus Heidelberg 
Date:   Fri Jan 16 22:42:33 2009 +0100

Documentation: let asciidoc align related options

Command line options can share the same paragraph of description, if
they are related or synonymous. In these cases they should be
written among each other, so that asciidoc can format them itself.

Signed-off-by: Markus Heidelberg 
Signed-off-by: Junio C Hamano 
--
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] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

2012-09-12 Thread Junio C Hamano
Elia Pinto  writes:

> Recent versions of Linux libc (later than 5.4.23) and glibc (2.x)
> include a malloc() implementation which is tunable via environment
> variables. When MALLOC_CHECK_ is set, a special (less efficient)
> implementation is used which is designed to be tolerant against
> simple errors, such as double calls of free() with the same argument,
> or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_
> is set to 3, a diagnostic message is printed on stderr
> and the program is aborted.
>
> Setting the MALLOC_PERTURB_ environment variable causes the malloc
> functions in libc to return memory which has been wiped and clear
> memory when it is returned.
> Of course this does not affect calloc which always does clear the memory.
>
> The reason for this exercise is, of course, to find code which uses
> memory returned by malloc without initializing it and code which uses
> code after it is freed. valgrind can do this but it's costly to run.
> The MALLOC_PERTURB_ exchanges the ability to detect problems in 100%
> of the cases with speed.
>
> The byte value used to initialize values returned by malloc is the byte
> value of the environment value. The value used to clear memory is the
> bitwise inverse. Setting MALLOC_PERTURB_ to zero disables the feature.
>
> This technique can find hard to detect bugs.
> It is therefore suggested to always use this flag (at least temporarily)
> when testing out code or a new distribution.
>
> Signed-off-by: Elia Pinto 
> ---
>  t/test-lib.sh | 6 ++
>  1 file changed, 6 insertions(+)

Interesting, but it bothers me to make it enabled unconditionally.
At least, this shouldn't be enabled under GIT_TEST_OPTS=--valgrind, no?

By the way, "export VAR=VAL" all on the same line, even though it is
in POSIX.1, is reported to be unsupported by some shells people care
about, and needs to be corrected to "VAR=VAL" and "export VAR" as
separate commands.  I think we saw a patch to fix an instance or two
that snuck in recently.

>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..98c90b0 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -93,6 +93,12 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
>  export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>  export EDITOR
>  
> +# Add libc malloc_check and MALLOC_PERTURB test 
> +export MALLOC_CHECK_=3
> +export MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
> +#
> +
> +
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
>  unset CDPATH
--
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 2/2] attr: "binary" attribute should choose built-in "binary" merge driver

2012-09-12 Thread Stephen Bash
- Original Message -
> From: "Junio C Hamano" 
> Sent: Wednesday, September 12, 2012 1:01:30 PM
> Subject: Re: [PATCH 2/2] attr: "binary" attribute should choose built-in 
> "binary" merge driver
> 
> >> Perhaps something like this makes it better.
> >
> > Patch didn't apply on top of the previous two for me,...
> 
> Look at 'pu' and see how it applies.

Ah, that seems to work better.
 
> > ...  The only remaining
> > question for me is should -Xtheirs resolve "deleted by them"
> > conflicts?
> 
> I do not know, and I do not care to worry about it too deeply, which
> means I would rather err on the safe side and have users inspect the
> situation, make the decision when such a conflict happens and
> resolve them themselves, instead of claiming a clean resolution that
> is possibly wrong.

A reasonable approach.  Thanks for clarifying.

Stephen
--
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] [git-am] Handle "git show" output correctly

2012-09-12 Thread Junio C Hamano
Matthieu Moy  writes:

> Peter Jones  writes:
>
>> Subject: [PATCH] [git-am] Handle "git show" output correctly
>
> The convention in Git is ": " (i.e. no
> brackets around git-am, just am: and no capital for Handle).
>
> My other concerns (name of stgit, multi-lines subject lines and lack of
> documentation) still hold.

We do not want to apply "git show" output that munges the log
message, period.

If you want to give patches to somebody (or to yourself) via e-mail
or via sneaker-net, "git format-patch" is there for you.  Do not
butcher "am" to accept a format that is not meant for patch
transport in the first place.

If you want to screw something in to your shelf, you would use a
screw and a screwdriver.  You do not try to hammer a nail using your
screwdriver, find that the screwdriver is not very useful as a
hammer and modify the screwdriver to hit your nail.

--
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] rev-list/log: document logic with several limiting options

2012-09-12 Thread Junio C Hamano
Junio C Hamano  writes:

> This is what I used to use when adding these generalized grep
> boolean expressions.
>
> With this applied,...

And this is the "this" X-<.

 grep.c | 90 +-
 1 file changed, 89 insertions(+), 1 deletion(-)

diff --git a/grep.c b/grep.c
index 04e3ec6..566c4cc 100644
--- a/grep.c
+++ b/grep.c
@@ -332,6 +332,87 @@ static struct grep_expr *compile_pattern_expr(struct 
grep_pat **list)
return compile_pattern_or(list);
 }
 
+static void indent(int in)
+{
+   while (in-- > 0)
+   fputc(' ', stderr);
+}
+
+static void dump_grep_pat(struct grep_pat *p)
+{
+   switch (p->token) {
+   case GREP_AND: fprintf(stderr, "*and*"); break;
+   case GREP_OPEN_PAREN: fprintf(stderr, "*(*"); break;
+   case GREP_CLOSE_PAREN: fprintf(stderr, "*)*"); break;
+   case GREP_NOT: fprintf(stderr, "*not*"); break;
+   case GREP_OR: fprintf(stderr, "*or*"); break;
+
+   case GREP_PATTERN: fprintf(stderr, "pattern"); break;
+   case GREP_PATTERN_HEAD: fprintf(stderr, "pattern_head"); break;
+   case GREP_PATTERN_BODY: fprintf(stderr, "pattern_body"); break;
+   }
+
+   switch (p->token) {
+   default: break;
+   case GREP_PATTERN_HEAD:
+   fprintf(stderr, "", p->field); break;
+   case GREP_PATTERN_BODY:
+   fprintf(stderr, ""); break;
+   }
+   switch (p->token) {
+   default: break;
+   case GREP_PATTERN_HEAD:
+   case GREP_PATTERN_BODY:
+   case GREP_PATTERN:
+   fprintf(stderr, "%.*s", (int)p->patternlen, p->pattern);
+   break;
+   }
+   fputc('\n', stderr);
+}
+
+static void dump_grep_expression_1(struct grep_expr *x, int in)
+{
+   indent(in);
+   switch (x->node) {
+   case GREP_NODE_TRUE:
+   fprintf(stderr, "true\n");
+   break;
+   case GREP_NODE_ATOM:
+   dump_grep_pat(x->u.atom);
+   break;
+   case GREP_NODE_NOT:
+   fprintf(stderr, "(not\n");
+   dump_grep_expression_1(x->u.unary, in+1);
+   indent(in);
+   fprintf(stderr, ")\n");
+   break;
+   case GREP_NODE_AND:
+   fprintf(stderr, "(and\n");
+   dump_grep_expression_1(x->u.binary.left, in+1);
+   dump_grep_expression_1(x->u.binary.right, in+1);
+   indent(in);
+   fprintf(stderr, ")\n");
+   break;
+   case GREP_NODE_OR:
+   fprintf(stderr, "(or\n");
+   dump_grep_expression_1(x->u.binary.left, in+1);
+   dump_grep_expression_1(x->u.binary.right, in+1);
+   indent(in);
+   fprintf(stderr, ")\n");
+   break;
+   }
+}
+
+void dump_grep_expression(struct grep_opt *opt)
+{
+   struct grep_expr *x = opt->pattern_expression;
+
+   if (opt->all_match)
+   fprintf(stderr, "[all-match]\n");
+   dump_grep_expression_1(x, 0);
+   fflush(NULL);
+}
+
 static struct grep_expr *grep_true_expr(void)
 {
struct grep_expr *z = xcalloc(1, sizeof(*z));
@@ -395,7 +476,7 @@ static struct grep_expr *prep_header_patterns(struct 
grep_opt *opt)
return header_expr;
 }
 
-void compile_grep_patterns(struct grep_opt *opt)
+static void compile_grep_patterns_real(struct grep_opt *opt)
 {
struct grep_pat *p;
struct grep_expr *header_expr = prep_header_patterns(opt);
@@ -432,9 +513,16 @@ void compile_grep_patterns(struct grep_opt *opt)
else
opt->pattern_expression = grep_or_expr(opt->pattern_expression,
   header_expr);
+
opt->all_match = 1;
 }
 
+void compile_grep_patterns(struct grep_opt *opt)
+{
+   compile_grep_patterns_real(opt);
+   dump_grep_expression(opt);
+}
+
 static void free_pattern_expr(struct grep_expr *x)
 {
switch (x->node) {
-- 
1.7.12.414.g1a62b7a

--
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] rev-list/log: document logic with several limiting options

2012-09-12 Thread Junio C Hamano
Michael J Gruber  writes:

> It was introduced in 0ab7befa with a clear meaning (AND everything),
> then the general logic (without --all-match) was modified in 80235ba7
> (to take headermatch AND (all greps ORed)), and 5aaeb733 finally made
> multiple authors resp. committers get ORed among each other. All of this
> in an attempt to make the standard usage most useful, of course. As a
> consequence, --all-match does not influence multiple --author options at
> all (contrary to the doc), e.g.
>
> I don't see any of this reflected in the doc, though. I noticed only by
> reading t/t7810-grep.sh. Before that, I had only gone by my own testing
> which didn't reveal the multiple author multiple special casing effect.
>
> I guess I'll have to wrap my head around the current implementation a
> few more times before trying to describe the status quo in the
> documentation...

This is what I used to use when adding these generalized grep
boolean expressions.

With this applied, you can try things like these:

$ git log --grep=regexp --grep=nosuch --all-match >/dev/null
$ git log --grep=regexp --grep=nosuch --author=Michael >/dev/null

For example, "--all-match --grep=regexp --author=Michael --author=Linus" turns
into

[all-match]
(or
 pattern_bodyregexp
 (or
  (or
   pattern_headLinus
   pattern_headMichael
  )
  true
 )
)

that says "body must have 'regexp' in it, and authored by either
Linus or Michael".

The semantics of "--all-match" is different from "--and" (which,
together with "--or", ")", and "(", is not availble to rev-list
command line parser primarily because "--not" is not available).
After applying all the "or"-ed terms, it checks the top-level nodes
that are "or"-ed and makes sure all of them fired (possibly and
usually on different lines).
--
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 2/2] attr: "binary" attribute should choose built-in "binary" merge driver

2012-09-12 Thread Junio C Hamano
Stephen Bash  writes:

>> Perhaps something like this makes it better.
>
> Patch didn't apply on top of the previous two for me,...

Look at 'pu' and see how it applies.

> ...  The only remaining
> question for me is should -Xtheirs resolve "deleted by them"
> conflicts?

I do not know, and I do not care to worry about it too deeply, which
means I would rather err on the safe side and have users inspect the
situation, make the decision when such a conflict happens and
resolve them themselves, instead of claiming a clean resolution that
is possibly wrong.

--
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: Interactive rebase with pre-built script?

2012-09-12 Thread Andrew Wong

On 09/11/2012 02:32 AM, Peter Krefting wrote:
Now, to my question. Is there an easy way to run interactive rebase on 
the upstream branch with this recipe? The best we have come up with so 
far is


  git checkout master # the upstream branch
  git rebase -i HEAD~

and then just append everything from the generated recipe.

Instead of rebasing to "HEAD~", you should be able to do:
git rebase -i HEAD
The default recipe should then just be "noop", and you can replace the 
whole default recipe with your recipe. This should also work even if the 
last commit was a merge.


Instead of appending your own recipe, you could also abuse the EDITOR 
environment variable.
Say your recipe is stored in a file called "my_recipe". Then, you could 
do this:

env EDITOR="cp my_recipe" git rebase -i HEAD

But this could potentially be dangerous because if "rebase" fires up a 
editor for any other reason (e.g. having a "reword" or "squash" in your 
recipe), then the commit message will be messed up. So you need to make 
sure your recipe won't trigger any editor except for the recipe.

--
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: Ambiguous date handling

2012-09-12 Thread Junio C Hamano
Chris Packham  writes:

> Consistent as long as you save as the default .txt. Some people have
> trained themselves to use the save as .eml option which uses RFC822
> style output.

Yuck.

> Could this be done in a applypatch-msg
> hook?

Isn't the hook about fixing up the log message?  Also I do not think
the name of the original file is given to the hook, so there is no
sufficient information to allow it to switch between two behaviours
based on .txt or .eml.

But if you are massaging the _input_ to "git am", then you can
certainly do the massaging even _before_ you feed it to "git am", no?

We could think about adding a new hook to "git am", though.  It
cannot just be an option to "git am" (or "git mailinfo") that says
"if the input is .txt, assume European date order for \d+/\d+/\d+
dates, and otherwise assume US style", as that is too specific to
your particular set-up and will not match general needs.  If we were
to add such a hook, $GIT_DIR/hooks/am-input-filter might look
something like this (it is left as an exercise to enhance it to
avoid munging a payload outside the header that happens to begin
with "Date: "):

#!/bin/sh
case "$#" in
0)
cat
;;
*)
for i
do
case "$i" in
*.txt)
sed -e 's/^\(Date: 
\)(\d+/)(\d+/)(\d+)/\1\3\2\4/' "$i"
;;
*)
cat "$i"
;;
esac
done
;;
esac

and then teach "am" to use the hook, perhaps like the attached.

But at that point, wouldn't it be far simpler and cleaner if you did

$ my-mbox-munge mail.txt | git am

in the first place?

 git-am.sh | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git i/git-am.sh w/git-am.sh
index c682d34..42654a0 100755
--- i/git-am.sh
+++ w/git-am.sh
@@ -265,7 +265,16 @@ split_patches () {
else
keep_cr=
fi
-   git mailsplit -d"$prec" -o"$dotest" -b $keep_cr -- "$@" > 
"$dotest/last" ||
+
+   if test -x "$GIT_DIR"/hooks/am-input-filter
+   then
+   mif="$GIT_DIR"/hooks/am-input-filter
+   else
+   mif=cat
+   fi
+
+   "$mif" "$@" |
+   git mailsplit -d"$prec" -o"$dotest" -b $keep_cr >"$dotest/last" 
||
clean_abort
;;
stgit-series)
--
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] [git-am] Handle "git show" output correctly

2012-09-12 Thread Matthieu Moy
Peter Jones  writes:

> Subject: [PATCH] [git-am] Handle "git show" output correctly

The convention in Git is ": " (i.e. no
brackets around git-am, just am: and no capital for Handle).

My other concerns (name of stgit, multi-lines subject lines and lack of
documentation) still hold.

-- 
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


[PATCH] [git-am] Handle "git show" output correctly

2012-09-12 Thread Peter Jones
This patch adds the ability for "git am" to accept patches in the format
generated by "git show".  Some people erroneously use "git show" instead
of "git format-patch", and it's nice as a maintainer to be able to
easily take their patch rather than going back and forth with them to
get a "correctly" formatted patch containing exactly the same actual
information.

Signed-off-by: Peter Jones 
---
 git-am.sh | 45 +
 1 file changed, 45 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index c682d34..cfd7b09 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+   "commit "*)
+   case "$l2" in
+   "Author: "*)
+   case "$l3" in
+   "Date: "*)
+   patch_format=gitshow
+   ;;
+   *)
+   ;;
+   esac
+   ;;
+   *)
+   ;;
+   esac
+   ;;
"From "* | "From: "*)
patch_format=mbox
;;
@@ -321,6 +336,36 @@ split_patches () {
this=
msgnum=
;;
+   gitshow)
+   this=0
+   for stgit in "$@"
+   do
+   this=`expr "$this" + 1`
+   msgnum=`printf "%0${prec}d" $this`
+   # The first nonemptyline after an empty line is the
+   # subject, and the body starts with the next nonempty
+   # line.
+   perl -ne 'BEGIN { $subject = 0 }
+   if ($subject > 1) { print ; }
+   elsif (/^\s+$/) { next ; }
+   elsif (/^Author:/) { s/Author/From/ ; print ;}
+   elsif (/^(From|Date)/) { print ; }
+   elsif (/^commit/) { next ; }
+   elsif ($subject) {
+   $subject = 2 ;
+   print "\n" ;
+   s/^// ;
+   print ;
+   } else {
+   print "Subject: ", $_ ;
+   $subject = 1;
+   }
+   ' < "$stgit" > "$dotest/$msgnum" || clean_abort
+   done
+   echo "$this" > "$dotest/last"
+   this=
+   msgnum=
+   ;;
hg)
this=0
for hg in "$@"
-- 
1.7.11.4

--
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] Handle "git show" output correctly.

2012-09-12 Thread Peter Jones
(this version with fixed tabs and the comment fixed to be actual English)

Signed-off-by: Peter Jones 
---
 git-am.sh | 45 +
 1 file changed, 45 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index c682d34..cfd7b09 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+   "commit "*)
+   case "$l2" in
+   "Author: "*)
+   case "$l3" in
+   "Date: "*)
+   patch_format=gitshow
+   ;;
+   *)
+   ;;
+   esac
+   ;;
+   *)
+   ;;
+   esac
+   ;;
"From "* | "From: "*)
patch_format=mbox
;;
@@ -321,6 +336,36 @@ split_patches () {
this=
msgnum=
;;
+   gitshow)
+   this=0
+   for stgit in "$@"
+   do
+   this=`expr "$this" + 1`
+   msgnum=`printf "%0${prec}d" $this`
+   # The first nonemptyline after an empty line is the
+   # subject, and the body starts with the next nonempty
+   # line.
+   perl -ne 'BEGIN { $subject = 0 }
+   if ($subject > 1) { print ; }
+   elsif (/^\s+$/) { next ; }
+   elsif (/^Author:/) { s/Author/From/ ; print ;}
+   elsif (/^(From|Date)/) { print ; }
+   elsif (/^commit/) { next ; }
+   elsif ($subject) {
+   $subject = 2 ;
+   print "\n" ;
+   s/^// ;
+   print ;
+   } else {
+   print "Subject: ", $_ ;
+   $subject = 1;
+   }
+   ' < "$stgit" > "$dotest/$msgnum" || clean_abort
+   done
+   echo "$this" > "$dotest/last"
+   this=
+   msgnum=
+   ;;
hg)
this=0
for hg in "$@"
-- 
1.7.11.4

--
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] Handle "git show" output correctly.

2012-09-12 Thread Matthieu Moy
Peter Jones  writes:

> (this version with fixed tabs)

This will end up being the commit message. If you add text here, then
the maintainer will have to manually fix it when applying (or reject
your patch). Please, be nice with him and put your comments below the
--- :

> Signed-off-by: Peter Jones 
> ---

(ie. here)

>  git-am.sh | 45 +

-- 
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: [PATCH] Handle "git show" output correctly.

2012-09-12 Thread Matthieu Moy
> Subject: Re: [PATCH] Handle "git show" output correctly.

No final period please.

This does not say which part of git is made to handle "git show". What
about

[PATCH] am: handle "git show" output correctly

Peter Jones  writes:

This lacks a proper commit message, i.e. an answer to the "why is this
change good?" question.

> Signed-off-by: Peter Jones 
> ---
>  git-am.sh | 46 ++

Documentation?

> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -216,6 +216,21 @@ check_patch_format () {
>   read l2
>   read l3
>   case "$l1" in
> +"commit "*)
> +case "$l2" in
> +"Author: "*)
> +case "$l3" in
> +"Date: "*)
> +patch_format=gitshow
> +;;
> +*)
> +;;
> +esac
> +;;
> +*)
> +;;
> +esac
> +;;

Your code is indented with space, Git indents with tabs. Please fix this
in your next version.

>   patch_format=mbox
>   ;;
> @@ -321,6 +336,37 @@ split_patches () {
>   this=
>   msgnum=
>   ;;
> +gitshow)
> + this=0
> + for stgit in "$@"

Probably a cut-and-paste from the stgit version, but your variable
naming doesn't make sense here.

> + do
> + this=`expr "$this" + 1`
> + msgnum=`printf "%0${prec}d" $this`
> + # Perl version of The first nonemptyline after an

Wrong cut-and-paste again, the sentense doesn't parse.

> +# empty line is the subject, and the body starts with
> +# the next nonempty line.
> + perl -ne 'BEGIN { $subject = 0 }
> + if ($subject > 1) { print ; }
> + elsif (/^\s+$/) { next ; }
> + elsif (/^Author:/) { s/Author/From/ ; print ;}
> + elsif (/^(From|Date)/) { print ; }
> +elsif (/^commit/) { next ; }
> + elsif ($subject) {
> + $subject = 2 ;
> + print "\n" ;
> +s/^// ;
> + print ;
> + } else {
> + print "Subject: ", $_ ;
> + $subject = 1;
> + }

How does this react to multi-line subject, e.g

This should be the
subject line.

And this is the body.

?

git format-patch will merge the lines in a single Subject: header, and
your version seems to take only the first line.

A test showing this would be welcome.

-- 
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


[PATCH] Handle "git show" output correctly.

2012-09-12 Thread Peter Jones
(this version with fixed tabs)

Signed-off-by: Peter Jones 
---
 git-am.sh | 45 +
 1 file changed, 45 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index c682d34..4a1a768 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+   "commit "*)
+   case "$l2" in
+   "Author: "*)
+   case "$l3" in
+   "Date: "*)
+   patch_format=gitshow
+   ;;
+   *)
+   ;;
+   esac
+   ;;
+   *)
+   ;;
+   esac
+   ;;
"From "* | "From: "*)
patch_format=mbox
;;
@@ -321,6 +336,36 @@ split_patches () {
this=
msgnum=
;;
+   gitshow)
+   this=0
+   for stgit in "$@"
+   do
+   this=`expr "$this" + 1`
+   msgnum=`printf "%0${prec}d" $this`
+   # Perl version of The first nonemptyline after an
+   # empty line is the subject, and the body starts with
+   # the next nonempty line.
+   perl -ne 'BEGIN { $subject = 0 }
+   if ($subject > 1) { print ; }
+   elsif (/^\s+$/) { next ; }
+   elsif (/^Author:/) { s/Author/From/ ; print ;}
+   elsif (/^(From|Date)/) { print ; }
+   elsif (/^commit/) { next ; }
+   elsif ($subject) {
+   $subject = 2 ;
+   print "\n" ;
+   s/^// ;
+   print ;
+   } else {
+   print "Subject: ", $_ ;
+   $subject = 1;
+   }
+   ' < "$stgit" > "$dotest/$msgnum" || clean_abort
+   done
+   echo "$this" > "$dotest/last"
+   this=
+   msgnum=
+   ;;
hg)
this=0
for hg in "$@"
-- 
1.7.11.4

--
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] Handle "git show" output correctly.

2012-09-12 Thread Peter Jones
Signed-off-by: Peter Jones 
---
 git-am.sh | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index c682d34..ebcbff7 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+"commit "*)
+case "$l2" in
+"Author: "*)
+case "$l3" in
+"Date: "*)
+patch_format=gitshow
+;;
+*)
+;;
+esac
+;;
+*)
+;;
+esac
+;;
"From "* | "From: "*)
patch_format=mbox
;;
@@ -321,6 +336,37 @@ split_patches () {
this=
msgnum=
;;
+gitshow)
+   this=0
+   for stgit in "$@"
+   do
+   this=`expr "$this" + 1`
+   msgnum=`printf "%0${prec}d" $this`
+   # Perl version of The first nonemptyline after an
+# empty line is the subject, and the body starts with
+# the next nonempty line.
+   perl -ne 'BEGIN { $subject = 0 }
+   if ($subject > 1) { print ; }
+   elsif (/^\s+$/) { next ; }
+   elsif (/^Author:/) { s/Author/From/ ; print ;}
+   elsif (/^(From|Date)/) { print ; }
+elsif (/^commit/) { next ; }
+   elsif ($subject) {
+   $subject = 2 ;
+   print "\n" ;
+s/^// ;
+   print ;
+   } else {
+   print "Subject: ", $_ ;
+   $subject = 1;
+   }
+   ' < "$stgit" > "$dotest/$msgnum" || clean_abort
+   done
+   echo "$this" > "$dotest/last"
+   this=
+   msgnum=
+   ;;
+
hg)
this=0
for hg in "$@"
-- 
1.7.11.4

--
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


GNU patch version 2.7 released

2012-09-12 Thread Andreas Grünbacher
I am pleased to announce that version 2.7 of GNU patch has been
released. The following significant changes have happened since the
last stable release in December 2009:

* Support for most features of the "diff --git" format, including renames and
  copies, permission changes, and symlink diffs.  Binary diffs are not
  supported yet; patch will complain and skip them.

* Support for double-quoted filenames: when a filename starts with a double
  quote, it is interpreted as a C string literal.  The escape sequences \\, \",
  \a, \b, \f, \n, \r, \t, \v, and \ooo (a three-digit octal number between 0 and
  255) are recognized.

* Ignore destination file names that are absolute or that contain a component
  of "..".  This addresses CVE-2010-4651.

* Refuse to apply a normal patch to a symlink.  (Previous versions of patch
  were replacing the symlink with a regular file.)

* When trying to modify a read-only file, warn about the potential problem
  by default.  The --read-only command line option allows to change this
  behavior.

* Files to be deleted are deleted once the entire input has been processed, not
  immediately.  This fixes a bug with numbered backup files.

* When a timestamp specifies a time zone, honor that instead of assuming the
  local time zone (--set-date) or Universal Coordinated Time (--set-utc).

* Support for nanosecond precision timestamps.

* Patch no longer gets a failed assertion for certain mangled patches.

* Many portability and bug fixes.

Release tarballs are available at:

  ftp://ftp.gnu.org/gnu/patch/

Please report bugs or suggestions on the  mailing list, or
in the project's bug tracker on Savannah:

  http://savannah.gnu.org/projects/patch

Thanks,
Andreas
--
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 0/2] Mixing English and a local language

2012-09-12 Thread Nguyen Thai Ngoc Duy
Should I interpret the silence as "I don't care, if you want it, go
for it" or "not acceptable, but no reasons given"? I'd like some form
of it in. Reverting the i18n diffstat patch is the last resort that I
really don't want to do.

On Sun, Aug 26, 2012 at 2:26 AM, Nguyễn Thái Ngọc Duy  wrote:
> The l10n effort leads to a situation where a contributor can submit a
> patch with some auto-generated information in his language, which may
> not be the team's language. We need to make sure exchange medium like
> patch is always in a common language that the team understands.
>
> Now this team language may not necessarily be English. However there
> are technical difficulties involved in switching between two
> languages. The only way I can think of, on top of gettext, is provide
> git translations in multiple domains. Say diff machinery uses
> "git-diff" domain while the rest is in "git". We can drive gettext to
> use language X for diff machinery, and Y for the rest. For that, we
> replace gettext() with dgettext().
>
> It's cumbersome. And there has not been any sign that there will be
> a real user for it. So I assume that the "team language" will always
> be English. It's simpler and should cover 90% of the user base. If
> someday people ask for that, supporting it is simply a matter of
> rewriting C_() and CQ_() macros in the first patch to use dgettext()
> instead.
>
> Switching between a language and English is easier. We just need an
> if/else to decide whether to call gettext(). Which is what the first
> patch does, just for certain parts of diff machinery. Error messages
> will alway be in native language.
>
> The second patch puts format-patch output in English unconditionally.
> Again I'm partly lazy and not so sure that there will be needs for
> format-patch to produce in native language. If someone needs it, we
> can introduce a new config key that flip no_l10n flag back to 0.
>
> More commands may follow format-patch. I think that 'apply' should also
> use English for non-tty output, unless users request it to be in local
> language. IOW local language is treated pretty much like coloring.
>
> Nguyễn Thái Ngọc Duy (2):
>   Allow to print diffstat in English regardless current locale
>   format-patch: always print diffstat in English
>
>  builtin/apply.c |  2 +-
>  builtin/log.c   |  1 +
>  diff.c  | 19 ---
>  diff.h  |  3 ++-
>  4 files changed, 16 insertions(+), 9 deletions(-)
>
> --
> 1.7.12.rc1.27.g6d3049b.dirty
>



-- 
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


[PATCH v3 6/6] api-string-list.txt: initialize the string_list the easy way

2012-09-12 Thread Michael Haggerty
In the demo code blurb, show how to initialize the string_list using
STRING_LIST_INIT_NODUP rather than memset().

Signed-off-by: Michael Haggerty 
---
 Documentation/technical/api-string-list.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/technical/api-string-list.txt 
b/Documentation/technical/api-string-list.txt
index 32b35d9..155ac8c 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -44,10 +44,9 @@ member (you need this if you add things later) and you 
should set the
 Example:
 
 
-struct string_list list;
+struct string_list list = STRING_LIST_INIT_NODUP;
 int i;
 
-memset(&list, 0, sizeof(struct string_list));
 string_list_append(&list, "foo");
 string_list_append(&list, "bar");
 for (i = 0; i < list.nr; i++)
-- 
1.7.11.3

--
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 4/6] string_list: add a new function, string_list_remove_duplicates()

2012-09-12 Thread Michael Haggerty
Add a function that deletes duplicate entries from a sorted
string_list.

Signed-off-by: Michael Haggerty 
---
 Documentation/technical/api-string-list.txt |  9 +
 string-list.c   | 17 +
 string-list.h   |  7 +++
 t/t0063-string-list.sh  | 17 +
 test-string-list.c  | 10 ++
 5 files changed, 60 insertions(+)

diff --git a/Documentation/technical/api-string-list.txt 
b/Documentation/technical/api-string-list.txt
index 300b301..0f8b7ce 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -30,6 +30,9 @@ member (you need this if you add things later) and you should 
set the
 
 . Can sort an unsorted list using `sort_string_list`.
 
+. Can remove duplicate items from a sorted list using
+  `string_list_remove_duplicates`.
+
 . Can remove individual items of an unsorted list using
   `unsorted_string_list_delete_item`.
 
@@ -108,6 +111,12 @@ write `string_list_insert(...)->util = ...;`.
Look up a given string in the string_list, returning the containing
string_list_item. If the string is not found, NULL is returned.
 
+`string_list_remove_duplicates`::
+
+   Remove all but the first of consecutive entries that have the
+   same string value.  If free_util is true, call free() on the
+   util members of any items that have to be deleted.
+
 * Functions for unsorted lists only
 
 `string_list_append`::
diff --git a/string-list.c b/string-list.c
index 179fde4..decfa74 100644
--- a/string-list.c
+++ b/string-list.c
@@ -92,6 +92,23 @@ struct string_list_item *string_list_lookup(struct 
string_list *list, const char
return list->items + i;
 }
 
+void string_list_remove_duplicates(struct string_list *list, int free_util)
+{
+   if (list->nr > 1) {
+   int src, dst;
+   for (src = dst = 1; src < list->nr; src++) {
+   if (!strcmp(list->items[dst - 1].string, 
list->items[src].string)) {
+   if (list->strdup_strings)
+   free(list->items[src].string);
+   if (free_util)
+   free(list->items[src].util);
+   } else
+   list->items[dst++] = list->items[src];
+   }
+   list->nr = dst;
+   }
+}
+
 int for_each_string_list(struct string_list *list,
 string_list_each_func_t fn, void *cb_data)
 {
diff --git a/string-list.h b/string-list.h
index 7d18e62..3a6a6dc 100644
--- a/string-list.h
+++ b/string-list.h
@@ -48,6 +48,13 @@ struct string_list_item *string_list_insert_at_index(struct 
string_list *list,
 int insert_at, const char 
*string);
 struct string_list_item *string_list_lookup(struct string_list *list, const 
char *string);
 
+/*
+ * Remove all but the first of consecutive entries with the same
+ * string value.  If free_util is true, call free() on the util
+ * members of any items that have to be deleted.
+ */
+void string_list_remove_duplicates(struct string_list *sorted_list, int 
free_util);
+
 
 /* Use these functions only on unsorted lists: */
 
diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh
index a5f05cd..dbfc05e 100755
--- a/t/t0063-string-list.sh
+++ b/t/t0063-string-list.sh
@@ -71,4 +71,21 @@ test_expect_success "test filter_string_list" '
test "x-" = "x$(test-string-list filter x1:x2 y)"
 '
 
+test_expect_success "test remove_duplicates" '
+   test "x-" = "x$(test-string-list remove_duplicates -)" &&
+   test "x" = "x$(test-string-list remove_duplicates "")" &&
+   test a = "$(test-string-list remove_duplicates a)" &&
+   test a = "$(test-string-list remove_duplicates a:a)" &&
+   test a = "$(test-string-list remove_duplicates a:a:a:a:a)" &&
+   test a:b = "$(test-string-list remove_duplicates a:b)" &&
+   test a:b = "$(test-string-list remove_duplicates a:a:b)" &&
+   test a:b = "$(test-string-list remove_duplicates a:b:b)" &&
+   test a:b:c = "$(test-string-list remove_duplicates a:b:c)" &&
+   test a:b:c = "$(test-string-list remove_duplicates a:a:b:c)" &&
+   test a:b:c = "$(test-string-list remove_duplicates a:b:b:c)" &&
+   test a:b:c = "$(test-string-list remove_duplicates a:b:c:c)" &&
+   test a:b:c = "$(test-string-list remove_duplicates a:a:b:b:c:c)" &&
+   test a:b:c = "$(test-string-list remove_duplicates a:a:a:b:b:b:c:c:c)"
+'
+
 test_done
diff --git a/test-string-list.c b/test-string-list.c
index 702276c..2d6eda7 100644
--- a/test-string-list.c
+++ b/test-string-list.c
@@ -87,6 +87,16 @@ int main(int argc, char **argv)
return 0;
}
 
+   if (argc == 3 && !strcmp(argv[1], "remove_duplicates")) {
+   struct string_list l

[PATCH v3 5/6] string_list: add a function string_list_longest_prefix()

2012-09-12 Thread Michael Haggerty
Add a function that finds the longest string from a string_list that
is a prefix of a given string.

Signed-off-by: Michael Haggerty 
---
 Documentation/technical/api-string-list.txt |  8 
 string-list.c   | 20 +++
 string-list.h   |  8 
 t/t0063-string-list.sh  | 30 +
 test-string-list.c  | 20 +++
 5 files changed, 86 insertions(+)

diff --git a/Documentation/technical/api-string-list.txt 
b/Documentation/technical/api-string-list.txt
index 0f8b7ce..32b35d9 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -75,6 +75,14 @@ Functions
to be deleted.  Preserve the order of the items that are
retained.
 
+`string_list_longest_prefix`::
+
+   Return the longest string within a string_list that is a
+   prefix (in the sense of prefixcmp()) of the specified string,
+   or NULL if no such prefix exists.  This function does not
+   require the string_list to be sorted (it does a linear
+   search).
+
 `print_string_list`::
 
Dump a string_list to stdout, useful mainly for debugging purposes. It
diff --git a/string-list.c b/string-list.c
index decfa74..c54b816 100644
--- a/string-list.c
+++ b/string-list.c
@@ -136,6 +136,26 @@ void filter_string_list(struct string_list *list, int 
free_util,
list->nr = dst;
 }
 
+char *string_list_longest_prefix(const struct string_list *prefixes,
+const char *string)
+{
+   int i, max_len = -1;
+   char *retval = NULL;
+
+   for (i = 0; i < prefixes->nr; i++) {
+   char *prefix = prefixes->items[i].string;
+   if (!prefixcmp(string, prefix)) {
+   int len = strlen(prefix);
+   if (len > max_len) {
+   retval = prefix;
+   max_len = len;
+   }
+   }
+   }
+
+   return retval;
+}
+
 void string_list_clear(struct string_list *list, int free_util)
 {
if (list->items) {
diff --git a/string-list.h b/string-list.h
index 3a6a6dc..5efd07b 100644
--- a/string-list.h
+++ b/string-list.h
@@ -38,6 +38,14 @@ int for_each_string_list(struct string_list *list,
 void filter_string_list(struct string_list *list, int free_util,
string_list_each_func_t want, void *cb_data);
 
+/*
+ * Return the longest string in prefixes that is a prefix (in the
+ * sense of prefixcmp()) of string, or NULL if no such prefix exists.
+ * This function does not require the string_list to be sorted (it
+ * does a linear search).
+ */
+char *string_list_longest_prefix(const struct string_list *prefixes, const 
char *string);
+
 
 /* Use these functions only on sorted lists: */
 int string_list_has_string(const struct string_list *list, const char *string);
diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh
index dbfc05e..41c8826 100755
--- a/t/t0063-string-list.sh
+++ b/t/t0063-string-list.sh
@@ -17,6 +17,14 @@ test_split () {
"
 }
 
+test_longest_prefix () {
+   test "$(test-string-list longest_prefix "$1" "$2")" = "$3"
+}
+
+test_no_longest_prefix () {
+   test_must_fail test-string-list longest_prefix "$1" "$2"
+}
+
 test_split "foo:bar:baz" ":" "-1" <|-  */
+   struct string_list prefixes = STRING_LIST_INIT_DUP;
+   int retval;
+   const char *prefix_string = argv[2];
+   const char *string = argv[3];
+   const char *match;
+
+   parse_string_list(&prefixes, prefix_string);
+   match = string_list_longest_prefix(&prefixes, string);
+   if (match) {
+   printf("%s\n", match);
+   retval = 0;
+   }
+   else
+   retval = 1;
+   string_list_clear(&prefixes, 0);
+   return retval;
+   }
+
fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
argv[1] ? argv[1] : "(there was none)");
return 1;
-- 
1.7.11.3

--
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 3/6] string_list: add a new function, filter_string_list()

2012-09-12 Thread Michael Haggerty
This function allows entries that don't match a specified criterion to
be discarded from a string_list while preserving the order of the
remaining entries.

Signed-off-by: Michael Haggerty 
---
 Documentation/technical/api-string-list.txt | 11 +++
 string-list.c   | 17 ++
 string-list.h   |  9 ++
 t/t0063-string-list.sh  | 11 +++
 test-string-list.c  | 48 +
 5 files changed, 96 insertions(+)

diff --git a/Documentation/technical/api-string-list.txt 
b/Documentation/technical/api-string-list.txt
index 1dcad47..300b301 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -33,6 +33,9 @@ member (you need this if you add things later) and you should 
set the
 . Can remove individual items of an unsorted list using
   `unsorted_string_list_delete_item`.
 
+. Can remove items not matching a criterion from a sorted or unsorted
+  list using `filter_string_list`.
+
 . Finally it should free the list using `string_list_clear`.
 
 Example:
@@ -61,6 +64,14 @@ Functions
 
 * General ones (works with sorted and unsorted lists as well)
 
+`filter_string_list`::
+
+   Apply a function to each item in a list, retaining only the
+   items for which the function returns true.  If free_util is
+   true, call free() on the util members of any items that have
+   to be deleted.  Preserve the order of the items that are
+   retained.
+
 `print_string_list`::
 
Dump a string_list to stdout, useful mainly for debugging purposes. It
diff --git a/string-list.c b/string-list.c
index acb1f5b..179fde4 100644
--- a/string-list.c
+++ b/string-list.c
@@ -102,6 +102,23 @@ int for_each_string_list(struct string_list *list,
return ret;
 }
 
+void filter_string_list(struct string_list *list, int free_util,
+   string_list_each_func_t want, void *cb_data)
+{
+   int src, dst = 0;
+   for (src = 0; src < list->nr; src++) {
+   if (want(&list->items[src], cb_data)) {
+   list->items[dst++] = list->items[src];
+   } else {
+   if (list->strdup_strings)
+   free(list->items[src].string);
+   if (free_util)
+   free(list->items[src].util);
+   }
+   }
+   list->nr = dst;
+}
+
 void string_list_clear(struct string_list *list, int free_util)
 {
if (list->items) {
diff --git a/string-list.h b/string-list.h
index dc5fbc8..7d18e62 100644
--- a/string-list.h
+++ b/string-list.h
@@ -29,6 +29,15 @@ int for_each_string_list(struct string_list *list,
 #define for_each_string_list_item(item,list) \
for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
 
+/*
+ * Apply want to each item in list, retaining only the ones for which
+ * the function returns true.  If free_util is true, call free() on
+ * the util members of any items that have to be deleted.  Preserve
+ * the order of the items that are retained.
+ */
+void filter_string_list(struct string_list *list, int free_util,
+   string_list_each_func_t want, void *cb_data);
+
 
 /* Use these functions only on sorted lists: */
 int string_list_has_string(const struct string_list *list, const char *string);
diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh
index fb85430..a5f05cd 100755
--- a/t/t0063-string-list.sh
+++ b/t/t0063-string-list.sh
@@ -60,4 +60,15 @@ test_split ":" ":" "-1" items[i].string);
 }
 
+void write_list_compact(const struct string_list *list)
+{
+   int i;
+   if (!list->nr)
+   printf("-\n");
+   else {
+   printf("%s", list->items[0].string);
+   for (i = 1; i < list->nr; i++)
+   printf(":%s", list->items[i].string);
+   printf("\n");
+   }
+}
+
+int prefix_cb(struct string_list_item *item, void *cb_data)
+{
+   const char *prefix = (const char *)cb_data;
+   return !prefixcmp(item->string, prefix);
+}
+
 int main(int argc, char **argv)
 {
if (argc == 5 && !strcmp(argv[1], "split")) {
@@ -39,6 +72,21 @@ int main(int argc, char **argv)
return 0;
}
 
+   if (argc == 4 && !strcmp(argv[1], "filter")) {
+   /*
+* Retain only the items that have the specified prefix.
+* Arguments: list|- prefix
+*/
+   struct string_li

[PATCH v3 1/6] string_list: add function string_list_append_nodup()

2012-09-12 Thread Michael Haggerty
Add a new function that appends a string to a string_list without
copying it.  This can be used to pass ownership of an already-copied
string to a string_list that has strdup_strings set.

Signed-off-by: Michael Haggerty 
---
 Documentation/technical/api-string-list.txt | 17 ++---
 string-list.c   | 20 +++-
 string-list.h   | 18 ++
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/Documentation/technical/api-string-list.txt 
b/Documentation/technical/api-string-list.txt
index 5a0c14f..113f841 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -20,8 +20,8 @@ If you need something advanced, you can manually malloc() the 
`items`
 member (you need this if you add things later) and you should set the
 `nr` and `alloc` members in that case, too.
 
-. Adds new items to the list, using `string_list_append` or
-  `string_list_insert`.
+. Adds new items to the list, using `string_list_append`,
+  `string_list_append_nodup`, or `string_list_insert`.
 
 . Can check if a string is in the list using `string_list_has_string` or
   `unsorted_string_list_has_string` and get it from the list using
@@ -100,7 +100,18 @@ write `string_list_insert(...)->util = ...;`.
 
 `string_list_append`::
 
-   Append a new string to the end of the string_list.
+   Append a new string to the end of the string_list.  If
+   `strdup_string` is set, then the string argument is copied;
+   otherwise the new `string_list_entry` refers to the input
+   string.
+
+`string_list_append_nodup`::
+
+   Append a new string to the end of the string_list.  The new
+   `string_list_entry` always refers to the input string, even if
+   `strdup_string` is set.  This function can be used to hand
+   ownership of a malloc()ed string to a `string_list` that has
+   `strdup_string` set.
 
 `sort_string_list`::
 
diff --git a/string-list.c b/string-list.c
index d9810ab..ad2aa5a 100644
--- a/string-list.c
+++ b/string-list.c
@@ -148,13 +148,23 @@ void print_string_list(const struct string_list *p, const 
char *text)
printf("%s:%p\n", p->items[i].string, p->items[i].util);
 }
 
-struct string_list_item *string_list_append(struct string_list *list, const 
char *string)
+struct string_list_item *string_list_append_nodup(struct string_list *list,
+ char *string)
 {
+   struct string_list_item *retval;
ALLOC_GROW(list->items, list->nr + 1, list->alloc);
-   list->items[list->nr].string =
-   list->strdup_strings ? xstrdup(string) : (char *)string;
-   list->items[list->nr].util = NULL;
-   return list->items + list->nr++;
+   retval = &list->items[list->nr++];
+   retval->string = string;
+   retval->util = NULL;
+   return retval;
+}
+
+struct string_list_item *string_list_append(struct string_list *list,
+   const char *string)
+{
+   return string_list_append_nodup(
+   list,
+   list->strdup_strings ? xstrdup(string) : (char 
*)string);
 }
 
 static int cmp_items(const void *a, const void *b)
diff --git a/string-list.h b/string-list.h
index 0684cb7..1b3915b 100644
--- a/string-list.h
+++ b/string-list.h
@@ -29,6 +29,7 @@ int for_each_string_list(struct string_list *list,
 #define for_each_string_list_item(item,list) \
for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
 
+
 /* Use these functions only on sorted lists: */
 int string_list_has_string(const struct string_list *list, const char *string);
 int string_list_find_insert_index(const struct string_list *list, const char 
*string,
@@ -38,11 +39,28 @@ struct string_list_item *string_list_insert_at_index(struct 
string_list *list,
 int insert_at, const char 
*string);
 struct string_list_item *string_list_lookup(struct string_list *list, const 
char *string);
 
+
 /* Use these functions only on unsorted lists: */
+
+/*
+ * Add string to the end of list.  If list->strdup_string is set, then
+ * string is copied; otherwise the new string_list_entry refers to the
+ * input string.
+ */
 struct string_list_item *string_list_append(struct string_list *list, const 
char *string);
+
+/*
+ * Like string_list_append(), except string is never copied.  When
+ * list->strdup_strings is set, this function can be used to hand
+ * ownership of a malloc()ed string to list without making an extra
+ * copy.
+ */
+struct string_list_item *string_list_append_nodup(struct string_list *list, 
char *string);
+
 void sort_string_list(struct string_list *list);
 int unsorted_string_list_has_string(struct string_list *list, const char 
*string);
 struct string_list_item *unsorted_string_list_lookup(struct string_list *list,

[PATCH v3 2/6] string_list: add two new functions for splitting strings

2012-09-12 Thread Michael Haggerty
Add two new functions, string_list_split() and
string_list_split_in_place().  These split a string into a string_list
on a separator character.  The first makes copies of the substrings
(leaving the input string untouched) and the second splits the
original string in place, overwriting the separator characters with
NULs and referring to the original string's memory.

These functions are similar to the strbuf_split_*() functions except
that they work with the more powerful string_list interface.

Signed-off-by: Michael Haggerty 
---
 .gitignore  |  1 +
 Documentation/technical/api-string-list.txt | 22 +-
 Makefile|  1 +
 string-list.c   | 53 
 string-list.h   | 29 +
 t/t0063-string-list.sh  | 63 +
 test-string-list.c  | 45 +
 7 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100755 t/t0063-string-list.sh
 create mode 100644 test-string-list.c

diff --git a/.gitignore b/.gitignore
index 68fe464..a188a82 100644
--- a/.gitignore
+++ b/.gitignore
@@ -194,6 +194,7 @@
 /test-run-command
 /test-sha1
 /test-sigchain
+/test-string-list
 /test-subprocess
 /test-svn-fe
 /common-cmds.h
diff --git a/Documentation/technical/api-string-list.txt 
b/Documentation/technical/api-string-list.txt
index 113f841..1dcad47 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -21,7 +21,8 @@ member (you need this if you add things later) and you should 
set the
 `nr` and `alloc` members in that case, too.
 
 . Adds new items to the list, using `string_list_append`,
-  `string_list_append_nodup`, or `string_list_insert`.
+  `string_list_append_nodup`, `string_list_insert`,
+  `string_list_split`, and/or `string_list_split_in_place`.
 
 . Can check if a string is in the list using `string_list_has_string` or
   `unsorted_string_list_has_string` and get it from the list using
@@ -135,6 +136,25 @@ counterpart for sorted lists, which performs a binary 
search.
is set. The third parameter controls if the `util` pointer of the
items should be freed or not.
 
+`string_list_split`::
+`string_list_split_in_place`::
+
+   Split a string into substrings on a delimiter character and
+   append the substrings to a `string_list`.  If `maxsplit` is
+   non-negative, then split at most `maxsplit` times.  Return the
+   number of substrings appended to the list.
++
+`string_list_split` requires a `string_list` that has `strdup_strings`
+set to true; it leaves the input string untouched and makes copies of
+the substrings in newly-allocated memory.
+`string_list_split_in_place` requires a `string_list` that has
+`strdup_strings` set to false; it splits the input string in place,
+overwriting the delimiter characters with NULs and creating new
+string_list_items that point into the original string (the original
+string must therefore not be modified or freed while the `string_list`
+is in use).
+
+
 Data structures
 ---
 
diff --git a/Makefile b/Makefile
index 26b697d..2e396b0 100644
--- a/Makefile
+++ b/Makefile
@@ -502,6 +502,7 @@ TEST_PROGRAMS_NEED_X += test-run-command
 TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
 TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sigchain
+TEST_PROGRAMS_NEED_X += test-string-list
 TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
 
diff --git a/string-list.c b/string-list.c
index ad2aa5a..acb1f5b 100644
--- a/string-list.c
+++ b/string-list.c
@@ -204,3 +204,56 @@ void unsorted_string_list_delete_item(struct string_list 
*list, int i, int free_
list->items[i] = list->items[list->nr-1];
list->nr--;
 }
+
+int string_list_split(struct string_list *list, const char *string,
+ int delim, int maxsplit)
+{
+   int count = 0;
+   const char *p = string, *end;
+
+   if (!list->strdup_strings)
+   die("internal error in string_list_split(): "
+   "list->strdup_strings must be set");
+   for (;;) {
+   count++;
+   if (maxsplit >= 0 && count > maxsplit) {
+   string_list_append(list, p);
+   return count;
+   }
+   end = strchr(p, delim);
+   if (end) {
+   string_list_append_nodup(list, xmemdupz(p, end - p));
+   p = end + 1;
+   } else {
+   string_list_append(list, p);
+   return count;
+   }
+   }
+}
+
+int string_list_split_in_place(struct string_list *list, char *string,
+  int delim, int maxsplit)
+{
+   int count = 0;
+   char *p = string, *end;
+
+   if (list->strdup_strings)
+   

[PATCH v3 0/6] Add some string_list-related functions

2012-09-12 Thread Michael Haggerty
Version 3 of a patch series that adds some functions to the
string_list API.  This patch series applies to current master.  Thanks
for Junio for lots of great feedback.

The patch series "Clean up how fetch_pack() handles the heads list"
v3, which requires some of the new string_list functionality, works
unmodified on top of this version of the patch series.

Changes since v2:

* Use "if (!...) die(...)" instead of assert() to check function
  preconditions.

* Remove unneeded cast.

* Fix formatting of api documentation.

Michael Haggerty (6):
  string_list: add function string_list_append_nodup()
  string_list: add two new functions for splitting strings
  string_list: add a new function, filter_string_list()
  string_list: add a new function, string_list_remove_duplicates()
  string_list: add a function string_list_longest_prefix()
  api-string-list.txt: initialize the string_list the easy way

 .gitignore  |   1 +
 Documentation/technical/api-string-list.txt |  68 +--
 Makefile|   1 +
 string-list.c   | 127 ++--
 string-list.h   |  71 
 t/t0063-string-list.sh  | 121 ++
 test-string-list.c  | 123 +++
 7 files changed, 502 insertions(+), 10 deletions(-)
 create mode 100755 t/t0063-string-list.sh
 create mode 100644 test-string-list.c

-- 
1.7.11.3

--
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] fetch: align new ref summary printout in UTF-8 locales

2012-09-12 Thread Nguyen Thai Ngoc Duy
Ping.. what happens to this patch? Do you want to support other
encodings as well via iconv()? I can't test that though.

On Tue, Sep 4, 2012 at 5:39 PM, Nguyễn Thái Ngọc Duy  wrote:
> fetch does printf("%-*s", width, "foo") where "foo" can be a utf-8
> string, but width is in bytes, not columns. For ASCII it's fine as one
> byte takes one column. For utf-8, this may result in misaligned ref
> summary table.
>
> Introduce gettext_width() function that returns the string length in
> columns (currently only supports utf-8 locales). Make the code use
> TRANSPORT_SUMMARY(x) where the length is compensated properly in
> non-English locales.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>   - rename gettext_length() to gettext_width()
>   - use "width" instead of letters
>   - leave other "%-*s" places unchanged if they always take ascii
> strings (i.e. no _() calls)
>   - note to self, may need to i18n-ize print_ref_status() in
> transport.c, looks like it's used by git-push only
>
>  builtin/fetch.c | 15 +++
>  gettext.c   | 15 +--
>  gettext.h   |  5 +
>  transport.h |  1 +
>  4 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index bb9a074..85e291f 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -255,9 +255,8 @@ static int update_local_ref(struct ref *ref,
> if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
> if (verbosity > 0)
> strbuf_addf(display, "= %-*s %-*s -> %s",
> -   TRANSPORT_SUMMARY_WIDTH,
> -   _("[up to date]"), REFCOL_WIDTH,
> -   remote, pretty_ref);
> +   TRANSPORT_SUMMARY(_("[up to date]")),
> +   REFCOL_WIDTH, remote, pretty_ref);
> return 0;
> }
>
> @@ -271,7 +270,7 @@ static int update_local_ref(struct ref *ref,
>  */
> strbuf_addf(display,
> _("! %-*s %-*s -> %s  (can't fetch in current 
> branch)"),
> -   TRANSPORT_SUMMARY_WIDTH, _("[rejected]"),
> +   TRANSPORT_SUMMARY(_("[rejected]")),
> REFCOL_WIDTH, remote, pretty_ref);
> return 1;
> }
> @@ -282,7 +281,7 @@ static int update_local_ref(struct ref *ref,
> r = s_update_ref("updating tag", ref, 0);
> strbuf_addf(display, "%c %-*s %-*s -> %s%s",
> r ? '!' : '-',
> -   TRANSPORT_SUMMARY_WIDTH, _("[tag update]"),
> +   TRANSPORT_SUMMARY(_("[tag update]")),
> REFCOL_WIDTH, remote, pretty_ref,
> r ? _("  (unable to update local ref)") : "");
> return r;
> @@ -317,7 +316,7 @@ static int update_local_ref(struct ref *ref,
> r = s_update_ref(msg, ref, 0);
> strbuf_addf(display, "%c %-*s %-*s -> %s%s",
> r ? '!' : '*',
> -   TRANSPORT_SUMMARY_WIDTH, what,
> +   TRANSPORT_SUMMARY(what),
> REFCOL_WIDTH, remote, pretty_ref,
> r ? _("  (unable to update local ref)") : "");
> return r;
> @@ -357,7 +356,7 @@ static int update_local_ref(struct ref *ref,
> return r;
> } else {
> strbuf_addf(display, "! %-*s %-*s -> %s  %s",
> -   TRANSPORT_SUMMARY_WIDTH, _("[rejected]"),
> +   TRANSPORT_SUMMARY(_("[rejected]")),
> REFCOL_WIDTH, remote, pretty_ref,
> _("(non-fast-forward)"));
> return 1;
> @@ -554,7 +553,7 @@ static int prune_refs(struct refspec *refs, int 
> ref_count, struct ref *ref_map)
> result |= delete_ref(ref->name, NULL, 0);
> if (verbosity >= 0) {
> fprintf(stderr, " x %-*s %-*s -> %s\n",
> -   TRANSPORT_SUMMARY_WIDTH, _("[deleted]"),
> +   TRANSPORT_SUMMARY(_("[deleted]")),
> REFCOL_WIDTH, _("(none)"), 
> prettify_refname(ref->name));
> warn_dangling_symref(stderr, dangling_msg, ref->name);
> }
> diff --git a/gettext.c b/gettext.c
> index f75bca7..71e9545 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -4,6 +4,8 @@
>
>  #include "git-compat-util.h"
>  #include "gettext.h"
> +#include "strbuf.h"
> +#include "utf8.h"
>
>  #ifndef NO_GETTEXT
>  #  include 
> @@ -27,10 +29,9 @@ int use_gettext_poison(void)
>  #endif
>
>  #ifndef NO_GETTEXT
> +static const char *charset;
>  static void init_gettext_charset(const char *domain)
>  {
> -   const char *c

Re: [PATCH] rev-list/log: document logic with several limiting options

2012-09-12 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 11.09.2012 18:22:
> Michael J Gruber  writes:
> 
>> The current behavior is probably as useful as it is confusing. In any
>> case it is going to stay. So, document it.
>>
>> Signed-off-by: Michael J Gruber 
>> ---
>> I would have written a test but don't really know where to stick it in.
>> rev-list has many small tests where it doesn't fit.
> 
> "git log" would be the more natural place, I would say.

Well, to me, "git log" is the ui to "git rev-list", so everything which
tests the revision walker should be tested using "git rev-list". But I
don't care as long as it fits in - and can be found later.

That being said, I now see lots of "log --grep" and "log --author" tests
in the grep-test. So I guess there it fits best (despite the misnomer).

>>
>>  Documentation/rev-list-options.txt | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/rev-list-options.txt 
>> b/Documentation/rev-list-options.txt
>> index 5436eba..9c13df3 100644
>> --- a/Documentation/rev-list-options.txt
>> +++ b/Documentation/rev-list-options.txt
>> @@ -6,6 +6,12 @@ special notations explained in the description, additional 
>> commit
>>  limiting may be applied. Note that they are applied before commit
>>  ordering and formatting options, such as '--reverse'.
>>  
>> +All occurrences of the same option are ORed: '--grep=foo --grep=bar'
>> +limits to commits which match at least one of these conditions.
> 
> How would this interact with "--all-match"?

Badly.

It was introduced in 0ab7befa with a clear meaning (AND everything),
then the general logic (without --all-match) was modified in 80235ba7
(to take headermatch AND (all greps ORed)), and 5aaeb733 finally made
multiple authors resp. committers get ORed among each other. All of this
in an attempt to make the standard usage most useful, of course. As a
consequence, --all-match does not influence multiple --author options at
all (contrary to the doc), e.g.

I don't see any of this reflected in the doc, though. I noticed only by
reading t/t7810-grep.sh. Before that, I had only gone by my own testing
which didn't reveal the multiple author multiple special casing effect.

I guess I'll have to wrap my head around the current implementation a
few more times before trying to describe the status quo in the
documentation...
--
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 1/6] string_list: add function string_list_append_nodup()

2012-09-12 Thread Michael Haggerty
On 09/10/2012 11:56 PM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> diff --git a/string-list.c b/string-list.c
>> index d9810ab..5594b7d 100644
>> --- a/string-list.c
>> +++ b/string-list.c
>> @@ -148,13 +148,23 @@ void print_string_list(const struct string_list *p, 
>> const char *text)
>>  printf("%s:%p\n", p->items[i].string, p->items[i].util);
>>  }
>>  
>> -struct string_list_item *string_list_append(struct string_list *list, const 
>> char *string)
>> +struct string_list_item *string_list_append_nodup(struct string_list *list,
>> +  char *string)
>>  {
>> +struct string_list_item *retval;
>>  ALLOC_GROW(list->items, list->nr + 1, list->alloc);
>> -list->items[list->nr].string =
>> -list->strdup_strings ? xstrdup(string) : (char *)string;
>> -list->items[list->nr].util = NULL;
>> -return list->items + list->nr++;
>> +retval = &list->items[list->nr++];
>> +retval->string = (char *)string;
> 
> Do you still need this cast, now the function takes non-const "char *string"?

Thanks.  Fixed in forthcoming re-roll.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 2/2] attr: "binary" attribute should choose built-in "binary" merge driver

2012-09-12 Thread Jeff King
On Wed, Sep 12, 2012 at 01:55:53AM -0700, Junio C Hamano wrote:

> > Yeah, that seems like the obviously correct thing to do. In practice,
> > most files would end up in the first few lines of ll_xdl_merge checking
> > buffer_is_binary anyway, so I think this would really only make a
> > difference when our "is it binary?" heuristic guesses wrong.
> 
> You made me look at that part again and then made me notice
> something unrelated.
> 
>   if (buffer_is_binary(orig->ptr, orig->size) ||
>   buffer_is_binary(src1->ptr, src1->size) ||
>   buffer_is_binary(src2->ptr, src2->size)) {
>   warning("Cannot merge binary files: %s (%s vs. %s)",
>   path, name1, name2);
>   return ll_binary_merge(drv_unused, result,
>  path,
>  orig, orig_name,
>  src1, name1,
>  src2, name2,
>  opts, marker_size);
>   }
> 
> Given that we now may know how to merge these things, the
> unconditional warning feels very wrong.
> 
> Perhaps something like this makes it better.
> 
> A path that is explicitly marked as binary did not get any such
> warning, but it will start to get warned just like a path that was
> auto-detected to be a binary.
> 
> It is a behaviour change, but I think it is a good one that makes
> two cases more consistent.
> 
> And we won't see the warning when -Xtheirs/-Xours large sledgehammer
> is in use, which tells us how to resolve these things "cleanly".

Yeah, I think it is the right thing to do. I noticed that the warning
would not trigger in the "-merge" case and wondered if it should, but
figured it was not a big deal either way.

However, I agree it is very bad for it to trigger with -Xours/theirs,
and that is worth fixing.  That it triggers in the "-merge" case
afterwards is a slight bonus.

-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 v2 2/6] string_list: add two new functions for splitting strings

2012-09-12 Thread Michael Haggerty
On 09/11/2012 12:33 AM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> +`string_list_split`, `string_list_split_in_place`::
>> +
>> +Split a string into substrings on a delimiter character and
>> +append the substrings to a `string_list`.  If `maxsplit` is
>> +non-negative, then split at most `maxsplit` times.  Return the
>> +number of substrings appended to the list.
> 
> 
> I recall that we favor
> 
> `A`::
> `B`::
> 
>   Description for A and B
> 
> for some reason but do not remember exactly why.

Will change.  Thanks.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 2/2] attr: "binary" attribute should choose built-in "binary" merge driver

2012-09-12 Thread Stephen Bash
- Original Message -
> From: "Junio C Hamano" 
> Sent: Wednesday, September 12, 2012 4:55:53 AM
> Subject: Re: [PATCH 2/2] attr: "binary" attribute should choose built-in 
> "binary" merge driver
> 
> Jeff King  writes:
> 
> > On Sat, Sep 08, 2012 at 09:40:39PM -0700, Junio C Hamano wrote:
> >
> >> The built-in "binary" attribute macro expands to "-diff -text", so
> >> that textual diff is not produced, and the contents will not go
> >> through any CR/LF conversion ever.  During a merge, it should also
> >> choose the "binary" low-level merge driver, but it didn't.
> >> 
> >> Make it expand to "-diff -merge -text".
> >
> > Yeah, that seems like the obviously correct thing to do. In
> > practice,
> > most files would end up in the first few lines of ll_xdl_merge
> > checking
> > buffer_is_binary anyway, so I think this would really only make a
> > difference when our "is it binary?" heuristic guesses wrong.
> 
> You made me look at that part again and then made me notice
> something unrelated.
> 
>   if (buffer_is_binary(orig->ptr, orig->size) ||
>   buffer_is_binary(src1->ptr, src1->size) ||
>   buffer_is_binary(src2->ptr, src2->size)) {
>   warning("Cannot merge binary files: %s (%s vs. %s)",
>   path, name1, name2);
>   return ll_binary_merge(drv_unused, result,
>  path,
>  orig, orig_name,
>  src1, name1,
>  src2, name2,
>  opts, marker_size);
>   }
> 
> Given that we now may know how to merge these things, the
> unconditional warning feels very wrong.
> 
> Perhaps something like this makes it better.

Patch didn't apply on top of the previous two for me, but after making the 
edits manually does what it claims to do (and makes the merge output much nicer 
to read, thanks!).  The only remaining question for me is should -Xtheirs 
resolve "deleted by them" conflicts?

Thanks,
Stephen
--
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] gitk: Rename 'tagcontents' to 'cached_tagcontent'

2012-09-12 Thread Paul Mackerras
On Wed, Sep 12, 2012 at 02:25:28AM -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > I've applied these two, on top of Paul's master branch at
> >
> > git://ozlabs.org/~paulus/gitk.git
> >
> > and tentatively queued in 'pu', but I would prefer to see it
> > eyeballed by and queued in his tree first.
> >
> > Thanks.
> 
> Pinging Paul...
> 
> The following changes since commit a135f214e371311f13807da637d492fd9642a2e3:
> 
>   gitk: Avoid Meta1-F5 (2012-04-25 13:44:31 +1000)
> 
> are available in the git repository at:
> 
>   git://github.com/gitster/git da/gitk-reload-tag-contents
> 
> for you to fetch changes up to 587277fea3bf3bfc4302480178bd88a277a69f05:
> 
>   gitk: Rename 'tagcontents' to 'cached_tagcontent' (2012-09-08 20:25:09 
> -0700)

Thanks, pulled and pushed out.  I had one other commit queued up, so
if you pull from git://ozlabs.org/~paulus/gitk.git you will get that
plus a merge.

Paul.
--
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] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

2012-09-12 Thread Elia Pinto
Recent versions of Linux libc (later than 5.4.23) and glibc (2.x)
include a malloc() implementation which is tunable via environment
variables. When MALLOC_CHECK_ is set, a special (less efficient)
implementation is used which is designed to be tolerant against
simple errors, such as double calls of free() with the same argument,
or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_
is set to 3, a diagnostic message is printed on stderr
and the program is aborted.

Setting the MALLOC_PERTURB_ environment variable causes the malloc
functions in libc to return memory which has been wiped and clear
memory when it is returned.
Of course this does not affect calloc which always does clear the memory.

The reason for this exercise is, of course, to find code which uses
memory returned by malloc without initializing it and code which uses
code after it is freed. valgrind can do this but it's costly to run.
The MALLOC_PERTURB_ exchanges the ability to detect problems in 100%
of the cases with speed.

The byte value used to initialize values returned by malloc is the byte
value of the environment value. The value used to clear memory is the
bitwise inverse. Setting MALLOC_PERTURB_ to zero disables the feature.

This technique can find hard to detect bugs.
It is therefore suggested to always use this flag (at least temporarily)
when testing out code or a new distribution.

Signed-off-by: Elia Pinto 
---
 t/test-lib.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 78c4286..98c90b0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -93,6 +93,12 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR
 
+# Add libc malloc_check and MALLOC_PERTURB test 
+export MALLOC_CHECK_=3
+export MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
+#
+
+
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
 unset CDPATH
-- 
1.7.11.rc1

--
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/3] sha1: clean pointer arithmetic

2012-09-12 Thread Yann Droneaud
One memcpy() argument is computed from a pointer added to an integer:
eg. int + pointer. It's unusal.
Let's write it in the correct order: pointer + offset.

Signed-off-by: Yann Droneaud 
---
 block-sha1/sha1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index c1af112..a7c4470 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -246,7 +246,7 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, 
unsigned long len)
unsigned int left = 64 - lenW;
if (len < left)
left = len;
-   memcpy(lenW + (char *)ctx->W, data, left);
+   memcpy((char *)ctx->W + lenW, data, left);
lenW = (lenW + left) & 63;
if (lenW)
return;
-- 
1.7.11.4

--
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/3] sha1: use char type for temporary work buffer

2012-09-12 Thread Yann Droneaud
The SHA context is holding a temporary buffer for partial block.

This block must 64 bytes long. It is currently described as
an array of 16 integers.

Signed-off-by: Yann Droneaud 
---
 block-sha1/sha1.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index b864df6..d29ff6a 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -9,7 +9,7 @@
 typedef struct {
unsigned long long size;
unsigned int H[5];
-   unsigned int W[16];
+   unsigned char W[64];
 } blk_SHA_CTX;
 
 void blk_SHA1_Init(blk_SHA_CTX *ctx);
-- 
1.7.11.4

--
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] completion: git branch --set-upstream-to=

2012-09-12 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 11.09.2012 19:13:
> Thanks; I picked up $gmane/204633 but forgot to queue.

I missed that one, thanks for reducing appropriately.

Michael
--
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: Ambiguous date handling

2012-09-12 Thread Chris Packham
On 09/12/2012 09:48 PM, Junio C Hamano wrote:
> Chris Packham  writes:
> 
>> Our default MUA has an annoying habit of using a non RFC822 date format when
>> saving an email as plaintext. This means the first 12 days of every month we
>> run into the ambiguous date problem (our date convention is dd/mm/yy).
>>
>> I see code in date.c for refusing a date in the future which would have 
>> caught
>> this...
> 
> The most sane thing to do when you know that your MUA *consistently*
> does dd/mm/yy (even though it may annoy you) is to massage its
> output before feeding it to Git.  And it should be a very simple
> matter of a one-liner filter, no?

Consistent as long as you save as the default .txt. Some people have
trained themselves to use the save as .eml option which uses RFC822
style output. sed 's|Date: (\d+)/(\d+)/(\d+)|\1.\2.\3|' should correct
the former and ignore the latter. Could this be done in a applypatch-msg
hook?

> 
> Regardless of the correctness of that "we reject timestamps way into
> the future" logic, it should be taken as the last resort.  If you
> are on September 1st, both 9/12 and 12/9 will look like into the
> future for more than ten days (which is the cut-off, I think).  If
> you are on December 28th, both look like sufficiently in the past.
> 

Duly noted. And I'm implying that the reject timestamps in future isn't
actually working. I've just started looking at t0006-date.sh so see if I
can prove 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


[PATCH 1/3] sha1: update pointer and remaining length after subfunction call

2012-09-12 Thread Yann Droneaud
There's no need to update the pointer and remaining length before
leaving or calling the SHA1 sub function.

Additionnaly, the partial block code could be looking more like
the full block handling branch.

Signed-off-by: Yann Droneaud 
---
 block-sha1/sha1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index a8d4bf9..c1af112 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -248,11 +248,11 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, 
unsigned long len)
left = len;
memcpy(lenW + (char *)ctx->W, data, left);
lenW = (lenW + left) & 63;
-   len -= left;
-   data = ((const char *)data + left);
if (lenW)
return;
blk_SHA1_Block(ctx, ctx->W);
+   data = ((const char *)data + left);
+   len -= left;
}
while (len >= 64) {
blk_SHA1_Block(ctx, data);
-- 
1.7.11.4

--
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/3] Janitor minor fixes on SHA1

2012-09-12 Thread Yann Droneaud
While looking to use the git SHA1 code, I've found some small oddities.
Please find little cosmetics fixes for them.

The patches are against 'next' and can be merged in one single patch
if needed.

Yann Droneaud (3):
  sha1: update pointer and remaining length after subfunction call
  sha1: clean pointer arithmetic
  sha1: use char type for temporary work buffer

 block-sha1/sha1.c | 6 +++---
 block-sha1/sha1.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
1.7.11.4

--
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: Ambiguous date handling

2012-09-12 Thread Junio C Hamano
Chris Packham  writes:

> Our default MUA has an annoying habit of using a non RFC822 date format when
> saving an email as plaintext. This means the first 12 days of every month we
> run into the ambiguous date problem (our date convention is dd/mm/yy).
>
> I see code in date.c for refusing a date in the future which would have caught
> this...

The most sane thing to do when you know that your MUA *consistently*
does dd/mm/yy (even though it may annoy you) is to massage its
output before feeding it to Git.  And it should be a very simple
matter of a one-liner filter, no?

Regardless of the correctness of that "we reject timestamps way into
the future" logic, it should be taken as the last resort.  If you
are on September 1st, both 9/12 and 12/9 will look like into the
future for more than ten days (which is the cut-off, I think).  If
you are on December 28th, both look like sufficiently in the past.
--
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] Add test for ambiguous patch dates

2012-09-12 Thread Chris Packham
--
This testcase is only good for the next couple of months. For a longer term
test the current time would need to be set in the test setup.

---
 t/t4255-am-author-date.sh |   85 +
 1 file changed, 85 insertions(+)
 create mode 100755 t/t4255-am-author-date.sh

diff --git a/t/t4255-am-author-date.sh b/t/t4255-am-author-date.sh
new file mode 100755
index 000..62bceee
--- /dev/null
+++ b/t/t4255-am-author-date.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+
+test_description='git am with ambiguous date'
+. ./test-lib.sh
+
+cat >patch.diff <
+To: C O Mmitter 
+Date:   12/9/2012 12:00 AM
+Subject:   [PATCH] add file.txt
+---
+ file.txt |7 +++
+ 1 file changed, 7 insertions(+)
+ create mode 100644 file.txt
+
+diff --git a/file.txt b/file.txt
+new file mode 100644
+index 000..fe745d6
+--- /dev/null
 b/file.txt
+@@ -0,0 +1,7 @@
++Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam pulvinar
++tempus ligula vitae ornare. Vestibulum ante ipsum primis in faucibus orci
++luctus et ultrices posuere cubilia Curae; Aenean dapibus mauris non quam
++commodo a porta sapien suscipit. Mauris venenatis, dui nec malesuada mattis,
++ante mauris ornare ipsum, ac tincidunt ipsum lectus aliquet tortor. Nulla 
ipsum
++felis, egestas at condimentum quis, accumsan nec arcu. Phasellus fringilla
++viverra tempus. Integer vel rhoncus odio.
+EOF
+
+test_expect_success 'apply patch with ambiguous date' '
+   git am patch.diff
+'
+
+cat >expected patch.diff <
+To: C O Mmitter 
+Date:   12.9.2012 12:00 AM
+Subject:   [PATCH] update file.txt
+---
+ file.txt |9 +
+ 1 file changed, 9 insertions(+)
+
+diff --git a/file.txt b/file.txt
+index fe745d6..cd45361 100644
+--- a/file.txt
 b/file.txt
+@@ -5,3 +5,12 @@ commodo a porta sapien suscipit. Mauris venenatis, dui nec 
malesuada mattis,
+ ante mauris ornare ipsum, ac tincidunt ipsum lectus aliquet tortor. Nulla 
ipsum
+ felis, egestas at condimentum quis, accumsan nec arcu. Phasellus fringilla
+ viverra tempus. Integer vel rhoncus odio.
++
++Donec et ante eu mi aliquam sodales non ut massa. Nullam a luctus dui. Etiam 
ac
++eros elit. Pellentesque habitant morbi tristique senectus et netus et 
malesuada
++fames ac turpis egestas. Curabitur commodo ligula id leo iaculis vel lobortis
++leo pulvinar. Aenean adipiscing cursus arcu quis consectetur. Morbi eget 
lectus
++nec neque interdum lacinia. Nam quis metus eget ligula faucibus imperdiet in 
et
++ligula. Aenean eu urna sit amet metus sagittis interdum non cursus orci.
++Maecenas imperdiet feugiat tellus, non ultrices nulla dictum sed. Nulla vel
++lorem ac massa euismod faucibus et ut leo.
+EOF
+
+test_expect_success 'apply patch with european date separator' '
+   git am patch.diff
+'
+
+cat >expected 

Ambiguous date handling

2012-09-12 Thread Chris Packham
Hi,

I think this has come up before [1],[2] but we ran into this at $dayjob today.
Our default MUA has an annoying habit of using a non RFC822 date format when
saving an email as plaintext. This means the first 12 days of every month we
run into the ambiguous date problem (our date convention is dd/mm/yy).

I see code in date.c for refusing a date in the future which would have caught
this but it doesn't appear to be working for us.

Following this is a patch adding a testcase for this. With the following
results:

  ok 1 - apply patch with ambiguous date
  not ok 2 - check ambiguous date # TODO known breakage
  ok 3 - apply patch with european date separator
  ok 4 - check european date
  # still have 1 known breakage(s)
  # passed all remaining 3 test(s)
  1..4

Thanks,
Chris

--
[1] - http://thread.gmane.org/gmane.comp.version-control.git/18412/focus=18417
[2] - http://thread.gmane.org/gmane.comp.version-control.git/84512/focus=85735

--
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] gitk: Rename 'tagcontents' to 'cached_tagcontent'

2012-09-12 Thread Junio C Hamano
Junio C Hamano  writes:

> I've applied these two, on top of Paul's master branch at
>
> git://ozlabs.org/~paulus/gitk.git
>
> and tentatively queued in 'pu', but I would prefer to see it
> eyeballed by and queued in his tree first.
>
> Thanks.

Pinging Paul...

The following changes since commit a135f214e371311f13807da637d492fd9642a2e3:

  gitk: Avoid Meta1-F5 (2012-04-25 13:44:31 +1000)

are available in the git repository at:

  git://github.com/gitster/git da/gitk-reload-tag-contents

for you to fetch changes up to 587277fea3bf3bfc4302480178bd88a277a69f05:

  gitk: Rename 'tagcontents' to 'cached_tagcontent' (2012-09-08 20:25:09 -0700)


David Aguilar (2):
  gitk: Teach "Reread references" to reload tags
  gitk: Rename 'tagcontents' to 'cached_tagcontent'

 gitk | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

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: [PATCH] Document the --done option.

2012-09-12 Thread Junio C Hamano
Junio C Hamano  writes:

> Junio C Hamano  writes:
>
>> "Eric S. Raymond"  writes:
>>
>>> ---
>>
>> A forgotten Sign-off?
>
> Ping?  Just telling us that this is Signed-off is fine.

Ping again.
--
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 2/2] attr: "binary" attribute should choose built-in "binary" merge driver

2012-09-12 Thread Junio C Hamano
Jeff King  writes:

> On Sat, Sep 08, 2012 at 09:40:39PM -0700, Junio C Hamano wrote:
>
>> The built-in "binary" attribute macro expands to "-diff -text", so
>> that textual diff is not produced, and the contents will not go
>> through any CR/LF conversion ever.  During a merge, it should also
>> choose the "binary" low-level merge driver, but it didn't.
>> 
>> Make it expand to "-diff -merge -text".
>
> Yeah, that seems like the obviously correct thing to do. In practice,
> most files would end up in the first few lines of ll_xdl_merge checking
> buffer_is_binary anyway, so I think this would really only make a
> difference when our "is it binary?" heuristic guesses wrong.

You made me look at that part again and then made me notice
something unrelated.

if (buffer_is_binary(orig->ptr, orig->size) ||
buffer_is_binary(src1->ptr, src1->size) ||
buffer_is_binary(src2->ptr, src2->size)) {
warning("Cannot merge binary files: %s (%s vs. %s)",
path, name1, name2);
return ll_binary_merge(drv_unused, result,
   path,
   orig, orig_name,
   src1, name1,
   src2, name2,
   opts, marker_size);
}

Given that we now may know how to merge these things, the
unconditional warning feels very wrong.

Perhaps something like this makes it better.

A path that is explicitly marked as binary did not get any such
warning, but it will start to get warned just like a path that was
auto-detected to be a binary.

It is a behaviour change, but I think it is a good one that makes
two cases more consistent.

And we won't see the warning when -Xtheirs/-Xours large sledgehammer
is in use, which tells us how to resolve these things "cleanly".

 ll-merge.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git i/ll-merge.c w/ll-merge.c
index 8535e2d..307315b 100644
--- i/ll-merge.c
+++ w/ll-merge.c
@@ -35,7 +35,7 @@ struct ll_merge_driver {
  */
 static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
   mmbuffer_t *result,
-  const char *path_unused,
+  const char *path,
   mmfile_t *orig, const char *orig_name,
   mmfile_t *src1, const char *name1,
   mmfile_t *src2, const char *name2,
@@ -53,6 +53,9 @@ static int ll_binary_merge(const struct ll_merge_driver 
*drv_unused,
} else {
switch (opts->variant) {
default:
+   warning("Cannot merge binary files: %s (%s vs. %s)\n",
+   path, name1, name2);
+   /* fallthru */
case XDL_MERGE_FAVOR_OURS:
stolen = src1;
break;
@@ -88,8 +91,6 @@ static int ll_xdl_merge(const struct ll_merge_driver 
*drv_unused,
if (buffer_is_binary(orig->ptr, orig->size) ||
buffer_is_binary(src1->ptr, src1->size) ||
buffer_is_binary(src2->ptr, src2->size)) {
-   warning("Cannot merge binary files: %s (%s vs. %s)\n",
-   path, name1, name2);
return ll_binary_merge(drv_unused, result,
   path,
   orig, orig_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


Re: How to update a cloned git repository

2012-09-12 Thread Matthieu Moy
"Joachim Schmitz"  writes:

>> I think it is the-merge-commit^2; contrib/git-resurrect.sh might be
>> of interest, too.
>
> Sorry you lost me, this is greek to me...

A commit is an object that contain pointers to its parents. The root
commit has no parent. For ordinary commits, there is one parent which is
the commit on top of which it was created. For merge commits, there are
N commits, the first is the one on top of which the merge was created,
and the N-1 next ones are the commits being merged.

commit^ => first parent of commit
commit^1 => same
commit^2 => second parent, i.e. the one merged in commit.

See in git.git:

$ git show cb10ae9433126ef < one commit to study
commit cb10ae9433126efbc4dcc46779d7ef2fe6b1f597
Merge: 13b608a 9aeaab6   <--- list of parents
Author: Junio C Hamano 
Date:   Tue Sep 11 15:57:04 2012 -0700

Merge branch 'jc/maint-blame-no-such-path' into pu

* jc/maint-blame-no-such-path:
  blame: allow "blame file" in the middle of a conflicted merge

$ git show cb10ae9433126ef^1 < "previous commit" in 
origin/pu
commit 13b608a063ce861929322e6bb3862b5364f3fbcf
Merge: fa17a26 bdee397
Author: Junio C Hamano 
Date:   Tue Sep 11 11:50:44 2012 -0700

Merge branch 'dg/run-command-child-cleanup' into pu

* dg/run-command-child-cleanup:
  run-command.c: fix broken list iteration in clear_child_for_cleanup

$ git show cb10ae9433126ef^2 <- commit being merged by 
cb10ae9433126ef
commit 9aeaab6811dce596b4f6141d76f5300359bfd009
Author: Junio C Hamano 
Date:   Tue Sep 11 14:30:03 2012 -0700

blame: allow "blame file" in the middle of a conflicted merge
[...]

-- 
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