RE: Please pull l10n updates for 1.8.1 round 1

2012-11-28 Thread Trần Ngọc Quân
Hello JX,
You missing pull from my repo (2 commits instead of one, v1.7 and v1.8): 
dcc52a0449c7ee10690e23152e63b9798f8a332f

$ git log -n 2
commit dcc52a0449c7ee10690e23152e63b9798f8a332f
Author: Tran Ngoc Quan 
Date:   Sat Nov 24 07:37:35 2012 +0700

l10n: vi.po: Update follow git-v1.8.0-273-g2d242

Signed-off-by: Tran Ngoc Quan 

commit 131fa518f10521b4a534863331decbfef2875f24
Author: Tran Ngoc Quan 
Date:   Wed Oct 31 08:19:59 2012 +0700

l10n: vi.po: update to git-v1.7.12-437-g1084f

 * updated all new messages (1967t0f0u)
 * make quote become more good-looking

Signed-off-by: Tran Ngoc Quan 

 https://github.com/vnwildman/git.git master
Thanks,
Trần Ngọc Quân

-Original Message-
From: Jiang Xin [mailto:worldhello@gmail.com] 
Sent: Thursday, November 29, 2012 8:19 AM
To: Junio C Hamano
Cc: Git List; Peter Krefting; Trần Ngọc Quân; Nguyễn Thái Ngọc Duy
Subject: Please pull l10n updates for 1.8.1 round 1

Hi, Junio

The following changes since commit 2d242fb3fc19fc9ba046accdd9210be8b9913f64:

  Update draft release notes for 1.8.1 (2012-11-21 13:32:58 -0800)

are available in the git repository at:

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

for you to fetch changes up to 647d5183b8dc36b38d19c7a3f388108f245b11d3:

  l10n: Update Swedish translation (1975t0f0u) (2012-11-23 08:59:11 +0100)


Jiang Xin (1):
  l10n: Update git.pot (14 new, 3 removed messages)

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

Tran Ngoc Quan (1):
  l10n: vi.po: update to git-v1.7.12-437-g1084f

 po/git.pot | 1224 --
 po/sv.po   | 1246 +--
 po/vi.po   | 1597 ++--
 3 files changed, 2097 insertions(+), 1970 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: Millisecond precision in timestamps?

2012-11-28 Thread Felipe Contreras
On Thu, Nov 29, 2012 at 8:11 AM, Junio C Hamano  wrote:
> Steven Michalske  writes:
>
>> Would having arbitrary key value pairs be useful in the git data
>> model?
>
> My answer to the question is that it is harmful to the data model,
> but the benefit of going against the data model _may_ outweigh the
> downside.  It is all relative.

If git doesn't provide the capability, people will keep using the
commit message to store that extra information, which I would think is
even more harmful. An standard 'commit-extra' note or something would
help deal with that.

Cheers.

-- 
Felipe Contreras
--
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: Millisecond precision in timestamps?

2012-11-28 Thread Junio C Hamano
Steven Michalske  writes:

> Would having arbitrary key value pairs be useful in the git data
> model?

My answer to the question is that it is harmful to the data model,
but the benefit of going against the data model _may_ outweigh the
downside.  It is all relative.

The first of very small number of principles of the git data model
is that the object name is derived solely from the contents, hence
we can tell two different things apart with object names without
looking at object contents.

This is actively broken by adding "junk" fields left and right.
Adding arbitrary pieces of data that are optional (and largely
ignored by core operations) means you can record objects with
essentially the same contents under different object names, so
object names no longer help us telling two moral-equivalent objects
apart.

But "if two objects have different names, they are not the same"
does not have to be the only and the absolute truth in all contexts;
the world is not so black and white.  Depending on the application
and the context, you may want to treat two things that are not the
same as equivalents.

For example, at the blob level, two blob objects that store the same
text (say, one original and the other typed in double-space) would
be different objects and have different object names, but you may
want to treat them as "equivalents" (not same but interchangeable),
by applying textconv filter to normalize their contents when
comparing them.  We still keep the "two objects with different names
are different" principle, but at the same time, allow users to treat
them as equivalent in specific contexts.

Introducing a hack to exclude selective "junk" fields from hashing
done for object name computation is not a solution and is out of the
question, but that does not necessarily mean that commit objects
should never be extended with new types of header fields.  When a
commit object is made with a "junk" field, it will have a name that
is different from the one it would get without the "junk" field, but
the benefit of the ability to store extra data _may_ outweigh the
downside of having to always compare the contents of two objects
with different names to find out that they are different but
equivalent.
--
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: Millisecond precision in timestamps?

2012-11-28 Thread Eric S. Raymond
Steven Michalske :
> Would having arbitrary key value pairs be useful in the git data
> model?  We could have ones that affect the sha1 and others that are
> transparent.

My tools would have several uses for these.

bzr's implementation of import streams has a commit-propperties extension.
reposurgeon can read, display.. and manipulate these key/value pairs.
I do wish they were in core git.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
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


reposurgeon now writes Subversion repositories

2012-11-28 Thread Eric S. Raymond
This is something that probably doesn't happen very often -
cross-posting to the Subversion and git dev lists that is on-topic for
both :-).

The repo head version of reposurgeon can now write Subversion
repositories from its common git-import-stream-based representation of
repository histories, as well as reading them in.  This joins full
support for git, hg, and bzr; it means that in theory reposurgeon
could now be used to move revision histories from these systems to
Subversion, as well as the other way around.

(For those of you who have been living under a rock, reposurgeon is a
multi-VCS surgery and conversion tool. Since 2.x it does a more
intelligent job of lifting from Subversion to anything else than any
other tool I know of. Much more at .)

Presently, writing (as opposed to reading) Subversion repos is more of
a stunt than a real production technique, and may always remain so.
It has serious limitations.  I am posting because I think the details
of those limitations will be of some technical interest to both
Subversion and git developers.

Indented paragraphs is the documentation from reposurgeon's manual
page.  I have added some further notes.

  In summary, Subversion repository histories do not round-trip through
  reposurgeon editing. File content changes are preserved but some
  metadata is unavoidably lost.  Furthermore, writing out a DVCS history
  in Subversion also loses significant portions of its metadata.

  Writing a Subversion repository or dump stream discards author
  information, the committer's name, and the hostname part of the commit
  address; only the commit timestamp and the local part of the
  committer's email address are preserved, the latter becoming the
  Subversion author field.  However, reading a Subversion repository and
  writing it out again will preserve the author fields.

Subversion's metadata doesn't have separate author and committer
properties, and doesn't store anything but a Unix user ID as
attribution.  I don't see any way around this.

  Import-stream timestamps have 1-second granularity. The subsecond
  parts of Subversion commit timestamps will be lost on their way through
  reposurgeon.

Unavoidable in moving from Subversion to git import streams, and one
of two places where git's data model requires us to throw away
information.  

However, I think I could preserve this information in a
Subversion-to-Subversion editing scenario by storing the incoming
timestamps as floats and only truncating them on import-stream output,
leaving the subseconds in place for Subversion output.

  Empty directories aren't represented in import streams. Consequently,
  reading and writing Subversion repositories preserves file content,
  but not empty directories.  It is also not guaranteed that after
  editing a Subverson repository that the sequence of directory
  creations and deletions relative to other operations will be
  identical; the only guarantee is that enclosing directories will be
  created before any files in them are.

  When reading a Subversion repository, reposurgeon discards the special
  directory-copy nodes associated with branch creations.  These can't be
  recreated if and when the repository is written back out to
  Subversion; rather, each branch copy node from the original translates
  into a branch creation plus the first set of file modifications on the
  branch.

In theory, I could relax the rules of reposurgeon's internal
representation so that empty directory-creation and deletion nodes are
not discarded at read time but only when outputting a git event stream.

That would bring Subversion repositories closer to round-tripping, but
not get all the way there.  One problem is botched branch copies -
directory copies with cp(1) followed by Subversion add operations.
This is not an uncommon malformation; reposurgeon takes it in stride,
treating these as though they had been real branch copies and
simplifying the backlinks appropriately.

  When reading a Subversion repository, reposurgeon also automatically
  breaks apart mixed-branch commits.

It has to.  These just can't be represented in the import-stream model of
branching.

  Because of the preceding two points, it is not guaranteed that 
  even revision numbers will be stable when a Subversion repository
  is read in and then written out!

So not only can Subversion repos fail to round-trip exactly, in the
presence of lots of branch copies and mixed-branch commits the
relationship between the read-in and written out revision numbers
could get pretty unpredictable.

  Subversion repositories are always written with a standard
  (trunk/tags/branches) layout. Thus, a repository with a nonstandard
  shape that has been analyzed by reposurgeon won't be written out with
  the same shape.

In particular, this means linear Subversion repositories with no trunk
(an organization some smaller projects used to use and might still)
will turn into branchy repos

Re: Please pull l10n updates for 1.8.1 round 1

2012-11-28 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 00/11] alternative unify appending of sob

2012-11-28 Thread Junio C Hamano
Nguyen Thai Ngoc Duy  writes:

> On Mon, Nov 26, 2012 at 8:45 AM, Brandon Casey  wrote:
>
>> I've integrated Duy's series with a few minor tweaks.  I added a couple
>> of additional tests to t4014 and corrected one of the tests which had
>> incorrect behavior.  I think his sign-off's should still be valid, so I
>> kept them in.  Sorry that I've been slow, and now the two of us are stepping
>> on each other's toes, but Duy please take a look and let me know if there's
>> anything you dislike.
>
> I'm still not sure whether format-patch should follow cherry-pick's
> rule in appending sob. If it does, it probably makes more sense to fix
> the sequencer.c code then delete log-tree.c (not fixes on log-tree.c
> then delete it). I see that your changes pass all the new tests I
> added in format-patch so sequencer.c is probably good enough,
> log-tree.c changes are probably not needed. Feel free take over the
> series :-)

After reading the series over, I agree with the above.

Patch #9 that fixes the copy in log-tree.c only to discard it in
patch #11 does not seem to be the best organization of the series.
Instead, perhaps we can salvage the tests in patch #9 (but mark
failing ones as expecting failure) without updating the one in
log-tree.c, adjust prototype in patch #10 (still broken in
log-tree.c) to avoid having to make changes to the callers in patch
#11, and then conclude the series with #11?

Other than the code in patches #06 and #07 that I already commented
on, i.e. assignments in if () condition that make it harder to
follow the logic, I did not find anything majorly objectionable in
the series.

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


Re: [PATCH 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer

2012-11-28 Thread Brandon Casey

On 11/28/2012 4:02 PM, Junio C Hamano wrote:

Brandon Casey  writes:


-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+static int is_rfc2822_line(const char *buf, int len)
+{
+   int i;
+
+   for (i = 0; i < len; i++) {
+   int ch = buf[i];
+   if (ch == ':')
+   break;
+   if (isalnum(ch) || (ch == '-'))
+   continue;
+   return 0;
+   }
+
+   return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+   return (strlen(cherry_picked_prefix) + 41) <= len &&
+   !prefixcmp(buf, cherry_picked_prefix);
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
  {
-   int ch;
int hit = 0;
-   int i, j, k;
+   int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;

@@ -1039,15 +1061,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int 
ignore_footer)
; /* do nothing */
k++;

-   for (j = 0; i + j < len; j++) {
-   ch = buf[i + j];
-   if (ch == ':')
-   break;
-   if (isalnum(ch) ||
-   (ch == '-'))
-   continue;
+   if (!(is_rfc2822_line(buf+i, k-i) ||
+   is_cherry_pick_from_line(buf+i, k-i)))
return 0;
-   }
}
return 1;
  }


Refactored code looks vastly more readable, but I think the
is_cherry_pick_from_line() function (by the way, shouldn't it be
named is_cherry_picked_from_line()?


Yes.


) shows its ambivalence.  It
insists that the line has to be long enough to hold 40-hex object
name plus a closing parenthesis, but it only makes sure that the
prefix matches, without checking if the line has 40-hex object name,
or the object name is immediately followed by a closing parenthesis.
It also does not care if there are other garbage after it.

If the code is trying to be strict to avoid misidentification, then
the check should be tightened (i.e. require the known fixed length,
make sure get_sha1_hex() is happy, 41st byte is a close parenthesis
that is followed by the end of line).  If the code is trying to be
more lenient to allow people hand-editing the cherry-picked-from
line that was generated, the check could be loosened (people may
truncate the 40-hex down to 12-hex or something).

I cannot read from this code which one was intended; the code must
make up its mind, whichever way (I do not have a strong preference).


The intention was a stricter-type match, but implemented somewhat
lazily.  Part of me doesn't think that anyone should need to modify
the string that 'cherry-pick -x' adds and that a strict match is
appropriate.  The other part of me doesn't want to reject a line that
looks like "(cherry picked from ...)" line that has a trimmed sha1
inside.

I think I'll submit a somewhat loosened check.


+test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match 
committer' '


Is the title of this test appropriate?  It looks like it is making
sure we do not add an extra blank line after the existing footer to
me.


It's really just part of the series of tests that checks for correct
operation when "the last sob doesn't match committer".  This test
has a few elements in it:

   * existing sob footer contains the committer's sob, but not last.
 ensure that we don't refrain from appending a sob just because
 one already exists in the footer.
   * ensure extra blank isn't inserted after existing footer and
 "(cherry picked from..."
   * ensure a blank isn't inserted between "(cherry picked from..."
 and new sob

The title of the test is me trying to fit "correctly adds \"(cherry
picked from...\" and sob when footer contains committer's sob but
not last" within a single line.


+   pristine_detach initial &&
+   sha1=`git rev-parse mesg-with-footer^0` &&
+   git cherry-pick -x -s mesg-with-footer &&
+   cat <<-EOF >expect &&
+   $mesg_with_footer
+   (cherry picked from commit $sha1)
+   Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+   EOF
+   git log -1 --pretty=format:%B >actual &&
+   test_cmp expect actual
+'
+



+test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists 
for committer' '
+   pristine_detach initial &&
+   sha1=`git rev-parse mesg-with-footer-sob^0` &&
+   git cherry-pick -x -s mesg-with-footer-sob &&
+   cat <<-EOF >expect &&
+   $mesg_with_footer_sob
+   (cherry picked from commit $sha1)
+   Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+   EOF
+   git log -1 --pretty=format:%B >actual &&
+   test_cmp expect actual
+'


For people on the sideline, $mesg_with_footer_sob en

Please pull l10n updates for 1.8.1 round 1

2012-11-28 Thread Jiang Xin
Hi, Junio

The following changes since commit 2d242fb3fc19fc9ba046accdd9210be8b9913f64:

  Update draft release notes for 1.8.1 (2012-11-21 13:32:58 -0800)

are available in the git repository at:

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

for you to fetch changes up to 647d5183b8dc36b38d19c7a3f388108f245b11d3:

  l10n: Update Swedish translation (1975t0f0u) (2012-11-23 08:59:11 +0100)


Jiang Xin (1):
  l10n: Update git.pot (14 new, 3 removed messages)

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

Tran Ngoc Quan (1):
  l10n: vi.po: update to git-v1.7.12-437-g1084f

 po/git.pot | 1224 --
 po/sv.po   | 1246 +--
 po/vi.po   | 1597 ++--
 3 files changed, 2097 insertions(+), 1970 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 v7 p2 1/2] fast-export: don't handle uninteresting refs

2012-11-28 Thread Max Horn

On 28.11.2012, at 23:23, Felipe Contreras wrote:

> They have been marked as UNINTERESTING for a reason, lets respect that.
> 
> Currently the first ref is handled properly, but not the rest:
> 
>  % git fast-export master ^uninteresting ^foo ^bar

All these refs are assumed to point to the same object, right? I think it would 
be better if the commit message stated that explicitly. To make up for the lost 
space, you could then get rid of one of the four refs, I think three are 
sufficient to drive the message home ;-).




> The reason this happens is that before traversing the commits,
> fast-export checks if any of the refs point to the same object, and any
> duplicated ref gets added to a list in order to issue 'reset' commands
> after the traversing. Unfortunately, it's not even checking if the
> commit is flagged as UNINTERESTING. The fix of course, is to do
> precisely that.

Hm... So this might be me being a stupid n00b (I am not yet that familiar with 
the internal rep of things in git and all...)... but I found the "precisely 
that" par very confusing, because right afterwards, you say:

> 
> However, in order to do it properly we need to get the UNINTERESTING flag
> from the command line ref, not from the commit object.

So this sounds like you are saying "we do *precisely* that, except we don't, 
because it is more complicated, so we actually don't do this *precisely*, just 
manner of speaking..."

Some details here are beyond my knowledge, I am afraid, so I have to resort to 
guess: In particular it is not clear to me why the "however" part pops up: 
Reading it makes it sound as if the commit object also carries an UNINTERESTING 
flag, but we can't use it because of some reason (perhaps it doesn't have the 
semantics we need?), so we have to look at revs.pending instead. Right? Wrong? 
Or is it because the commit objects actually do *not* carry the UNINTERESTING 
bits, hence we need to look at revs.pending. Or is it due to yet another reason?

I would find it helpful if that could be clarified. E.g. like so:

 "The fix is to add such a check. However, we cannot just use the UNINTERESTING 
flag of the commit object, because INSERT-REASON."

or

 "The fix is to add such a check. However, the commit object does not contain 
the UNINTERESTING flag directly."

or something.

Anyway, other than these nitpicky questions, this whole thing looks very 
logical to me, description and code alike. I also played around with tons of 
"fast-export" invocations, with and without this patch, and it seems to do what 
the description says. Finally, I went to the various long threads discussion 
prior versions of this patch, in particular those starting at
  http://thread.gmane.org/gmane.comp.version-control.git/208725
and
  http://thread.gmane.org/gmane.comp.version-control.git/209355/focus=209370

These contained some concerns. Sadly, several of those discussions ultimately 
degenerated into not-so-pleasant exchanges :-(, and my impression is that as a 
result some people are not so inclined to comment on these patches anymore at 
all. Which is a pity :-(. But overall, it seems this patch makes nothing worse, 
but fixes some things; and it is simple enough that it shouldn't make future 
improvements harder.

So *I* at least am quite happy with this, it helps me! My impression is that 
Felipe's latest patch addresses most concerns people raised by means of an 
improved description. I couldn't find any in those threads that I feel still 
applies -- but of course those people should speak for themselves, I am simply 
afraid they don't want to be part of this anymore :-(.


Still, for what little it might be worth, I think this patch is good and a real 
improvement. I hope it can be merged soon.


Cheers,
Max


> Fortunately we
> can simply use revs.pending, which contains all the information we need
> for get_tags_and_duplicates(), plus the ref flag. This way the rest of
> the positive refs will remain untouched; it's only the negative ones
> that change in behavior.
> 
> Signed-off-by: Felipe Contreras 
> ---
> builtin/fast-export.c | 11 +++
> t/t5801-remote-helpers.sh |  8 
> t/t9350-fast-export.sh| 30 ++
> 3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 191936c..2547e6c 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -474,18 +474,21 @@ static void handle_tag(const char *name, struct tag 
> *tag)
>  (int)message_size, (int)message_size, message ? message : "");
> }
> 
> -static void get_tags_and_duplicates(struct object_array *pending,
> +static void get_tags_and_duplicates(struct rev_cmdline_info *info,
>   struct string_list *extra_refs)
> {
>   struct tag *tag;
>   int i;
> 
> - for (i = 0; i < pending->nr; i++) {
> - struct object_array_entry *e = pending->objects + i;
> + for (i = 0; i < 

Re: [PATCH 06/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b

2012-11-28 Thread Junio C Hamano
Brandon Casey  writes:

> +/* Returns 0 for non-conforming footer

Please format it like this:

/*
 * Returns 0 for ...

> + * Returns 1 for conforming footer
> + * Returns 2 when sob exists within conforming footer
> + * Returns 3 when sob exists within conforming footer as last entry
> + */
> +static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
> + int ignore_footer)
>  {
>   int hit = 0;
> - int i, k;
> + int i, k = 0;

This is not wrong per-se, but I do not think it is necessary ('k' is
always initialized at least to 'i' before it gets used).  If we need
to do this, I'd prefer to see it done in 1/11 that cleaned up the
containing function as a preparation for this series.

>   int len = sb->len - ignore_footer;
>   const char *buf = sb->buf;
> + int found_sob = 0;
>  
>   for (i = len - 1; i > 0; i--) {
>   if (hit && buf[i] == '\n')
> @@ -63,14 +70,24 @@ static int has_conforming_footer(struct strbuf *sb, int 
> ignore_footer)
>   i++;
>  
>   for (; i < len; i = k) {
> + int found_rfc2822;
> +
>   for (k = i; k < len && buf[k] != '\n'; k++)
>   ; /* do nothing */
>   k++;
>  
> - if (!(is_rfc2822_line(buf+i, k-i) ||
> - is_cherry_pick_from_line(buf+i, k-i)))
> + found_rfc2822 = is_rfc2822_line(buf+i, k-i);

Not limited to this place but please have SP around binary operators, i.e.

found_rfc2822 = is_rfc2822_line(buf + i, k - i);

> + if (found_rfc2822 && sob &&
> + !strncasecmp(buf+i, sob->buf, sob->len))
> + found_sob = k;

Are we sure we want strncasecmp() here?  I *think* you are trying to
catch "signed-off-By:" and other misspellings, and even though I
know in practice we know "Brandon Casey" and "brandon casey" are the
same person, it still feels somewhat sloppy.  Perhaps it is just me.

> +
> + if (!(found_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))
>   return 0;
>   }
> + if (found_sob == i)
> + return 3;
> + if (found_sob)
> + return 2;
>   return 1;
>  }
>  
> @@ -291,7 +308,7 @@ static int do_recursive_merge(struct commit *base, struct 
> commit *next,
>   rollback_lock_file(&index_lock);
>  
>   if (opts->signoff)
> - append_signoff(msgbuf, 0);
> + append_signoff(msgbuf, 0, 0);
>  
>   if (!clean) {
>   int i;
> @@ -547,7 +564,7 @@ static int do_pick_commit(struct commit *commit, struct 
> replay_opts *opts)
>   }
>  
>   if (opts->record_origin) {
> - if (!has_conforming_footer(&msgbuf, 0))
> + if (!has_conforming_footer(&msgbuf, NULL, 0))
>   strbuf_addch(&msgbuf, '\n');
>   strbuf_addstr(&msgbuf, cherry_picked_prefix);
>   strbuf_addstr(&msgbuf, 
> sha1_to_hex(commit->object.sha1));
> @@ -1074,9 +1091,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
>   return pick_commits(todo_list, opts);
>  }
>  
> -void append_signoff(struct strbuf *msgbuf, int ignore_footer)
> +void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
>  {
>   struct strbuf sob = STRBUF_INIT;
> + int has_footer = 0;
>   int i;
>  
>   strbuf_addstr(&sob, sign_off_header);
> @@ -1085,10 +1103,11 @@ void append_signoff(struct strbuf *msgbuf, int 
> ignore_footer)
>   strbuf_addch(&sob, '\n');
>   for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] 
> != '\n'; i--)
>   ; /* do nothing */
> - if (prefixcmp(msgbuf->buf + i, sob.buf)) {
> - if (!i || !has_conforming_footer(msgbuf, ignore_footer))
> - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, 
> "\n", 1);
> - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, 
> sob.len);
> - }
> + if (!i || !(has_footer =
> + has_conforming_footer(msgbuf, &sob, ignore_footer)))
> + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
> + if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
> + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
> + sob.buf, sob.len);

Avoid assignment inside if () conditional.  It is not immediately
obvious what value is compared against 3 in the second one, as the
above makes it appear as if has_footer is uninitialized when i is
zero.

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 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer

2012-11-28 Thread Junio C Hamano
Brandon Casey  writes:

> -static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
> +static int is_rfc2822_line(const char *buf, int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++) {
> + int ch = buf[i];
> + if (ch == ':')
> + break;
> + if (isalnum(ch) || (ch == '-'))
> + continue;
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int is_cherry_pick_from_line(const char *buf, int len)
> +{
> + return (strlen(cherry_picked_prefix) + 41) <= len &&
> + !prefixcmp(buf, cherry_picked_prefix);
> +}
> +
> +static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
>  {
> - int ch;
>   int hit = 0;
> - int i, j, k;
> + int i, k;
>   int len = sb->len - ignore_footer;
>   const char *buf = sb->buf;
>  
> @@ -1039,15 +1061,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int 
> ignore_footer)
>   ; /* do nothing */
>   k++;
>  
> - for (j = 0; i + j < len; j++) {
> - ch = buf[i + j];
> - if (ch == ':')
> - break;
> - if (isalnum(ch) ||
> - (ch == '-'))
> - continue;
> + if (!(is_rfc2822_line(buf+i, k-i) ||
> + is_cherry_pick_from_line(buf+i, k-i)))
>   return 0;
> - }
>   }
>   return 1;
>  }

Refactored code looks vastly more readable, but I think the
is_cherry_pick_from_line() function (by the way, shouldn't it be
named is_cherry_picked_from_line()?) shows its ambivalence.  It
insists that the line has to be long enough to hold 40-hex object
name plus a closing parenthesis, but it only makes sure that the
prefix matches, without checking if the line has 40-hex object name,
or the object name is immediately followed by a closing parenthesis.
It also does not care if there are other garbage after it.

If the code is trying to be strict to avoid misidentification, then
the check should be tightened (i.e. require the known fixed length,
make sure get_sha1_hex() is happy, 41st byte is a close parenthesis
that is followed by the end of line).  If the code is trying to be
more lenient to allow people hand-editing the cherry-picked-from
line that was generated, the check could be loosened (people may
truncate the 40-hex down to 12-hex or something).

I cannot read from this code which one was intended; the code must
make up its mind, whichever way (I do not have a strong preference).

> +test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match 
> committer' '

Is the title of this test appropriate?  It looks like it is making
sure we do not add an extra blank line after the existing footer to
me.

> + pristine_detach initial &&
> + sha1=`git rev-parse mesg-with-footer^0` &&
> + git cherry-pick -x -s mesg-with-footer &&
> + cat <<-EOF >expect &&
> + $mesg_with_footer
> + (cherry picked from commit $sha1)
> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
> + EOF
> + git log -1 --pretty=format:%B >actual &&
> + test_cmp expect actual
> +'
> +

> +test_expect_success 'cherry-pick -x -s adds sob even when trailing sob 
> exists for committer' '
> + pristine_detach initial &&
> + sha1=`git rev-parse mesg-with-footer-sob^0` &&
> + git cherry-pick -x -s mesg-with-footer-sob &&
> + cat <<-EOF >expect &&
> + $mesg_with_footer_sob
> + (cherry picked from commit $sha1)
> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
> + EOF
> + git log -1 --pretty=format:%B >actual &&
> + test_cmp expect actual
> +'

For people on the sideline, $mesg_with_footer_sob ends with s-o-b by
the same "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" we are adding
here.  This is probably a sensible thing to do.

One thing I am not so happy about this whole "(cherry picked from"
thing (and I am again agreeing with Linus who made me change the
default long time ago not to add this line by default) is this.  If
you use "cherry-pick -s -x $commit" to transplant a commit from an
unrelated part of the history, you will get the object name in the
resulting commit.  But you can do the same in at least two different
ways.  The easiest is:

git format-patch -1 --stdout $commit | git am -s3

and a bit closer to what "cherry-pick" does is:

git checkout $commit^0 &&
git rebase --onto @{-1} HEAD^ &&
git checkout -B @{-1}

i.e. rebase the single commit on top of the branch you were on.

In either way, you don't get "cherry picked from", even though you
just did the moral equivalent of it.  Also we need to realize that
the first one is essentially what happens all the time here; the "|"
is implemented by the mailing list.  And nobody misses "cherry
picked

Re: git-svn: What is --follow-parent / --no-follow-parent for?

2012-11-28 Thread Sebastian Leske
Hi Steven, hi Eric,

thanks for your explanations. I'll try to update my doc patch to include
them.

--
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 v7 p2 2/2] fast-export: make sure updated refs get updated

2012-11-28 Thread Felipe Contreras
When an object has already been exported (and thus is in the marks) it's
flagged as SHOWN, so it will not be exported again, even if in a later
time it's exported through a different ref.

We don't need the object to be exported again, but we want the ref
updated, which doesn't happen.

Since we can't know if a ref was exported or not, let's just assume that
if the commit was marked (flags & SHOWN), the user still wants the ref
updated.

IOW: If it's specified in the command line, it will get updated,
regardless of whether or not the object was marked.

So:

 % git branch test master
 % git fast-export $mark_flags master
 % git fast-export $mark_flags test

Would export 'test' properly.

Additionally, this fixes issues with remote helpers; now they can push
refs whose objects have already been exported, and a few other issues as
well. Update the tests accordingly.

Signed-off-by: Felipe Contreras 
---
 builtin/fast-export.c | 10 +++---
 t/t5801-remote-helpers.sh | 20 
 t/t9350-fast-export.sh| 15 +++
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2547e6c..77dffd1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -526,10 +526,14 @@ static void get_tags_and_duplicates(struct 
rev_cmdline_info *info,
typename(e->item->type));
continue;
}
-   if (commit->util)
-   /* more than one name for the same object */
+
+   /*
+* This ref will not be updated through a commit, lets make
+* sure it gets properly updated eventually.
+*/
+   if (commit->util || commit->object.flags & SHOWN)
string_list_append(extra_refs, full_name)->util = 
commit;
-   else
+   if (!commit->util)
commit->util = full_name;
}
 }
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index ece8fd5..f387027 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -63,18 +63,6 @@ test_expect_success 'fetch new branch' '
compare_refs server HEAD local FETCH_HEAD
 '
 
-#
-# This is only needed because of a bug not detected by this script. It will be
-# fixed shortly, but for now lets not cause regressions.
-#
-test_expect_success 'bump commit in server' '
-   (cd server &&
-   git checkout master &&
-   echo content >>file &&
-   git commit -a -m four) &&
-   compare_refs server HEAD server HEAD
-'
-
 test_expect_success 'fetch multiple branches' '
(cd local &&
 git fetch
@@ -170,4 +158,12 @@ test_expect_success 'push all with existing object' '
compare_refs local dup2 server dup2
 '
 
+test_expect_success 'push ref with existing object' '
+   (cd local &&
+   git branch dup master &&
+   git push origin dup
+   ) &&
+   compare_refs local dup server dup
+'
+
 test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index c8e41c1..9320b4f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -484,4 +484,19 @@ test_expect_success 'avoid uninteresting refs' '
test_cmp expected actual
 '
 
+cat > expected << EOF
+reset refs/heads/master
+from :14
+
+EOF
+
+test_expect_success 'refs are updated even if no commits need to be exported' '
+   > tmp-marks &&
+   git fast-export --import-marks=tmp-marks \
+   --export-marks=tmp-marks master > /dev/null &&
+   git fast-export --import-marks=tmp-marks \
+   --export-marks=tmp-marks master > actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.0.1

--
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 v7 p2 1/2] fast-export: don't handle uninteresting refs

2012-11-28 Thread Felipe Contreras
They have been marked as UNINTERESTING for a reason, lets respect that.

Currently the first ref is handled properly, but not the rest:

  % git fast-export master ^uninteresting ^foo ^bar
  reset refs/heads/bar
  from :0

  reset refs/heads/foo
  from :0

  reset refs/heads/uninteresting
  from :0

  % git fast-export ^uninteresting ^foo ^bar master
  reset refs/heads/master
  from :0

  reset refs/heads/bar
  from :0

  reset refs/heads/foo
  from :0

Clearly this is wrong; the negative refs should be ignored.

After this patch:

  % git fast-export ^uninteresting ^foo ^bar master
  # nothing
  % git fast-export master ^uninteresting ^foo ^bar
  # nothing

And even more, it would only happen if the ref is pointing to exactly
the same commit, but not otherwise:

 % git fast-export ^next next
 reset refs/heads/next
 from :0

 % git fast-export ^next next^{commit}
 # nothing
 % git fast-export ^next next~0
 # nothing
 % git fast-export ^next next~1
 # nothing
 % git fast-export ^next next~2
 # nothing

The reason this happens is that before traversing the commits,
fast-export checks if any of the refs point to the same object, and any
duplicated ref gets added to a list in order to issue 'reset' commands
after the traversing. Unfortunately, it's not even checking if the
commit is flagged as UNINTERESTING. The fix of course, is to do
precisely that.

However, in order to do it properly we need to get the UNINTERESTING flag
from the command line ref, not from the commit object. Fortunately we
can simply use revs.pending, which contains all the information we need
for get_tags_and_duplicates(), plus the ref flag. This way the rest of
the positive refs will remain untouched; it's only the negative ones
that change in behavior.

Signed-off-by: Felipe Contreras 
---
 builtin/fast-export.c | 11 +++
 t/t5801-remote-helpers.sh |  8 
 t/t9350-fast-export.sh| 30 ++
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 191936c..2547e6c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -474,18 +474,21 @@ static void handle_tag(const char *name, struct tag *tag)
   (int)message_size, (int)message_size, message ? message : "");
 }
 
-static void get_tags_and_duplicates(struct object_array *pending,
+static void get_tags_and_duplicates(struct rev_cmdline_info *info,
struct string_list *extra_refs)
 {
struct tag *tag;
int i;
 
-   for (i = 0; i < pending->nr; i++) {
-   struct object_array_entry *e = pending->objects + i;
+   for (i = 0; i < info->nr; i++) {
+   struct rev_cmdline_entry *e = info->rev + i;
unsigned char sha1[20];
struct commit *commit;
char *full_name;
 
+   if (e->flags & UNINTERESTING)
+   continue;
+
if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
continue;
 
@@ -681,7 +684,7 @@ int cmd_fast_export(int argc, const char **argv, const char 
*prefix)
if (import_filename && revs.prune_data.nr)
full_tree = 1;
 
-   get_tags_and_duplicates(&revs.pending, &extra_refs);
+   get_tags_and_duplicates(&revs.cmdline, &extra_refs);
 
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 12ae256..ece8fd5 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -162,4 +162,12 @@ test_expect_failure 'pushing without marks' '
compare_refs local2 HEAD server HEAD
 '
 
+test_expect_success 'push all with existing object' '
+   (cd local &&
+   git branch dup2 master &&
+   git push origin --all
+   ) &&
+   compare_refs local dup2 server dup2
+'
+
 test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 1f59862..c8e41c1 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -454,4 +454,34 @@ test_expect_success 'test bidirectionality' '
git fast-import --export-marks=marks-cur --import-marks=marks-cur
 '
 
+cat > expected << EOF
+blob
+mark :13
+data 5
+bump
+
+commit refs/heads/master
+mark :14
+author A U Thor  1112912773 -0700
+committer C O Mitter  1112912773 -0700
+data 5
+bump
+from :12
+M 100644 :13 file
+
+EOF
+
+test_expect_success 'avoid uninteresting refs' '
+   > tmp-marks &&
+   git fast-export --import-marks=tmp-marks \
+   --export-marks=tmp-marks master > /dev/null &&
+   git tag v1.0 &&
+   git branch uninteresting &&
+   echo bump > file &&
+   git commit -a -m bump &&
+   git fast-export --import-marks=tmp-marks \
+   --export-marks=tmp-marks ^uninteresting ^v1.0 master > actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.0.1

--
To unsubscribe from this li

[PATCH v7 p2 0/2] fast-export fixes

2012-11-28 Thread Felipe Contreras
Hi,

Here's version 7 part 2; I've dropped all the unnecessary patches, nobody seems
to care about the current brokedness, and I did them only to show that these
are correct, and that remote helpers without marks just don't work. These are
the ones I care about.

Below is a summary of what happens when you apply both patches, and how the new
behavior is obviously correct. All the refs point to the same object.

== before ==

  % git fast-export --export--marks=marks master
  # exported stuff
  % git fast-export --{import,export}-marks=marks test
  # nothing
  % git fast-export --{import,export}-marks=marks master ^uninteresting
  reset refs/heads/uninteresting
  from :6

  % git fast-export --{import,export}-marks=marks ^uninteresting master ^foo 
test
  reset refs/heads/test
  from :6

  reset refs/heads/foo
  from :6

  reset refs/heads/master
  from :6

  % git fast-export --{import,export}-marks=marks uninteresting..master

== after ==

  % git fast-export --export--marks=marks master
  # exported stuff
  % git fast-export --{import,export}-marks=marks test
  reset refs/heads/test
  from :6

  % git fast-export --{import,export}-marks=marks master ^uninteresting
  reset refs/heads/master
  from :6

  % git fast-export --{import,export}-marks=marks ^uninteresting master ^foo 
test
  reset refs/heads/test
  from :6

  reset refs/heads/master
  from :6

  % git fast-export --{import,export}-marks=marks uninteresting..master
  reset refs/heads/master
  from :6

Changes since v6:

  * Drop all the extra patches
  * Reorder patches so tests never fail

Felipe Contreras (2):
  fast-export: don't handle uninteresting refs
  fast-export: make sure updated refs get updated

 builtin/fast-export.c | 21 ++---
 t/t5801-remote-helpers.sh | 28 
 t/t9350-fast-export.sh| 45 +
 3 files changed, 75 insertions(+), 19 deletions(-)

-- 
1.8.0.1

--
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 v7 p1 04/13] Rename git-remote-testgit to git-remote-testpy

2012-11-28 Thread Felipe Contreras
This script is not really exercising the remote-helper functionality,
but more the python framework for remote helpers that live in
git_remote_helpers.

It's also not a good example of how to write remote-helpers, unless you
are planning to use python, and even then you might not want to use this
framework.

So let's use a more appropriate name: git-remote-testpy.

A patch that replaces git-remote-testgit with a simpler version is on
the way.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 .gitignore|   2 +-
 Makefile  |   2 +-
 git-remote-testgit.py | 272 --
 git-remote-testpy.py  | 272 ++
 t/t5800-remote-helpers.sh | 148 -
 t/t5800-remote-testpy.sh  | 148 +
 6 files changed, 422 insertions(+), 422 deletions(-)
 delete mode 100644 git-remote-testgit.py
 create mode 100644 git-remote-testpy.py
 delete mode 100755 t/t5800-remote-helpers.sh
 create mode 100755 t/t5800-remote-testpy.sh

diff --git a/.gitignore b/.gitignore
index eb5114f..827fe48 100644
--- a/.gitignore
+++ b/.gitignore
@@ -124,7 +124,7 @@
 /git-remote-ftps
 /git-remote-fd
 /git-remote-ext
-/git-remote-testgit
+/git-remote-testpy
 /git-remote-testsvn
 /git-repack
 /git-replace
diff --git a/Makefile b/Makefile
index 2ab0747..83a36b7 100644
--- a/Makefile
+++ b/Makefile
@@ -470,7 +470,7 @@ SCRIPT_PERL += git-relink.perl
 SCRIPT_PERL += git-send-email.perl
 SCRIPT_PERL += git-svn.perl
 
-SCRIPT_PYTHON += git-remote-testgit.py
+SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
diff --git a/git-remote-testgit.py b/git-remote-testgit.py
deleted file mode 100644
index ade797b..000
--- a/git-remote-testgit.py
+++ /dev/null
@@ -1,272 +0,0 @@
-#!/usr/bin/env python
-
-# This command is a simple remote-helper, that is used both as a
-# testcase for the remote-helper functionality, and as an example to
-# show remote-helper authors one possible implementation.
-#
-# This is a Git <-> Git importer/exporter, that simply uses git
-# fast-import and git fast-export to consume and produce fast-import
-# streams.
-#
-# To understand better the way things work, one can activate debug
-# traces by setting (to any value) the environment variables
-# GIT_TRANSPORT_HELPER_DEBUG and GIT_DEBUG_TESTGIT, to see messages
-# from the transport-helper side, or from this example remote-helper.
-
-# hashlib is only available in python >= 2.5
-try:
-import hashlib
-_digest = hashlib.sha1
-except ImportError:
-import sha
-_digest = sha.new
-import sys
-import os
-import time
-sys.path.insert(0, os.getenv("GITPYTHONLIB","."))
-
-from git_remote_helpers.util import die, debug, warn
-from git_remote_helpers.git.repo import GitRepo
-from git_remote_helpers.git.exporter import GitExporter
-from git_remote_helpers.git.importer import GitImporter
-from git_remote_helpers.git.non_local import NonLocalGit
-
-def get_repo(alias, url):
-"""Returns a git repository object initialized for usage.
-"""
-
-repo = GitRepo(url)
-repo.get_revs()
-repo.get_head()
-
-hasher = _digest()
-hasher.update(repo.path)
-repo.hash = hasher.hexdigest()
-
-repo.get_base_path = lambda base: os.path.join(
-base, 'info', 'fast-import', repo.hash)
-
-prefix = 'refs/testgit/%s/' % alias
-debug("prefix: '%s'", prefix)
-
-repo.gitdir = os.environ["GIT_DIR"]
-repo.alias = alias
-repo.prefix = prefix
-
-repo.exporter = GitExporter(repo)
-repo.importer = GitImporter(repo)
-repo.non_local = NonLocalGit(repo)
-
-return repo
-
-
-def local_repo(repo, path):
-"""Returns a git repository object initalized for usage.
-"""
-
-local = GitRepo(path)
-
-local.non_local = None
-local.gitdir = repo.gitdir
-local.alias = repo.alias
-local.prefix = repo.prefix
-local.hash = repo.hash
-local.get_base_path = repo.get_base_path
-local.exporter = GitExporter(local)
-local.importer = GitImporter(local)
-
-return local
-
-
-def do_capabilities(repo, args):
-"""Prints the supported capabilities.
-"""
-
-print "import"
-print "export"
-print "refspec refs/heads/*:%s*" % repo.prefix
-
-dirname = repo.get_base_path(repo.gitdir)
-
-if not os.path.exists(dirname):
-os.makedirs(dirname)
-
-path = os.path.join(dirname, 'git.marks')
-
-print "*export-marks %s" % path
-if os.path.exists(path):
-print "*import-marks %s" % path
-
-print # end capabilities
-
-
-def do_list(repo, args):
-"""Lists all known references.
-
-Bug: This will always set the remote head to master for non-local
-repositories, since we have no way of determining what the remote
-head is at clone time.
-"""
-
-for ref in repo.revs:
-debug("? refs/heads/%s", ref)
-print "? refs/he

[PATCH v7 p1 11/13] remote-testgit: implement the "done" feature manually

2012-11-28 Thread Felipe Contreras
People who want to write their own remote-helper will find it more
useful to see clearly how they are supposed to advertise and implement
the "done" feature themselves.

Right now we are relying on fast-export to do that by using the
--use-done-feature argument. However, people writing their own
remote-helper would probably not have such an option, as they would
probably be writing the fast-export functionality themselves.

It should now be clearer to them.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 git-remote-testgit | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index 6fb8780..b395c8d 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -61,8 +61,10 @@ do
echo "feature import-marks=$gitmarks"
echo "feature export-marks=$gitmarks"
fi
-   git fast-export --use-done-feature "${testgitmarks_args[@]}" 
$refs |
+   echo "feature done"
+   git fast-export "${testgitmarks_args[@]}" $refs |
sed -e "s#refs/heads/#${prefix}/heads/#g"
+   echo "done"
;;
export)
before=$(git for-each-ref --format='%(refname) %(objectname)')
-- 
1.8.0.1

--
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 v7 p1 12/13] fast-export: trivial cleanup

2012-11-28 Thread Felipe Contreras
Setting 'commit' to 'commit' is a no-op. It might have been there to
avoid a compiler warning, but if so, it was the compiler to blame, and
it's certainly not there any more.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 builtin/fast-export.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9b70ec1..191936c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array 
*pending,
for (i = 0; i < pending->nr; i++) {
struct object_array_entry *e = pending->objects + i;
unsigned char sha1[20];
-   struct commit *commit = commit;
+   struct commit *commit;
char *full_name;
 
if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
-- 
1.8.0.1

--
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 v7 p1 10/13] remote-testgit: report success after an import

2012-11-28 Thread Felipe Contreras
Doesn't make a difference for the tests, but it does for the ones
seeking reference.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 git-remote-testgit | 13 +
 1 file changed, 13 insertions(+)

diff --git a/git-remote-testgit b/git-remote-testgit
index efda74b..6fb8780 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -65,7 +65,20 @@ do
sed -e "s#refs/heads/#${prefix}/heads/#g"
;;
export)
+   before=$(git for-each-ref --format='%(refname) %(objectname)')
+
git fast-import "${testgitmarks_args[@]}" --quiet
+
+   after=$(git for-each-ref --format='%(refname) %(objectname)')
+
+   # figure out which refs were updated
+   join -e 0 -o '0 1.2 2.2' -a 2 <(echo "$before") <(echo 
"$after") |
+   while read ref a b
+   do
+   test $a == $b && continue
+   echo "ok $ref"
+   done
+
echo
;;
'')
-- 
1.8.0.1

--
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 v7 p1 09/13] remote-testgit: exercise more features

2012-11-28 Thread Felipe Contreras
Unfortunately a lot of these tests fail.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 git-remote-testgit| 38 +++---
 t/t5801-remote-helpers.sh | 52 +++
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index 5117ab5..efda74b 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -6,17 +6,25 @@ url=$2
 
 dir="$GIT_DIR/testgit/$alias"
 prefix="refs/testgit/$alias"
-refspec="refs/heads/*:${prefix}/heads/*"
 
-gitmarks="$dir/git.marks"
-testgitmarks="$dir/testgit.marks"
+default_refspec="refs/heads/*:${prefix}/heads/*"
+
+refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}"
+
+test -z "$refspec" && prefix="refs"
 
 export GIT_DIR="$url/.git"
 
 mkdir -p "$dir"
 
-test -e "$gitmarks" || > "$gitmarks"
-test -e "$testgitmarks" || > "$testgitmarks"
+if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
+then
+   gitmarks="$dir/git.marks"
+   testgitmarks="$dir/testgit.marks"
+   test -e "$gitmarks" || >"$gitmarks"
+   test -e "$testgitmarks" || >"$testgitmarks"
+   testgitmarks_args=( "--"{import,export}"-marks=$testgitmarks" )
+fi
 
 while read line
 do
@@ -24,9 +32,12 @@ do
capabilities)
echo 'import'
echo 'export'
-   echo "refspec $refspec"
-   echo "*import-marks $gitmarks"
-   echo "*export-marks $gitmarks"
+   test -n "$refspec" && echo "refspec $refspec"
+   if test -n "$gitmarks"
+   then
+   echo "*import-marks $gitmarks"
+   echo "*export-marks $gitmarks"
+   fi
echo
;;
list)
@@ -45,13 +56,16 @@ do
test "${line%% *}" != "import" && break
done
 
-   echo "feature import-marks=$gitmarks"
-   echo "feature export-marks=$gitmarks"
-   git fast-export --use-done-feature 
--{import,export}-marks="$testgitmarks" $refs |
+   if test -n "$gitmarks"
+   then
+   echo "feature import-marks=$gitmarks"
+   echo "feature export-marks=$gitmarks"
+   fi
+   git fast-export --use-done-feature "${testgitmarks_args[@]}" 
$refs |
sed -e "s#refs/heads/#${prefix}/heads/#g"
;;
export)
-   git fast-import --{import,export}-marks="$testgitmarks" --quiet
+   git fast-import "${testgitmarks_args[@]}" --quiet
echo
;;
'')
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index bc0b5f7..12ae256 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -110,4 +110,56 @@ test_expect_failure 'push new branch with old:new refspec' 
'
compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'cloning without refspec' '
+   GIT_REMOTE_TESTGIT_REFSPEC="" \
+   git clone "testgit::${PWD}/server" local2 &&
+   compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling without refspecs' '
+   (cd local2 &&
+   git reset --hard &&
+   GIT_REMOTE_TESTGIT_REFSPEC="" git pull) &&
+   compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing without refspecs' '
+   test_when_finished "(cd local2 && git reset --hard origin)" &&
+   (cd local2 &&
+   echo content >>file &&
+   git commit -a -m ten &&
+   GIT_REMOTE_TESTGIT_REFSPEC="" git push) &&
+   compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling with straight refspec' '
+   (cd local2 &&
+   GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) &&
+   compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing with straight refspec' '
+   test_when_finished "(cd local2 && git reset --hard origin)" &&
+   (cd local2 &&
+   echo content >>file &&
+   git commit -a -m eleven &&
+   GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) &&
+   compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling without marks' '
+   (cd local2 &&
+   GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) &&
+   compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing without marks' '
+   test_when_finished "(cd local2 && git reset --hard origin)" &&
+   (cd local2 &&
+   echo content >>file &&
+   git commit -a -m twelve &&
+   GIT_REMOTE_TESTGIT_NO_MARKS=1 git push) &&
+   compare_refs local2 HEAD server HEAD
+'
+
 test_done
-- 
1.8.0.1

--
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 v7 p1 07/13] remote-testgit: remove irrelevant test

2012-11-28 Thread Felipe Contreras
This was only to cover a bug that was fixed in remote-testpy not to
resurface.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 t/t5801-remote-helpers.sh | 13 -
 1 file changed, 13 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 2f7fc10..6801529 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -53,19 +53,6 @@ test_expect_success 'pushing to local repo' '
compare_refs localclone HEAD server HEAD
 '
 
-# Generally, skip this test.  It demonstrates a now-fixed race in
-# git-remote-testgit, but is too slow to leave in for general use.
-: test_expect_success 'racily pushing to local repo' '
-   test_when_finished "rm -rf server2 localclone2" &&
-   cp -R server server2 &&
-   git clone "testgit::${PWD}/server2" localclone2 &&
-   (cd localclone2 &&
-   echo content >>file &&
-   git commit -a -m three &&
-   GIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&
-   compare_refs localclone2 HEAD server2 HEAD
-'
-
 test_expect_success 'fetch new branch' '
(cd public &&
 git checkout -b new &&
-- 
1.8.0.1

--
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 v7 p1 08/13] remote-testgit: cleanup tests

2012-11-28 Thread Felipe Contreras
We don't need a bare 'server' and an intermediary 'public'. The repos
can talk to each other directly; that's what we want to exercise.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 t/t5801-remote-helpers.sh | 63 ++-
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 6801529..bc0b5f7 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -19,100 +19,95 @@ compare_refs() {
 }
 
 test_expect_success 'setup repository' '
-   git init --bare server/.git &&
-   git clone server public &&
-   (cd public &&
+   git init server &&
+   (cd server &&
 echo content >file &&
 git add file &&
-git commit -m one &&
-git push origin master)
+git commit -m one)
 '
 
 test_expect_success 'cloning from local repo' '
-   git clone "testgit::${PWD}/server" localclone &&
-   test_cmp public/file localclone/file
+   git clone "testgit::${PWD}/server" local &&
+   test_cmp server/file local/file
 '
 
 test_expect_success 'create new commit on remote' '
-   (cd public &&
+   (cd server &&
 echo content >>file &&
-git commit -a -m two &&
-git push)
+git commit -a -m two)
 '
 
 test_expect_success 'pulling from local repo' '
-   (cd localclone && git pull) &&
-   test_cmp public/file localclone/file
+   (cd local && git pull) &&
+   test_cmp server/file local/file
 '
 
 test_expect_success 'pushing to local repo' '
-   (cd localclone &&
+   (cd local &&
echo content >>file &&
git commit -a -m three &&
git push) &&
-   compare_refs localclone HEAD server HEAD
+   compare_refs local HEAD server HEAD
 '
 
 test_expect_success 'fetch new branch' '
-   (cd public &&
+   (cd server &&
+git reset --hard &&
 git checkout -b new &&
 echo content >>file &&
-git commit -a -m five &&
-git push origin new
+git commit -a -m five
) &&
-   (cd localclone &&
+   (cd local &&
 git fetch origin new
) &&
-   compare_refs public HEAD localclone FETCH_HEAD
+   compare_refs server HEAD local FETCH_HEAD
 '
 
 #
 # This is only needed because of a bug not detected by this script. It will be
 # fixed shortly, but for now lets not cause regressions.
 #
-test_expect_success 'bump commit in public' '
-   (cd public &&
+test_expect_success 'bump commit in server' '
+   (cd server &&
git checkout master &&
-   git pull &&
echo content >>file &&
-   git commit -a -m four &&
-   git push) &&
-   compare_refs public HEAD server HEAD
+   git commit -a -m four) &&
+   compare_refs server HEAD server HEAD
 '
 
 test_expect_success 'fetch multiple branches' '
-   (cd localclone &&
+   (cd local &&
 git fetch
) &&
-   compare_refs server master localclone refs/remotes/origin/master &&
-   compare_refs server new localclone refs/remotes/origin/new
+   compare_refs server master local refs/remotes/origin/master &&
+   compare_refs server new local refs/remotes/origin/new
 '
 
 test_expect_success 'push when remote has extra refs' '
-   (cd localclone &&
+   (cd local &&
 git reset --hard origin/master &&
 echo content >>file &&
 git commit -a -m six &&
 git push
) &&
-   compare_refs localclone master server master
+   compare_refs local master server master
 '
 
 test_expect_success 'push new branch by name' '
-   (cd localclone &&
+   (cd local &&
 git checkout -b new-name  &&
 echo content >>file &&
 git commit -a -m seven &&
 git push origin new-name
) &&
-   compare_refs localclone HEAD server refs/heads/new-name
+   compare_refs local HEAD server refs/heads/new-name
 '
 
 test_expect_failure 'push new branch with old:new refspec' '
-   (cd localclone &&
+   (cd local &&
 git push origin new-name:new-refspec
) &&
-   compare_refs localclone HEAD server refs/heads/new-refspec
+   compare_refs local HEAD server refs/heads/new-refspec
 '
 
 test_done
-- 
1.8.0.1

--
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 v7 p1 06/13] remote-testgit: remove non-local functionality

2012-11-28 Thread Felipe Contreras
This only makes sense for the python remote helpers framework. The tests
don't exercise any feature of transport helper. Remove them.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 git-remote-testgit|  3 ---
 t/t5801-remote-helpers.sh | 50 ---
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index 5042f5a..5117ab5 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -4,9 +4,6 @@
 alias=$1
 url=$2
 
-# huh?
-url="${url#file://}"
-
 dir="$GIT_DIR/testgit/$alias"
 prefix="refs/testgit/$alias"
 refspec="refs/heads/*:${prefix}/heads/*"
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index f52ab14..2f7fc10 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -33,11 +33,6 @@ test_expect_success 'cloning from local repo' '
test_cmp public/file localclone/file
 '
 
-test_expect_success 'cloning from remote repo' '
-   git clone "testgit::file://${PWD}/server" clone &&
-   test_cmp public/file clone/file
-'
-
 test_expect_success 'create new commit on remote' '
(cd public &&
 echo content >>file &&
@@ -50,11 +45,6 @@ test_expect_success 'pulling from local repo' '
test_cmp public/file localclone/file
 '
 
-test_expect_success 'pulling from remote remote' '
-   (cd clone && git pull) &&
-   test_cmp public/file clone/file
-'
-
 test_expect_success 'pushing to local repo' '
(cd localclone &&
echo content >>file &&
@@ -76,19 +66,6 @@ test_expect_success 'pushing to local repo' '
compare_refs localclone2 HEAD server2 HEAD
 '
 
-test_expect_success 'synch with changes from localclone' '
-   (cd clone &&
-git pull)
-'
-
-test_expect_success 'pushing remote local repo' '
-   (cd clone &&
-   echo content >>file &&
-   git commit -a -m four &&
-   git push) &&
-   compare_refs clone HEAD server HEAD
-'
-
 test_expect_success 'fetch new branch' '
(cd public &&
 git checkout -b new &&
@@ -102,6 +79,20 @@ test_expect_success 'fetch new branch' '
compare_refs public HEAD localclone FETCH_HEAD
 '
 
+#
+# This is only needed because of a bug not detected by this script. It will be
+# fixed shortly, but for now lets not cause regressions.
+#
+test_expect_success 'bump commit in public' '
+   (cd public &&
+   git checkout master &&
+   git pull &&
+   echo content >>file &&
+   git commit -a -m four &&
+   git push) &&
+   compare_refs public HEAD server HEAD
+'
+
 test_expect_success 'fetch multiple branches' '
(cd localclone &&
 git fetch
@@ -111,29 +102,30 @@ test_expect_success 'fetch multiple branches' '
 '
 
 test_expect_success 'push when remote has extra refs' '
-   (cd clone &&
+   (cd localclone &&
+git reset --hard origin/master &&
 echo content >>file &&
 git commit -a -m six &&
 git push
) &&
-   compare_refs clone master server master
+   compare_refs localclone master server master
 '
 
 test_expect_success 'push new branch by name' '
-   (cd clone &&
+   (cd localclone &&
 git checkout -b new-name  &&
 echo content >>file &&
 git commit -a -m seven &&
 git push origin new-name
) &&
-   compare_refs clone HEAD server refs/heads/new-name
+   compare_refs localclone HEAD server refs/heads/new-name
 '
 
 test_expect_failure 'push new branch with old:new refspec' '
-   (cd clone &&
+   (cd localclone &&
 git push origin new-name:new-refspec
) &&
-   compare_refs clone HEAD server refs/heads/new-refspec
+   compare_refs localclone HEAD server refs/heads/new-refspec
 '
 
 test_done
-- 
1.8.0.1

--
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 v7 p1 05/13] Add new simplified git-remote-testgit

2012-11-28 Thread Felipe Contreras
Exercising the python remote helper framework is for another tool and
another test. This is about testing the remote-helper interface.

It's way simpler, it exercises the same features of remote helpers, it's
easy to read and understand, and it doesn't depend on python.

For now let's just copy the old remote-helpers test script, although
some of those tests don't make sense. In addition, this script would be
able to test other features not currently being tested.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-remote-testgit.txt |   2 +-
 git-remote-testgit   |  64 
 t/t5801-remote-helpers.sh| 139 +++
 3 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100755 git-remote-testgit
 create mode 100755 t/t5801-remote-helpers.sh

diff --git a/Documentation/git-remote-testgit.txt 
b/Documentation/git-remote-testgit.txt
index 2a67d45..612a625 100644
--- a/Documentation/git-remote-testgit.txt
+++ b/Documentation/git-remote-testgit.txt
@@ -19,7 +19,7 @@ testcase for the remote-helper functionality, and as an 
example to
 show remote-helper authors one possible implementation.
 
 The best way to learn more is to read the comments and source code in
-'git-remote-testgit.py'.
+'git-remote-testgit'.
 
 SEE ALSO
 
diff --git a/git-remote-testgit b/git-remote-testgit
new file mode 100755
index 000..5042f5a
--- /dev/null
+++ b/git-remote-testgit
@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+# Copyright (c) 2012 Felipe Contreras
+
+alias=$1
+url=$2
+
+# huh?
+url="${url#file://}"
+
+dir="$GIT_DIR/testgit/$alias"
+prefix="refs/testgit/$alias"
+refspec="refs/heads/*:${prefix}/heads/*"
+
+gitmarks="$dir/git.marks"
+testgitmarks="$dir/testgit.marks"
+
+export GIT_DIR="$url/.git"
+
+mkdir -p "$dir"
+
+test -e "$gitmarks" || > "$gitmarks"
+test -e "$testgitmarks" || > "$testgitmarks"
+
+while read line
+do
+   case $line in
+   capabilities)
+   echo 'import'
+   echo 'export'
+   echo "refspec $refspec"
+   echo "*import-marks $gitmarks"
+   echo "*export-marks $gitmarks"
+   echo
+   ;;
+   list)
+   git for-each-ref --format='? %(refname)' 'refs/heads/'
+   head=$(git symbolic-ref HEAD)
+   echo "@$head HEAD"
+   echo
+   ;;
+   import*)
+   # read all import lines
+   while true
+   do
+   ref="${line#* }"
+   refs="$refs $ref"
+   read line
+   test "${line%% *}" != "import" && break
+   done
+
+   echo "feature import-marks=$gitmarks"
+   echo "feature export-marks=$gitmarks"
+   git fast-export --use-done-feature 
--{import,export}-marks="$testgitmarks" $refs |
+   sed -e "s#refs/heads/#${prefix}/heads/#g"
+   ;;
+   export)
+   git fast-import --{import,export}-marks="$testgitmarks" --quiet
+   echo
+   ;;
+   '')
+   exit
+   ;;
+   esac
+done
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
new file mode 100755
index 000..f52ab14
--- /dev/null
+++ b/t/t5801-remote-helpers.sh
@@ -0,0 +1,139 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Sverre Rabbelier
+#
+
+test_description='Test remote-helper import and export commands'
+
+. ./test-lib.sh
+
+if ! type "${BASH-bash}" >/dev/null 2>&1; then
+   skip_all='skipping remote-testgit tests, bash not available'
+   test_done
+fi
+
+compare_refs() {
+   git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
+   git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
+   test_cmp expect actual
+}
+
+test_expect_success 'setup repository' '
+   git init --bare server/.git &&
+   git clone server public &&
+   (cd public &&
+echo content >file &&
+git add file &&
+git commit -m one &&
+git push origin master)
+'
+
+test_expect_success 'cloning from local repo' '
+   git clone "testgit::${PWD}/server" localclone &&
+   test_cmp public/file localclone/file
+'
+
+test_expect_success 'cloning from remote repo' '
+   git clone "testgit::file://${PWD}/server" clone &&
+   test_cmp public/file clone/file
+'
+
+test_expect_success 'create new commit on remote' '
+   (cd public &&
+echo content >>file &&
+git commit -a -m two &&
+git push)
+'
+
+test_expect_success 'pulling from local repo' '
+   (cd localclone && git pull) &&
+   test_cmp public/file localclone/file
+'
+
+test_expect_success 'pulling from remote remote' '
+   (cd clone && git pull) &&
+   test_cmp public/file clone/file
+'
+
+test_expect_success 'pushing to local repo' '
+   (cd localclone &&
+   echo content >>file &&
+   git co

[PATCH v7 p1 02/13] remote-testgit: fix direction of marks

2012-11-28 Thread Felipe Contreras
Basically this is what we want:

  == pull ==

testgit transport-helper

* export -> import

# testgit.marks git.marks

  == push ==

testgit transport-helper

* import<- export

# testgit.marks git.marks

Each side should be agnostic of the other side. Because testgit.marks
(our helper marks) could be anything, not necessarily a format parsable
by fast-export or fast-import. In this test they happen to be compatible,
because we use those tools, but in the real world it would be something
completely different. For example, they might be mapping marks to
mercurial revisions (certainly not parsable by fast-import/export).

This is what we have:

  == pull ==

testgit transport-helper

* export -> import

# testgit.marks git.marks

  == push ==

testgit transport-helper

* import<- export

# git.marks testgit.marks

The only reason this is working is that git.marks and testgit.marks are
roughly the same.

This new behavior used to not be possible before due to a bug in
fast-export, but with the bug fixed, it works fine.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 git-remote-testgit.py  | 2 +-
 git_remote_helpers/git/importer.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..ade797b 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -91,7 +91,7 @@ def do_capabilities(repo, args):
 if not os.path.exists(dirname):
 os.makedirs(dirname)
 
-path = os.path.join(dirname, 'testgit.marks')
+path = os.path.join(dirname, 'git.marks')
 
 print "*export-marks %s" % path
 if os.path.exists(path):
diff --git a/git_remote_helpers/git/importer.py 
b/git_remote_helpers/git/importer.py
index 5c6b595..e28cc8f 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -39,7 +39,7 @@ class GitImporter(object):
 gitdir = self.repo.gitpath
 else:
 gitdir = os.path.abspath(os.path.join(dirname, '.git'))
-path = os.path.abspath(os.path.join(dirname, 'git.marks'))
+path = os.path.abspath(os.path.join(dirname, 'testgit.marks'))
 
 if not os.path.exists(dirname):
 os.makedirs(dirname)
-- 
1.8.0.1

--
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 v7 p1 03/13] remote-helpers: fix failure message

2012-11-28 Thread Felipe Contreras
This is remote-testgit, not remote-hg.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 t/t5800-remote-helpers.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index e7dc668..d46fa40 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -8,7 +8,7 @@ test_description='Test remote-helper import and export commands'
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON ; then
-   skip_all='skipping git-remote-hg tests, python not available'
+   skip_all='skipping remote-testgit tests, python not available'
test_done
 fi
 
@@ -17,7 +17,7 @@ import sys
 if sys.hexversion < 0x0204:
 sys.exit(1)
 ' || {
-   skip_all='skipping git-remote-hg tests, python version < 2.4'
+   skip_all='skipping remote-testgit tests, python version < 2.4'
test_done
 }
 
-- 
1.8.0.1

--
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 v7 p1 01/13] fast-export: avoid importing blob marks

2012-11-28 Thread Felipe Contreras
We want to be able to import, and then export, using the same marks, so
that we don't push things that the other side already received.

Unfortunately, fast-export doesn't store blobs in the marks, but
fast-import does. This creates a mismatch when fast export is reusing a
mark that was previously stored by fast-import.

There is no point in one tool saving blobs, and the other not, but for
now let's just check in fast-export that the objects are indeed commits.

Signed-off-by: Felipe Contreras 
Signed-off-by: Junio C Hamano 
---
 builtin/fast-export.c  |  4 
 t/t9350-fast-export.sh | 14 ++
 2 files changed, 18 insertions(+)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 12220ad..9b70ec1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -614,6 +614,10 @@ static void import_marks(char *input_file)
if (object->flags & SHOWN)
error("Object %s already has a mark", 
sha1_to_hex(sha1));
 
+   if (object->type != OBJ_COMMIT)
+   /* only commits */
+   continue;
+
mark_object(object, mark);
if (last_idnum < mark)
last_idnum = mark;
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 3e821f9..5948b65 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -440,4 +440,18 @@ test_expect_success 'fast-export quotes pathnames' '
)
 '
 
+test_expect_success 'test bidirectionality' '
+   >marks-cur &&
+   >marks-new &&
+   git init marks-test &&
+   git fast-export --export-marks=marks-cur --import-marks=marks-cur 
--branches | \
+   git --git-dir=marks-test/.git fast-import --export-marks=marks-new 
--import-marks=marks-new &&
+   (cd marks-test &&
+   git reset --hard &&
+   echo Wohlauf > file &&
+   git commit -a -m "back in time") &&
+   git --git-dir=marks-test/.git fast-export --export-marks=marks-new 
--import-marks=marks-new --branches | \
+   git fast-import --export-marks=marks-cur --import-marks=marks-cur
+'
+
 test_done
-- 
1.8.0.1

--
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 v7 p1 00/13] fast-export and remote-testgit improvements

2012-11-28 Thread Felipe Contreras
Hi,

Basically the same as v6, except the last patch was dropped, and testgit
switched to '#!/usr/bin/env bash'.

Shouldn't break any tests now.

Felipe Contreras (13):
  fast-export: avoid importing blob marks
  remote-testgit: fix direction of marks
  remote-helpers: fix failure message
  Rename git-remote-testgit to git-remote-testpy
  Add new simplified git-remote-testgit
  remote-testgit: remove non-local functionality
  remote-testgit: remove irrelevant test
  remote-testgit: cleanup tests
  remote-testgit: exercise more features
  remote-testgit: report success after an import
  remote-testgit: implement the "done" feature manually
  fast-export: trivial cleanup
  fast-export: fix comparison in tests

 .gitignore   |   2 +-
 Documentation/git-remote-testgit.txt |   2 +-
 Makefile |   2 +-
 builtin/fast-export.c|   6 +-
 git-remote-testgit   |  90 
 git-remote-testgit.py| 272 ---
 git-remote-testpy.py | 272 +++
 git_remote_helpers/git/importer.py   |   2 +-
 t/t5800-remote-helpers.sh| 148 ---
 t/t5800-remote-testpy.sh | 148 +++
 t/t5801-remote-helpers.sh| 165 +
 t/t9350-fast-export.sh   |  20 ++-
 12 files changed, 701 insertions(+), 428 deletions(-)
 create mode 100755 git-remote-testgit
 delete mode 100644 git-remote-testgit.py
 create mode 100644 git-remote-testpy.py
 delete mode 100755 t/t5800-remote-helpers.sh
 create mode 100755 t/t5800-remote-testpy.sh
 create mode 100755 t/t5801-remote-helpers.sh

-- 
1.8.0.1

--
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 0/5] jk/send-email-sender-prompt loose ends

2012-11-28 Thread Felipe Contreras
On Wed, Nov 28, 2012 at 7:25 PM, Jeff King  wrote:
> Here are the cleanups and refactorings split out from my
> jk/send-email-sender-prompt series. They can go right on master and are
> independent of Felipe's fc/send-email-no-sender-prompt topic.
>
>   [1/5]: test-lib: allow negation of prerequisites
>
> Same as before. I think this is a useful feature for the test suite (and
> 2/5 depends on it).
>
>   [2/5]: t7502: factor out autoident prerequisite
>
> Same as before. Patch 5/5 depends on it.
>
>   [3/5]: ident: make user_ident_explicitly_given static
>
> Same as before. Cleanup.
>
>   [4/5]: ident: keep separate "explicit" flags for author and committer
>
> Same as before. Cleanup. I do not know if anybody will ever care about
> the corner cases it fixes, so it is really just being defensive for
> future code.
>
>   [5/5]: t: add tests for "git var"
>
> Tests split out from he "git var can take multiple values" patch.
>
> Dropped were:
>
>   - "git var" can take multiple values. Nobody really cares about doing
> so, and it's an external interface, so we'd have to support it
> forever.
>
>   - exporting "explicit ident" flag via "git var"; same reasoning as
> above
>
>   - Git.pm supporting explicit ident; ditto
>
>   - send-email prompting change; obsoleted by Felipe's patch

For what it's worth; they look good to me. However, I think it's worth
mentioning that there are no functional changes.

-- 
Felipe Contreras
--
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] fsck: warn about ".git" in trees

2012-11-28 Thread Jeff King
On Wed, Nov 28, 2012 at 01:25:05PM -0800, Junio C Hamano wrote:

> >> * jk/fsck-dot-in-trees (2012-11-28) 1 commit
> >>  - fsck: warn about '.' and '..' in trees
> >> 
> >>  Will merge to 'next'.
> >
> > Do you have an opinion on warning about '.git', as well? It probably
> > would make more sense as a patch on top, but I thought I'd ask before
> > this got merged to next.
> 
> Yeah, it would make sense to reject what we would not record
> ourselves when the tools are used in a sane manner.

Here's the patch on top of jk/fsck-dot-in-trees.

-- >8 --
Subject: [PATCH] fsck: warn about ".git" in trees

Having a ".git" entry inside a tree can cause confusing
results on checkout. At the top-level, you could not
checkout such a tree, as it would complain about overwriting
the real ".git" directory. In a subdirectory, you might
check it out, but performing operations in the subdirectory
would confusingly consider the in-tree ".git" directory as
the repository.

The regular git tools already make it hard to accidentally
add such an entry to a tree, and do not allow such entries
to enter the index at all. Teaching fsck about it provides
an additional safety check, and let's us avoid propagating
any such bogosity when transfer.fsckObjects is on.

Signed-off-by: Jeff King 
---
 fsck.c  |  5 +
 t/t1450-fsck.sh | 15 +++
 2 files changed, 20 insertions(+)

diff --git a/fsck.c b/fsck.c
index 31c9a51..99c0497 100644
--- a/fsck.c
+++ b/fsck.c
@@ -144,6 +144,7 @@ static int fsck_tree(struct tree *item, int strict, 
fsck_error error_func)
int has_empty_name = 0;
int has_dot = 0;
int has_dotdot = 0;
+   int has_dotgit = 0;
int has_zero_pad = 0;
int has_bad_modes = 0;
int has_dup_entries = 0;
@@ -174,6 +175,8 @@ static int fsck_tree(struct tree *item, int strict, 
fsck_error error_func)
has_dot = 1;
if (!strcmp(name, ".."))
has_dotdot = 1;
+   if (!strcmp(name, ".git"))
+   has_dotgit = 1;
has_zero_pad |= *(char *)desc.buffer == '0';
update_tree_entry(&desc);
 
@@ -227,6 +230,8 @@ static int fsck_tree(struct tree *item, int strict, 
fsck_error error_func)
retval += error_func(&item->object, FSCK_WARN, "contains '.'");
if (has_dotdot)
retval += error_func(&item->object, FSCK_WARN, "contains '..'");
+   if (has_dotgit)
+   retval += error_func(&item->object, FSCK_WARN, "contains 
'.git'");
if (has_zero_pad)
retval += error_func(&item->object, FSCK_WARN, "contains 
zero-padded file modes");
if (has_bad_modes)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0b5c30b..d730734 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -253,4 +253,19 @@ test_expect_success 'fsck notices "." and ".." in trees' '
)
 '
 
+test_expect_success 'fsck notices ".git" in trees' '
+   (
+   git init dotgit &&
+   cd dotgit &&
+   blob=$(echo foo | git hash-object -w --stdin) &&
+   tab=$(printf "\\t") &&
+   git mktree <<-EOF &&
+   100644 blob $blob$tab.git
+   EOF
+   git fsck 2>out &&
+   cat out &&
+   grep "warning.*\\.git" out
+   )
+'
+
 test_done
-- 
1.8.0.207.gdf2154c

--
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] svnrdump_sim: start the script with /usr/bin/env python

2012-11-28 Thread Felipe Contreras
On Wed, Nov 28, 2012 at 8:33 PM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano  wrote:
>> ...
>>> You need a fix for that; didn't I already say "you need a bit more
>>> than that"?
>>
>> I disagree. Most of the contrib scripts are expected to be used as
>> they are.
>
> You are only looking at one of the uses for this script, when there
> are two.
>
> You are correct that distros may install with whatever tweaks of
> their own, and to help their tweak process (like the one that
> specifically notices "/usr/bin/env python" as you wrote), changing
> the "#!/usr/bin/python" to match others would be a good change.
>
> But that change alone is not sufficient for this one, which is used
> from t/ script.  You cannot treat this one like import-zips and
> hg-to-git that we do not use in-tree.  Somewhere before t9020 uses
> it, it needs the treatment similar to the rewriting that is done for
> git-p4.py to git-p4.

Unless the tests are moved to contrib, which I think is a good
practice: should anything in contrib break 'make test'? I don't think
so.

Cheers.

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


Re: What's cooking in git.git (Nov 2012, #09; Wed, 28)

2012-11-28 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Nov 28, 2012 at 11:54:27AM -0800, Junio C Hamano wrote:
>
>> * jk/fsck-dot-in-trees (2012-11-28) 1 commit
>>  - fsck: warn about '.' and '..' in trees
>> 
>>  Will merge to 'next'.
>
> Do you have an opinion on warning about '.git', as well? It probably
> would make more sense as a patch on top, but I thought I'd ask before
> this got merged to next.

Yeah, it would make sense to reject what we would not record
ourselves when the tools are used in a sane manner.

>> * pf/editor-ignore-sigint (2012-11-11) 5 commits
>>  - launch_editor: propagate SIGINT from editor to git
>>  - run-command: do not warn about child death by SIGINT
>>  - run-command: drop silent_exec_failure arg from wait_or_whine
>>  - launch_editor: ignore SIGINT while the editor has control
>>  - launch_editor: refactor to use start/finish_command
>> 
>>  Avoid confusing cases where the user hits Ctrl-C while in the editor
>>  session, not realizing git will receive the signal. Since most editors
>>  will take over the terminal and will block SIGINT, this is not likely
>>  to confuse anyone.
>> 
>>  Some people raised issues with emacsclient, which are addressed by this
>>  re-roll. It should probably also handle SIGQUIT, and there were a
>>  handful of other review comments.
>> 
>>  Expecting a re-roll.
>
> I'm slowly going through my post-travel/vacation/illness backlog. I hope
> to re-roll this one today or tomorrow.

Thanks.

>> * jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
>>  - config: exit on error accessing any config file
>>  - doc: advertise GIT_CONFIG_NOSYSTEM
>>  - config: treat user and xdg config permission problems as errors
>>  - config, gitignore: failure to access with ENOTDIR is ok
>> 
>>  An RFC to deal with a situation where .config/git is a file and we
>>  notice .config/git/config is not readable due to ENOTDIR, not
>>  ENOENT; I think a bit more refactored approach to consistently
>>  address permission errors across config, exclude and attrs is
>>  desirable.  Don't we also need a check for an opposite situation
>>  where we open .config/git/config or .gitattributes for reading but
>>  they turn out to be directories?
>
> I am not sure about the refactored approach you mention. We
> fundamentally need to treat in-tree attributes and exclude files more
> leniently, because we may find arbitrary paths in the tree.

OK.

> As far as the opposite situation, I do not know if it is worth worrying
> about. If $GIT_DIR/config or $HOME/.config/git/config is a directory,
> that is an error and we should flag it[1]. In-tree is more hazy, but I
> think complaining is still the right thing. You cannot expect to store
> arbitrary crap at .gitattributes inside your tree. If you have crap in a
> file at such a path, we would read it and complain when its syntax is
> not that of a .gitattributes file. We should similarly complain when it
> is a directory.

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


Re: git-svn: What is --follow-parent / --no-follow-parent for?

2012-11-28 Thread Eric Wong
Sebastian Leske  wrote:
> However, this does not make sense to me: This sounds like there is no
> good reason *not* to enable this option.  So why is it there? And in
> what situation might I want to use "--no-follow-parent"?

Speed.  Following long/convoluted histories can take a long time.
Sometimes the user doesn't care about ancient history.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

2012-11-28 Thread Simon Oosthoek
On 28/11/12 21:47, Junio C Hamano wrote:
> Simon Oosthoek  writes:
> 
>> perhaps the point should read like this:
>> #3a) In ~/.bashrc set PROMPT_COMMAND
>> #To customize the prompt, provide start/end arguments
>> #PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>>
>> Which would not be confusing at all, I think...
> 
> It says "to customize", so a user who just wants the default (which
> does not exist but the comment does not say so) would be left
> without instruction, no?
> 
> In $HOME/.bashrc, PROMPT_COMMAND can be set to
> '__git_ps1  ', where  and 
> are strings you would put in $PS1 before and after
> the status string generated by git-prompt machinery.
> e.g.
> PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> or something?
> 

Looks better than my suggestion :-)

thanks

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


Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

2012-11-28 Thread Junio C Hamano
Simon Oosthoek  writes:

> perhaps the point should read like this:
> #3a) In ~/.bashrc set PROMPT_COMMAND
> #To customize the prompt, provide start/end arguments
> #PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>
> Which would not be confusing at all, I think...

It says "to customize", so a user who just wants the default (which
does not exist but the comment does not say so) would be left
without instruction, no?

In $HOME/.bashrc, PROMPT_COMMAND can be set to
'__git_ps1  ', where  and 
are strings you would put in $PS1 before and after
the status string generated by git-prompt machinery.
e.g.
PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'

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


Re: [PATCH 6/5] t9001: check send-email behavior with implicit sender

2012-11-28 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Nov 28, 2012 at 03:06:26PM -0500, Jeff King wrote:
>
>> Here's a cleaned up version that makes it more obvious the commands are
>> the same (it also fixes a few minor whitespace problems on the
>> indentation, which you can see from the quoting above).
>
> I wondered how painful it would be to actually run the command and then
> conditionally check the results inside the test. I ended up with this:
> ...
> which does work, though it is less nice that the protections and nice
> syntax of "test_must_fail" must be abandoned (unless we want to do
> something horrible like 'test_must_fail sh -c "exit $exit_code"'.
> I could go either way.

A change along that line did cross my mind, and I agree that it does
clarify that we are testing the same command produces an expected
result, the expectation may be different depending on external
conditions, and we can figure out what the expected values are
before running the test.  I do not think we use such a pattern in
very many places, though.

The differences in "expected results" are generally not limited to
the final outcome in $? (e.g. your "else" side not just checks $?
indicates an error, but makes sure that we got an expected error
message), which means that the if statement at the end has to be
there in some form anyway, which in turn means that a new test
helper function to abstract this pattern out further would not
buy us very much (either it would be too complex and bug prone to
implement, or it would be too simplistic and limited).

I'd prefer to leave these as two separate tests for now like your
previous patch.  People interested in consolidating/simplifying can
first audit the existing tests to find other instances of this
pattern to see if it is worth doing, and then come up with an
abstraction to be shared among them.

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: What's cooking in git.git (Nov 2012, #09; Wed, 28)

2012-11-28 Thread Jeff King
On Wed, Nov 28, 2012 at 11:54:27AM -0800, Junio C Hamano wrote:

> * jk/fsck-dot-in-trees (2012-11-28) 1 commit
>  - fsck: warn about '.' and '..' in trees
> 
>  Will merge to 'next'.

Do you have an opinion on warning about '.git', as well? It probably
would make more sense as a patch on top, but I thought I'd ask before
this got merged to next.

> * pf/editor-ignore-sigint (2012-11-11) 5 commits
>  - launch_editor: propagate SIGINT from editor to git
>  - run-command: do not warn about child death by SIGINT
>  - run-command: drop silent_exec_failure arg from wait_or_whine
>  - launch_editor: ignore SIGINT while the editor has control
>  - launch_editor: refactor to use start/finish_command
> 
>  Avoid confusing cases where the user hits Ctrl-C while in the editor
>  session, not realizing git will receive the signal. Since most editors
>  will take over the terminal and will block SIGINT, this is not likely
>  to confuse anyone.
> 
>  Some people raised issues with emacsclient, which are addressed by this
>  re-roll. It should probably also handle SIGQUIT, and there were a
>  handful of other review comments.
> 
>  Expecting a re-roll.

I'm slowly going through my post-travel/vacation/illness backlog. I hope
to re-roll this one today or tomorrow.

> * jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
>  - config: exit on error accessing any config file
>  - doc: advertise GIT_CONFIG_NOSYSTEM
>  - config: treat user and xdg config permission problems as errors
>  - config, gitignore: failure to access with ENOTDIR is ok
> 
>  An RFC to deal with a situation where .config/git is a file and we
>  notice .config/git/config is not readable due to ENOTDIR, not
>  ENOENT; I think a bit more refactored approach to consistently
>  address permission errors across config, exclude and attrs is
>  desirable.  Don't we also need a check for an opposite situation
>  where we open .config/git/config or .gitattributes for reading but
>  they turn out to be directories?

I am not sure about the refactored approach you mention. We
fundamentally need to treat in-tree attributes and exclude files more
leniently, because we may find arbitrary paths in the tree. Whereas if
something in $GIT_DIR is inaccessible, it's probably a serious problem.
So I think we have to manually use access_or_{warn,die} from each
callsite.

As far as the opposite situation, I do not know if it is worth worrying
about. If $GIT_DIR/config or $HOME/.config/git/config is a directory,
that is an error and we should flag it[1]. In-tree is more hazy, but I
think complaining is still the right thing. You cannot expect to store
arbitrary crap at .gitattributes inside your tree. If you have crap in a
file at such a path, we would read it and complain when its syntax is
not that of a .gitattributes file. We should similarly complain when it
is a directory.

-Peff

[1] We might want to eventually allow "config directories" where we
would read all files in lexical order or something. So it is
tempting to think of ignoring such entries as a
forward-compatibility thing. But ignoring is the wrong thing; you
probably would not want an old version of git to _ignore_ your
config; it is better if it says "I am too old to understand your
config format".
--
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 v6 07/16] remote-hg: add support for hg-git compat mode

2012-11-28 Thread W. Trevor King
I'm not sure if this is the most recent patch iteration for this
feature, but I just saw this typo in `pu`.

On Sun, Nov 04, 2012 at 03:13:29AM +0100, Felipe Contreras wrote:
> +# Commits are modified to preserve hg information and allow biridectionality.
   
s/biridectionality/bidirectionality/

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git config key bug or by design?

2012-11-28 Thread Jeff King
On Wed, Nov 28, 2012 at 07:11:47AM -0500, Peter van der Does wrote:

> I am writing a tool, it needs to store branch names in a separate config
> file.
> 
> It's clear git doesn't respect those values, hence my question. I
> understand how to work around the problem, I would just prefix the key.
> I was just wondering if it was by design, which I guess it is as the
> parsing of the file will die if the key starts with a non-alpha
> character.

In that case, I would definitely say to use some prefix section like
"section.$branchname.key". That is how git stores per-branch information
(e.g., branch.master.merge), and it was always designed to let there be
arbitrary data in the "middle" section, whereas the section and key are
restricted and case-insensitive.

So no, I do not recall "cannot start with number" as a specific design
decision, but it is definitely a design decision that the section name
would not allow arbitrary content.

-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 6/5] t9001: check send-email behavior with implicit sender

2012-11-28 Thread Jeff King
On Wed, Nov 28, 2012 at 03:06:26PM -0500, Jeff King wrote:

> Here's a cleaned up version that makes it more obvious the commands are
> the same (it also fixes a few minor whitespace problems on the
> indentation, which you can see from the quoting above).

I wondered how painful it would be to actually run the command and then
conditionally check the results inside the test. I ended up with this:

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c5d66cf..9c8fac1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -201,6 +201,30 @@ test_expect_success $PREREQ 'Prompting works' '
grep "^To: t...@example.com\$" msgtxt1
 '
 
+test_expect_success $PREREQ 'handle implicit ident' '
+   clean_fake_sendmail &&
+   (
+   sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
+   {
+   git send-email \
+   --smtp-server="$(pwd)/fake.sendmail" \
+   --to=t...@example.com \
+   $patches errors;
+   exit_code=$?
+   } &&
+   if test_have_prereq AUTOIDENT; then
+   test $exit_code = 0
+   else
+   test $exit_code != 0 &&
+   test_i18ngrep "tell me who you are" errors
+   fi
+   )
+'
+
 test_expect_success $PREREQ 'tocmd works' '
clean_fake_sendmail &&
cp $patches tocmd.patch &&

which does work, though it is less nice that the protections and nice
syntax of "test_must_fail" must be abandoned (unless we want to do
something horrible like 'test_must_fail sh -c "exit $exit_code"'.
I could go either way.

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


Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

2012-11-28 Thread Simon Oosthoek
On 28/11/12 19:02, Piotr Krukowiecki wrote:
> Is your setting?:
> PROMPT_COMMAND=__git_ps1
> 
> 
> That's right
>  
> 
> I believe you need to give 2 parameters in order to use it in
> PROMPT_COMMAND mode.
> 
> 
> Are you sure? git-prompt.sh says:
> 
> #3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
> #To customize the prompt, provide start/end arguments
> #PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> I interpret this as: if you don't want to customize the prompt, the
> simple "PROMPT_COMMAND=__git_ps1" is enough.
>  

I'm sure, although you're right this documentation is wrong...
I guess it must have slipped by when the final changes were made to the
code. (The requirement to have 2 arguments for PROMPT_COMMAND mode were
one of the last changes before being accepted into the release process.)

I've been too busy with other stuff to take another look at this in the
meantime.

perhaps the point should read like this:
#3a) In ~/.bashrc set PROMPT_COMMAND
#To customize the prompt, provide start/end arguments
#PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'

Which would not be confusing at all, I think...

> 
> 
> These last 2 lines say: if 2 arguments are given, use pcmode.
> Otherwise you get command-subtitution mode, which gives weird
> effects when being called from PROMPT_COMMAND.
> 
> 
> Still, it seems to work with above "patch"..

It only works in your specific case, since you're expecting the branch
info before the rest of your prompt. The output comes from the part of
the code that is meant for substitution mode ;-)

Cheers

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


Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option

2012-11-28 Thread Junio C Hamano
"W. Trevor King"  writes:

>> As the command takes other options whose names begin with 'r', I
>> thought the longer term plan was to stop letting "--rebase" squat on
>> short and sweet "-r" and leaving it undocumented (even though the
>> short one was added by mistake) was meant to be the first step in
>> that process.
>> 
>> But maybe I am confusing an undocumented single-letter option from
>> some other subcommand.  Anybody remembers?
>
> Perhaps you are remembering:
>
> On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote:
>> Ah, this reminds me of another thing I noticed when I saw that
>> ...

No.  The discussion might or might not be the "-r" option to
"submodule update", but even if it were so, I wasn't refering to
that exchange but something more in the further 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


Re: [PATCH 6/5] t9001: check send-email behavior with implicit sender

2012-11-28 Thread Jeff King
On Wed, Nov 28, 2012 at 10:55:02AM -0800, Junio C Hamano wrote:

> > +test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
> > +   clean_fake_sendmail &&
> > +   (sane_unset GIT_AUTHOR_NAME &&
> > +   sane_unset GIT_AUTHOR_EMAIL &&
> > +   sane_unset GIT_COMMITTER_NAME &&
> > +   sane_unset GIT_COMMITTER_EMAIL &&
> > +   GIT_SEND_EMAIL_NOTTY=1 git send-email \
> > +   --smtp-server="$(pwd)/fake.sendmail" \
> > +   --to=t...@example.com \
> > +   $patches \
> > +   errors
> > +   )
> > +'
> > +
> > +test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts 
> > send-email' '
> > +   clean_fake_sendmail &&
> > +   (sane_unset GIT_AUTHOR_NAME &&
> > +   sane_unset GIT_AUTHOR_EMAIL &&
> > +   sane_unset GIT_COMMITTER_NAME &&
> > +   sane_unset GIT_COMMITTER_EMAIL &&
> > +   GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
> > +   test_must_fail git send-email \
> > +   --smtp-server="$(pwd)/fake.sendmail" \
> > +   $patches errors &&
> > +   test_i18ngrep "tell me who you are" errors
> > +   )
> > +'
> 
> The difference between these two tests should solely come from
> AUTOIDENT and nothing else, so it would be better to see their
> command line arguments to match; the former is with --to and the
> latter is without in this patch but I do not think you meant them to
> differ that way.

Yeah, that makes sense. The top one originally was testing that we
still prompted in the autoident case, and so had some echos piped into
send-email. I simplified it to use --to, but I agree it is even better
to show that the commands are identical.

Here's a cleaned up version that makes it more obvious the commands are
the same (it also fixes a few minor whitespace problems on the
indentation, which you can see from the quoting above).

-- >8 --
Subject: [PATCH] t9001: check send-email behavior with implicit sender

We allow send-email to use an implicitly-defined identity
for the sender (because there is still a confirmation step),
but we abort when we cannot generate such an identity. Let's
make sure that we test this.

Signed-off-by: Jeff King 
---
 t/t9001-send-email.sh | 28 
 1 file changed, 28 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c5d66cf..97d6f4c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -201,6 +201,34 @@ test_expect_success $PREREQ 'Prompting works' '
grep "^To: t...@example.com\$" msgtxt1
 '
 
+test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
+   clean_fake_sendmail &&
+   (sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   GIT_SEND_EMAIL_NOTTY=1 git send-email \
+   --smtp-server="$(pwd)/fake.sendmail" \
+   --to=t...@example.com \
+   $patches errors
+   )
+'
+
+test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts 
send-email' '
+   clean_fake_sendmail &&
+   (sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
+   test_must_fail git send-email \
+   --smtp-server="$(pwd)/fake.sendmail" \
+   --to=t...@example.com \
+   $patches errors &&
+   test_i18ngrep "tell me who you are" errors
+   )
+'
+
 test_expect_success $PREREQ 'tocmd works' '
clean_fake_sendmail &&
cp $patches tocmd.patch &&
-- 
1.8.0.207.gdf2154c

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


What's cooking in git.git (Nov 2012, #09; Wed, 28)

2012-11-28 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Hopefully 1.8.1-rc0 preview will be tagged this weekend.  Many
topics are marked to be cooked in 'next' during the feature freeze,
but some topics in flight should be in 'master' before -rc1 happens.

You can find the changes described here in the integration branches of the
repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* bc/append-signed-off-by (2012-11-26) 11 commits
 - Unify appending signoff in format-patch, commit and sequencer
 - format-patch: update append_signoff prototype
 - format-patch: stricter S-o-b detection
 - t4014: more tests about appending s-o-b lines
 - sequencer.c: teach append_signoff to avoid adding a duplicate newline
 - sequencer.c: teach append_signoff how to detect duplicate s-o-b
 - sequencer.c: always separate "(cherry picked from" from commit body
 - sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
 - t/t3511: add some tests of 'cherry-pick -s' functionality
 - t/test-lib-functions.sh: allow to specify the tag name to test_commit
 - sequencer.c: remove broken support for rfc2822 continuation in footer

 Will merge to 'next'.


* er/doc-add-new-commands (2012-11-26) 1 commit
 - Documentation: how to add a new command

 Will merge to 'next'.


* fc/send-email-no-sender-prompt (2012-11-26) 1 commit
 - send-email: avoid questions when user has an ident
 (this branch is used by jk/send-email-sender-prompt.)

 Will merge to 'next'.


* jl/submodule-rm (2012-11-23) 1 commit
  (merged to 'next' on 2012-11-28 at 0e4115f)
 + Teach rm to remove submodules when given with a trailing '/'

 Finishing touches to the topic already in 'master'.
 Will merge to 'master'.


* km/send-email-remove-cruft-in-address (2012-11-26) 5 commits
 - git-send-email: allow edit invalid email address
 - git-send-email: ask what to do with an invalid email address
 - git-send-email: remove invalid addresses earlier
 - git-send-email: fix fallback code in extract_valid_address()
 - git-send-email: remove garbage after email address

 Will merge to 'next'.


* mh/unify-xml-in-imap-send-and-http-push (2012-11-26) 8 commits
 - wrap_in_html(): process message in bulk rather than line-by-line
 - wrap_in_html(): use strbuf_addstr_xml_quoted()
 - imap-send: change msg_data from storing (char *, len) to storing strbuf
 - imap-send: correctly report errors reading from stdin
 - imap-send: store all_msgs as a strbuf
 - lf_to_crlf(): NUL-terminate msg_data::data
 - xml_entities(): use function strbuf_addstr_xml_quoted()
 - Add new function strbuf_add_xml_quoted()


* pw/p4-various-fixes (2012-11-26) 6 commits
 - git p4: remove unneeded cmd initialization
 - git p4: fix labelDetails typo in exception
 - git p4 test: display unresolvable host error
 - git p4: catch p4 errors when streaming file contents
 - git p4: handle servers without move support
 - git p4: catch p4 describe errors

 Will merge to 'next'.


* rr/t4041-cleanup (2012-11-27) 4 commits
 - t4041 (diff-submodule-option): modernize style
 - t4041 (diff-submodule-option): rewrite add_file() routine
 - t4041 (diff-submodule-option): parse digests sensibly
 - t4041 (diff-submodule-option): don't hardcode SHA-1 in expected outputs

 As a clean-up, it still misses some.


* jc/doc-maintainer (2012-11-27) 1 commit
 - update "howto maintain git"

 An early draft that is still incomplete.


* jc/doc-push-satellite (2012-11-27) 1 commit
 - Documentation/git-push.txt: clarify the "push from satellite" workflow

 Will merge to 'next'.


* jk/fsck-dot-in-trees (2012-11-28) 1 commit
 - fsck: warn about '.' and '..' in trees

 Will merge to 'next'.


* lt/diff-stat-show-0-lines (2012-11-27) 6 commits
 - diff --shortstat: do not count "unmerged" entries
 - diff --stat: do not count "unmerged" entries
 - diff --stat: move the "total count" logic to the last loop
 - diff --stat: use "file" temporary variable to refer to data->files[i]
 - diff --stat: status of unmodified pair in diff-q is not zero
 - test: add failing tests for "diff --stat" to t4049

 Will merge to 'next'.


* mh/doc-remote-helpers (2012-11-27) 6 commits
 - git-remote-helpers.txt: clarify options & ref list attributes
 - git-remote-helpers.txt: clarify command <-> capability correspondences
 - git-remote-helpers.txt: rearrange description of capabilities
 - git-remote-helpers.txt: minor grammar fix
 - git-remote-helpers.txt: document missing capabilities
 - git-remote-helpers.txt: document invocation before input format

 Need comment and Ack from people who have worked on remote-helpers
 before this goes forward.


* mh/pthreads-autoconf (2012-11-27) 1 commit
 - configure.ac: fix pthreads detection on Mac OS X

 Will merge to 'next'.


* mk/complete-tcsh (2012-11-27) 1 commit
 - Support for git aliasing for tcsh completion

 Wi

Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option

2012-11-28 Thread W. Trevor King
On Wed, Nov 28, 2012 at 11:02:45AM -0800, Junio C Hamano wrote:
> "W. Trevor King"  writes:
> 
> > From: "W. Trevor King" 
> >
> > Signed-off-by: W. Trevor King 
> > ---
> >  Documentation/git-submodule.txt | 3 ++-
> >  git-submodule.sh| 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Hmm, I wonder why I have this funny feeling that this was proposed
> and rejected already...
> 
> As the command takes other options whose names begin with 'r', I
> thought the longer term plan was to stop letting "--rebase" squat on
> short and sweet "-r" and leaving it undocumented (even though the
> short one was added by mistake) was meant to be the first step in
> that process.
> 
> But maybe I am confusing an undocumented single-letter option from
> some other subcommand.  Anybody remembers?

Perhaps you are remembering:

On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote:
> Ah, this reminds me of another thing I noticed when I saw that
> patch.  The change seems to think "branch" is the _only_ thing the
> user might want to record per submodule upon "git submodule add".
> As an interface to muck with an uninterpreted random configuration,
> it squats on a good option name for setting one single and arbitrary
> variable---quite a selfish change that is not acceptable.
> 
> Calling the option "--record-branch-for-submodule" or something more
> specific might alleviate the problem, but then it would become even
> less useful as a short-hand for "config submodule.$name.branch", I
> would suspect.

With this recent patch, I'm just documenting someone else's squatting
;).  But yes, the reason I noticed was because I was tempted to make
the same mistake again :p.  In my defense, I think `update --remote`
is a good deal more general than my earlier `add --record`.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
--
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] svnrdump_sim: start the script with /usr/bin/env python

2012-11-28 Thread Junio C Hamano
Felipe Contreras  writes:

> On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano  wrote:
> ...
>> You need a fix for that; didn't I already say "you need a bit more
>> than that"?
>
> I disagree. Most of the contrib scripts are expected to be used as
> they are.

You are only looking at one of the uses for this script, when there
are two.

You are correct that distros may install with whatever tweaks of
their own, and to help their tweak process (like the one that
specifically notices "/usr/bin/env python" as you wrote), changing
the "#!/usr/bin/python" to match others would be a good change.

But that change alone is not sufficient for this one, which is used
from t/ script.  You cannot treat this one like import-zips and
hg-to-git that we do not use in-tree.  Somewhere before t9020 uses
it, it needs the treatment similar to the rewriting that is done for
git-p4.py to git-p4.
--
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 v5 0/2] submodule update: add --remote for submodule's upstream changes

2012-11-28 Thread W. Trevor King
From: "W. Trevor King" 

On Wed, Nov 28, 2012 at 11:53:34AM -0500, W. Trevor King wrote:
> I thought of a better idea on the train.  How about adding `--remote`
> to `submodule update` that overrides the gitlinked SHA-1 with the
> SHA-1 for origin/$branch?  All of the other checkout/merge/rebase
> functionality is untouched.  The only thing that changes is where we
> look for the update.  I'm working up the patch now, and will submit v5
> shortly.

Here it is.

Changes since v4:

* Remove `update --branch` in favor of the new `update --remote` logic
  as mentioned above.
* Optional config overrides for submodule..branch (as suggested
  by Jens)
* Save --branch as submodule..branch instead of requiring
  --local-branch.
* Restructure doc edits.

I'm a lot happier with this two-patch proposal.  The first patch
implements --remote and documents submodule..branch.  The second
patch just gives you a convenient way to set it.  I haven't heard from
anyone other than Heiko recently about the --branch/--remote-branch
choice, so I returned to the simpler --branch side effect storage from
v1.  I'd be happy to submit v6 with explicit --remote-branch recording
if desired.

W. Trevor King (2):
  submodule update: add --remote for submodule's upstream changes
  submodule add: If --branch is given, record it in .gitmodules

 Documentation/config.txt|  9 +
 Documentation/git-submodule.txt | 26 +-
 Documentation/gitmodules.txt|  5 +
 git-submodule.sh| 30 +-
 t/t7400-submodule-basic.sh  |  1 +
 t/t7406-submodule-update.sh | 31 +++
 6 files changed, 96 insertions(+), 6 deletions(-)

-- 
1.8.0.2.gad10246.dirty

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


[PATCH v5 1/2] submodule update: add --remote for submodule's upstream changes

2012-11-28 Thread W. Trevor King
From: "W. Trevor King" 

The current `update` command incorporates the superproject's gitlinked
SHA-1 ($sha1) into the submodule HEAD ($subsha1).  Depending on the
options you use, it may checkout $sha1, rebase the $subsha1 onto
$sha1, or merge $sha1 into $subsha1.  This helps you keep up with
changes in the upstream superproject.

However, it's also useful to stay up to date with changes in the
upstream subproject.  Previous workflows for incorporating such
changes include the ungainly:

  $ git submodule foreach 'git checkout $(git config --file 
$toplevel/.gitmodules submodule.$name.branch) && git pull'

With this patch, all of the useful functionality for incorporating
superproject changes can be reused to incorporate upstream subproject
updates.  When you specify --remote, the target $sha1 is replaced with
a $sha1 of the submodule's origin/master tracking branch.  If you want
to merge a different tracking branch, you can configure the
`submodule..branch` option in `.gitmodules`.  You can override
the `.gitmodules` configuration setting for a particular superproject
by configuring the option in that superproject's default configuration
(using the usual configuration hierarchy, e.g. `.git/config`,
`~/.gitconfig`, etc.).

Previous use of submodule..branch
===

Because we're adding a new configuration option, it's a good idea to
check if anyone else is already using the option.  The foreach-pull
example above was described by Ævar in

  commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f
  Author: Ævar Arnfjörð Bjarmason 
  Date:   Fri May 21 16:10:10 2010 +

git-submodule foreach: Add $toplevel variable

Gerrit uses the same interpretation for the setting, but because
Gerrit has direct access to the subproject repositories, it updates
the superproject repositories automatically when a subproject changes.
Gerrit also accepts the special value '.', which it expands into the
superproject's branch name.

Although the --remote functionality is using `submodule..branch`
slightly differently, the effect is the same.  The foreach-pull
example uses the option to record the name of the local branch to
checkout before pulls.  The tracking branch to be pulled is recorded
in `.git/modules//config`, which was initialized by the module
clone during `submodule add` or `submodule init`.  Because the branch
name stored in `submodule..branch` was likely the same as the
branch name used during the initial `submodule add`, the same branch
will be pulled in each workflow.

Implementation details
==

In order to ensure a current tracking branch state, `update --remote`
fetches the submodule's remote repository before calculating the
SHA-1.  However, I didn't change the logic guarding the existing fetch:

  if test -z "$nofetch"
  then
# Run fetch only if $sha1 isn't present or it
# is not reachable from a ref.
(clear_local_git_env; cd "$path" &&
  ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
   test -z "$rev") || git-fetch)) ||
die "$(eval_gettext "Unable to fetch in submodule path '\$path'")"
  fi

There will not be a double-fetch, because the new $sha1 determined
after the `--remote` triggered fetch should always exist in the
repository.  If it doesn't, it's because some racy process removed it
from the submodule's repository and we *should* be re-fetching.

Signed-off-by: W. Trevor King 
---
 Documentation/config.txt|  9 +
 Documentation/git-submodule.txt | 24 +++-
 Documentation/gitmodules.txt|  5 +
 git-submodule.sh| 26 +-
 t/t7406-submodule-update.sh | 31 +++
 5 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 11f320b..de39b1c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1994,10 +1994,11 @@ status.submodulesummary::
 submodule..path::
 submodule..url::
 submodule..update::
-   The path within this project, URL, and the updating strategy
-   for a submodule.  These variables are initially populated
-   by 'git submodule init'; edit them to override the
-   URL and other values found in the `.gitmodules` file.  See
+submodule..branch::
+   The path within this project, URL, the updating strategy, and the
+   remote branch name for a submodule.  These variables are initially
+   populated by 'git submodule init'; edit them to override the URL and
+   other values found in the `.gitmodules` file.  See
linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
 submodule..fetchRecurseSubmodules::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..39aa02d 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,7 +13,7 @@ SYNOPSIS
  [--reference ] [--]  []
 'git submodule' [--quiet] st

[PATCH v5 2/2] submodule add: If --branch is given, record it in .gitmodules

2012-11-28 Thread W. Trevor King
From: "W. Trevor King" 

This allows you to easily record a submodule..branch option in
.gitmodules when you add a new submodule.  With this patch,

  $ git submodule add -b   []
  $ git config -f .gitmodules submodule..branch 

reduces to

  $ git submodule add -b   []

This means that future calls to

  $ git submodule update --remote ...

will get updates from the same branch that you used to initialize the
submodule, which is usually what you want.

Signed-off-by: W. Trevor King 
---
 Documentation/git-submodule.txt | 2 ++
 git-submodule.sh| 4 
 t/t7400-submodule-basic.sh  | 1 +
 3 files changed, 7 insertions(+)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 39aa02d..43e5b4b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -208,6 +208,8 @@ OPTIONS
 -b::
 --branch::
Branch of repository to add as submodule.
+   The name of the branch is recorded as `submodule..branch` in
+   `.gitmodules` for `update --remote`.
 
 -f::
 --force::
diff --git a/git-submodule.sh b/git-submodule.sh
index b63d869..1f76893 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -368,6 +368,10 @@ Use -f if you really want to add it." >&2
 
git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
git config -f .gitmodules submodule."$sm_path".url "$repo" &&
+   if test -n "$branch"
+   then
+   git config -f .gitmodules submodule."$sm_path".branch "$branch"
+   fi &&
git add --force .gitmodules ||
die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 }
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5397037..90e2915 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -133,6 +133,7 @@ test_expect_success 'submodule add --branch' '
(
cd addtest &&
git submodule add -b initial "$submodurl" submod-branch &&
+   test "initial" = "$(git config -f .gitmodules 
submodule.submod-branch.branch)" &&
git submodule init
) &&
 
-- 
1.8.0.2.gad10246.dirty

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


Re: git fetch pack freezes

2012-11-28 Thread Shawn Pearce
On Wed, Nov 28, 2012 at 9:25 AM, Ivan Kanis  wrote:
> Shawn Pearce  wrote:
>
>> On Wed, Nov 28, 2012 at 6:12 AM, Ivan Kanis  
>> wrote:
>>>
>>> On the server we are seeing the following error message:
>>
>> Upgrade your server.
>
> OK we'll look into it. I have a question: will a 1.8 server still serve
> 1.7 git client?

Yes.

>> So the stack frames are correct. Its just a problem that the parent
>> didn't identify the server crashing and closing its side of the pipe
>> on stdin to force it to EOF to prevent the child from getting hung
>> here in a read.
>
> It sounds like a bug on the client, doesn't it?

Yes.
--
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] svnrdump_sim: start the script with /usr/bin/env python

2012-11-28 Thread Felipe Contreras
On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras
>>  wrote:

>>> That works if somebody managed to export PYTHON_PATH, which very very
>>> often is not the case for me.
>>
>> Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh,
>> svnrdump.py is used as is.
>
> You need a fix for that; didn't I already say "you need a bit more
> than that"?

I disagree. Most of the contrib scripts are expected to be used as
they are. There's no step in the Makefile that will convert them, and
it's up to each distribution to decide what to do with them. This is
what Arch Linux does:

  # more contrib stuff
  cp -a ./contrib/* $pkgdir/usr/share/git/
  # scripts are for python 2.x
  sed -i 's|#![ ]*/usr/bin/env python|#!/usr/bin/env python2|' \
$(find "$pkgdir" -name '*.py') \
"$pkgdir"/usr/lib/git-core/git-p4 \
"$pkgdir"/usr/share/git/gitview/gitview

At some point we might decide to change this, but at the moment
contrib scripts are pretty much stand-alone.

Cheers.

-- 
Felipe Contreras
--
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] submodule update: document exisiting -r form for --rebase

2012-11-28 Thread Junio C Hamano
"W. Trevor King"  writes:

> From: "W. Trevor King" 
>
> Signed-off-by: W. Trevor King 
> ---
>  Documentation/git-submodule.txt | 3 ++-
>  git-submodule.sh| 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Hmm, I wonder why I have this funny feeling that this was proposed
and rejected already...

As the command takes other options whose names begin with 'r', I
thought the longer term plan was to stop letting "--rebase" squat on
short and sweet "-r" and leaving it undocumented (even though the
short one was added by mistake) was meant to be the first step in
that process.

But maybe I am confusing an undocumented single-letter option from
some other subcommand.  Anybody remembers?

> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index b4683bb..ec78fa7 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
> [--reference ] [--]  []
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [...]
>  'git submodule' [--quiet] init [--] [...]
> -'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> +'git submodule' [--quiet] update [--init] [-N|--no-fetch] [-r|--rebase]
> [--reference ] [--merge] [--recursive] [--] 
> [...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) 
> ]
> [commit] [--] [...]
> @@ -251,6 +251,7 @@ OPTIONS
>   If the key `submodule.$name.update` is set to `merge`, this option is
>   implicit.
>  
> +-r::
>  --rebase::
>   This option is only valid for the update command.
>   Rebase the current branch onto the commit recorded in the
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ab6b110..f2e8026 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
>  USAGE="[--quiet] add [-b branch] [-f|--force] [--reference ] 
> [--]  []
> or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]
> or: $dashless [--quiet] init [--] [...]
> -   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
> [--rebase] [--reference ] [--merge] [--recursive] [--] [...]
> +   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
> [-r|--rebase] [--reference ] [--merge] [--recursive] [--] 
> [...]
> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] 
> [commit] [--] [...]
> or: $dashless [--quiet] foreach [--recursive] 
> or: $dashless [--quiet] sync [--] [...]"
--
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: add options --single-branch and --branch to "git clone"

2012-11-28 Thread Junio C Hamano
Ralf Thielow  writes:

> Signed-off-by: Ralf Thielow 
> ---
>  contrib/completion/git-completion.bash | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 48c3abd..cda095d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -989,6 +989,8 @@ _git_clone ()
>   --upload-pack
>   --template=
>   --depth
> + --single-branch
> + --branch
>   "

Hmph OK, will queue.
--
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 6/5] t9001: check send-email behavior with implicit sender

2012-11-28 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Nov 28, 2012 at 01:25:35PM -0500, Jeff King wrote:
>
>> Here are the cleanups and refactorings split out from my
>> jk/send-email-sender-prompt series. They can go right on master and are
>> independent of Felipe's fc/send-email-no-sender-prompt topic.
>> [...]
>> Dropped were:
>> [...]
>>   - send-email prompting change; obsoleted by Felipe's patch
>
> Here is one more patch pulling out the extra tests from my final commit.
> It needs to go on top of the merge of Felipe's series and the one I just
> sent. The former because of the new prompting behavior, and the latter
> because of the AUTOIDENT prerequisites.
>
> If it's simpler, my whole series can just go on top of Felipe's patch
> (or vice versa).
>
> -- >8 --
> Subject: [PATCH] t9001: check send-email behavior with implicit sender
>
> We allow send-email to use an implicitly-defined identity
> for the sender (because there is still a confirmation step),
> but we abort when we cannot generate such an identity. Let's
> make sure that we test this.
>
> Signed-off-by: Jeff King 
> ---
>  t/t9001-send-email.sh | 28 
>  1 file changed, 28 insertions(+)
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index c5d66cf..27edfa8 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -201,6 +201,34 @@ test_expect_success $PREREQ 'Prompting works' '
>   grep "^To: t...@example.com\$" msgtxt1
>  '
>  
> +test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
> +   clean_fake_sendmail &&
> +   (sane_unset GIT_AUTHOR_NAME &&
> + sane_unset GIT_AUTHOR_EMAIL &&
> + sane_unset GIT_COMMITTER_NAME &&
> + sane_unset GIT_COMMITTER_EMAIL &&
> + GIT_SEND_EMAIL_NOTTY=1 git send-email \
> + --smtp-server="$(pwd)/fake.sendmail" \
> + --to=t...@example.com \
> + $patches \
> + errors
> +   )
> +'
> +
> +test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts 
> send-email' '
> +   clean_fake_sendmail &&
> +   (sane_unset GIT_AUTHOR_NAME &&
> + sane_unset GIT_AUTHOR_EMAIL &&
> + sane_unset GIT_COMMITTER_NAME &&
> + sane_unset GIT_COMMITTER_EMAIL &&
> + GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
> + test_must_fail git send-email \
> + --smtp-server="$(pwd)/fake.sendmail" \
> + $patches errors &&
> + test_i18ngrep "tell me who you are" errors
> +   )
> +'

The difference between these two tests should solely come from
AUTOIDENT and nothing else, so it would be better to see their
command line arguments to match; the former is with --to and the
latter is without in this patch but I do not think you meant them to
differ that way.



>  test_expect_success $PREREQ 'tocmd works' '
>   clean_fake_sendmail &&
>   cp $patches tocmd.patch &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/5] t9001: check send-email behavior with implicit sender

2012-11-28 Thread Jeff King
On Wed, Nov 28, 2012 at 01:25:35PM -0500, Jeff King wrote:

> Here are the cleanups and refactorings split out from my
> jk/send-email-sender-prompt series. They can go right on master and are
> independent of Felipe's fc/send-email-no-sender-prompt topic.
> [...]
> Dropped were:
> [...]
>   - send-email prompting change; obsoleted by Felipe's patch

Here is one more patch pulling out the extra tests from my final commit.
It needs to go on top of the merge of Felipe's series and the one I just
sent. The former because of the new prompting behavior, and the latter
because of the AUTOIDENT prerequisites.

If it's simpler, my whole series can just go on top of Felipe's patch
(or vice versa).

-- >8 --
Subject: [PATCH] t9001: check send-email behavior with implicit sender

We allow send-email to use an implicitly-defined identity
for the sender (because there is still a confirmation step),
but we abort when we cannot generate such an identity. Let's
make sure that we test this.

Signed-off-by: Jeff King 
---
 t/t9001-send-email.sh | 28 
 1 file changed, 28 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c5d66cf..27edfa8 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -201,6 +201,34 @@ test_expect_success $PREREQ 'Prompting works' '
grep "^To: t...@example.com\$" msgtxt1
 '
 
+test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
+   clean_fake_sendmail &&
+   (sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   GIT_SEND_EMAIL_NOTTY=1 git send-email \
+   --smtp-server="$(pwd)/fake.sendmail" \
+   --to=t...@example.com \
+   $patches \
+   errors
+   )
+'
+
+test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts 
send-email' '
+   clean_fake_sendmail &&
+   (sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
+   test_must_fail git send-email \
+   --smtp-server="$(pwd)/fake.sendmail" \
+   $patches errors &&
+   test_i18ngrep "tell me who you are" errors
+   )
+'
+
 test_expect_success $PREREQ 'tocmd works' '
clean_fake_sendmail &&
cp $patches tocmd.patch &&
-- 
1.8.0.207.gdf2154c

--
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] submodule update: document exisiting -r form for --rebase

2012-11-28 Thread W. Trevor King
From: "W. Trevor King" 

Signed-off-by: W. Trevor King 
---
 Documentation/git-submodule.txt | 3 ++-
 git-submodule.sh| 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..ec78fa7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,7 +13,7 @@ SYNOPSIS
  [--reference ] [--]  []
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...]
 'git submodule' [--quiet] init [--] [...]
-'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
+'git submodule' [--quiet] update [--init] [-N|--no-fetch] [-r|--rebase]
  [--reference ] [--merge] [--recursive] [--] 
[...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]
  [commit] [--] [...]
@@ -251,6 +251,7 @@ OPTIONS
If the key `submodule.$name.update` is set to `merge`, this option is
implicit.
 
+-r::
 --rebase::
This option is only valid for the update command.
Rebase the current branch onto the commit recorded in the
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..f2e8026 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b branch] [-f|--force] [--reference ] [--] 
 []
or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]
or: $dashless [--quiet] init [--] [...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
[--rebase] [--reference ] [--merge] [--recursive] [--] [...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
[-r|--rebase] [--reference ] [--merge] [--recursive] [--] 
[...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] 
[commit] [--] [...]
or: $dashless [--quiet] foreach [--recursive] 
or: $dashless [--quiet] sync [--] [...]"
-- 
1.8.0.2.gcc766b6

--
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] completion: add options --single-branch and --branch to "git clone"

2012-11-28 Thread Ralf Thielow
Signed-off-by: Ralf Thielow 
---
 contrib/completion/git-completion.bash | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 48c3abd..cda095d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -989,6 +989,8 @@ _git_clone ()
--upload-pack
--template=
--depth
+   --single-branch
+   --branch
"
return
;;
-- 
1.8.0

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


[PATCH 5/5] t: add tests for "git var"

2012-11-28 Thread Jeff King
We do not currently have any explicit tests for "git var" at
all (though we do exercise it to some degree as a part of
other tests). Let's add a few basic sanity checks.

Signed-off-by: Jeff King 
---
 t/t0007-git-var.sh | 49 +
 1 file changed, 49 insertions(+)
 create mode 100755 t/t0007-git-var.sh

diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
new file mode 100755
index 000..5868a87
--- /dev/null
+++ b/t/t0007-git-var.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='basic sanity checks for git var'
+. ./test-lib.sh
+
+test_expect_success 'get GIT_AUTHOR_IDENT' '
+   test_tick &&
+   echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
+   git var GIT_AUTHOR_IDENT >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'get GIT_COMMITTER_IDENT' '
+   test_tick &&
+   echo "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" 
>expect &&
+   git var GIT_COMMITTER_IDENT >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success !AUTOIDENT 'requested identites are strict' '
+   (
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   test_must_fail git var GIT_COMMITTER_IDENT
+   )
+'
+
+# For git var -l, we check only a representative variable;
+# testing the whole output would make our test too brittle with
+# respect to unrelated changes in the test suite's environment.
+test_expect_success 'git var -l lists variables' '
+   git var -l >actual &&
+   echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
+   sed -n s/GIT_AUTHOR_IDENT=//p actual.author &&
+   test_cmp expect actual.author
+'
+
+test_expect_success 'git var -l lists config' '
+   git var -l >actual &&
+   echo false >expect &&
+   sed -n s/core\\.bare=//p actual.bare &&
+   test_cmp expect actual.bare
+'
+
+test_expect_success 'listing and asking for variables are exclusive' '
+   test_must_fail git var -l GIT_COMMITTER_IDENT
+'
+
+test_done
-- 
1.8.0.207.gdf2154c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/5] ident: keep separate "explicit" flags for author and committer

2012-11-28 Thread Jeff King
We keep track of whether the user ident was given to us
explicitly, or if we guessed at it from system parameters
like username and hostname. However, we kept only a single
variable. This covers the common cases (because the author
and committer will usually come from the same explicit
source), but can miss two cases:

  1. GIT_COMMITTER_* is set explicitly, but we fallback for
 GIT_AUTHOR. We claim the ident is explicit, even though
 the author is not.

  2. GIT_AUTHOR_* is set and we ask for author ident, but
 not committer ident. We will claim the ident is
 implicit, even though it is explicit.

This patch uses two variables instead of one, updates both
when we set the "fallback" values, and updates them
individually when we read from the environment.

Rather than keep user_ident_sufficiently_given as a
compatibility wrapper, we update the only two callers to
check the committer_ident, which matches their intent and
what was happening already.

Signed-off-by: Jeff King 
---
 builtin/commit.c |  4 ++--
 cache.h  |  3 ++-
 ident.c  | 32 +---
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1dd2ec5..d6dd3df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -755,7 +755,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
ident_shown++ ? "" : "\n",
author_ident->buf);
 
-   if (!user_ident_sufficiently_given())
+   if (!committer_ident_sufficiently_given())
status_printf_ln(s, GIT_COLOR_NORMAL,
_("%s"
"Committer: %s"),
@@ -1265,7 +1265,7 @@ static void print_summary(const char *prefix, const 
unsigned char *sha1,
strbuf_addstr(&format, "\n Author: ");
strbuf_addbuf_percentquote(&format, &author_ident);
}
-   if (!user_ident_sufficiently_given()) {
+   if (!committer_ident_sufficiently_given()) {
strbuf_addstr(&format, "\n Committer: ");
strbuf_addbuf_percentquote(&format, &committer_ident);
if (advice_implicit_identity) {
diff --git a/cache.h b/cache.h
index 50d9eea..18fdd18 100644
--- a/cache.h
+++ b/cache.h
@@ -1149,7 +1149,8 @@ struct config_include_data {
 #define CONFIG_INCLUDE_INIT { 0 }
 extern int git_config_include(const char *name, const char *value, void *data);
 
-extern int user_ident_sufficiently_given(void);
+extern int committer_ident_sufficiently_given(void);
+extern int author_ident_sufficiently_given(void);
 
 extern const char *git_commit_encoding;
 extern const char *git_log_output_encoding;
diff --git a/ident.c b/ident.c
index 733d69d..ac9672f 100644
--- a/ident.c
+++ b/ident.c
@@ -14,7 +14,8 @@ static char git_default_date[50];
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-static int user_ident_explicitly_given;
+static int committer_ident_explicitly_given;
+static int author_ident_explicitly_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -113,7 +114,8 @@ const char *ident_default_email(void)
 
if (email && email[0]) {
strbuf_addstr(&git_default_email, email);
-   user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
} else
copy_email(xgetpwuid_self(), &git_default_email);
strbuf_trim(&git_default_email);
@@ -331,6 +333,10 @@ const char *fmt_name(const char *name, const char *email)
 
 const char *git_author_info(int flag)
 {
+   if (getenv("GIT_AUTHOR_NAME"))
+   author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   if (getenv("GIT_AUTHOR_EMAIL"))
+   author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return fmt_ident(getenv("GIT_AUTHOR_NAME"),
 getenv("GIT_AUTHOR_EMAIL"),
 getenv("GIT_AUTHOR_DATE"),
@@ -340,16 +346,16 @@ const char *git_author_info(int flag)
 const char *git_committer_info(int flag)
 {
if (getenv("GIT_COMMITTER_NAME"))
-   user_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
if (getenv("GIT_COMMITTER_EMAIL"))
-   user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return fmt_ident(getenv("GIT_COMMITTER_NAME"),
 getenv("GIT_COMMITTER_EMAIL"),
 getenv("GIT_COMMITTER_DATE"),
 flag);
 }
 
-int user_ident_sufficiently_given(void)
+static int ident_is_sufficient(int us

[PATCH 3/5] ident: make user_ident_explicitly_given static

2012-11-28 Thread Jeff King
In v1.5.6-rc0~56^2 (2008-05-04) "user_ident_explicitly_given"
was introduced as a global for communication between config,
ident, and builtin-commit.  In v1.7.0-rc0~72^2 (2010-01-07)
readers switched to using the common wrapper
user_ident_sufficiently_given().  After v1.7.11-rc1~15^2~18
(2012-05-21), the var is only written in ident.c.

Now we can make it static, which will enable further
refactoring without worrying about upsetting other code.

Signed-off-by: Jeff King 
Reviewed-by: Jonathan Nieder 
---
 cache.h | 4 
 ident.c | 6 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index dbd8018..50d9eea 100644
--- a/cache.h
+++ b/cache.h
@@ -1149,10 +1149,6 @@ struct config_include_data {
 #define CONFIG_INCLUDE_INIT { 0 }
 extern int git_config_include(const char *name, const char *value, void *data);
 
-#define IDENT_NAME_GIVEN 01
-#define IDENT_MAIL_GIVEN 02
-#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-extern int user_ident_explicitly_given;
 extern int user_ident_sufficiently_given(void);
 
 extern const char *git_commit_encoding;
diff --git a/ident.c b/ident.c
index a4bf206..733d69d 100644
--- a/ident.c
+++ b/ident.c
@@ -10,7 +10,11 @@
 static struct strbuf git_default_name = STRBUF_INIT;
 static struct strbuf git_default_email = STRBUF_INIT;
 static char git_default_date[50];
-int user_ident_explicitly_given;
+
+#define IDENT_NAME_GIVEN 01
+#define IDENT_MAIL_GIVEN 02
+#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
+static int user_ident_explicitly_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
-- 
1.8.0.207.gdf2154c

--
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/5] t7502: factor out autoident prerequisite

2012-11-28 Thread Jeff King
t7502 checks the behavior of commit when we can and cannot
determine a valid committer ident. Let's move that into
test-lib as a lazy prerequisite so other scripts can use it.

Signed-off-by: Jeff King 
---
 t/t7502-commit.sh | 12 +---
 t/test-lib.sh |  6 ++
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index deb187e..1a5cb69 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -243,16 +243,6 @@ test_expect_success 'message shows author when it is not 
equal to committer' '
  .git/COMMIT_EDITMSG
 '
 
-test_expect_success 'setup auto-ident prerequisite' '
-   if (sane_unset GIT_COMMITTER_EMAIL &&
-   sane_unset GIT_COMMITTER_NAME &&
-   git var GIT_COMMITTER_IDENT); then
-   test_set_prereq AUTOIDENT
-   else
-   test_set_prereq NOAUTOIDENT
-   fi
-'
-
 test_expect_success AUTOIDENT 'message shows committer when it is automatic' '
 
echo >>negative &&
@@ -271,7 +261,7 @@ echo editor started > "$(pwd)/.git/result"
 exit 0
 EOF
 
-test_expect_success NOAUTOIDENT 'do not fire editor when committer is bogus' '
+test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
>.git/result
>expect &&
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..0334a9e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -738,6 +738,12 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
esac
 '
 
+test_lazy_prereq AUTOIDENT '
+   sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   git var GIT_AUTHOR_IDENT
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY
-- 
1.8.0.207.gdf2154c

--
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/5] test-lib: allow negation of prerequisites

2012-11-28 Thread Jeff King
You can set and test a prerequisite like this:

  test_set_prereq FOO
  test_have_prereq FOO && echo yes

You can negate the test in the shell like this:

  ! test_have_prereq && echo no

However, when you are using the automatic prerequisite
checking in test_expect_*, there is no opportunity to use
the shell negation.  This patch introduces the syntax "!FOO"
to indicate that the test should only run if a prerequisite
is not meant.

One alternative is to set an explicit negative prerequisite,
like:

  if system_has_foo; then
  test_set_prereq FOO
  else
  test_set_prereq NO_FOO
  fi

However, this doesn't work for lazy prerequisites, which
associate a single test with a single name. We could teach
the lazy prereq evaluator to set both forms, but the code
change ends up quite similar to this one (because we still
need to convert NO_FOO into FOO to find the correct lazy
script).

Signed-off-by: Jeff King 
---
 t/t-basic.sh| 32 
 t/test-lib-functions.sh | 21 -
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/t/t-basic.sh b/t/t-basic.sh
index 08677df..562cf41 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -115,6 +115,38 @@ then
exit 1
 fi
 
+test_lazy_prereq LAZY_TRUE true
+havetrue=no
+test_expect_success LAZY_TRUE 'test runs if lazy prereq is satisfied' '
+   havetrue=yes
+'
+donthavetrue=yes
+test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '
+   donthavetrue=no
+'
+
+if test "$havetrue$donthavetrue" != yesyes
+then
+   say 'bug in test framework: lazy prerequisites do not work'
+   exit 1
+fi
+
+test_lazy_prereq LAZY_FALSE false
+nothavefalse=no
+test_expect_success !LAZY_FALSE 'negative lazy prereqs checked' '
+   nothavefalse=yes
+'
+havefalse=yes
+test_expect_success LAZY_FALSE 'missing negative lazy prereqs will skip' '
+   havefalse=no
+'
+
+if test "$nothavefalse$havefalse" != yesyes
+then
+   say 'bug in test framework: negative lazy prerequisites do not work'
+   exit 1
+fi
+
 clean=no
 test_expect_success 'tests clean up after themselves' '
test_when_finished clean=yes
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..22a4f8f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -275,6 +275,15 @@ test_have_prereq () {
 
for prerequisite
do
+   case "$prerequisite" in
+   !*)
+   negative_prereq=t
+   prerequisite=${prerequisite#!}
+   ;;
+   *)
+   negative_prereq=
+   esac
+
case " $lazily_tested_prereq " in
*" $prerequisite "*)
;;
@@ -294,10 +303,20 @@ test_have_prereq () {
total_prereq=$(($total_prereq + 1))
case "$satisfied_prereq" in
*" $prerequisite "*)
+   satisfied_this_prereq=t
+   ;;
+   *)
+   satisfied_this_prereq=
+   esac
+
+   case "$satisfied_this_prereq,$negative_prereq" in
+   t,|,t)
ok_prereq=$(($ok_prereq + 1))
;;
*)
-   # Keep a list of missing prerequisites
+   # Keep a list of missing prerequisites; restore
+   # the negative marker if necessary.
+   prerequisite=${negative_prereq:+!}$prerequisite
if test -z "$missing_prereq"
then
missing_prereq=$prerequisite
-- 
1.8.0.207.gdf2154c

--
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/5] jk/send-email-sender-prompt loose ends

2012-11-28 Thread Jeff King
Here are the cleanups and refactorings split out from my
jk/send-email-sender-prompt series. They can go right on master and are
independent of Felipe's fc/send-email-no-sender-prompt topic.

  [1/5]: test-lib: allow negation of prerequisites

Same as before. I think this is a useful feature for the test suite (and
2/5 depends on it).

  [2/5]: t7502: factor out autoident prerequisite

Same as before. Patch 5/5 depends on it.

  [3/5]: ident: make user_ident_explicitly_given static

Same as before. Cleanup.

  [4/5]: ident: keep separate "explicit" flags for author and committer

Same as before. Cleanup. I do not know if anybody will ever care about
the corner cases it fixes, so it is really just being defensive for
future code.

  [5/5]: t: add tests for "git var"

Tests split out from he "git var can take multiple values" patch.

Dropped were:

  - "git var" can take multiple values. Nobody really cares about doing
so, and it's an external interface, so we'd have to support it
forever.

  - exporting "explicit ident" flag via "git var"; same reasoning as
above

  - Git.pm supporting explicit ident; ditto

  - send-email prompting change; obsoleted by Felipe's patch

-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: [RFC/PATCH] l10n: de.po: translate 825 new messages

2012-11-28 Thread Ralf Thielow
> Hi Ralf,
>
> This is the middle third of my review.  Sorry for the long wait!  I hope
> it can still be useful.
>

Hi Thomas,

no problem. Thanks for your review. Of course it's very useful.
Some of the mistakes I made are so obvious that I can't say what
I've had in mind when translation these messages :/ But some aren't,
so thanks for the further explanations within your review.

> I don't really share your apparent aversion to just using "" :-)
I don't really have one :) so I'm fine with using .

> This would be a good time to settle on a good translation for
> "rewriting".  Perhaps "neu schreiben".  "Überschreiben" to me implies
On some other places we actually use "neu schreiben".

>> -msgstr ""
>> +msgstr "erzeugt kleinere Pakete"
>
> Smaller is not really the point: they are packs that do not have the
[...]
> You could call them "abgespeckt" ;-)
I used "dünner"!?

Furthermore I've unified the translation of "email" to "Email",
not "eMail". You'll see the result below. I hope I haven't missed
something.

Thanks,
Ralf

---
 po/de.po | 95 
 1 file changed, 47 insertions(+), 48 deletions(-)

diff --git a/po/de.po b/po/de.po
index fe6e8cf..1a75ea2 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1978,7 +1978,7 @@ msgstr "Pfade sind getrennt durch NUL Zeichen"
 #: builtin/apply.c:4336
 msgid "ensure at least  lines of context match"
 msgstr ""
-"stellt sicher, dass mindestens  Zeilen des Kontextes übereinstimmen"
+"stellt sicher, dass mindestens  Zeilen des Kontextes übereinstimmen"
 
 #: builtin/apply.c:4337
 msgid "action"
@@ -2167,7 +2167,7 @@ msgstr "Unterdrückt den Namen des Autors und den 
Zeitstempel (Standard: aus)"
 
 #: builtin/blame.c:2329
 msgid "Show author email instead of name (Default: off)"
-msgstr "Zeigt anstatt des Namens die eMail-Adresse des Autors (Standard: aus)"
+msgstr "Zeigt anstatt des Namens die Email-Adresse des Autors (Standard: aus)"
 
 #: builtin/blame.c:2330
 msgid "Ignore whitespace differences"
@@ -4072,7 +4072,7 @@ msgstr "gibt nur exakte Übereinstimmungen aus"
 
 #: builtin/describe.c:412
 msgid "consider  most recent tags (default: 10)"
-msgstr "betrachtet die jüngsten  Markierungen (Standard: 10)"
+msgstr "betrachtet die jüngsten  Markierungen (Standard: 10)"
 
 #: builtin/describe.c:414
 msgid "only consider tags matching "
@@ -4143,7 +4143,7 @@ msgstr "git fast-export [rev-list-opts]"
 
 #: builtin/fast-export.c:635
 msgid "show progress after  objects"
-msgstr "zeigt Fortschritt nach  Objekten an"
+msgstr "zeigt Fortschritt nach  Objekten an"
 
 #: builtin/fast-export.c:637
 msgid "select handling of signed tags"
@@ -4422,7 +4422,7 @@ msgstr ""
 #: builtin/fmt-merge-msg.c:13
 msgid "git fmt-merge-msg [-m ] [--log[=]|--no-log] [--file ]"
 msgstr ""
-"git fmt-merge-msg [-m ] [--log[=]|--no-log] [--file "
+"git fmt-merge-msg [-m ] [--log[=]|--no-log] [--file "
 "]"
 
 #: builtin/fmt-merge-msg.c:653 builtin/fmt-merge-msg.c:656 builtin/grep.c:786
@@ -4433,7 +4433,7 @@ msgstr "Anzahl"
 
 #: builtin/fmt-merge-msg.c:654
 msgid "populate log with at most  entries from shortlog"
-msgstr "fügt Historie mit höchstens  Einträgen von \"shortlog\" hinzu"
+msgstr "fügt Historie mit höchstens  Einträgen von \"shortlog\" hinzu"
 
 #: builtin/fmt-merge-msg.c:657
 msgid "alias for --log (deprecated)"
@@ -4457,23 +4457,23 @@ msgstr "git for-each-ref [Optionen] []"
 
 #: builtin/for-each-ref.c:994
 msgid "quote placeholders suitably for shells"
-msgstr "quotiert Platzhalter geeignet für Shells"
+msgstr "formatiert Platzhalter als Shell-String"
 
 #: builtin/for-each-ref.c:996
 msgid "quote placeholders suitably for perl"
-msgstr "quotiert Platzhalter geeignet für Perl"
+msgstr "formatiert Platzhalter als Perl-String"
 
 #: builtin/for-each-ref.c:998
 msgid "quote placeholders suitably for python"
-msgstr "quotiert Platzhalter geeignet für Python"
+msgstr "formatiert Platzhalter als Python-String"
 
 #: builtin/for-each-ref.c:1000
 msgid "quote placeholders suitably for tcl"
-msgstr "quotiert Platzhalter geeignet für TCL"
+msgstr "formatiert Platzhalter als TCL-String"
 
 #: builtin/for-each-ref.c:1003
 msgid "show only  matched refs"
-msgstr "zeigt nur  passende Referenzen"
+msgstr "zeigt nur  passende Referenzen"
 
 #: builtin/for-each-ref.c:1004
 msgid "format"
@@ -4489,7 +4489,7 @@ msgstr "Schüssel"
 
 #: builtin/for-each-ref.c:1006
 msgid "field name to sort on"
-msgstr "Name des Felder zum Sortieren"
+msgstr "sortiere nach diesem Feld"
 
 #: builtin/fsck.c:608
 msgid "git fsck [options] [...]"
@@ -4521,7 +4521,7 @@ msgstr "erzeugt Kopfknoten des Referenzprotokolls 
(Standard)"
 
 #: builtin/fsck.c:620
 msgid "also consider packs and alternate objects"
-msgstr "betrachtet auch Pakete und wechselnde Objekte"
+msgstr ""
 
 #: builtin/fsck.c:621
 msgid "enable more strict checking"
@@ -4645,7 +4645,7 @@ msgstr "findet Übereinstimmungen unabhängig von Groß- und 
Kleinschreibung"
 
 #: builtin/grep.c:741
 msgid "match patterns on

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

2012-11-28 Thread Piotr Krukowiecki
Re-sending mail.

On Wed, Nov 28, 2012 at 7:02 PM, Piotr Krukowiecki
 wrote:
>
> On Wed, Nov 28, 2012 at 2:20 PM, Simon Oosthoek  wrote:
>>
>> * Piotr Krukowiecki  [2012-11-28 11:03:29 
>> +0100]:
>>
>> > Hi,
>> >
>> > when I set PROMPT_COMMAND to __git_ps1 I get a space at the beginning:
>> >
>>
>> Is your setting?:
>> PROMPT_COMMAND=__git_ps1
>
>
> That's right
>
>
>> I believe you need to give 2 parameters in order to use it in PROMPT_COMMAND 
>> mode.
>
>
> Are you sure? git-prompt.sh says:
>
> #3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
> #To customize the prompt, provide start/end arguments
> #PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>
> I interpret this as: if you don't want to customize the prompt, the simple 
> "PROMPT_COMMAND=__git_ps1" is enough.
>
>>
>> >  (master)pkruk@foobar ~/dir$
>> > ^ space
>> >
>> > Is there a reason for this? It looks like a waste of space. If I'm not in
>> > git repository I don't have the space:
>> >
>> > pkruk@foobar ~/other$
>> >
>> > I noticed the space is explicitly specified in printf_format in
>> > git-prompt.sh. Is it needed? If I remove it, everything seems to work fine
>> > (no leading space)...
>> >
>> > --- /usr/local/src/git/git/contrib/completion/git-prompt.sh 2012-11-28
>> > 10:27:05.728939201 +0100
>> > +++ /home/pkruk/.git-prompt.sh 2012-11-28 10:52:56.852629745 +0100
>> > @@ -218,7 +218,7 @@ __git_ps1 ()
>> >   local detached=no
>> >   local ps1pc_start='\u@\h:\w '
>> >   local ps1pc_end='\$ '
>> > - local printf_format=' (%s)'
>> > + local printf_format='(%s)'
>> >
>> >   case "$#" in
>> >   2) pcmode=yes
>>
>>
>> These last 2 lines say: if 2 arguments are given, use pcmode. Otherwise you 
>> get command-subtitution mode, which gives weird effects when being called 
>> from PROMPT_COMMAND.
>>
>
> Still, it seems to work with above "patch"..
>




--
Piotr Krukowiecki
--
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: Millisecond precision in timestamps?

2012-11-28 Thread Junio C Hamano
Thomas Berg  writes:

> If roundtripping to other version control systems is an argument,
> adding sub-second timestamps could potentially create as many problems
> as it solves. For example, I've been using the hg-git bridge, and it
> supports roundtripping between git and mercurial today (for most repos
> I've tried anyway). I may have missed something,...

What I left unsaid was that the use of extra subsecond resolution is
optional.  I do not see any reason for *us* to create commits with
subsecond resolution when we are writing native commits.  Only when
the end users and/or import tools tell us to.  If you assume all
foreign SCM you care about have at least one second resolution, you
would be fine.

Having said all that, given that this, if implemented, would not be
used by us but only for recording other people's times, and that the
set of meta information we record in our history will never be
superset of everybody else's anyway, I do not see much point in
supporting subsecond timestamps in the first place.
--
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 4/7] remote-bzr: update working tree

2012-11-28 Thread Junio C Hamano
Felipe Contreras  writes:

> Signed-off-by: Felipe Contreras 
> ---
>  contrib/remote-helpers/git-remote-bzr | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-bzr 
> b/contrib/remote-helpers/git-remote-bzr
> index 2c05f35..5b89a05 100755
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -571,6 +571,8 @@ def do_export(parser):
>  repo.generate_revision_history(revid, marks.get_tip('master'))
>  revno, revid = repo.last_revision_info()
>  peer.import_last_revision_info_and_tags(repo, revno, revid)
> +wt = peer.bzrdir.open_workingtree()
> +wt.update()
>  print "ok %s" % ref
>  print

Shouldn't this be squashed as part of one of the earlier patches?
The split between 1/7 (import first) and 2/7 (then support export)
makes sense, but this one looks more like "oops, we forgot to touch
the working tree while updating the history in the previous steps
and here is a fix" to me.
--
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 5/7] remote-bzr: add simple tests

2012-11-28 Thread Junio C Hamano
Felipe Contreras  writes:

> Signed-off-by: Felipe Contreras 
> ---
>  contrib/remote-helpers/test-bzr.sh | 111 
> +
>  1 file changed, 111 insertions(+)
>  create mode 100755 contrib/remote-helpers/test-bzr.sh
>
> diff --git a/contrib/remote-helpers/test-bzr.sh 
> b/contrib/remote-helpers/test-bzr.sh
> new file mode 100755
> index 000..8594ffc
> --- /dev/null
> +++ b/contrib/remote-helpers/test-bzr.sh
> @@ -0,0 +1,111 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Felipe Contreras
> +#
> +
> +test_description='Test remote-bzr'
> +
> +. ./test-lib.sh
> +
> +if ! test_have_prereq PYTHON; then
> + skip_all='skipping remote-bzr tests; python not available'
> + test_done
> +fi
> +
> +if ! "$PYTHON_PATH" -c 'import bzrlib'; then
> + skip_all='skipping remote-bzr tests; bzr not available'
> + test_done
> +fi

> +cmd=< +import bzrlib
> +bzrlib.initialize()
> +import bzrlib.plugin
> +bzrlib.plugin.load_plugins()
> +import bzrlib.plugins.fastimport
> +EOF
> +if ! "$PYTHON_PATH" -c "$cmd"; then

I cannot see how this could have ever worked.  It could be that you
wrote it for zsh and tested only with the version of zsh you have;
zsh I have here does not grok it (and of course dash and bash won't).

In any case, I do not think this is a POSIX shell.  Just replace
"=< + echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
> + skip_all='skipping remote-bzr tests; bzr-fastimport not available'
> + test_done
> +fi
> +
> +check () {
> + (cd $1 &&
> + git log --format='%s' -1 &&
> + git symbolic-ref HEAD) > actual &&
> + (echo $2 &&
> + echo "refs/heads/$3") > expected &&

Style: s/> expected/>expected/;
I won't repeat this but there are many others in this file.

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


Re: git fetch pack freezes

2012-11-28 Thread Ivan Kanis
Hi Shawn,

Thanks for the quick reply.

Shawn Pearce  wrote:

> On Wed, Nov 28, 2012 at 6:12 AM, Ivan Kanis  wrote:
>>
>> On the server we are seeing the following error message:
>
> Upgrade your server.

OK we'll look into it. I have a question: will a 1.8 server still serve
1.7 git client?

> So the stack frames are correct. Its just a problem that the parent
> didn't identify the server crashing and closing its side of the pipe
> on stdin to force it to EOF to prevent the child from getting hung
> here in a read.

It sounds like a bug on the client, doesn't it?
-- 
Ivan Kanis, Release Manager, Vision Objects,
Tel +33 2 28 01 49 50,  Fax +33 2 40 25 89 20
http://www.visionobjects.com

Let a fool hold his tongue and he will pass for a sage.
-- Publilius Syrus
--
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] push: cleanup push rules comment

2012-11-28 Thread Junio C Hamano
Chris Rorvick  writes:

> ---
>
> I ended up rewriting most of the comment.  The new version removes
> inter-rule dependencies (e.g., rule 5 overrides rule 3) which I think
> makes it more readable.

Nice; 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] svnrdump_sim: start the script with /usr/bin/env python

2012-11-28 Thread Junio C Hamano
Christian Couder  writes:

> On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras
>  wrote:
>> On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano  wrote:
>>> Christian Couder  writes:
>>>
 All the python scripts except contrib/svn-fe/svnrdump_sim.py
 start with "#!/usr/bin/env python".

 This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.
>>>
>>> I suspect you need a bit more than that.
>>>
>>> $ make git-p4
>>> $ diff -u git-p4.py git-p4
>>>
>>> shows you how we tell the scripts how to find their interpreters
>>> (that way, there is no need to rely on the existence of
>>> /usr/bin/env).
>>
>> That works if somebody managed to export PYTHON_PATH, which very very
>> often is not the case for me.
>
> Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh,
> svnrdump.py is used as is.

You need a fix for that; didn't I already say "you need a bit more
than 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 v4 0/4] git-submodule add: Add --local-branch option (summary)

2012-11-28 Thread W. Trevor King
On Wed, Nov 28, 2012 at 08:09:03AM -0500, W. Trevor King wrote:
> * A new 'submodule pull' for tracking the submodule's remote, which is
>   pulling --ff-only origin/$branch into a whatever state the submodule
>   is currently in.  If any changes were made to submodule $shas,
>   optionally commit them with a shortlog-generated commit message.

I thought of a better idea on the train.  How about adding `--remote`
to `submodule update` that overrides the gitlinked SHA-1 with the
SHA-1 for origin/$branch?  All of the other checkout/merge/rebase
functionality is untouched.  The only thing that changes is where we
look for the update.  I'm working up the patch now, and will submit v5
shortly.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
--
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 3/3] Add option to transpose parents of merge commit

2012-11-28 Thread Junio C Hamano
Johannes Sixt  writes:

> Am 11/28/2012 0:00, schrieb Kacper Kornet:
>> When the changes are pushed upstream, and in the meantime someone else
>> updated upstream branch git advises to use git pull. This results in
>> history:
>> 
>>  ---A---B---C--
>>  \ /
>>   D---E
>
> The commit message will say:
>
>   Merge branch 'master' of /that/remote
>
>   * 'master' of /that/remote:
> E
> D
>
>> where B is the local commit. D, E are commits pushed by someone else
>> when the developer was working on B. However sometimes the following
>> history is preferable:
>> 
>> ---A---D---C'--
>> \ /
>>  '-B-'
>
> Better:
>
>  ---A--D--E--C'
>  \  /
>   `B

Yup, that topology is what Kacper's workflow wants.

Stepping back a bit, however, I am not sure if that is really true.
The goal of this topic seems to be to keep one integration branch
and always merge *into* that integration branch, never *from* it,
but for what purpose?  Making the "log --first-parent" express the
integration branch as a linear series of progress?  If so, I suspect
a project with such a policy would dictate that D and E also be on a
side branch, i.e. the history would look more like this:

  D---E
 / \
  --A---X---C---
 \ /
  `---B

with X being a --no-ff merge of the topic that consists of these two
commits.

> In this case, the commit message should say... what? Certainly not the
> same thing. But I do not see that you changed anything in this regard.

True.  If the goal is to emulate a merge of B from a side branch
into _the_ integration branch, the summary should also emulate the
message that would be given when the remote pulled from your current
branch.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git fetch pack freezes

2012-11-28 Thread Shawn Pearce
On Wed, Nov 28, 2012 at 6:12 AM, Ivan Kanis  wrote:
> We are using git with the smart HTTP protocol. We are seeing the client
> freeze with Jenkins poll. It stops the continuous build which is a bit
> of a pain. It happens with the latest release 1.8.0 on the client side.
> The server is using 1.7.0.4.
>
> On the server we are seeing the following error message:
>
> var/log/apache2/error.log.1:[Mon Nov 19 15:17:12 2012] [error] [client 
> 192.168.50.15] fatal: git upload-pack: not our ref 
> aa92ca2076c2421433f89ee0b0ae89cd36d27481 multi_ack_detailed side-band-64k 
> thin-pack no-progress ofs-delta

Upgrade your server. What happened here is a race condition that is
only possible in the smart HTTP protocol. The client makes an initial
request to ask where the branches are. Then it makes a second (and
possibly more) subsequent request to negotiate a common ancestor and
actually obtain the necessary objects. If a branch is updated between
that initial request and any subsequent request, the server fails with
this error message.

More recent servers handle this more gracefully by looking to see if
the object the client wants is reachable within some reasonable
distance from a current branch. If so, the request is still allowed to
continue, rather than die() with a failure.

> On the client side I compiled git with debugging symbols. I then
> attached the debugger to the process that gets hosed. I get the
> following backtrace:
>
> #5  0x00010f8a1246 in packet_read_line (fd=0, buffer=0x10f981eb0 "", 
> size=1000) at pkt-line.c:211
> #6  0x00010f7b5196 in get_ack (fd=0, result_sha1=0x7fff6f377450 "") at 
> builtin/fetch-pack.c:227
> #7  0x00010f7b4074 in find_common (fd=0x7fff6f377668, 
> result_sha1=0x7fff6f377450 "", refs=0x0) at builtin/fetch-pack.c:416
>
> The file descriptor being 0 in #6 seems a bit suspicious. I am not
> familiar with the git protocol. Could I get some guidance on how to
> troubleshoot some more so that I come up with a patch?

There is actually a parent process in front of this one called
git-remote-http (or git-remote-https depending on protocol). That
parent is doing the HTTP transport and is sending whatever the server
provides into this child's stdin. I'm not surprised there is yet
another deadlock condition in here, its a rather heinous process setup
and way to handle the protocol.

fd in frame #7 is an address of an int array. fd in frame #6 is an
actual fd, the one to read from, which in this case should be 0 to
read from stdin, because its trying to get data from the HTTP server
by way of reading it from the pipe the parent supplied to it on stdin.

So the stack frames are correct. Its just a problem that the parent
didn't identify the server crashing and closing its side of the pipe
on stdin to force it to EOF to prevent the child from getting hung
here in a read.
--
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: Python extension commands in git - request for policy change

2012-11-28 Thread Magnus Bäck
On Tuesday, November 27, 2012 at 19:10 EST,
 Felipe Contreras  wrote:

> On Tue, Nov 27, 2012 at 3:35 PM, Magnus Bäck  wrote:
>
> > While "constant traffic" probably overstates the issue, these are
> > not theoretical problems. I recall at least three cases in the last
> > year or so where Git has seen breakage with Solaris or Mac OS X
> > because of sed or tr incompatibilities, and I don't even read this
> > list that thoroughly.
>
> Most of the *constant* traffic is about *theoretical*
> incompatibilities, how much of that are real incompatibilities, it's
> not known. _Some_ of the traffic is about real incompatibilities,
> sure, but you could count only three cases *in a year*. It's not a
> huge amount. And then, how man this year?
>
> Also, I would like references to those incompatibilities.

I don't remember the details of the Mac OS X problem, but just searching
the archives for "xpg4" revealed the following Solaris problems since
April:

http://article.gmane.org/gmane.comp.version-control.git/195010
http://article.gmane.org/gmane.comp.version-control.git/195966
http://article.gmane.org/gmane.comp.version-control.git/207944

-- 
Magnus Bäck
ba...@google.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 4/4] git-svn: Note about tags.

2012-11-28 Thread Michael J Gruber
Sebastian Leske venit, vidit, dixit 23.11.2012 08:29:
> Document that 'git svn' will import SVN tags as branches.
> 
> Signed-off-by: Sebastian Leske 
> ---
>  Documentation/git-svn.txt |6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 92780ef..6212b24 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -938,6 +938,12 @@ the possible corner cases (git doesn't do it, either).  
> Committing
>  renamed and copied files is fully supported if they're similar enough
>  for git to detect them.
>  
> +In SVN, it is possible (though discouraged) to commit changes to a tag
> +(because a tag is just a directory copy, thus technically the same as a
> +branch). When cloning an SVN repository, 'git svn' cannot know if such a
> +commit to a tag will happen in the future. Thus it acts conservatively
> +and imports all SVN tags as branches, prefixing the tag name with 'tags/'.
> +

If git-svn turned SVN tags in git lightweight tags it would be just as
conservative, because they can be rewritten also... It's just a ref.
Note that the above assumes standard refspecs.

But, importing SVN tags as branches conveys their character much more
precisely. If r8 is being tagged then svn creates r9, which may or may
not be treesame to r8; different projects do different things in this step.

Even if they are treesame, r9 still carries a separate commit message.
This is probably the most typical case, and converting the r9 commit
into a git tag object would be appropriate (but is left to the user of
git-svn).

A lightweight tag pointing to an "off-branch" commit (r9) would be a
strange thing, so git-svn creates a new branch instead, pointing at r9,
branching off r8.

>  CONFIGURATION
>  -
>  
> 

--
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/4] git-svn: Expand documentation for --follow-parent

2012-11-28 Thread Michael J Gruber
Sebastian Leske venit, vidit, dixit 23.11.2012 08:13:
> Describe what the option --follow-parent does, and what happens if it is
> set or unset.
> 
> Signed-off-by: Sebastian Leske 
> ---
>  Documentation/git-svn.txt |   16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 928a961..92780ef 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -628,11 +628,19 @@ ADVANCED OPTIONS
>   Default: "svn"
>  
>  --follow-parent::
> + This option is only relevant if we are tracking branches (using
> + one of the repository layout options --trunk, --tags,
> + --branches, --stdlayout). For each tracked branch, try to find
> + out where its revision was copied (i.e. branched) from, and set
> + a suitable parent in the first git commit for the branch.
>   This is especially helpful when we're tracking a directory
> - that has been moved around within the repository, or if we
> - started tracking a branch and never tracked the trunk it was
> - descended from. This feature is enabled by default, use
> - --no-follow-parent to disable it.
> + that has been moved around within the repository (note that you
> + must track both the old and the new name for the whole history
> + to be imported).  If this feature is disabled, the branches
> + created by 'git svn' will all be linaear and not share any

linear

> + history, meaning that there will be no information on where
> + branches where branched off or merged. This feature is enabled
> + by default, use --no-follow-parent to disable it.
>  +
>  [verse]
>  config key: svn.followparent
> 

--
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/4] Recommend use of structure options for git svn.

2012-11-28 Thread Michael J Gruber
Sebastian Leske venit, vidit, dixit 21.11.2012 08:13:
> Document that when using git svn, one should usually either use the
> directory structure options to import branches as branches, or only
> import one subdirectory. The default behaviour of cloning all branches
> and tags as subdirectories in the working copy is usually not what the
> user wants.

"Usually" assumes that usually svn repos follow a branch/tag structure.
That may be quite true nowadays (but wasn't in the beginning).

In fact I would suggest to emphasize "--stdlayout" as the most standard
choice nowadays, followed by specifying a specific branch (usually
trunk) as the repo uri.

> Signed-off-by: Sebastian Leske 
> ---
>  Documentation/git-svn.txt |   10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 482d60d..928a961 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -871,6 +871,16 @@ already dcommitted.  It is considered bad practice to 
> --amend commits
>  you've already pushed to a remote repository for other users, and
>  dcommit with SVN is analogous to that.
>  
> +When cloning an SVN repository, if none of the options for describing
> +the repository layout is used (--trunk, --tags, --branches,
> +--stdlayout), 'git svn clone' will create a git repository with
> +completely linear history, where branches and tags appear as separate
> +folders in the working copy.  For projects with many branches this will
> +lead to a working copy many times larger than just the trunk.  It is
> +recommended to either use the the options for trunk / tag / branch
> +directory, or to only clone the trunk (or a subdirectory of the
> +trunk).
> +
>  When using multiple --branches or --tags, 'git svn' does not automatically
>  handle name collisions (for example, if two branches from different paths 
> have
>  the same name, or if a branch and a tag have the same name).  In these cases,
> 

--
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/4] git-svn: Document branches with at-sign(@).

2012-11-28 Thread Michael J Gruber
Sebastian Leske venit, vidit, dixit 18.11.2012 18:08:
> git svn will sometimes create branches with an at-sign in the name
> (branchname@revision). These branches confuse many users and it is a FAQ
> why they are created. Document when git svn will create them.
> 
> Signed-off-by: Sebastian Leske 
> ---
> 
> I found various important features of git-svn that are not documented.
> Thus I created a series of patches to add and update documentation for
> git-svn. I am sending this as four patches (of which this mail is the
> first). I have tried my best to make the patches ready for inclusion
> into git, but if there's any feedback, I'll gladly incorporate it.
> 
> 
>  Documentation/git-svn.txt |   26 ++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 8b0d3ad..482d60d 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -881,6 +881,32 @@ different name spaces.  For example:
>   branches = stable/*:refs/remotes/svn/stable/*
>   branches = debug/*:refs/remotes/svn/debug/*
>  
> +If 'git svn' is configured to fetch branches (and --follow-branches
> +is in effect), it will sometimes create multiple branches for one SVN
> +branch, where the addtional branches have names of the form
> +'branchname@nnn' (with nnn an SVN revision number).  These additional
> +branches are created if 'git svn' cannot find a parent commit for the
> +first commit in the branch, to connect the branch to the history of the
> +other branches. Normally, 'git svn' will find the git commit that
> +corresponds to the SVN revision a branch was created (copied) from and
> +use that as the parent. However, it is possible that there is no
> +suitable git commit to serve as parent.  This will happen, among other
> +reasons, if the SVN branch is a copy of a revision that was not fetched
> +by 'git svn' (e.g. because it is an old revision that was skipped with
> +'--revision'), or if in SVN a directory was copied that is not tracked
> +by 'git svn' (a branch that is not tracked at all, or a subdirectory of
> +a tracked branch). In these cases, 'git svn' will still create a git
> +branch, but the branch will not be connected to the history of the other
> +branches.  'git svn' will also create a commit from the SVN revision the
> +branch was copied from, and use that as the parent commit of the branch
> +(this is indicated by the message "Initializing parent: ").
> +Additionally, it will create a special branch named
> +'@', where  is the SVN revision
> +number the branch was copied from.  This branch will point to the
> +specially created parent commit.
> +If in SVN the branch was deleted and later recreated from a different
> +version, there will be multiple such branches with an '@'.
> +
>  BUGS
>  
>  
> 

In fact, the most typical reason for at-branches is a botched branching
or tagging operation on the svn side: since they are simply "copy"
operations it's too easy to copy a tree into the wrong directory.

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


git fetch pack freezes

2012-11-28 Thread Ivan Kanis
Hello,

We are using git with the smart HTTP protocol. We are seeing the client
freeze with Jenkins poll. It stops the continuous build which is a bit
of a pain. It happens with the latest release 1.8.0 on the client side.
The server is using 1.7.0.4.

On the server we are seeing the following error message:

var/log/apache2/error.log.1:[Mon Nov 19 15:17:12 2012] [error] [client 
192.168.50.15] fatal: git upload-pack: not our ref 
aa92ca2076c2421433f89ee0b0ae89cd36d27481 multi_ack_detailed side-band-64k 
thin-pack no-progress ofs-delta

The error is located in upload-pack.c:

 * Hmph.  We may later want to allow "want" line that
 * asks for something like "master~10" (symbolic)...
 * would it make sense?  I don't know.
 */
o = lookup_object(sha1_buf);
if (!o || !(o->flags & OUR_REF))
die("git upload-pack: not our ref %s", line+5);

It calls die with I believe is doing the right thing(TM).

On the client side I compiled git with debugging symbols. I then
attached the debugger to the process that gets hosed. I get the
following backtrace:

#0  0x7fff85389af0 in read ()
#1  0x00010f900913 in xread (fd=0, buf=0x7fff6f376e2c, len=4) at 
wrapper.c:142
#2  0x00010f900a11 in read_in_full (fd=0, buf=0x7fff6f376e2c, count=4) at 
wrapper.c:171
#3  0x00010f8a14e4 in safe_read (fd=0, buffer=0x7fff6f376e2c, size=4, 
return_line_fail=0) at pkt-line.c:140
#4  0x00010f8a110e in packet_read_internal (fd=0, buffer=0x10f981eb0 "", 
size=1000, return_line_fail=0) at pkt-line.c:183
#5  0x00010f8a1246 in packet_read_line (fd=0, buffer=0x10f981eb0 "", 
size=1000) at pkt-line.c:211
#6  0x00010f7b5196 in get_ack (fd=0, result_sha1=0x7fff6f377450 "") at 
builtin/fetch-pack.c:227
#7  0x00010f7b4074 in find_common (fd=0x7fff6f377668, 
result_sha1=0x7fff6f377450 "", refs=0x0) at builtin/fetch-pack.c:416
#8  0x00010f7b32a5 in do_fetch_pack (fd=0x7fff6f377668, 
orig_ref=0x10fb007a0, sought=0x7fff6f377670, pack_lockfile=0x7fff6f377660)
at builtin/fetch-pack.c:832
#9  0x00010f7b2cbd in fetch_pack (my_args=0x10f973fe8, fd=0x7fff6f377668, 
conn=0x0, ref=0x10fb007a0, 
dest=0x7fff6f377a78 "https://foo.visionobjects.com/git/a-project-name/";, 
sought=0x7fff6f377670, pack_lockfile=0x7fff6f377660)
at builtin/fetch-pack.c:1069
#10 0x00010f7b2a25 in cmd_fetch_pack (argc=7, argv=0x7fff6f377890, 
prefix=0x0) at builtin/fetch-pack.c:1012
#11 0x00010f77aa5b in run_builtin (p=0x10f971990, argc=7, 
argv=0x7fff6f377890) at git.c:306
#12 0x00010f779ac7 in handle_internal_command (argc=7, argv=0x7fff6f377890) 
at git.c:467
#13 0x00010f77a299 in run_argv (argcp=0x7fff6f377858, argv=0x7fff6f377850) 
at git.c:513
#14 0x00010f7798fd in main (argc=7, argv=0x7fff6f377890) at git.c:588

The file descriptor being 0 in #6 seems a bit suspicious. I am not
familiar with the git protocol. Could I get some guidance on how to
troubleshoot some more so that I come up with a patch?
-- 
Ivan Kanis, Release Manager, Vision Objects,
Tel +33 2 28 01 49 50,  Fax +33 2 40 25 89 20
http://www.visionobjects.com

The essence of science: ask an impertinent question, and you are on
the way to a pertinent answer.
-- Jacob Bronowski
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

2012-11-28 Thread Simon Oosthoek
* Piotr Krukowiecki  [2012-11-28 11:03:29 +0100]:

> Hi,
> 
> when I set PROMPT_COMMAND to __git_ps1 I get a space at the beginning:
> 

Is your setting?:
PROMPT_COMMAND=__git_ps1

I believe you need to give 2 parameters in order to use it in PROMPT_COMMAND 
mode.

In my .bashrc I have:
if [ -f ~/.gitprompt.sh ]
then
. ~/.gitprompt.sh
GIT_PS1_SHOWDIRTYSTATE=true
GIT_PS1_SHOWCOLORHINTS=true
GIT_PS1_SHOWUNTRACKEDFILES=true
PROMPT_COMMAND="__git_ps1 '\u@\[\e[1;34m\]\h\[\e[0m\]:\w' '\\\$ '"
fi


>  (master)pkruk@foobar ~/dir$
> ^ space
> 
> Is there a reason for this? It looks like a waste of space. If I'm not in
> git repository I don't have the space:
> 
> pkruk@foobar ~/other$
> 
> I noticed the space is explicitly specified in printf_format in
> git-prompt.sh. Is it needed? If I remove it, everything seems to work fine
> (no leading space)...
> 
> --- /usr/local/src/git/git/contrib/completion/git-prompt.sh 2012-11-28
> 10:27:05.728939201 +0100
> +++ /home/pkruk/.git-prompt.sh 2012-11-28 10:52:56.852629745 +0100
> @@ -218,7 +218,7 @@ __git_ps1 ()
>   local detached=no
>   local ps1pc_start='\u@\h:\w '
>   local ps1pc_end='\$ '
> - local printf_format=' (%s)'
> + local printf_format='(%s)'
> 
>   case "$#" in
>   2) pcmode=yes


These last 2 lines say: if 2 arguments are given, use pcmode. Otherwise you get 
command-subtitution mode, which gives weird effects when being called from 
PROMPT_COMMAND.

Cheers

Simon
--
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 v4 0/4] git-submodule add: Add --local-branch option (summary)

2012-11-28 Thread W. Trevor King
On Tue, Nov 27, 2012 at 09:42:05PM -0500, W. Trevor King wrote:
> On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
> > https://github.com/hvoigt/git/commits/hv/floating_submodules_draft
> 
> I looked over this before, but maybe not thoroughly enough ;).

Heiko pointed out that I likely hadn't looked at this branch at all,
which is true.  I'd confused it with his earlier:

On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote:
> https://github.com/hvoigt/git/commits/hv/floating_submodules

but the new floating_submodules_draft branch adds Heiko's function
reorganization and floating submodule pull (with 'update --branch') on
top of my v4 commits (instead of my branch-checkout with 'update
--branch').

> This looks fine, but my current --branch implementation (which doesn't
> pull) is only a thin branch-checkout layer on top of the standard
> `update` functionality.  I'm still unsure if built-in pulls are worth
> the configuration trouble.  I'll sleep on it.  Maybe I'll feel better
> about them tomorrow ;).

I do feel better about them today.  To get a better feel for how
people see this going forward, here is a summary of proposals to date:

v1. Record submodule..branch with 'submodule add --branch …',
leave interpretation up to the user.

Feedback (paraphrased):
  Phil Hord: that clobbers a configure option used by Gerritt and
possibly others, rename to --record-branch.  Maybe we should
export submodule..* as $submodule_* in 'foreach'?
  Nahor: what about corner cases (e.g. upstream renames branches)?
  Jens Lehmann: if you record it, people will expect Git to use
it.  What about automatic pulls & commits?

v2. Add --record instead of using --branch to address Phil's v1
comment.

Feedback (paraphrased):
  Jens Lehmann: this is still a tiny optimization over using 'git
config'.  People still use this option in different ways.
  Shawn Pearce: the Gerrit use is basically the same as Ævar's,
but Gerrit has special handling for '.'.
  Jeff King: if we set up submodule..branch, we should
either use it in Git, or make it very clear that we do not use
it.  If we use Phil's $submodule_* export, we should clear the
variables for recursive submodules.

v3. Renamed Added $submodule_* export to v2.  Give an example of
Ævar's pull workflow when explaining that Git does not use the
value internally.

Feedback (paraphrased):
  Heiko Voigt: what about automatic pulls & commits?  You should
allow .git/config overrides of the .gitmodules settings.  if
we set up submodule..branch, we should use it in Git.
How does the submodule know which remote branch to track?
Junio's proposed 'diff' changes may hide $sha1 information.
  Junio C Hamano: if we set up submodule..branch, we should
use it in Git.  Use something more specific than --record.
Update 'git diff [$path]' and friends in the superproject
compares the HEAD of the checkout of the submodule at $path
with the tip of the branch named by submodule.$name.branch in
.gitmodules of the superproject, instead of the commit that is
recorded in the index of the superproject.
  Sascha Cunz: you can use 'git shortlog $OLD_SHA1..$NEW_SHA1' for
the automatic commit message.
  Trevor King: actually, Ævar's update command only specifies the
local branch name.  The remote is recorded for that branch in
.git/modules//config.

v4. Rename --record to --local-branch.  Remove $submodule_* export.
Add .git/config overrides for submodule..branch.  Add
'submodule update --branch' to check out $sha1 as
submodule..branch.

Feedback (paraphrased):
  Heiko Voigt: who cares about the local branch name?  You should
be pulling origin/$branch, but still using
.git/modules//config to record the tracked branch.  You
should also use 'submodule add --branch[=branch]' instead of
'--local-branch'.  You should pull the 'update --branch'
functionality out into a sub-function like
'handle_tracked_branch_update'.
  Jens Lehmann: .git/config overrides are nice, but they should be
optional.
  Trevor King: floating branches are following the submodule's
remote, while 'update --rebase/--merge' are following the
superproject's recorded $sha1.  Bundling remote-following and
superproject-following in the same command may be confusing.

Here's what I think we need in v5:

* Jens' optional .git/config overrides.
* Return --local-branch handling to a side effect of --branch (as in
  v1).
* A new 'submodule pull' for tracking the submodule's remote, which is
  pulling --ff-only origin/$branch into a whatever state the submodule
  is currently in.  If any changes were made to submodule $shas,
  optionally commit them with a shortlog-generated commit message.
* Remove my c

Re: git config key bug or by design?

2012-11-28 Thread Peter van der Does
On Tue, 27 Nov 2012 22:34:09 -0500
Jeff King  wrote:
> 
> I don't recall ever discussing it. But what is it that you want to
> store in a key starting with a number? Git does not respect any such
> config values[1].
> 
> Are you writing a new tool that will store its config alongside git's?
> Even if the behavior is loosened, you would probably want to avoid
> starting your config keys with numbers, as older git versions would be
> around for a while and would choke on it.
> 
> -Peff
> 
> [1] You can still store arbitrary bytes in the subsection name (e.g.,
> "foo.123.bar").

I am writing a tool, it needs to store branch names in a separate config
file. 

It's clear git doesn't respect those values, hence my question. I
understand how to work around the problem, I would just prefix the key.
I was just wondering if it was by design, which I guess it is as the
parsing of the file will die if the key starts with a non-alpha
character.


-- 
Peter van der Does

GPG key: CB317D6E

IRC: Ganseki on irc.freenode.net
Twitter: @petervanderdoes
--
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] configure.ac: fix pthreads detection on Mac OS X

2012-11-28 Thread Max Horn

On 28.11.2012, at 07:38, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> The configure script checks whether certain flags are required to use
>> pthreads. But it did not consider that *none* might be needed (as is the
>> case on Mac OS X). This lead to configure adding "-mt" to the list of
>> flags (which does nothing on OS X except producing a warning). This in
>> turn triggered a compiler warning on every single file.
>> 
>> To solve this, we now first check if pthreads work without extra flags.
>> This means the check is now order dependant, hence a comment is added
>> explaining this, and the reasons for it.
>> 
>> Note that it might be possible to write an order independent test, but
>> it does not seem worth the extra effort required for implementing and
>> testing such a solution, when this simple solution exists and works.
>> 
>> Signed-off-by: Max Horn 
>> ---
>> 
>> This is actually a revised version from my patch
>> "Change configure to check if pthreads are usable without any extra flags"
>> from July. I simply had forgotten all about it :-(.
> 
> Will queue,

OK

> but we would need wider testing to avoid "compiles well
> without an option but fails to link" issues similar to cea13a8
> (Improve test for pthreads flag, 2011-03-28) on other people's
> platforms (I know you tested on Mac OS X and over there it compiles
> and links well---I am worried about others).

Sure, understood. Though note that the test in question performs a compile & 
link test. So I have a hard time to see how this could break something. Then 
again, I dabbled in portable code long enough to never say never ;-).

BTW, is there such a thing as a build farm for git which automatically compiles 
and runs tests for pu / next / main, across a variety of platforms? Or does it 
all rely on devs test building everything regularly?


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


Re: git-svn: What is --follow-parent / --no-follow-parent for?

2012-11-28 Thread Steven Michalske

On Nov 19, 2012, at 11:31 PM, Sebastian Leske  
wrote:
> 
> Commit graph of git-svn result:
> 
> --follow-parent:   --no-follow-parent:
> 
> 
>   |   |
>  /| | |
> / | | |
> | | | |
> | | | |
> | | | |
> \ | | |
>  \| | |
>   |   | 
> 
> 
> (please excuse cheap ASCII art)
> 
> Is that the only effect of --no-follow-parent? And again, why would I
> want that?

I suppose that you would want this for a SVN repo that holds many different and 
possibly loosely related projects.
The you can apply git filter branch then to split them up cleanly.

e.g. Imagine that the two root commits of the branches do not have true common 
history.

Steve

--
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: Millisecond precision in timestamps?

2012-11-28 Thread Andreas Ericsson
On 11/28/2012 09:04 AM, David Aguilar wrote:
> On Tue, Nov 27, 2012 at 11:58 PM, Eric S. Raymond  wrote:
>> Junio C Hamano :
>>> Roundtrip conversions may benefit from sub-second timestamps, but
>>> personally I think negative timestamps are more interesting and of
>>> practical use.
>>
>> You mean, as in times before the Unix epoch 1970-01-01T00:00:00Z?
>>
>> Interesting.  I hadn't thought of that.  I've never seen a software
>> project under version control with bits that old, which is significant
>> because I've probably done more digging into ancient software than
>> anybody other than a specialist historian or two.
> 
> One example I've heard is someone wanting to throw the history
> of a country's laws into git so they can diff them.
> 

That'll get tricky if you try it in Sweden. Our oldest written law
dates back to 1281. Quite fun reading. Apparently it was against the
law to shoot your slaves with stone arrows back then.

See my other proposal for how this could be done, which would only
affect the output layer (and some care would have to be taken with
the input, naturally).

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


Re: Millisecond precision in timestamps?

2012-11-28 Thread Andreas Ericsson
On 11/28/2012 08:29 AM, Junio C Hamano wrote:
> Jeff King  writes:
> 
>> There is room for new headers, and older versions of git will ignore
>> them. You could add a new "committer-timestamp" field that elaborates on
>> the timestamp included on the committer line. Newer versions of git
>> would respect it, and older versions would fall back to using the
>> committer timestamp.
>>
>> But I really wonder if anybody actually cares about adding sub-second
>> timestamp support, or if it is merely "because SVN has it".
> 
> Roundtrip conversions may benefit from sub-second timestamps, but
> personally I think negative timestamps are more interesting and of
> practical use.  Prehistoric projects need them even if they intend
> to switch to Git, never to go back to their original tarballs and
> collection of RCS ,v files.
> 
> And if we were to add "committer-timestamp" and friends to support
> negative timestamps anyway (because older tools will not support
> them), supporting sub-second part might be something we want to
> think about at the same time.
> 
> We would however need to be extra careful.  How should we express
> half-second past Tue Nov 27 23:24:16 2012 (US/Pacific)?  Would we
> spell it 1354087456.5?  1354087456.500?  Would we require decimal
> representation of floating point numbers to be normalized in some
> way (e.g. minimum number of digits without losing precision)?  The
> same timestamp needs to be expressed the same way, or we will end up
> with different commit objects, which defeats the whole purpose of
> introducing subsecond timestamps to support round-trip conversions.
> 
> If we were to use a separate "subsecond" fields, another thing we
> need to be careful about is the order of these extra fields, exactly
> for the same reason.
> 

If we're going to support pre-epoch timestamps, we'll have to do that
for 2.0 anyway, since we'll otherwise have two conflicting dates in
the commit object.

Adding support for parsing them now and start writing them in 2.0
would make sense.

In that case, we'd have to print timestamps as
printf("%lu.%06lu", tv.tv_sec, tv.tv_usec);

I'm unsure how useful it is to support pre-epoch dates though. It's
hard to find software where anyone really cares about the code from
43 years ago with anything but historical interest, and for those
who take the museum road, I'm betting it's more interesting to see
how people worked back then than it is to see what they wrote.

Aside from that, it would be trivial to support museum style history
viewing with a special flag that treats the timestamps as minutes
since 1900-01-01 or some such, giving us plenty of time before even
the first punch-card was invented. It wouldn't be much harder to
let the user specify the timeunit and the start-point either, and
then we could store the history of carbon-based lifeforms on earth
in git.

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


Re: Possible vulnerability to SHA-1 collisions

2012-11-28 Thread Andreas Ericsson
On 11/28/2012 01:27 AM, Jeff King wrote:
> On Tue, Nov 27, 2012 at 06:30:17PM -0500, Aaron Schrab wrote:
> 
>> At 18:07 -0500 27 Nov 2012, Jeff King  wrote:
>>> PS I also think the OP's "sockpuppet creates innocuous bugfix" above is
>>>   easier said than done. We do not have SHA-1 collisions yet, but if
>>>   the md5 attacks are any indication, the innocuous file will not be
>>>   completely clean; it will need to have some embedded binary goo that
>>>   is mutated randomly during the collision process (which is why the
>>>   md5 attacks were demonstrated with postscript files which _rendered_
>>>   to look good, but contained a chunk of random bytes in a spot ignored
>>>   by the postscript interpreter).
>>
>> I don't think that really saves us though.  Many formats have parts
>> of the file which will be ignored, such as comments in source code.
> 
> Agreed, it does not save us unconditionally. It just makes it harder to
> execute the attack. Would you take a patch from a stranger that had a
> kilobyte of binary garbage in a comment?
> 
> A more likely avenue would be a true binary file where nobody is
> expected to read the diff.
> 
>> With the suggested type of attack, there isn't a requirement about
>> which version of the file is modified.  So the attacker should be
>> able to generate a version of a file with an innocuous change, get
>> the SHA-1 for that, then add garbage comments to their malicious
>> version of the file to try to get the same SHA-1.
> 
> That's not how birthday collision attacks usually work, though. You do
> not get to just mutate the malicious side and leave the innocuous side
> untouched. You are mutating both sides over and over and hoping to find
> a matching sha1 from the "good" and "evil" sides.
> 
> Of course, I have not been keeping up too closely with the efforts to
> break sha-1. Maybe there is something more nefarious about the current
> attacks. I am just going off my recollection of the md5 collision
> attacks.
> 

AFAIR, collision attacks can be executed with a 2^51 probability (with
a 2^80 claim, that's pretty bad), but preimage attacks are still stuck
very close to the claimed 2^160.

That means every attack involving SHA1 means Mr. Malicious creates
both the involved files or does exceptional research without sharing
it.

I think git's job is to make sure that write access to only one of
the repositories is insufficient to launch an attack. If the attacker
manages to change all repositories involved then the hash function
used is really quite irrelevant.

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


Re: Millisecond precision in timestamps?

2012-11-28 Thread Thomas Berg
On Wed, Nov 28, 2012 at 9:44 AM, Felipe Contreras
 wrote:
>> If roundtripping to other version control systems is an argument,
>> adding sub-second timestamps could potentially create as many problems
>> as it solves. For example, I've been using the hg-git bridge, and it
>> supports roundtripping between git and mercurial today (for most repos
>> I've tried anyway). I may have missed something, but this could imply
>> that mercurial doesn't care about sub-second timestamps either. If so,
>> and if git suddenly were to record it, it would no longer be as
>> straight forward to represent git history in hg.
>
> I'm not entirely sure. The API seems to return a float for the time,
> but at least as far I can see, it never has any decimals anyway.
>
> But it doesn't really matter, mercurial doesn't have a committer
> information either. This is solved by tools like hg-git by storing the
> information in an 'extra' field, which can store anything.

True. For many commits though, hg-git doesn't need any extra fields,
as far as I've seen. A timestamp incompatibility would require extra
info on every commit.

> Either way, I don't see the point in changing git's commit format for
> external tools. The git-notes functionality works just fine for that,
> it just needs to be attached in the relevant places, like 'git
> fast-export'.

I agree. Even encoding info in the commit message works fine, and
git-svn already does that.

> BTW. Have you checked git's native support for hg?[1]

That's been added after I played with this last, I'll have a look.

Cheers,
Thomas
--
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] svnrdump_sim: start the script with /usr/bin/env python

2012-11-28 Thread Christian Couder
On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras
 wrote:
> On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano  wrote:
>> Christian Couder  writes:
>>
>>> All the python scripts except contrib/svn-fe/svnrdump_sim.py
>>> start with "#!/usr/bin/env python".
>>>
>>> This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.
>>
>> I suspect you need a bit more than that.
>>
>> $ make git-p4
>> $ diff -u git-p4.py git-p4
>>
>> shows you how we tell the scripts how to find their interpreters
>> (that way, there is no need to rely on the existence of
>> /usr/bin/env).
>
> That works if somebody managed to export PYTHON_PATH, which very very
> often is not the case for me.

Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh,
svnrdump.py is used as is.
So if your python is not /usr/bin/python, you cannot just add
something to $PATH to pass the test.

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


Re: Millisecond precision in timestamps?

2012-11-28 Thread Felipe Contreras
On Wed, Nov 28, 2012 at 9:19 AM, Thomas Berg  wrote:
> On Wed, Nov 28, 2012 at 8:29 AM, Junio C Hamano  wrote:
>> Jeff King  writes:
>>
>>> There is room for new headers, and older versions of git will ignore
>>> them. You could add a new "committer-timestamp" field that elaborates on
>>> the timestamp included on the committer line. Newer versions of git
>>> would respect it, and older versions would fall back to using the
>>> committer timestamp.
>>>
>>> But I really wonder if anybody actually cares about adding sub-second
>>> timestamp support, or if it is merely "because SVN has it".
>>
>> Roundtrip conversions may benefit from sub-second timestamps, but
>> personally I think negative timestamps are more interesting and of
>> practical use.  Prehistoric projects need them even if they intend
>> to switch to Git, never to go back to their original tarballs and
>> collection of RCS ,v files.
>
> If roundtripping to other version control systems is an argument,
> adding sub-second timestamps could potentially create as many problems
> as it solves. For example, I've been using the hg-git bridge, and it
> supports roundtripping between git and mercurial today (for most repos
> I've tried anyway). I may have missed something, but this could imply
> that mercurial doesn't care about sub-second timestamps either. If so,
> and if git suddenly were to record it, it would no longer be as
> straight forward to represent git history in hg.

I'm not entirely sure. The API seems to return a float for the time,
but at least as far I can see, it never has any decimals anyway.

But it doesn't really matter, mercurial doesn't have a committer
information either. This is solved by tools like hg-git by storing the
information in an 'extra' field, which can store anything.

Unfortunately git doesn't have a similar field, so people have been
using the commit message to store extra information.

Either way, I don't see the point in changing git's commit format for
external tools. The git-notes functionality works just fine for that,
it just needs to be attached in the relevant places, like 'git
fast-export'.

BTW. Have you checked git's native support for hg?[1]

Cheers.

[1] http://felipec.wordpress.com/2012/11/13/git-remote-hg-bzr-2/

-- 
Felipe Contreras
--
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: Millisecond precision in timestamps?

2012-11-28 Thread Thomas Berg
On Wed, Nov 28, 2012 at 8:29 AM, Junio C Hamano  wrote:
> Jeff King  writes:
>
>> There is room for new headers, and older versions of git will ignore
>> them. You could add a new "committer-timestamp" field that elaborates on
>> the timestamp included on the committer line. Newer versions of git
>> would respect it, and older versions would fall back to using the
>> committer timestamp.
>>
>> But I really wonder if anybody actually cares about adding sub-second
>> timestamp support, or if it is merely "because SVN has it".
>
> Roundtrip conversions may benefit from sub-second timestamps, but
> personally I think negative timestamps are more interesting and of
> practical use.  Prehistoric projects need them even if they intend
> to switch to Git, never to go back to their original tarballs and
> collection of RCS ,v files.

If roundtripping to other version control systems is an argument,
adding sub-second timestamps could potentially create as many problems
as it solves. For example, I've been using the hg-git bridge, and it
supports roundtripping between git and mercurial today (for most repos
I've tried anyway). I may have missed something, but this could imply
that mercurial doesn't care about sub-second timestamps either. If so,
and if git suddenly were to record it, it would no longer be as
straight forward to represent git history in hg.

In my opinion it would be a shame to sacrifice this compatibility just
to reduce the distance to svn, which is much larger anyway.

- Thomas
--
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: Millisecond precision in timestamps?

2012-11-28 Thread David Aguilar
On Tue, Nov 27, 2012 at 11:58 PM, Eric S. Raymond  wrote:
> Junio C Hamano :
>> Roundtrip conversions may benefit from sub-second timestamps, but
>> personally I think negative timestamps are more interesting and of
>> practical use.
>
> You mean, as in times before the Unix epoch 1970-01-01T00:00:00Z?
>
> Interesting.  I hadn't thought of that.  I've never seen a software
> project under version control with bits that old, which is significant
> because I've probably done more digging into ancient software than
> anybody other than a specialist historian or two.

One example I've heard is someone wanting to throw the history
of a country's laws into git so they can diff them.
-- 
David
--
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] svnrdump_sim: start the script with /usr/bin/env python

2012-11-28 Thread Felipe Contreras
On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> All the python scripts except contrib/svn-fe/svnrdump_sim.py
>> start with "#!/usr/bin/env python".
>>
>> This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.
>
> I suspect you need a bit more than that.
>
> $ make git-p4
> $ diff -u git-p4.py git-p4
>
> shows you how we tell the scripts how to find their interpreters
> (that way, there is no need to rely on the existence of
> /usr/bin/env).

That works if somebody managed to export PYTHON_PATH, which very very
often is not the case for me.

./git-p4
zsh: ./git-p4: bad interpreter: /usr/bin/python: no such file or directory

In this case git-p4.py is correct, and git-p4 is not.

Either way, this is for contrib, and we don't have a standard
procedure for python scripts there. /usr/bin/env is better than
nothing, and in the vast majority of cases, more than enough.

Cheers.

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