Re: [PATCH 10/17] get_revision_internal(): make check less mysterious

2013-05-22 Thread Michael Haggerty
On 05/21/2013 07:38 PM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> The condition under which gc_boundary() is called was previously
>>
>> if (alloc <= nr)
>>
>> .  But by construction, nr can never exceed alloc, so the check looks
>> unnecessarily mysterious.  In fact, the purpose of the check is to try
>> to avoid a realloc() call by shrinking the array if possible if it is
>> at its allocation limit when a new element is about to be added.  So
>> change the check to
>>
>> if (nr == alloc)
>>
>> and add a comment to explain what's going on.
>>
>> Signed-off-by: Michael Haggerty 
>> ---
>> Please check that I have properly described the purpose of this check.
>>
>> The way the code is written, it looks like a bad pattern of growth and
>> shrinkage of the array (namely, just under the resize limit) could
>> cause gc_boundary() to be called over and over again with (most of)
>> the same data.  I hope that the author had some reason to believe that
>> such a pattern is unlikely.
> 
> That is about comparing with "alloc", not having high and low
> watermarks, right?
> 
> I do not see "alloc <= nr" is mysterious at all; it is merely being
> defensive.

If nr would ever exceed alloc, then the code would be broken and would
probably have already performed an illegal memory access.  Pretending to
support nr > alloc here is not defensive but misleading, because by that
time the ship is going down anyway.

On a more practical level, when I saw this code I thought to myself
"that's strange, I'd better look into it because it suggests that I
don't understand the meaning of nr and alloc".  I think that the
suggested change will help prevent the next reader from repeating the
same pointless investigation.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/17] revision: split some overly-long lines

2013-05-22 Thread Michael Haggerty
On 05/21/2013 07:34 PM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> Signed-off-by: Michael Haggerty 
>> ---
>>  revision.c | 20 ++--
>>  revision.h | 32 +---
>>  2 files changed, 35 insertions(+), 17 deletions(-)
> 
> Looks obviously good for *.c file, but I am on the fence for *.h
> one, as the reason we kept these long single lines in *.h files was
> to help those who want to grep in *.h files to let them view the
> full function signature.  It probably is OK to tell them to use
> "git grep -A$n" instead, though.

My goal with this patch was not to set a new policy but rather just to
make the code conform a little better to the existing policy as
described in CodingGuidelines.  *If* it is preferred that header files
list all parameters on a single line, then by all means adjust the
CodingGuidelines and I will just as happily make header files conform to
*that* policy when I touch them :-)

(That being said, my personal preference is to apply the 80-character
limit for header files too.)

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Your windows version has virus ?

2013-05-22 Thread Felipe Contreras
On Thu, May 23, 2013 at 12:40 AM, Peter Krefting  wrote:
> Klavs Rommedahl:
>
>>  I just downloaded your Windows version, and got a virus attack, and a
>> trojan.
>>  So sad to try "a new world" on the net, and then getting attacked. What
>> to do ? Any clues ?
>
>
> Which URL did you download from, exactly?
>
> 
> checks out clean, at least according to VirusTotal:
> 

This is not spam?

-- 
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: Your windows version has virus ?

2013-05-22 Thread Peter Krefting

Klavs Rommedahl:


 I just downloaded your Windows version, and got a virus attack, and a trojan.
 So sad to try "a new world" on the net, and then getting attacked. What to do 
? Any clues ?


Which URL did you download from, exactly?

 
checks out clean, at least according to VirusTotal: 



--
\\// Peter - http://www.softwolves.pp.se/
--
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] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 11:07 PM, Felipe Contreras
 wrote:
> On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano  wrote:
>> Felipe Contreras  writes:
>>
 IIRC, git-gui runs two blames, one without any -C and one with (I do
 not offhand recall how many -C it uses) to show both.
>>>
>>> 'git blame' is a very expensive operation, perhaps we should add
>>> another option so users don't need to run two blames to find this.
>>
>> Yes, you could add a "struct origin *suspect_in_the_same_file" next
>> to the current "struct origin *suspect" to the blame_entry in order
>> to represent the origin you would get if you were to stop at a "move
>> across files" event, and keep digging, when you are operating under
>> "-C -C" or higher mode.
>
> No, this is what I meant:

But that would not print the right line numbers, so perhaps:

--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1500,25 +1500,16 @@ static int emit_one_suspect_detail(struct
origin *suspect, int repeat)
return 1;
 }

-/*
- * The blame_entry is found to be guilty for the range.  Mark it
- * as such, and show it in incremental output.
- */
-static void found_guilty_entry(struct blame_entry *ent)
+static void print_guilty_entry(struct blame_entry *ent)
 {
-   if (ent->guilty)
-   return;
-   ent->guilty = 1;
-   if (incremental) {
-   struct origin *suspect = ent->suspect;
-
-   printf("%s %d %d %d\n",
-  sha1_to_hex(suspect->commit->object.sha1),
-  ent->s_lno + 1, ent->lno + 1, ent->num_lines);
-   emit_one_suspect_detail(suspect, 0);
-   write_filename_info(suspect->path);
-   maybe_flush_or_die(stdout, "stdout");
-   }
+   struct origin *suspect = ent->suspect;
+
+   printf("%s %d %d %d\n",
+   sha1_to_hex(suspect->commit->object.sha1),
+   ent->s_lno + 1, ent->lno + 1, ent->num_lines);
+   emit_one_suspect_detail(suspect, 0);
+   write_filename_info(suspect->path);
+   maybe_flush_or_die(stdout, "stdout");
 }

 /*
@@ -1531,14 +1522,19 @@ static void assign_blame(struct scoreboard *sb, int opt)
struct rev_info *revs = sb->revs;

while (1) {
-   struct blame_entry *ent;
+   struct blame_entry *ent, tmp_ent;
struct commit *commit;
struct origin *suspect = NULL;
+   int found_guilty = 0;

/* find one suspect to break down */
-   for (ent = sb->ent; !suspect && ent; ent = ent->next)
-   if (!ent->guilty)
+   for (ent = sb->ent; ent; ent = ent->next)
+   if (!ent->guilty) {
+   tmp_ent = *ent;
suspect = ent->suspect;
+   break;
+   }
+
if (!suspect)
return; /* all done */

@@ -1564,9 +1560,20 @@ static void assign_blame(struct scoreboard *sb, int opt)
commit->object.flags |= UNINTERESTING;

/* Take responsibility for the remaining entries */
-   for (ent = sb->ent; ent; ent = ent->next)
-   if (same_suspect(ent->suspect, suspect))
-   found_guilty_entry(ent);
+   for (ent = sb->ent; ent; ent = ent->next) {
+   if (same_suspect(ent->suspect, suspect)) {
+   if (ent->guilty)
+   continue;
+   found_guilty = ent->guilty = 1;
+   if (incremental)
+   print_guilty_entry(ent);
+   }
+   }
+
+   if (incremental && !found_guilty &&
+   !is_null_sha1(suspect->commit->object.sha1))
+   print_guilty_entry(&tmp_ent);
+
origin_decref(suspect);

if (DEBUG) /* sanity */

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


Your windows version has virus ?

2013-05-22 Thread Klavs Rommedahl
Hi.

  I just downloaded your Windows version, and got a virus attack, and a trojan.

  So sad to try "a new world" on the net, and then getting attacked. What to do 
? Any clues ?

Sincerely
  A very very very disapointed - Klavs.

Ps : both McAfee, Symantec and Kaspersky find these attacks.

Sendt fra min iPhone--
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] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>>> IIRC, git-gui runs two blames, one without any -C and one with (I do
>>> not offhand recall how many -C it uses) to show both.
>>
>> 'git blame' is a very expensive operation, perhaps we should add
>> another option so users don't need to run two blames to find this.
>
> Yes, you could add a "struct origin *suspect_in_the_same_file" next
> to the current "struct origin *suspect" to the blame_entry in order
> to represent the origin you would get if you were to stop at a "move
> across files" event, and keep digging, when you are operating under
> "-C -C" or higher mode.

No, this is what I meant:

--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1509,16 +1509,6 @@ static void found_guilty_entry(struct blame_entry *ent)
if (ent->guilty)
return;
ent->guilty = 1;
-   if (incremental) {
-   struct origin *suspect = ent->suspect;
-
-   printf("%s %d %d %d\n",
-  sha1_to_hex(suspect->commit->object.sha1),
-  ent->s_lno + 1, ent->lno + 1, ent->num_lines);
-   emit_one_suspect_detail(suspect, 0);
-   write_filename_info(suspect->path);
-   maybe_flush_or_die(stdout, "stdout");
-   }
 }

 /*
@@ -1536,12 +1526,24 @@ static void assign_blame(struct scoreboard *sb, int opt)
struct origin *suspect = NULL;

/* find one suspect to break down */
-   for (ent = sb->ent; !suspect && ent; ent = ent->next)
-   if (!ent->guilty)
+   for (ent = sb->ent; ent; ent = ent->next)
+   if (!ent->guilty) {
suspect = ent->suspect;
+   break;
+   }
+
if (!suspect)
return; /* all done */

+   if (incremental &&
!is_null_sha1(suspect->commit->object.sha1)) {
+   printf("%s %d %d %d\n",
+
sha1_to_hex(suspect->commit->object.sha1),
+   ent->s_lno + 1, ent->lno + 1,
ent->num_lines);
+   emit_one_suspect_detail(suspect, 0);
+   write_filename_info(suspect->path);
+   maybe_flush_or_die(stdout, "stdout");
+   }
+
/*
 * We will use this suspect later in the loop,
 * so hold onto it in the meantime.

-- 
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 v6] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 10:23 PM, Felipe Contreras
 wrote:
> This doesn't make any sense:

Ah, never mind, it's COPYING the one being modified, not EXTRACTING.

-- 
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 v6] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
This doesn't make any sense:

---
cd /tmp
rm -rf blame

git init blame
cd blame

cp ~/dev/git/COPYING COPYING
git add COPYING
git commit -m initial

sed -ne 120,140p COPYING >EXTRACTING
git add EXTRACTING
git commit -m second

git log --oneline
git blame -C EXTRACTING

echo >>COPYING
git commit --amend -a --no-edit

git log --oneline
git blame -C EXTRACTING
---

Why would the first instance find the blame in commit #2, and the
second one at commit #1? If anything, the second instance should have
more trouble finding the original commit.

-- 
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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Theodore Ts'o
On Wed, May 22, 2013 at 11:55:00AM -0700, Junio C Hamano wrote:
> But in a triangular workflow, the way to make the result reach the
> "upstream" is *not* by pushing there yourself.  For developers at
> the leaf level, it is to push to their own repository (often on
> GitHub), which is different from where they (initially) clone from
> in order to bootstrap themselves, and (subsequently) pull from in
> order to keep them up-to-date.  And then they request the published
> work to be pulled by the "upstream".

Yep, what I do personally is to call the destination of this "publish", i.e.:

[remote "publish"]
url = ssh://gitol...@ra.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.g
push = +master:master
push = +origin:origin
push = +dev:dev

So my typical work flow when I am ready to submit to Linus is:

   git tag -s ext4_for_linus
   git push publish

   git request-pull 
git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git origin > /tmp/pull


But actually, it's much more common that I am doing a "git push
publish" so that (a) it can get picked up by the daily linux-next tree
(for integration testing even before Linus pulls it into his tree),
and (b) so other ext4 developers so they can either test or develop
against the ext4 tree in progress.

I suppose it would be convenient for "git push" to push to the
"publish" target, but I don't get confused about pushing to origin,
since semantically what I am doing is publishing the current state of
the ext4 tree so other people can see it.  So "git push publish" makes
a lot of sense to me.

> Even in a triangular workflow, @{u} should still refer to the place
> you integrate with, i.e. your "upstream", not to the place you push
> to publish the result of your work.
> 
> This branch..rewindable safety however cannot be tied to
> @{u}.  The bottom boundary you want to be warned when you cross is
> the change you pushed out to your publishing repository, and it may
> not have reached remotes.origin. yet.

Indeed, and in fact for my use case what I promise people is that all
of the commits between origin..master are non-rewindable.  It's the
commits betewen master..dev which are rewindable.  So for me, I'd
still use the safety feature even for my rewindable branch, but
instead of using remotes/publish/dev the "no-rewind" point, I'd want
to use remotes/publish/master as the "no-rewind" point.

Right now I do this just by being careful, but if there was an
automatic safety mechanism, it would save me a bit of work, since
otherwise I might not catch my mistake until I do the "git push
publish", at which point I curse and then start consulting the reflog
to back the state of my tree out, and then reapplying the work I had
to the right tree.

> We will be introducing remote.pushdefault configuration in the
> upcoming 1.8.3 release, so that you can say.
>
> and hopefully it would let you do this:
> 
>   git checkout master
> ... after working on it ...
> git push

Yes, that would be convenient.  BTW, one of the other things which I
do for e2fsprogs is that I use multiple publishing points, which is
mostly for historical reasons --- it used to be that repo.or.cz wasn't
all that reliable, and the 10-15 minute replication time from
ra.kernel.org to git.kernel.org got really old.

So what I do is something like this:

git push publish ; git push repo ; git push code

where

[remote "publish"]
url = ssh://gitol...@ra.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
fetch = +refs/heads/*:refs/heads/*
push = next
push = master
push = maint
push = debian
push = +pu

[remote "code"]
url = https://code.google.com/p/e2fsprogs/
fetch = +refs/heads/*:refs/heads/*
push = next
push = master
push = maint
push = debian
push = +pu

[remote "repo"]
url = ssh://repo.or.cz/srv/git/e2fsprogs.git
push = next
push = master
push = maint
push = debian
push = +pu

I don't know if this is something you'd want git to encourage, or
support explicitly, but I thought I'd mention it.

- Ted


--
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] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Felipe Contreras  writes:

>> IIRC, git-gui runs two blames, one without any -C and one with (I do
>> not offhand recall how many -C it uses) to show both.
>
> 'git blame' is a very expensive operation, perhaps we should add
> another option so users don't need to run two blames to find this.

Yes, you could add a "struct origin *suspect_in_the_same_file" next
to the current "struct origin *suspect" to the blame_entry in order
to represent the origin you would get if you were to stop at a "move
across files" event, and keep digging, when you are operating under
"-C -C" or higher mode.

That would make the result just as expensive as a single run of "-C
-C" (or higher mode) [*1*] without having to run an extra "git
blame" without any -C (even though that mode is much cheaper).

Representing the result for a terminal with reasonable width might
become unwieldy, but for --porcelain output format that should not
be a problem.


[Footnote]

*1* It would make it slightly more expensive than the current code,
as it is very likely that you have to split a single blame_entry
into many separate pieces.
--
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] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 6:42 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
 The person who moved the code will be on the list regardless,
>>>
>>> That is exactly the point I have been trying to raise.  Does the
>>> person appear in the list when you run blame with -CCC?  You ask for
>>
>> s/person/commit/;
>>
>>> the body of the function, and the -C mode of blame sees through the
>>> block-of-line movement across file boundaries, and goes straight to
>>> the last commit that touched the body of the function in its original
>>> file, no?
>
> -- >8 --
> cd /var/tmp/
> git init blame
> cd blame
>
> cp /src/git/COPYING COPYING
> git add COPYING
> git commit -m initial
>
> sed -ne 120,140p COPYING >EXTRACTING
> git add EXTRACTING
> git commit -m second
>
> git blame -C -C -C EXTRACTING
> -- 8< --

I just realized that -CCC does not do the same thing as -C -C -C.

> then the commit that did add these lines to EXTRACTING touched
> COPYING, and the origin of these lines are traced to it (this is
> designed to be useful in a typical "refactor by moving code";
> "cut and paste without changing the original" people need heavier
> copy detection with more -C).
>
> IIRC, git-gui runs two blames, one without any -C and one with (I do
> not offhand recall how many -C it uses) to show both.

'git blame' is a very expensive operation, perhaps we should add
another option so users don't need to run two blames to find this.

-- 
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 v6] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Junio C Hamano  writes:

>>> The person who moved the code will be on the list regardless,
>>
>> That is exactly the point I have been trying to raise.  Does the
>> person appear in the list when you run blame with -CCC?  You ask for
>
> s/person/commit/;
>
>> the body of the function, and the -C mode of blame sees through the
>> block-of-line movement across file boundaries, and goes straight to
>> the last commit that touched the body of the function in its original
>> file, no?

-- >8 --
cd /var/tmp/
git init blame
cd blame

cp /src/git/COPYING COPYING
git add COPYING
git commit -m initial

sed -ne 120,140p COPYING >EXTRACTING
git add EXTRACTING
git commit -m second

git blame -C -C -C EXTRACTING
-- 8< --

will show that all lines from EXTRACTING came from the original
revision of COPYING, and we will miss the line-move event.

"blame -C" with a single -C does not look at the files that did not
change in the commit that added these lines to EXTRACTING
(i.e. COPYING), so the digging stops there.

After this, if you do this:

-- >8 --
echo >>COPYING
git commit --amend -a --no-edit

git blame -C EXTRACTING
-- 8< --

then the commit that did add these lines to EXTRACTING touched
COPYING, and the origin of these lines are traced to it (this is
designed to be useful in a typical "refactor by moving code";
"cut and paste without changing the original" people need heavier
copy detection with more -C).

IIRC, git-gui runs two blames, one without any -C and one with (I do
not offhand recall how many -C it uses) to show both.

HTH.

--
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] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 5:53 PM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> On Wed, May 22, 2013 at 5:38 PM, Junio C Hamano  wrote:
>> ...
>>> As I already said in the above, the answer is no, when you are
>>> trying to find who moved the code from the original place.
>>
>> But I'm not trying to find who moved the code, I'm trying to find
>> related commits; hence the name 'git related'.
>>
>> The person who moved the code will be on the list regardless,
>
> That is exactly the point I have been trying to raise.  Does the
> person appear in the list when you run blame with -CCC?  You ask for
> the body of the function, and the -C mode of blame sees through the
> block-of-line movement across file boundaries, and goes straight to
> the last commit that touched the body of the function in its original
> file, no?

I'm not familiar how the different -C options work, but I'm testing
right now and I see the commit that moved a file with both -C and
-CCC, but strangely enough, not if it's the previous commit (with
both).

-- 
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 v6] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Junio C Hamano  writes:

> Felipe Contreras  writes:
>
>> On Wed, May 22, 2013 at 5:38 PM, Junio C Hamano  wrote:
>> ...
>>> As I already said in the above, the answer is no, when you are
>>> trying to find who moved the code from the original place.
>>
>> But I'm not trying to find who moved the code, I'm trying to find
>> related commits; hence the name 'git related'.
>>
>> The person who moved the code will be on the list regardless,
>
> That is exactly the point I have been trying to raise.  Does the
> person appear in the list when you run blame with -CCC?  You ask for

s/person/commit/;

> the body of the function, and the -C mode of blame sees through the
> block-of-line movement across file boundaries, and goes straight to
> the last commit that touched the body of the function in its original
> file, no?
--
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] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Felipe Contreras  writes:

> On Wed, May 22, 2013 at 5:38 PM, Junio C Hamano  wrote:
> ...
>> As I already said in the above, the answer is no, when you are
>> trying to find who moved the code from the original place.
>
> But I'm not trying to find who moved the code, I'm trying to find
> related commits; hence the name 'git related'.
>
> The person who moved the code will be on the list regardless,

That is exactly the point I have been trying to raise.  Does the
person appear in the list when you run blame with -CCC?  You ask for
the body of the function, and the -C mode of blame sees through the
block-of-line movement across file boundaries, and goes straight to
the last commit that touched the body of the function in its original
file, no?

--
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] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 5:38 PM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>>> Depending on the nature of the change in question, it may match well
>>> or worse to what you are trying to find out.  When you are trying to
>>> say "What were you smoking when you implemented this broken logic?",
>>> using -C may be good, but when your question is "Even though all the
>>> callers of this function live in that other file, somebody moved
>>> this function that used to be file static in that file to here and
>>> made it public. Why?", you do not want to use -C.
>>>
>>> I am reasonably sure that in the finished code later in the series
>>> it will become configurable, but a fallback default is better to be
>>> not so expensive one.
>>
>> The script's purpose is to find related commits, -CCC does that, does it not?
>
> As I already said in the above, the answer is no, when you are
> trying to find who moved the code from the original place.

But I'm not trying to find who moved the code, I'm trying to find
related commits; hence the name 'git related'.

The person who moved the code will be on the list regardless, so 'git
related' does find the person who moved the code indirectly; by
finding the persons that touched the code.

>>> Makes sense to start from the preimage so that you can find out who
>>> wrote the original block of lines your patch is removing.
>>>
>>> But then if source is /dev/null, wouldn't you be able to stop
>>> without running blame at all?  You know the patch is creating a new
>>> file at that point and there is nobody to point a finger at.
>>
>> A patch can touch multiple files.
>
> So?
>
> What line range would you be feeding "blame" with, for such a file
> creation event?

I thought it was skipping git blame in such cases, perhaps it got
dropped in one of the other countless versions of this script.

Good catch.

-- 
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 v6] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Felipe Contreras  writes:

>> Depending on the nature of the change in question, it may match well
>> or worse to what you are trying to find out.  When you are trying to
>> say "What were you smoking when you implemented this broken logic?",
>> using -C may be good, but when your question is "Even though all the
>> callers of this function live in that other file, somebody moved
>> this function that used to be file static in that file to here and
>> made it public. Why?", you do not want to use -C.
>>
>> I am reasonably sure that in the finished code later in the series
>> it will become configurable, but a fallback default is better to be
>> not so expensive one.
>
> The script's purpose is to find related commits, -CCC does that, does it not?

As I already said in the above, the answer is no, when you are
trying to find who moved the code from the original place.

>> Makes sense to start from the preimage so that you can find out who
>> wrote the original block of lines your patch is removing.
>>
>> But then if source is /dev/null, wouldn't you be able to stop
>> without running blame at all?  You know the patch is creating a new
>> file at that point and there is nobody to point a finger at.
>
> A patch can touch multiple files.

So?

What line range would you be feeding "blame" with, for such a file
creation event?
--
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] bisect: Fix log output for multi-parent skip ranges

2013-05-22 Thread Torstein Hegge
On Mon, Apr 22, 2013 at 23:02:29 +0200, Torstein Hegge wrote:
> There has to be a better way to get the range of possible first bad
> commits, similar to the output of 'git log --bisect --format="%H"'.

I just realized that this felt clunky because I didn't understand what
'--not' does in git rev-list.

In the case where the range of skipped commits include a merge and
points in each parent marked good, I want

git rev-list bad --not good-1 good-2

or 

git rev-list bad ^good-1 ^good-2

but instead I did

git rev-list bad --not good-1 --not good-2

which will include commits outside the range of skipped commits. Sorry
about that :/

--- >8 ---
Subject: [PATCH] bisect: Fix log output for multi-parent skip ranges

The bisect log output of skipped commits introduced in f989cac "bisect:
Log possibly bad, skipped commits at bisection end" should obtain the range of
skipped commits from

git rev-list bad --not good-1 good-2

not

git rev-list bad --not good-1 --not good-2

when the skipped range contains a merge with good points in each parent.

Signed-off-by: Torstein Hegge 
---
 git-bisect.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index d7518e9..9f064b6 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -320,8 +320,8 @@ bisect_next() {
elif test $res -eq 2
then
echo "# only skipped commits left to test" 
>>"$GIT_DIR/BISECT_LOG"
-   good_revs=$(git for-each-ref --format="--not %(objectname)" 
"refs/bisect/good-*")
-   for skipped in $(git rev-list refs/bisect/bad $good_revs)
+   good_revs=$(git for-each-ref --format="%(objectname)" 
"refs/bisect/good-*")
+   for skipped in $(git rev-list refs/bisect/bad --not $good_revs)
do
skipped_commit=$(git show-branch $skipped)
echo "# possible first bad commit: $skipped_commit" 
>>"$GIT_DIR/BISECT_LOG"
-- 
1.8.3.rc1.377.g7010c6b

--
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] Add new git-related helper to contrib

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 2:23 PM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> diff --git a/contrib/related/git-related b/contrib/related/git-related
>> new file mode 100755
>> index 000..b96dcdd
>> --- /dev/null
>> +++ b/contrib/related/git-related
>> @@ -0,0 +1,124 @@
>> +#!/usr/bin/env ruby
>> +
>> +# This script finds people that might be interested in a patch
>> +# usage: git related 
>> +
>> +$since = '5-years-ago'
>> +$min_percent = 10
>> +
>> +def fmt_person(name, email)
>> +  '%s <%s>' % [name, email]
>> +end
>
> Micronit.  I suspect you do not need this helper, unless later
> patches start using it.

It's not *needed*, but it makes if fulfills the role of a function: to
avoid typing that code in multiple places.

>> +  def import
>> +return if @items.empty?
>> +File.popen(%w[git cat-file --batch], 'r+') do |p|
>> +  p.write(@items.keys.join("\n"))
>> +  p.close_write
>> +  p.each do |line|
>> +if line =~ /^(\h{40}) commit (\d+)/
>> +  id, len = $1, $2
>> +  data = p.read($2.to_i)
>> +  @items[id].parse(data)
>> +end
>> +  end
>> +end
>> +  end
>> +
>> +  def get_blame(source, start, len, from)
>> +return if len == 0
>> +len ||= 1
>> +File.popen(['git', 'blame', '--incremental', '-CCC',
>
> I am torn on the hardcoded use of "-CCC" here.
>
> Depending on the nature of the change in question, it may match well
> or worse to what you are trying to find out.  When you are trying to
> say "What were you smoking when you implemented this broken logic?",
> using -C may be good, but when your question is "Even though all the
> callers of this function live in that other file, somebody moved
> this function that used to be file static in that file to here and
> made it public. Why?", you do not want to use -C.
>
> I am reasonably sure that in the finished code later in the series
> it will become configurable, but a fallback default is better to be
> not so expensive one.

The script's purpose is to find related commits, -CCC does that, does it not?

>> +   '-L', '%u,+%u' % [start, len],
>> +   '--since', $since, from + '^',
>
> Is "from" unconditionally set?
>
> Perhaps that nil + '^' magically disappear and this code is relying
> on that, but it smells like a too much magic to me.

I personally don't care. You decide what's the behavior when no 'From
' line is available in the patch. I don't see the point in worrying
about non-git patches.

>> +   '--', source]) do |p|
>> +  p.each do |line|
>> +if line =~ /^(\h{40})/
>> +  id = $&
>> +  @items[id] = Commit.new(id)
>> +end
>> +  end
>> +end
>> +  end
>> +
>> +  def from_patch(file)
>> +from = source = nil
>> +File.open(file) do |f|
>> +  f.each do |line|
>> +case line
>> +when /^From (\h+) (.+)$/
>> +  from = $1
>> +when /^---\s+(\S+)/
>> +  source = $1 != '/dev/null' ? $1[2..-1] : nil
>> +when /^@@ -(\d+)(?:,(\d+))?/
>> +  get_blame(source, $1, $2, from)
>> +end
>
> Makes sense to start from the preimage so that you can find out who
> wrote the original block of lines your patch is removing.
>
> But then if source is /dev/null, wouldn't you be able to stop
> without running blame at all?  You know the patch is creating a new
> file at that point and there is nobody to point a finger at.

A patch can touch multiple files.

-- 
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: [RFC/PATCH 2/2] doc: command line interface (cli) dot-repository dwimmery

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 5:09 PM, Philip Oakley  wrote:
> From: "Felipe Contreras" 
> Sent: Wednesday, May 22, 2013 12:03 AM

>>> The value of the trick was acknowledged as now being in use
>>> http://article.gmane.org/gmane.comp.version-control.git/223572
>>
>>
>> How is that more useful than 'git branch -f master $sha1'?
>
>
> The 'trick' checks for a fast forward, while the branch update is forced. It
> depends on what checks are desired.

If that was truly useful, surely we could add an option for 'git
branch' to do just that.

> My original patch was to simply document Git's dot repository capability
> that does not appear to be that well known. Let's not keep it as an Easter
> Egg.

I know, all I said is that I think nobody cares about that
implementation detail. Instead of explaining to the user why Git has
so many quirks, we should get rid of them and make it work more in
line with users' expectations.

-- 
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: [RFC/PATCH 2/2] doc: command line interface (cli) dot-repository dwimmery

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 11:50 AM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> She told Git that her local svn-branch was the basis for svn-next. She
>> DIT NOT TELL Git to fetch from there. She told Git to fetch from any
>> location Git thought best to fetch from, either a) or b) would fetch
>> from the wrong location, but a) would be wronger, you just don't want
>> to admit it.
>
> "(a) is more wrong" is just your opinion, and you are simply wrong.

My opinion based on very solid grounds; the whole purpose of 'git
fetch; is to FETCH from a REMOTE. a) is not doing that at all.

In addition, the vast majority of users don't have a clue as to what  the hell

>From .
* branchmaster -> FETCH_HEAD

means.

a) is wronger. Period.

You say it's not, but give no explanation at all. This is no way to argue.

> She is working on svn-ext based on her local git-svn the latter of
> which has some changes of her own on top of Eric's tree.
>
> When working on _any_ branch that bases its work on something else,
> you have @{u} available, but that @{u} will not stay up-to-date if
> you forked from work that is done outside your repository.  That is
> what an unqualified "git fetch" is designed to help when run on a
> branch that bases its work on something else.

The fact that it's designed that way doesn't mean it's a good design,
and it doesn't mean the user expects that.

> If you happen to know
> that yoru current branch is forked from git-svn that is a local
> branch,

That's a very big *IF*.

> then running "git fetch" becomes unnecessary for the purpose
> of updating @{u} (it already and always is up to date), so doing no
> object transfer and no ref update is absolutely the right thing to
> do.  That is what "remote = ." gives you.

Jumping to conclusions based on assumptions again.

Sally doesn't know what the designers intended, Sally doesn't remember
what is the upstream of the current branch, of it has any upstream at
all. Sally does 'git fetch' instinctively, and expects Git to do the
right thing, but it doesn't, it does an utterly irrelevant and useless
action; non-fetching from a local-remote.

> In addition, that does not break the "pull = fetch + merge"
> equivalence you seem to be ignoring.

Do you want me to count to you the many times I've proved to you that
pull is NOT fetch + merge?

YOU are the one ignoring the fact that it's not: it's only that way in
very specific circumstances, certaily ver far from being a universal
truth.

> If she wants to check what Eric has been doing, she can do "git
> fetch git-svn", giving the remote name she calls Eric's tree with.
> At that point, she is not saying "I want to check what is happening
> to the upstream of my _current_ branch" (and the fetched result is
> not something she can immediately use while on her current branch).

Irrelevant.

> On the other hand, an unqualified "git fetch" that slurps from my
> tree, which is your (b), is just plain wrong.

But that's *EXACTLY* what we do when there's no upstream branch, is it not?

> My tree is not even
> related to what she is working on.

Unless you are prepared to say fetching from any other tree that @{u}
is wrong, and 'git fetch' should forbit it, this is irrelevant.

The user can fetch from wherever they want.

> Of course, when she is interested in what have been happening in my
> tree, she can say "git fetch origin".

Irrelevant. We are not changing that behavior.

-- 
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: [RFC/PATCH 2/2] doc: command line interface (cli) dot-repository dwimmery

2013-05-22 Thread Philip Oakley

From: "Felipe Contreras" 
Sent: Wednesday, May 22, 2013 12:03 AM
On Tue, May 21, 2013 at 5:33 PM, Philip Oakley  
wrote:

From: "Felipe Contreras" 
Sent: Tuesday, May 21, 2013 10:21 PM


On Tue, May 21, 2013 at 11:23 AM, Junio C Hamano 
wrote:


"Philip Oakley"  writes:



On Sat, May 4, 2013 at 2:51 PM, Jonathan Nieder 


wrote:


Another trick is to use "git push":
git push . $production_sha1:refs/heads/master



It all falls out naturally from the "Git is distributed and no
repository is special" principle.  I think that word "trick" merely
refers to "those who do not realize that the local repository is 
not

all that special and merely is _a_ repository just like anybody
else's may not realize they can do this", nothing more.


Nobody cares.


The value of the trick was acknowledged as now being in use
http://article.gmane.org/gmane.comp.version-control.git/223572


How is that more useful than 'git branch -f master $sha1'?


The 'trick' checks for a fast forward, while the branch update is 
forced. It depends on what checks are desired.


My original patch was to simply document Git's dot repository capability 
that does not appear to be that well known. Let's not keep it as an 
Easter Egg.





Not sure if that was the caring you were commenting on.


My point is that nobody uses '.' as a remote. Yes, you can find the
occasional esoteric person in the Git mailing list that might find
some weird command useful, but that's the fringe user-base.


You say it's "mistaken", but you are not the arbiter of truth; the
fact that you say it's so doesn't make it so. It's just rhetoric.

You haven't shown that it's indeed mistaken.



An aside: in some domains (e.g. Human Error taxonomy) a 'mistake' is 
a
planned action which later turns out to not be the action that would 
now
have, in retrospect, been chosen. The intent was good, but is later 
classed

(within the taxonomy) as a 'mistake'. (It is not related to 'blame').


Yeah, that's what a mistake is, in my mind.

If I understand the extended thread correctly, the approach moved on 
and

alternatives were found, so in that sense the intent was good.


No, the approach didn't move on, there are no better alternatives, the
"intent" is irrelevant, the approach is good, there is no mistake.

Junio simply ignored the fact that he was proven wrong.

I still haven't received a response: which makes more sense?

a)

% git checkout svn-ext
% git fetch
From .
* branchmaster -> FETCH_HEAD
# oops
% git fetch git-svn
% git log ..FETCH_HEAD
% git merge FETCH_HEAD

b)

% git checkout svn-ext
% git fetch
From git://git.kernel.org/pub/scm/git/git
   680ed3e..de3a5c6  master -> origin/master
# oops
% git fetch svn-ext
% git log ..FETCH_HEAD
% git merge FETCH_HEAD

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


-
No virus found in this message.
Checked by AVG - www.avg.com
Version: 2013.0.3343 / Virus Database: 3162/6344 - Release Date: 
05/21/13




--
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: is it just me, or does --all ignore --no-tags?

2013-05-22 Thread Geoff Thorpe
On 22 May 2013 17:31, Junio C Hamano  wrote:
>> So instead of doing;
>> git fetch --all --no-tags
>> I'm now doing this to avoid the problem;
>> git remote | xargs -n 1 git fetch --no-tags
>
> I suspect that this is 8556646 (fetch --all: pass --tags/--no-tags through to
> each remote, 2012-09-05) which is in 1.7.12.2 and upwards.

Awesome, that sounds like the exact problem. Much appreciated.
--
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: is it just me, or does --all ignore --no-tags?

2013-05-22 Thread Junio C Hamano
> So instead of doing;
> git fetch --all --no-tags
> I'm now doing this to avoid the problem;
> git remote | xargs -n 1 git fetch --no-tags

I suspect that this is 8556646 (fetch --all: pass --tags/--no-tags through to
each remote, 2012-09-05) which is in 1.7.12.2 and upwards.
--
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


is it just me, or does --all ignore --no-tags?

2013-05-22 Thread Geoff Thorpe
I have some peculiar reasons for doing this, but nonetheless I don't
believe there's anything illegal here. I have a repo with a bunch of
remotes set up to pull in branches and tags from different places, and
I need all those branches and tags name-spaced. Eg. to illustrate, the
config looks something like;

[core]
repositoryformatversion = 0
filemode = true
bare = true
[remote "foo"]
url = git://some.address/foo
fetch = +refs/heads/*:refs/remotes/foo/heads/*
fetch = +refs/tags/*:refs/remotes/foo/tags/*
[remote "bar"]
url = git://somewhere.else/bar
fetch = +refs/heads/*:refs/remotes/bar/heads/*
fetch = +refs/tags/*:refs/remotes/bar/tags/*

If I do "git fetch --no-tags foo && git fetch --no-tags bar",
everything's fine - all the branches and tags from "foo" sit under
refs/remotes/foo/{heads,tags}/, and similarly for "bar". OTOH, if I do
"git fetch --all --no-tags", it's as though the --no-tags wasn't
there. Inverting the argument order doesn't seem to help.

What this means is that in addition to putting branches and tags in
the locations specified in the refspecs, it also puts copies of all
the tags in refs/tags/. In my case, "foo" and "bar" will sometimes
define the same tag name but they don't refer to the same thing,
that's (one of the reasons) why I need them name-spaced. The current
situation with "--all" is that refs/tags/ contains an ad-hoc union of
tags from the different remotes, and if there are any collisions they
seem to resolve in favour of whichever was fetched last.

So instead of doing;
git fetch --all --no-tags
I'm now doing this to avoid the problem;
git remote | xargs -n 1 git fetch --no-tags

BTW, I've hit this problem on a system I don't administer, so I can't
easily verify this with the latest source. It's a ubuntu system and
"git --version" indicates 1.7.9.5. Apologies in advance if this is
just noise about an already-fixed-bug, but I didn't find anything via
a quick search (of the mail-list and the net more generally) so
figured I'd send this in.

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


Re: [PATCH 2/2] sha1_name: fix error message for @{}, @{}

2013-05-22 Thread Eric Sunshine
On Wed, May 22, 2013 at 6:39 AM, Ramkumar Ramachandra
 wrote:
> Currently, when we try to resolve @{} or @{} when the reflog
> doesn't go back far enough, we get errors like:
>
>   # on branch master
>   $ git show @{1}
>   fatal: Log for '' only has 7 entries.
>
>   $ git show @{1.days.ago}
>   warning: Log for '' only goes back to Tue, 21 May 2013 14:14:45 +0530.
>   ...
>
>   # detached HEAD case
>   $ git show @{1}
>   fatal: Log for '' only has 2005 entries.
>
>   $ git show master@{1}
>   fatal: Log for 'master' only has 7 entries.
>
> The empty string '' is ugly, inconsistent, and failing to convey

s/failing/fails/

> information about whose logs we are inspecting.  Change this so that we
> get:
>
>   # on branch master
>   $ git show @{1}
>   fatal: Log for 'master' only has 7 entries.
>
>   $ git show @{1.days.ago}
>   warning: Log for 'master' only goes back to Tue, 21 May 2013 14:14:45 +0530.
>   ...
>
>   # detached HEAD case
>   $ git show @{1}
>   fatal: Log for 'HEAD' only has 2005 entries.
>
>   $ git show master@{1}
>   fatal: Log for 'master' only has 7 entries.
>
> Simple, consistent, and informative; suitable for output even from
> plumbing commands like rev-parse.
>
> Signed-off-by: Ramkumar Ramachandra 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Document push --no-verify

2013-05-22 Thread Thomas Rast
Junio C Hamano  writes:

> Thomas Rast  writes:
>
>> ec9 (push: Add support for pre-push hooks, 2013-01-13) forgot to
>> add a note to git-push(1) about the new --no-verify option.
>
> Does it take --verify option (that may well be the default) so that
> somebody with
>
>  [alias] put = push --no-verify
>
> can say "git put --verify args..."?

Yes.  Doesn't parse-options implicitly do the correct negation for all
boolean options, even those that are declared in their negative form?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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-submodule nested subrepo bug (Segmentation fault)

2013-05-22 Thread Kirill Berezin
Ok, version is: 1.8.1.msysgit.1
Segmentation fault at the git clone --recursive
http://github.com/Exsul/al_server
0 [main] mkdir 6596 open_stackdumpfile: Dumping stack trace to
mkdir.exe.stackdump
C:\Users\\libexec\git-core\git-submodule: line 181: 6596
Segmentation fault (core dumped) mkdir -p "$ditdir_base"
fatal: Could not switch to 's:/USER//al_server/.git/modules/': No
such file or directory
Clone of 'https://.../Exsul/chat.git' into submodule path 'chat' failed

2013/5/20 John Keeping :
> On Mon, May 20, 2013 at 09:32:21AM +0400, Kirill Berezin wrote:
>> When you trying to add submodule, that already has submodule, it craches.
>> For example you could try: git clone --recursive
>> http://github.com/Exsul/al_server
>
> Which version of Git were you using?  I was not able to reproduce this
> with 1.8.3-rc3.
--
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: Reading commit objects

2013-05-22 Thread Shawn Pearce
On Wed, May 22, 2013 at 7:20 AM, Chico Sokol  wrote:
> I'm not criticizing JGit, guys. It simply doesn't fit into our needs.
> We're not interested in mapping git commands in java and don't have
> the same RAM limitations.

I guess you aren't trying to process the WebKit or Linux kernel
repositories. Or you can afford more RAM than I can[1]. :-)

[1] $DAY_JOB has lots of RAM.  Lots.

> Are you guys contributors of JGit?

Not really. I had nothing to do with JGit.  :-)

> Can you guys point me out to the
> code that unpacks git objects? The closest I could get was that class:
> https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObject.java

This class handles the loose object format in $GIT_DIR/objects, but
does not handle objects contained in pack files. That is elsewhere,
and well, more complex. Look at PackFile.java.

> It seems to be a standard and a non standard format of the packed
> object, as I read the comments of this method:
> https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObject.java#L272

There are two formats, the official format that is used, and an
experimental format that was discarded but is still supported for
legacy reasons.

> I suspect that the default inflater class of java api expect the
> object to be in the standard format.
>
> What the following comment mean? What's the "Experimental pack-based"
> format? Is there any docs on the specs of that?

Read the code. This is the dead format that is no longer written, but
is still supported.
--
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: Reading commit objects

2013-05-22 Thread Shawn Pearce
On Wed, May 22, 2013 at 7:25 AM, Chico Sokol  wrote:
>> Your code is broken. IOUtils is probably corrupting what you get back.
>> After inflating the stream you should see the object type ("commit"),
>> space, its length in bytes as a base 10 string, and then a NUL ('\0').
>> Following that is the tree line, and parent(s) if any. I wonder if
>> IOUtils discarded the remainder of the line after the NUL and did not
>> consider the tree line.
>
...
> Is the contents of a unpacked object utf-8
> encoded?

Its more complicated than that. Commit objects are usually in utf-8,
unless a repository configuration setting told you otherwise, or an
encoding header appears in the commit. And sometimes that data lies
anyway. ISO-8859-1 is one of the safer forms of reading a commit, but
that also isn't always accurate.
--
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] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Junio C Hamano  writes:

> Felipe Contreras  writes:
>
>> diff --git a/contrib/related/git-related b/contrib/related/git-related
>> new file mode 100755
>> index 000..b96dcdd
>> --- /dev/null
>> +++ b/contrib/related/git-related
>> @@ -0,0 +1,124 @@
>> +#!/usr/bin/env ruby
>> +
>> +# This script finds people that might be interested in a patch
>> +# usage: git related 
>> +
>> +$since = '5-years-ago'
>> +$min_percent = 10
>> +
>> +def fmt_person(name, email)
>> +  '%s <%s>' % [name, email]
>> +end
>
> Micronit.  I suspect you do not need this helper, unless later
> patches start using it.

Not that matters terribly, but "later patches start using it in a
different way" may be needed as a clarification.

The current two callers capture both "%s <%s>" as two separate
groups but they do not need to; they can do "(%s <%s>)" and pass $1
to this function, I think.


--
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] Add new git-related helper to contrib

2013-05-22 Thread Junio C Hamano
Felipe Contreras  writes:

> diff --git a/contrib/related/git-related b/contrib/related/git-related
> new file mode 100755
> index 000..b96dcdd
> --- /dev/null
> +++ b/contrib/related/git-related
> @@ -0,0 +1,124 @@
> +#!/usr/bin/env ruby
> +
> +# This script finds people that might be interested in a patch
> +# usage: git related 
> +
> +$since = '5-years-ago'
> +$min_percent = 10
> +
> +def fmt_person(name, email)
> +  '%s <%s>' % [name, email]
> +end

Micronit.  I suspect you do not need this helper, unless later
patches start using it.

> +  def import
> +return if @items.empty?
> +File.popen(%w[git cat-file --batch], 'r+') do |p|
> +  p.write(@items.keys.join("\n"))
> +  p.close_write
> +  p.each do |line|
> +if line =~ /^(\h{40}) commit (\d+)/
> +  id, len = $1, $2
> +  data = p.read($2.to_i)
> +  @items[id].parse(data)
> +end
> +  end
> +end
> +  end
> +
> +  def get_blame(source, start, len, from)
> +return if len == 0
> +len ||= 1
> +File.popen(['git', 'blame', '--incremental', '-CCC',

I am torn on the hardcoded use of "-CCC" here.

Depending on the nature of the change in question, it may match well
or worse to what you are trying to find out.  When you are trying to
say "What were you smoking when you implemented this broken logic?",
using -C may be good, but when your question is "Even though all the
callers of this function live in that other file, somebody moved
this function that used to be file static in that file to here and
made it public. Why?", you do not want to use -C.

I am reasonably sure that in the finished code later in the series
it will become configurable, but a fallback default is better to be
not so expensive one.

> +   '-L', '%u,+%u' % [start, len],
> +   '--since', $since, from + '^',

Is "from" unconditionally set?

Perhaps that nil + '^' magically disappear and this code is relying
on that, but it smells like a too much magic to me.

> +   '--', source]) do |p|
> +  p.each do |line|
> +if line =~ /^(\h{40})/
> +  id = $&
> +  @items[id] = Commit.new(id)
> +end
> +  end
> +end
> +  end
> +
> +  def from_patch(file)
> +from = source = nil
> +File.open(file) do |f|
> +  f.each do |line|
> +case line
> +when /^From (\h+) (.+)$/
> +  from = $1
> +when /^---\s+(\S+)/
> +  source = $1 != '/dev/null' ? $1[2..-1] : nil
> +when /^@@ -(\d+)(?:,(\d+))?/
> +  get_blame(source, $1, $2, from)
> +end

Makes sense to start from the preimage so that you can find out who
wrote the original block of lines your patch is removing.

But then if source is /dev/null, wouldn't you be able to stop
without running blame at all?  You know the patch is creating a new
file at that point and there is nobody to point a finger at.

> +  end
> +end
> +  end
> +
> +end
> +
> +exit 1 if ARGV.size != 1
> +
> +commits = Commits.new
> +commits.from_patch(ARGV[0])
> +commits.import
> +
> +count_per_person = Hash.new(0)
> +
> +commits.each do |id, commit|
> +  commit.persons.each do |person|
> +count_per_person[person] += 1
> +  end
> +end
> +
> +count_per_person.each do |person, count|
> +  percent = count.to_f * 100 / commits.size
> +  next if percent < $min_percent
> +  puts person
> +end
--
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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Junio C Hamano
Theodore Ts'o  writes:

> On Wed, May 22, 2013 at 10:58:49AM -0700, Junio C Hamano wrote:
>> Theodore Ts'o  writes:
>> 
>> > I was actually thinking that it might be interesting to have a
>> > branch..rewindable, which would change the guilt defaults, and
>> > could also key changes in key git behavior which makes it less likely
>> > that a user shoots him or herself in the foot --- i.e., give warnings
>> > if he or she has modified the branch in such a way that
>> > remotes.origin. is no longer contained within the branch head.
>> 
>> At least "rebase" can pay attention to it and might make the world a
>> better place.
>
> Yeah, rebase was the primary command I was thinking about.  The other
> one would be "git commit --amend" after the branch had been pushed
> out.

It may or may not matter for the kernel folks, but let me pick your
brain while we are on this subject.

The "upstream" (your remotes.origin.) is that on top of
which you build your work.  You clone from there to bootstrap
yourself, you add your work (which may include integrating the work
of your contributors, if you are a mid-tier maintainer/integrator
aka a lieutenant) on top of it, and arrange the result to reach the
"upstream" in some way.

For the simplest (and still widely used) workflow that employs a
central shared repository, the way to make the result to reach the
"upstream" is by directly pushing into it yourself.  In that sense,
the word "upstream" and the traditional behaviour of "git push" that
pushes back to the 'origin' (or branch..remote) to update
your "upstream" (or branch..merge at 'origin') both make
perfect sense.

Also, if you are rebasing, @{u} refers to that place you integrate
with, i.e. your "upstream", in the central shared repository
workflow.

But in a triangular workflow, the way to make the result reach the
"upstream" is *not* by pushing there yourself.  For developers at
the leaf level, it is to push to their own repository (often on
GitHub), which is different from where they (initially) clone from
in order to bootstrap themselves, and (subsequently) pull from in
order to keep them up-to-date.  And then they request the published
work to be pulled by the "upstream".

Even in a triangular workflow, @{u} should still refer to the place
you integrate with, i.e. your "upstream", not to the place you push
to publish the result of your work.

This branch..rewindable safety however cannot be tied to
@{u}.  The bottom boundary you want to be warned when you cross is
the change you pushed out to your publishing repository, and it may
not have reached remotes.origin. yet.

We will be introducing remote.pushdefault configuration in the
upcoming 1.8.3 release, so that you can say:

[remote "origin"]
url = git://g.k.o/pub/scm/linux/kernel/git/torvalds/linux.git/
fetch = +refs/heads/*:refs/remotes/origin/*

[remote "ext4"]
url = g.k.o:/pub/scm/linux/kernel/git/tytso/ext4.git/
fetch = +refs/heads/*:refs/remotes/ext4/*

[remote]
pushdefault = ext4

and hopefully it would let you do this:

git checkout master
... after working on it ...
git push

As remote.pushdefault is set to ext4, without any extra arguments,
the result will pushed to the "ext4" remote.  If you are using the
traditional push.default=matching, it may also try to push out dev,
dev-next and other branches you may have in your local repository
and at k.org; if you are using push.default=simple or other "single
branch" modes like "current", "upstream", etc, it will only push out
your current branch (i.e. "master") to "ext4" remote.

You may however be using your local "master" branch for your
development, and pushing the result out to "dev".  With only the
remote.pushdefault setting to push to ext4 (instead of origin), you
still would have to say

git push ext4 master:dev

There is another change discussed on the list recently to also let
you configure your local "master" branch to update "dev" in your
publishing repository.  It may go like this:

[branch "master"]
push = refs/heads/dev

In any case, refs/remotes/ext4/dev would be the remote tracking
branch (not refs/remotes/origin/anything) that keeps track of what
you pushed out there the last time.  And that would be what your new
safety based on "branch.master.rewindable = no" needs to check
against, not "refs/remotes/origin/master" which is your master@{u}.
--
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: override merge.ff = false using --ff-only

2013-05-22 Thread Junio C Hamano
Matt McClure  writes:

> I naively tried to override merge.ff = false using --ff-only on the
> command line. I expected that it would override the configured default
> and perform a fast-forward merge. Instead, it said:
>
> $ git config -l | grep -F 'merge.ff'
> merge.ff=false
>
> $ git merge --ff-only foo
> fatal: You cannot combine --no-ff with --ff-only.
>
> On the other hand, I see that --ff works just fine in the same initial state.
>
> $ git merge --ff foo
> Updating b869407..17b5495
> Fast-forward
> ...
>  4 files changed, 2 insertions(+), 5 deletions(-)
>
> Would it be better if --ff-only refused to merge only if the commits
> themselves prevented fast-forwarding?

In general it would be better if any --ff related command line
options made us ignore the configured default like merge.ff the user
may have in the repository, not just --ff-only vs merge.ff
combination, and your "On the other hand" demonstrates that it is
the case for --ff from the command line.

I do not offhand see why --ff-only should behave differently from
that expectation.
--
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: first parent, commit graph layout, and pull merge direction

2013-05-22 Thread Junio C Hamano
Andreas Krey  writes:

> A short trial showed that representing first parent chains as
> straight lines in the graph does actually improve understandability,
> as feature branches clearly stand out as separate lines even when
> they no longer carry a branch name.

If you have a four-commit segment in your commit ancestry graph
(time flows from left to right; turn your head 90-degrees to the
right if you want a gitk representation):

---A--X
\/
/\
---B--Y

where X and Y are both merges between A and B, having A as their
first parent, how would you express such a graph with first-parent
chain going a straight line?

> Also, there is an implication with 'git pull': You'd expect the
> master branch to be a first parent line, but when I do a small
> thing directly on master and need to pull before pushing back,
> then origin/master is merged into my branch, and thus my side
> branch becomes the first parent line.

Don't do that, then.
--
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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Theodore Ts'o
On Wed, May 22, 2013 at 10:58:49AM -0700, Junio C Hamano wrote:
> Theodore Ts'o  writes:
> 
> > I was actually thinking that it might be interesting to have a
> > branch..rewindable, which would change the guilt defaults, and
> > could also key changes in key git behavior which makes it less likely
> > that a user shoots him or herself in the foot --- i.e., give warnings
> > if he or she has modified the branch in such a way that
> > remotes.origin. is no longer contained within the branch head.
> 
> At least "rebase" can pay attention to it and might make the world a
> better place.

Yeah, rebase was the primary command I was thinking about.  The other
one would be "git commit --amend" after the branch had been pushed
out.

- Ted
--
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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Junio C Hamano
Theodore Ts'o  writes:

> I was actually thinking that it might be interesting to have a
> branch..rewindable, which would change the guilt defaults, and
> could also key changes in key git behavior which makes it less likely
> that a user shoots him or herself in the foot --- i.e., give warnings
> if he or she has modified the branch in such a way that
> remotes.origin. is no longer contained within the branch head.

At least "rebase" can pay attention to it and might make the world a
better place.

Your final "git push" needs to be forced if you rewound beyond
remotes.origin. so in that sense, there already is a sefety,
but it is better to give the user a chance to notice that before the
user spends more time to polish the rewound topic to perfection,
only to see the push rejected.


--
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: Avoiding broken Gitweb links and deleted objects

2013-05-22 Thread Junio C Hamano
Matt McClure  writes:

> On Fri, May 10, 2013 at 12:22 PM, Junio C Hamano  wrote:
>> I think what I missed is that the same logic to ignore side branches
>> whose history gets cauterised with such an "ours" merge may apply to
>> an "ours" merge that people already make, but the latter may want to
>> take both histories into account.
>>
>> So I guess it is not such a great idea.
>
> The particular proposed implementation? Or the broader idea to save
> loose commits more permanently? I'm still interested in a solution for
> the latter.

Recording such an "otherwise should not be recorded as a merge" side
history as if it were "-s ours" merge is what I judged as "not a
great idea".

If you want to keep older commits, either you make sure you point at
them with some refs, or not prune the repository.  I do not think of
any other solution offhand.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] sha1_name: fix error message for @{u}

2013-05-22 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> 2. Callers calling in with programmatic data, and expecting the function
>to return and not die().  In this case, why would anyone ever
>construct a string containing "@{u}" programmatically in the first
>place?

If you have to ask why, and cannot answer the question yourself,
then you would not know if such a caller exists.  After a code
audit, I know there is no such caller that appends @{u} but if you
were writing a quick-and-dirty caller, I would not be surprised if
you find it more convenient to form a textual extended SHA-1
expression and have get_sha1() do its work, instead of asking the
same question programmatically.

In this case, I think you already checked there is no such problem,
and it is a more straight-forward justification to say that you did
a code-audit and made sure that all the callers that used to hit one
of these errors() want to die().

Also such a caller, if existed, would either

(1) want to die itself, in which case these error() messages are
superfluous; or

(2) want to continue (possibly dying with its own message), in
which case these error() messages are unwanted.

Because you are changing only the existing call sites of error()
into die(), and not changing silent -1 returns to die(), this change
is safe for both kinds of such callers, I think.

> Signed-off-by: Ramkumar Ramachandra 
> ---
>  sha1_name.c   | 13 +++--
>  t/t1507-rev-parse-upstream.sh | 15 +--
>  2 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 3820f28..416a673 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1033,14 +1033,15 @@ int interpret_branch_name(const char *name, struct 
> strbuf *buf)
>* points to something different than a branch.
>*/
>   if (!upstream)
> - return error(_("HEAD does not point to a branch"));
> + die(_("HEAD does not point to a branch"));

OK.

>   if (!upstream->merge || !upstream->merge[0]->dst) {
>   if (!ref_exists(upstream->refname))
> + die(_("No such branch: '%s'"), cp);

OK.

> + if (!upstream->merge) {
> + die(_("No upstream configured for branch '%s'"),
> + upstream->name);
> + }

OK, but I would not add extra {} if I were doing this change.

> + die(
>   _("Upstream branch '%s' not stored as a remote-tracking 
> branch"),
>   upstream->merge[0]->src);

OK, but I would fix the indentation while at it if I were doing this change.

> diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
> index b27a720..2a19e79 100755
> --- a/t/t1507-rev-parse-upstream.sh
> +++ b/t/t1507-rev-parse-upstream.sh
> @@ -129,8 +129,7 @@ test_expect_success 'branch@{u} works when tracking a 
> local branch' '
>  
>  test_expect_success 'branch@{u} error message when no upstream' '
>   cat >expect <<-EOF &&
> - error: No upstream configured for branch ${sq}non-tracking${sq}
> - fatal: Needed a single revision
> + fatal: No upstream configured for branch ${sq}non-tracking${sq}

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


Re: [PATCH] Document push --no-verify

2013-05-22 Thread Junio C Hamano
Thomas Rast  writes:

> ec9 (push: Add support for pre-push hooks, 2013-01-13) forgot to
> add a note to git-push(1) about the new --no-verify option.

Does it take --verify option (that may well be the default) so that
somebody with

 [alias] put = push --no-verify

can say "git put --verify args..."?

>
> Signed-off-by: Thomas Rast 
> ---
>
> The insertion spot is at the end, because the existing ordering is
> indistinguishable from random.  This should also be fixed, but is a
> much bigger change and probably not suitable for an -rc period.
>
>
>  Documentation/git-push.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index d514813..426b3d2 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -11,7 +11,7 @@ SYNOPSIS
>  [verse]
>  'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] 
> [--receive-pack=]
>  [--repo=] [-f | --force] [--prune] [-v | --verbose] [-u 
> | --set-upstream]
> -[ [...]]
> +[--no-verify] [ [...]]
>  
>  DESCRIPTION
>  ---
> @@ -195,6 +195,9 @@ useful if you write an alias or script around 'git push'.
>   be pushed. If on-demand was not able to push all necessary
>   revisions it will also be aborted and exit with non-zero status.
>  
> +--no-verify::
> + Bypass the pre-push hook (see linkgit:githooks[5]).
> +
>  
>  include::urls-remotes.txt[]
--
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/3] Fixing volatile HEAD in push.default = current

2013-05-22 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> Junio C Hamano wrote:
>> The patch may have been done by a wrong motivation, in that it does
>> not fundamentally "fix" the itch.  The particular "itch" is not
>> something we are going to promise to the end users, ever, anyway.
>
> Just out of curiosity, is it possible to write a correct fix at all?

Is there anything to "fix" in the first place, you have to wonder.

Your "git push there HEAD:master" would roughly do the following:

(1) read HEAD to learn what commit you are pushing;

(2) contact the other side and find where there tips are;

(3) send a packfile that should be enough for the other side to
have complete history leading to the commit you read in (1);

(4) tell the other side to update its 'master' to the commit you
read in (1).

If you drop step (1) and replace "the commit you read in (1)" in
steps (3) and (4) with "the commit you see in HEAD at this point by
re-reading HEAD", then such a "git push" that races with something
else you do in your other terminal may break---you can cause it to
see different commits at steps (3) and (4), potentially getting the
other side out of sync (but the receiving end does an independent
connectivity check so your push will likely to be rejected).

And the fix to such a breakage is to structure the code like the
above four steps to make it race-free.

If I understand your example correctly, you are talking about a
quite different thing.  "git push there HEAD:master" racing with
your other terminal that changes the HEAD sees different HEAD
depending on the order:

(a) if the other terminal changes the HEAD first, step (1) will
see that updated HEAD; or

(b) if the step (1) reads HEAD before you change it in the other
terminal, it will see the original HEAD.

But that is very much to be expected, isn't it?  It sounds similar
to

I have "largedir" I want to get rid of, but there is a directory
I want to save, "largedir/precious", in it, so I do

cp -R largedir/precious precious

and then run 'rm -rf largedir' in another terminal in parallel.

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


Re: [RFC/PATCH 2/2] doc: command line interface (cli) dot-repository dwimmery

2013-05-22 Thread Junio C Hamano
Felipe Contreras  writes:

> She told Git that her local svn-branch was the basis for svn-next. She
> DIT NOT TELL Git to fetch from there. She told Git to fetch from any
> location Git thought best to fetch from, either a) or b) would fetch
> from the wrong location, but a) would be wronger, you just don't want
> to admit it.

"(a) is more wrong" is just your opinion, and you are simply wrong.

She is working on svn-ext based on her local git-svn the latter of
which has some changes of her own on top of Eric's tree.

When working on _any_ branch that bases its work on something else,
you have @{u} available, but that @{u} will not stay up-to-date if
you forked from work that is done outside your repository.  That is
what an unqualified "git fetch" is designed to help when run on a
branch that bases its work on something else.  If you happen to know
that yoru current branch is forked from git-svn that is a local
branch, then running "git fetch" becomes unnecessary for the purpose
of updating @{u} (it already and always is up to date), so doing no
object transfer and no ref update is absolutely the right thing to
do.  That is what "remote = ." gives you.

In addition, that does not break the "pull = fetch + merge"
equivalence you seem to be ignoring.

If she wants to check what Eric has been doing, she can do "git
fetch git-svn", giving the remote name she calls Eric's tree with.
At that point, she is not saying "I want to check what is happening
to the upstream of my _current_ branch" (and the fetched result is
not something she can immediately use while on her current branch).

On the other hand, an unqualified "git fetch" that slurps from my
tree, which is your (b), is just plain wrong.  My tree is not even
related to what she is working on.

Of course, when she is interested in what have been happening in my
tree, she can say "git fetch origin".
--
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: English/German terminology, git.git's de.po, and pro-git

2013-05-22 Thread Jan Engelhardt

On Wednesday 2013-05-22 17:52, Holger Hellmuth (IKS) wrote:
>>
>> Not sure if German users would know what "hunk" means, in case we
>> leave it untranslated. And I'm not sure if I would understand "Kontext".
>> I tend to leave it untranslated.
>
> I don't think "Bereich" is a bad choice. As "hunk" is not a word with special
> meaning in cvs and not used in any commands I don't see a lot of reasons to
> keep it in english.

hunk is chunk without a c, but otherwise with pretty much the same meaning.
Especially when it rejects :)
--
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 (May 2013, #05; Mon, 20)

2013-05-22 Thread Junio C Hamano
Michael J Gruber  writes:

>> * mg/more-textconv (2013-05-10) 7 commits
>>  - grep: honor --textconv for the case rev:path
>>  - grep: allow to use textconv filters
>>  - t7008: demonstrate behavior of grep with textconv
>>  - cat-file: do not die on --textconv without textconv filters
>>  - show: honor --textconv for blobs
>>  - diff_opt: track whether flags have been set explicitly
>>  - t4030: demonstrate behavior of show with textconv
>> 
>>  I think this is ready for 'next'; not that it matters during the
>>  prerelease feature freeze.
>
> Oh, I'm sorry, I thought we were still in discussions about the default
> mechanism (config or attributes) and the implementation (tacking context
> onto each object)? Therefore, I didn't hurry to polish and follow up
> over my vacation. I'm not sure I had smoothed out all minor things
> (honor/obey and such) when the object struct size issue came up. I'll
> check today or tomorrow. (Freeze, yes, but we don't want too many next
> rewrites, and one is coming soon...)

I thought this was fine as-is, but we can kick it back to 'pu' and
replace it with a reroll after 1.8.3 if that is necessary.

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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Josef 'Jeff' Sipek
On Wed, May 22, 2013 at 10:45:31AM -0400, Theodore Ts'o wrote:
> I just had another idea (although I haven't had a chance to code up
> anything yet).  Perhaps instead of, or in addition to, a global
> setting (i.e., guilt.reusebranch), perhaps we should have a per-branch
> setting, such as branch..guiltReuseBranch?
> 
> I was actually thinking that it might be interesting to have a
> branch..rewindable, which would change the guilt defaults, and
> could also key changes in key git behavior which makes it less likely
> that a user shoots him or herself in the foot --- i.e., give warnings
> if he or she has modified the branch in such a way that
> remotes.origin. is no longer contained within the branch head.

Interesting!  I wonder what git people have to say about this.

Jeff.

-- 
We have joy, we have fun, we have Linux on a Sun...
--
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 v13 02/15] path.c: refactor relative_path(), not only strip prefix

2013-05-22 Thread Junio C Hamano
Michael Haggerty  writes:

>> Different results for relative_path() before and after this refactor:
>> 
>> abs path  base path  relative (original)  relative (refactor)
>>   =  ===  ===
>> /a/b/c/   /a/b   c/   c/
>> /a/b//c/  //a///b/   c/   c/
>> /a/b  /a/b   ../
>> /a/b/ /a/b   ../
>> /a/a/b/  /a   ../
>> / /a/b/  /../../
>> /a/c  /a/b/  /a/c ../c
>> /a/b  (empty)/a/b /a/b
>> /a/b  (null) /a/b /a/b
>> (empty)   /a/b   (empty)  ./
>> (null)(empty)(null)   ./
>> (null)/a/b   (segfault)   ./
>
> The old and new versions both seem to be (differently) inconsistent
> about when the output has a trailing slash.  What is the rule?

That is a good point.  At least adding / at the end of "." seems
unneeded, given that the output in some cases have no trailing
slash, forcing a caller who wanted to get a directory to append a
trailing path components to it to check if it needs to add one
before doing so.  Always adding a slash / to the output may sound
consistent, but it is not quite; e.g. "/a/c based on /a/b is ../c"
case may be referring to a non directory /a/c and ensuring a
trailing slash to produce ../c/ is actively wrong.

"The caller knows" rule might work (I am thinking aloud, without
looking at existing callers to see what would break, only to see if
a consistent and simple-to-explain rule is possible).

When the caller asks to turn /a/b relative to /a/b (or /a/b/,
/a//b/./), then we do not know (or we do not want to know) if the
caller means it to be a directory with the intention of appending
something after it, so just return ".".  When the caller asks to
turn /a/b/ relative to the same base, return "./" with the trailing
slash.  Remember if the "abs path" side had a trailing slash,
normalize both input (turning "/./" into "/" and "foo/bar/../" into
foo/", squashing multiple slashes into one, etc.) and then strip the
trailing slash from them, do the usual "comparison and replacement
of leading path components with series of ../" and then append a
slash if the original had one, or something?

>> index 04ff..0174d 100644
>> --- a/path.c
>> +++ b/path.c
>> @@ -441,42 +441,104 @@ int adjust_shared_perm(const char *path)
>>  return 0;
>>  }
>>  
>> -const char *relative_path(const char *abs, const char *base)
>> +/*
>> + * Give path as relative to prefix.
>> + *
>> + * The strbuf may or may not be used, so do not assume it contains the
>> + * returned path.
>> + */
>> +const char *relative_path(const char *abs, const char *base,
>> +  struct strbuf *sb)
>
> Thanks for adding documentation.  But I think it could be improved:
>
> * The comment refers to "path" and "prefix" but the function parameters
> are "abs" and "base".  I suggest making them agree.
>
> * Who owns the memory pointed to by the return value?
>
> * The comment says that "the strbuf may or may not be used".  So why is
> it part of the interface?  (I suppose it is because the strbuf might be
> given ownership of the returned memory if it has to be allocated.)
> Would it be more straightforward to *always* return the result in the
> strbuf?

This comes from the original in quote.c, I think.  The caller
supplies a strbuf as a scratchpad area and releasing it is the
caller's responsibility.  If the function does not need any
scratchpad area (i.e. the answer is a substring of the input), the
result may point into the abs.

So the calling convention is:

struct strbuf scratch = STRBUF_INIT;
const char *result = relative(abs, base, &scratch);
use(result);
strbuf_release(&scratch);

and the lifetime rule is "consume it before either abs or scratch
is changed".

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


Remote branch can not be resolved as commit?

2013-05-22 Thread Kendall Shaw
I am trying to setup a repository for use inside the LAN, but I have 
been unable to checkout any branch so far. I am very new to git.


The repository is being served from gitblit over https. I have 
GIT_SSL_NO_VERIFY=true. The repository was created from git svn.


git ls-remote

shows the remote branches, e.g.:

... refs/remotes/2.0.3
... refs/remotes/trunk

git branch -r

shows none of the remote branches.

git checkout -b new-2.0.3 origin/2.0.3

produces:

fatal: git checkout: updating paths is incompatible with switching branches.
Did you intend to checkout 'origin/2.0.3' which can not be resolved as 
commit?


What does that mean?

I get the same result after each of these:

git fetch

git remote update
git fetch

git add remote stage-repo https://example.com:8443/git/blah-tools.git
git fetch stage-repo
git checkout -b new-2.0.3 stage-repo/2.0.3

Can you explain what the error message means, and what I am doing wrong?

Kendall


--
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: English/German terminology, git.git's de.po, and pro-git

2013-05-22 Thread Holger Hellmuth (IKS)

Am 22.05.2013 17:16, schrieb Ralf Thielow:

 hunk   = Bereich


IMHO "Kontext" is better if you use a German word. Technically the context is
something else, but in a German text IMHO it fits nicer when explaining to the
user where he/she can select the n-th hunk.



Not sure if German users would know what "hunk" means, in case we
leave it untranslated. And I'm not sure if I would understand "Kontext".
I tend to leave it untranslated.


I don't think "Bereich" is a bad choice. As "hunk" is not a word with 
special meaning in cvs and not used in any commands I don't see a lot of 
reasons to keep it in english.


Alternative translations might be "Teilbereich", "Dateibereich". 
"Kontext" would be very confusing IMHO



--
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: Avoiding broken Gitweb links and deleted objects

2013-05-22 Thread William Swanson
On Wed, May 22, 2013 at 6:32 AM, Matt McClure  wrote:
> Is there a way to push/pull reflogs among different repositories?

Not that I am aware of, at least not in core git.

> In my original scenario:
>
> 1. the commits are created on a developer machine
> 2. pushed to a central origin repository running Gitweb
> 3. the branch is rebased on the developer machine
> 4. the branch is push --force'd to the origin
>
> Later, git push tells me:
>
> warning: There are too many unreachable loose objects; run 'git
> prune' to remove them.

You don't need to share reflogs in this case. Assuming the server were
to keep logs of its own, the forced update would create a new reflog
entry showing something like "   Forced
push", so the pre-rebase version would still be reachable from the
reflogs, keeping it around.

> or I want to delete old topic branch HEADs to improve performance.
>
> But I never want to let Git delete the underlying commit objects since
> there could be Gitweb links pointing at them.

The reflog thing won't help you in this case, since reflogs are
deleted when their branches are deleted. it sounds like you never want
to delete anything, so it would make more sense to just disable
garbage collection entirely.

-William
--
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: English/German terminology, git.git's de.po, and pro-git

2013-05-22 Thread Ralf Thielow
2013/5/20 Christian Stimming :
> Thanks for the update. I would like to add some comments on this G+E glossary
> and I hope you are interested in reading those, even though it is known that I
> prefer a "pure Ger" translation. However, as I wrote in my other message I
> agree that for the command line tool the criteria for choosing the translation
> approach are different from those for a GUI tool. So I can very well envision
> a good G+E translation for git core and subsequently all related books.
>

Thanks for your comments.

> Am Sonntag, 19. Mai 2013, 18:53:18 schrieb Ralf Thielow:
>> Basic repository objects:
>>
>> blob   = Blob
>> tree   = Baum, Baum-Objekt (bevorzugt), "Tree"-Objekt
>> submodule  = Submodul
>> pack(noun) = Pack-Datei
>> pack(verb) = packen (ggf. Pack-Datei erstellen)
>> ancestor   = Vorfahre, Vorgänger, Vorgänger-Commit (bevorzugt)
>
> Yes. Does the "Pack-Datei" appear anywhere in the book? I wouldn't understand
> the term, but then again, this is probably because I don't understand the
> semantic of this thingy as a repository object regardless of the language...
>

The book has this word in it's index ("9.4 Pack-Dateien")
http://git-scm.com/book/de
so we're fine here.

While at there, I just read "Die Refspec" in the index. The current glossary
doesn't contain "refspec" and we translate is as "Referenzspezifikation".
So if we want to match the book, we should add "refspec = Refspec" to
the glossary.

>> Content in a repository:
>>
>> file(s)= Datei(en)
>> tracked file   = beobachtete Datei
>> track file = beobachte Datei
>> untracked file = unbeobachtete Datei
>> directory  = Verzeichnis
>
> Yes.
>
>> Repositories / tracking concepts:
>>
>> clone (verb)   = klonen
>> clone (noun)   = der Klon
>> repository = Repository
>> bare repository= Bare Repository
>
> Yes. After some evaluation of the git-gui translation I think using
> "Repository" there as well is probably the better choice.
>
>> working directory  = Arbeitsverzeichnis
>> working tree   = -||-
>>
>> remote branch  = Remote-Branch
>> remote-tracking branch = Remote-Tracking-Branch
>> upstream branch= Upstream-Branch
>
> Yes. What's the main reason for using "Branch" in the German text? Consistency
> with the commands, or assumed familiarity of the term within the target
> audience? "Zweig" is available.
>

I think it's at the same level as "Commit" and a well known SCM-term. Users
(even beginners) who know "Commit" and "Tag" do also know "Branch". And
I think it sounds better in combination with "Remote-", "Remote-Tracking-" and
"Upstream-" which are english words.

>> remote repository  = Remote-Repository
>> remote(noun)   = -||-
>> remote(adj)= extern, entfernt liegend
>>
>> Authorship:
>>
>> author= Autor
>> committer = Commit-Ersteller
>> tagger= Tag-Ersteller
>
> Yes.
>
>> Commits, tags and other references:
>>
>> HEAD   = HEAD
>> Konzept aus der Git-Welt, daher nicht zu übersetzen.
>> detached HEAD  = losgelöster HEAD
>>
>> commit(noun)  = Commit
>> commit(verb)  = committen
>> commit the result = das Ergebnis committen
>> parent commit = Eltern-Commit
>> child commit  = Kind-Commit
>> commit message= Commit-Beschreibung
>
> Yes, for the G+E approach.
>
>> stash(noun)   = der Stash
>> stash(verb)   = "stashen", "stash" benutzen (bevorzugt)
>> unstash(verb) = "unstashen", "zurückladen", "aus 'stash'
>> zurückladen" (bevorzugt)
>
> Using "Stash" in G+E is quite ugly, but the noun is probably unavoidable
> because the feature is pretty much unique to git. I'd suggest to use only the
> noun and use the verbs as "stash benutzen" and "aus stash zurückladen" as
> proposed.
>

Yes.

>> reference  = Referenz
>> revision   = Commit
>> branch = Branch
>> tag(noun)  = Tag
>> tag(verb)  = taggen, Tag erstellen
>> annotated tag  = annotierter Tag
>> tag message= Tag-Beschreibung
>
> I've commented on "Branch" above. As for "Tag": Yes, the term is familiar
> among the target audience. However, do you really want this noun which is the
> same word as "Tag wie in Datum"? Some more disambiguation between the tag and
> the date would be helpful, wouldn't it?
> The derived forms are fine, and also here I'd suggest to use only the G+E noun
> but construct the verbs with other German words: "Tag erstellen".
>
>> stage/index (noun) = Staging-Area, Index
>> stage/index (verb) = (für einen | zum) Commit vormerken
>> (bevorzugt), zur Staging Area hinzufügen, dem Index hinzufügen
>> unstage (verb) = aus Staging Area entfernen, aus Index entfernen
>
> I'd strongly suggest not to use "Index". I've never understood why this term
> showed up in the E

Re: Reading commit objects

2013-05-22 Thread Chico Sokol
Solved! It was exaclty the problem pointed by Shawn.

Here is the working code:

File dotGit = new File("objects/25/0f67ef017fcb97b5371a302526872cfcadad21");
InflaterInputStream inflaterInputStream = new InflaterInputStream(new
FileInputStream(dotGit));
Integer read = inflaterInputStream.read();
while(read != 0) { //reading the bytes from 'commit \0'
read = inflaterInputStream.read();
System.out.println((char)read.byteValue());
}
ByteArrayOutputStream os = new ByteArrayOutputStream();
IOUtils.copyLarge(inflaterInputStream, os);
System.out.println(new String(os.toByteArray()));

Thank you all!



--
Chico Sokol


On Wed, May 22, 2013 at 11:25 AM, Chico Sokol  wrote:
>> Your code is broken. IOUtils is probably corrupting what you get back.
>> After inflating the stream you should see the object type ("commit"),
>> space, its length in bytes as a base 10 string, and then a NUL ('\0').
>> Following that is the tree line, and parent(s) if any. I wonder if
>> IOUtils discarded the remainder of the line after the NUL and did not
>> consider the tree line.
>
>
> Maybe you're right, Shawn. I've also tried the following code:
>
> File dotGit = new File("objects/25/0f67ef017fcb97b5371a302526872cfcadad21");
> InflaterInputStream inflaterInputStream = new InflaterInputStream(new
> FileInputStream(dotGit));
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> IOUtils.copyLarge(inflaterInputStream, os);
> System.out.println(new String(os.toByteArray()));
>
> But we got the same result, I'll try to read the bytes by myself
> (without apache IOUtils). Is the contents of a unpacked object utf-8
> encoded?
--
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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Theodore Ts'o
I just had another idea (although I haven't had a chance to code up
anything yet).  Perhaps instead of, or in addition to, a global
setting (i.e., guilt.reusebranch), perhaps we should have a per-branch
setting, such as branch..guiltReuseBranch?

I was actually thinking that it might be interesting to have a
branch..rewindable, which would change the guilt defaults, and
could also key changes in key git behavior which makes it less likely
that a user shoots him or herself in the foot --- i.e., give warnings
if he or she has modified the branch in such a way that
remotes.origin. is no longer contained within the branch head.

  - Ted
--
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/3] Fixing volatile HEAD in push.default = current

2013-05-22 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> The patch may have been done by a wrong motivation, in that it does
> not fundamentally "fix" the itch.  The particular "itch" is not
> something we are going to promise to the end users, ever, anyway.

Just out of curiosity, is it possible to write a correct fix at all?
Even if the first statement in do_push() locks the HEAD ref, it's not
enough: the program may be wading around in git.c/setup.c when I
switch terminals and change HEAD, right?

So, our position on the matter is: no git command makes any guarantees
with respect to races, correct?
--
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/3] Fixing volatile HEAD in push.default = current

2013-05-22 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> Why should I lie in the patch?  The terminal flipping was a very big
> itch I had, and the patch fixes exactly that issue.  Showing the real
> branch name was an unintended side-effect.
>
> I just said "early" and showed a nice end-user example in which it
> works, not "theoretically impossible to race with".  Better wording
> (while not lying about the motivation behind the patch)?

The patch may have been done by a wrong motivation, in that it does
not fundamentally "fix" the itch.  The particular "itch" is not
something we are going to promise to the end users, ever, anyway.

The only remaining justification for the change is, even though the
user cannot _safely_ flip the branches with this patch, it improves
the output.

That does not make the patch wrong, but the original motivation is
an irrelevant, lost cause.  "Even though this started to address an
itch, the patch does not fundamentally fix that itch at all." may be
a honest statement to make, but that alone is not a justification to
have this change.

The "side effect" is the only improvement this patch gives us, and
that happens to be a good enough justification.  At that point, is
the original itch the patch does not correctly address even worth
mentioning?  I answered "no" to that question.

So I do not think you are lying anything.
--
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: Reading commit objects

2013-05-22 Thread Chico Sokol
> Your code is broken. IOUtils is probably corrupting what you get back.
> After inflating the stream you should see the object type ("commit"),
> space, its length in bytes as a base 10 string, and then a NUL ('\0').
> Following that is the tree line, and parent(s) if any. I wonder if
> IOUtils discarded the remainder of the line after the NUL and did not
> consider the tree line.


Maybe you're right, Shawn. I've also tried the following code:

File dotGit = new File("objects/25/0f67ef017fcb97b5371a302526872cfcadad21");
InflaterInputStream inflaterInputStream = new InflaterInputStream(new
FileInputStream(dotGit));
ByteArrayOutputStream os = new ByteArrayOutputStream();
IOUtils.copyLarge(inflaterInputStream, os);
System.out.println(new String(os.toByteArray()));

But we got the same result, I'll try to read the bytes by myself
(without apache IOUtils). Is the contents of a unpacked object utf-8
encoded?
--
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: Reading commit objects

2013-05-22 Thread Chico Sokol
I'm not criticizing JGit, guys. It simply doesn't fit into our needs.
We're not interested in mapping git commands in java and don't have
the same RAM limitations.

I know JGit team is doing a great job and we do not intend to build a
library with such completeness.

Are you guys contributors of JGit? Can you guys point me out to the
code that unpacks git objects? The closest I could get was that class:
https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObject.java

It seems to be a standard and a non standard format of the packed
object, as I read the comments of this method:
https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObject.java#L272

I suspect that the default inflater class of java api expect the
object to be in the standard format.

What the following comment mean? What's the "Experimental pack-based"
format? Is there any docs on the specs of that?

We must determine if the buffer contains the standard
zlib-deflated stream or the experimental format based
on the in-pack object format. Compare the header byte
for each format:
RFC1950 zlib w/ deflate : 0www1000 : 0 <= www <= 7
Experimental pack-based : Sttt : ttt = 1,2,3,4


--
Chico Sokol


On Wed, May 22, 2013 at 2:59 AM, Shawn Pearce  wrote:
> On Tue, May 21, 2013 at 3:18 PM, Chico Sokol  wrote:
>> Ok, we discovered that the commit object actually contains the tree
>> object's sha1, by reading its contents with python zlib library.
>>
>> So the bug must be with our java code (we're building a java lib).
>>
>> Is there any non-standard issue in git's zlib compression? We're
>> decompressing its contents with java default zlib api, so it should
>> work normally, here's our code, that's printing that wrong output:
>>
>> import java.io.File;
>> import java.io.FileInputStream;
>> import java.util.zip.InflaterInputStream;
>> import org.apache.commons.io.IOUtils;
>> ...
>> File obj = new 
>> File(".git/objects/25/0f67ef017fcb97b5371a302526872cfcadad21");
>> InflaterInputStream inflaterInputStream = new InflaterInputStream(new
>> FileInputStream(obj));
>> System.out.println(IOUtils.readLines(inflaterInputStream));
> ...
 Currently, we're trying to parse commit objects. After decompressing
 the contents of a commit object file we got the following output:

 commit 191
 author Francisco Sokol  1369140112 -0300
 committer Francisco Sokol  1369140112 -0300

 first commit
>
> Your code is broken. IOUtils is probably corrupting what you get back.
> After inflating the stream you should see the object type ("commit"),
> space, its length in bytes as a base 10 string, and then a NUL ('\0').
> Following that is the tree line, and parent(s) if any. I wonder if
> IOUtils discarded the remainder of the line after the NUL and did not
> consider the tree line.
>
> And you wonder why JGit code is confusing. We can't rely on "standard
> Java APIs" to do the right thing, because commonly used libraries have
> made assumptions that disagree with the way Git works.
--
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: English/German terminology, git.git's de.po, and pro-git

2013-05-22 Thread Ralf Thielow
2013/5/20 Holger Hellmuth :
> Am 19.05.2013 18:56, schrieb Ralf Thielow:
>
>> 2013/5/16 Holger Hellmuth (IKS) :
>>>
>>>
>> [...]

 +reset = neu setzen (maybe "umsetzen"?)
>>>
>>>
>>>
>>> "zurücksetzen"
>>>
>>
>> "reset" can be used with every existing commit. "zurücksetzen"
>> would imply that it have to be a recent commit, no?
>
>
> It implies that it sets to something that already existed or came before.
> So it even fits in a case where you reset to an older commit and reset back
> to HEAD because the HEAD commit existed already.
>
> If you still don't like it, I would prefer "umsetzen" to "neu setzen".
>

I'd still understand "zurücksetzen" as "set something *back* to" but this
"back" can also be something that was made after HEAD perhaps on
another branch and HEAD (or the current ref) was never at this point before,
so "zurücksetzen" is not true in this case.

I prefer "umsetzen" to "neu setzen", too. I'll change the glossary to this.

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
--
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: override merge.ff = false using --ff-only

2013-05-22 Thread Yann Droneaud

Hi,

Le 22.05.2013 15:21, Matt McClure a écrit :

I naively tried to override merge.ff = false using --ff-only on the
command line. I expected that it would override the configured 
default

and perform a fast-forward merge. Instead, it said:

$ git config -l | grep -F 'merge.ff'
merge.ff=false

$ git merge --ff-only foo
fatal: You cannot combine --no-ff with --ff-only.

On the other hand, I see that --ff works just fine in the same 
initial state.




You might want to read the following messages from thread "git merge 
 behavior"


http://article.gmane.org/gmane.comp.version-control.git/218519
http://article.gmane.org/gmane.comp.version-control.git/218755
http://article.gmane.org/gmane.comp.version-control.git/218786


Regards.

--
Yann Droneaud
OPTEYA

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


Re: [PATCH] Added guilt.reusebranch configuration option.

2013-05-22 Thread Josef 'Jeff' Sipek
On Wed, May 22, 2013 at 03:01:36PM +0200, Per Cederqvist wrote:
> When the option is true (the default), Guilt does not create a new Git
> branch when patches are applied.  This way, you can switch between
> Guilt 0.35 and the current version of Guilt with no issues.
> 
> At a future time, maybe a year after Guilt with guilt.reusebranch
> support is released, the default should be changed to "false" to take
> advantage of the ability to use a separate Git branch when patches are
> applied.

So, I've been using the always-on prefix code and I do like it.  It makes me
reasonably happy that other people that work on the same repo won't screw it
up.  So, with that said, I'm tempted to actually make the default the
new-style prefix.  If someone (there's at least Ted) wants the old behavior,
they'll have the config.

IOW, I'm tempted to apply this patch with a minor tweak: change the default
to new-style.

> Signed-off-by: Per Cederqvist 
> ---
> 
> This is an alternative solution to the same problem.  I've been running
> with this code for a while.  I don't remember if I sent it to the list
> before, but if I did it was apparently lost.  Sorry if I never sent it.

For all I know, I just dropped this patch on the floor by accident :/

Jeff.

> This version includes some regression tests.
> 
> (I'm having mail problems. Apologies if you receive this patch twice.)
> 
> /ceder
> 
>  guilt|  29 +++-
>  regression/scaffold  |   1 +
>  regression/t-062.out | 457 
> +++
>  regression/t-062.sh  | 150 +
>  4 files changed, 632 insertions(+), 5 deletions(-)
>  create mode 100644 regression/t-062.out
>  create mode 100755 regression/t-062.sh
> 
> diff --git a/guilt b/guilt
> index 66a671a..108d4e7 100755
> --- a/guilt
> +++ b/guilt
> @@ -836,6 +836,9 @@ guilt_push_diff_context=1
>  # default diffstat value: true or false
>  DIFFSTAT_DEFAULT="false"
>  
> +# default old_style_prefix value: true or false
> +REUSE_BRANCH_DEFAULT="true"
> +
>  # Prefix for guilt branches.
>  GUILT_PREFIX=guilt/
>  
> @@ -847,6 +850,10 @@ GUILT_PREFIX=guilt/
>  diffstat=`git config --bool guilt.diffstat`
>  [ -z "$diffstat" ] && diffstat=$DIFFSTAT_DEFAULT
>  
> +# reuse Git branch?
> +reuse_branch=`git config --bool guilt.reusebranch`
> +[ -z "$reuse_branch" ] && reuse_branch=$REUSE_BRANCH_DEFAULT
> +
>  #
>  # The following gets run every time this file is source'd
>  #
> @@ -911,13 +918,25 @@ else
>   die "Unsupported operating system: $UNAME_S"
>  fi
>  
> -if [ "$branch" = "$raw_git_branch" ] && [ -n "`get_top 2>/dev/null`" ]
> +if [ -n "`get_top 2>/dev/null`" ]
>  then
> -# This is for compat with old repositories that still have a
> -# pushed patch without the new-style branch prefix.
> -old_style_prefix=true
> + # If there is at least one pushed patch, we set
> + # old_style_prefix according to how it was pushed.  It is only
> + # possible to change the prefix style while no patches are
> + # applied.
> + if [ "$branch" = "$raw_git_branch" ]
> + then
> + old_style_prefix=true
> + else
> + old_style_prefix=false
> + fi
>  else
> -old_style_prefix=false
> + if $reuse_branch
> + then
> + old_style_prefix=true
> + else
> + old_style_prefix=false
> + fi
>  fi
>  
>  _main "$@"
> diff --git a/regression/scaffold b/regression/scaffold
> index 5c8b73e..acddb07 100644
> --- a/regression/scaffold
> +++ b/regression/scaffold
> @@ -88,6 +88,7 @@ function setup_git_repo
>   git config log.date default
>   git config log.decorate no
>   git config guilt.diffstat false
> + git config guilt.reusebranch false
>  }
>  
>  function setup_guilt_repo
> diff --git a/regression/t-062.out b/regression/t-062.out
> new file mode 100644
> index 000..d00b3f6
> --- /dev/null
> +++ b/regression/t-062.out
> @@ -0,0 +1,457 @@
> +% setup_repo
> +% git config guilt.reusebranch true
> +% guilt push -a
> +Applying patch..modify
> +Patch applied.
> +Applying patch..add
> +Patch applied.
> +Applying patch..remove
> +Patch applied.
> +Applying patch..mode
> +Patch applied.
> +% list_files
> +d .git/patches
> +d .git/patches/master
> +d .git/refs/patches
> +d .git/refs/patches/master
> +f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
> +f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
> +f 71596bf71b72c2717e1aee378aabefbfa19ab7c8  .git/patches/master/status
> +f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
> +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
> +f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
> +r 33633e7a1aa31972f125878baf7807be57b1672d  .git/refs/patches/master/modify
> +r 37d588cc39848368810e88332bd03b083f2ce3ac  .git/refs/patches/master/add
> +r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba  .git/refs/patches/master/mode
> +r ffb7faa126a6d9

Re: Avoiding broken Gitweb links and deleted objects

2013-05-22 Thread Matt McClure
On Fri, May 10, 2013 at 3:34 AM, William Swanson  wrote:
> I started working on something like this a few weeks ago, but
> eventually came to the conclusion that this information does not
> belong in the commit graph itself.
>
> A better approach, I think, would be to enhance the reflogs to the
> point where they can provide this information in a reliable manner.

Is there a way to push/pull reflogs among different repositories?

In my original scenario:

1. the commits are created on a developer machine
2. pushed to a central origin repository running Gitweb
3. the branch is rebased on the developer machine
4. the branch is push --force'd to the origin

Later, git push tells me:

warning: There are too many unreachable loose objects; run 'git
prune' to remove them.

or I want to delete old topic branch HEADs to improve performance.

But I never want to let Git delete the underlying commit objects since
there could be Gitweb links pointing at them.

-- 
Matt McClure
http://matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
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] guilt: fix date parsing

2013-05-22 Thread Josef 'Jeff' Sipek
On Wed, May 22, 2013 at 08:10:10AM -0400, Theodore Ts'o wrote:
> On Tue, May 21, 2013 at 11:39:21PM -0400, Josef 'Jeff' Sipek wrote:
> > I applied this one and the "guilt: skip empty line after..." patch.
> 
> Thanks!  BTW, it looks like you are not using "git am -s" to apply
> these patches?  The reason why I ask is that whatever you're using
> isn't removing the [XXX] subject prefix (e.g., [PATCH] or [PATCH -v2]
> which is useful for mailing lists, but less useful in the git commit
> descriptions.
> 
> If you're using guilt, do you have some script that preformats a Unix
> mbox into guilt-friendly files?  If so, maybe it would be good to
> modify it to strip out the [PATCH] annotations.  If not, let me know,
> since I've been thinking about writing a script to take a Unix mbox,
> and bursts it into a separate patch-per-file with a series file
> suitable for use by guilt, removing mail headers and doing other
> appropriate pre-parsing --- basically, a "guilt am" which works much
> like "git am".  But if someone else has done this already, no point
> duplicating effort.  :-)

You are correct.  I just `guilt import -P blah /tmp/mdir/cur/X` and then
hand-edit the patch to remove headers.  Last night I was thinking about
making a `guilt import-mbox` that'd import a mbox or maildir.  I don't know
off the top of my head how much `git am` would help in this instance.

Feel free to make the import-mbox or whatever command :)

Jeff.

-- 
Only two things are infinite, the universe and human stupidity, and I'm not
sure about the former.
- Albert Einstein
--
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: Avoiding broken Gitweb links and deleted objects

2013-05-22 Thread Matt McClure
On Fri, May 10, 2013 at 12:22 PM, Junio C Hamano  wrote:
> I think what I missed is that the same logic to ignore side branches
> whose history gets cauterised with such an "ours" merge may apply to
> an "ours" merge that people already make, but the latter may want to
> take both histories into account.
>
> So I guess it is not such a great idea.

The particular proposed implementation? Or the broader idea to save
loose commits more permanently? I'm still interested in a solution for
the latter.

-- 
Matt McClure
http://matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
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] Geolocation support

2013-05-22 Thread Alessandro Di Marco
Hi all,

this is a hack I made a couple of years ago in order to store my current
location in git commits (I travel a lot and being able to associate a
place with the commit date helps me to quickly recover what were doing
at that time). Long story short, the screeenshot at
http://tinypic.com/r/wars40/5 shows the new gitk interface once this
patch has been integrated. Geolocation is controlled by two envvars
GIT_AUTHOR_PLACE and COMMITTER_PLACE, respectively. You can set them via
something like this:

function gitenv {
DATA="`tail -n1 /etc/geotags.dat`"

CITY="`echo \"$DATA\" | cut -d ',' -f 6 | sed -r 's/^\s+//'`"
COUNTRY="`echo \"$DATA\" | cut -d ',' -f 5 | sed -r 's/^\s+//'`"
LAT="`echo \"$DATA\" | cut -d ',' -f 2 | sed -r 's/^\s+//'`"
LON="`echo \"$DATA\" | cut -d ',' -f 3 | sed -r 's/^\s+//'`"

if [ -n "$CITY" ] ; then
export GIT_AUTHOR_PLACE="$CITY, $COUNTRY ($LAT, $LON)"
export GIT_COMMITTER_PLACE="$CITY, $COUNTRY ($LAT, $LON)"
else
export GIT_AUTHOR_PLACE="$COUNTRY ($LAT, $LON)"
export GIT_COMMITTER_PLACE="$COUNTRY ($LAT, $LON)"
fi
}

alias git='gitenv ; git'

Finally, here it is a sample of my geotags data (produced by a bash
script appending a line per wlan gateway change).

Mon Mar 4 19:18:00 CET 2013, 44.4167, 8.95, Europe, Italy, Genoa, Europe/Rome, 
2.230.XXX.XX, Fastweb, Fastweb SpA
Wed Mar 6 01:14:43 CET 2013, 25.7743, -80.1937, North America, United States, 
Miami, America/New_York, 96.47.XXX.XXX, IPTelligent LLC, IPTelligent LLC
Mon May 6 22:29:01 UTC 2013, 35.685, 139.7514, Asia, Japan, Tokyo, Asia/Tokyo, 
183.77.XXX.XXX, Asahi Net, Asahi Net
Thu May 16 21:55:20 UTC 2013, 44.4167, 8.95, Europe, Italy, Genoa, Europe/Rome, 
2.230.XXX.XXX, Fastweb, Fastweb SpA

That's all folks.

Regards,
Alessandro

Signed-off-by: Alessandro Di Marco 
---
 builtin/blame.c  |   2 +-
 builtin/commit.c |  32 ++--
 cache.h  |   3 +-
 fsck.c   |  13 +-
 gitk-git/gitk| 571 ++-
 ident.c  |  30 ++-
 6 files changed, 619 insertions(+), 32 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 57a487e..6ed145c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2136,7 +2136,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,

origin = make_origin(commit, path);

-   ident = fmt_ident("Not Committed Yet", "not.committed.yet", NULL, 0);
+   ident = fmt_ident("Not Committed Yet", "not.committed.yet", NULL, NULL, 
0);
strbuf_addstr(&msg, "tree \n");
for (parent = commit->parents; parent; parent = parent->next)
strbuf_addf(&msg, "parent %s\n",
diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..a19ae23 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -497,40 +497,46 @@ static int sane_ident_split(struct ident_split *person)

 static void determine_author_info(struct strbuf *author_ident)
 {
-   char *name, *email, *date;
+   char *name, *email, *date, *place;
struct ident_split author;

name = getenv("GIT_AUTHOR_NAME");
email = getenv("GIT_AUTHOR_EMAIL");
date = getenv("GIT_AUTHOR_DATE");
+   place = getenv("GIT_AUTHOR_PLACE");

if (author_message) {
-   const char *a, *lb, *rb, *eol;
+   const char *a, *lb1, *rb1, *lb2, *rb2, *eol;
size_t len;

a = strstr(author_message_buffer, "\nauthor ");
if (!a)
die(_("invalid commit: %s"), author_message);

-   lb = strchrnul(a + strlen("\nauthor "), '<');
-   rb = strchrnul(lb, '>');
-   eol = strchrnul(rb, '\n');
-   if (!*lb || !*rb || !*eol)
+   lb1 = strchrnul(a + strlen("\nauthor "), '<');
+   rb1 = strchrnul(lb1, '>');
+
+   lb2 = strchrnul(rb1, '(');
+   rb2 = strchrnul(lb2, ')');
+
+   eol = strchrnul(rb1, '\n');
+   if (!*lb1 || !*rb1 || !*lb2 || !*rb2 || !*eol)
die(_("invalid commit: %s"), author_message);

-   if (lb == a + strlen("\nauthor "))
+   if (lb1 == a + strlen("\nauthor "))
/* \nauthor  */
name = xcalloc(1, 1);
else
name = xmemdupz(a + strlen("\nauthor "),
-   (lb - strlen(" ") -
+   (lb1 - strlen(" ") -
 (a + strlen("\nauthor ";
-   email = xmemdupz(lb + strlen("<"), rb - (lb + strlen("<")));
-   date = xmemdupz(rb + strlen("> "), eol - (rb + strlen("> ")));
-   len = eol - (rb + strlen("> "));
+   email = xmemdupz(lb1 + strlen("<"), rb1 - (lb1 + strlen("<")));
+   date = xmemdupz(rb1 + strlen("> "), lb

override merge.ff = false using --ff-only

2013-05-22 Thread Matt McClure
I naively tried to override merge.ff = false using --ff-only on the
command line. I expected that it would override the configured default
and perform a fast-forward merge. Instead, it said:

$ git config -l | grep -F 'merge.ff'
merge.ff=false

$ git merge --ff-only foo
fatal: You cannot combine --no-ff with --ff-only.

On the other hand, I see that --ff works just fine in the same initial state.

$ git merge --ff foo
Updating b869407..17b5495
Fast-forward
...
 4 files changed, 2 insertions(+), 5 deletions(-)

Would it be better if --ff-only refused to merge only if the commits
themselves prevented fast-forwarding?

--
Matt McClure
http://matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
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] Added guilt.reusebranch configuration option.

2013-05-22 Thread Per Cederqvist
When the option is true (the default), Guilt does not create a new Git
branch when patches are applied.  This way, you can switch between
Guilt 0.35 and the current version of Guilt with no issues.

At a future time, maybe a year after Guilt with guilt.reusebranch
support is released, the default should be changed to "false" to take
advantage of the ability to use a separate Git branch when patches are
applied.

Signed-off-by: Per Cederqvist 
---

This is an alternative solution to the same problem.  I've been running
with this code for a while.  I don't remember if I sent it to the list
before, but if I did it was apparently lost.  Sorry if I never sent it.

This version includes some regression tests.

(I'm having mail problems. Apologies if you receive this patch twice.)

/ceder

 guilt|  29 +++-
 regression/scaffold  |   1 +
 regression/t-062.out | 457 +++
 regression/t-062.sh  | 150 +
 4 files changed, 632 insertions(+), 5 deletions(-)
 create mode 100644 regression/t-062.out
 create mode 100755 regression/t-062.sh

diff --git a/guilt b/guilt
index 66a671a..108d4e7 100755
--- a/guilt
+++ b/guilt
@@ -836,6 +836,9 @@ guilt_push_diff_context=1
 # default diffstat value: true or false
 DIFFSTAT_DEFAULT="false"
 
+# default old_style_prefix value: true or false
+REUSE_BRANCH_DEFAULT="true"
+
 # Prefix for guilt branches.
 GUILT_PREFIX=guilt/
 
@@ -847,6 +850,10 @@ GUILT_PREFIX=guilt/
 diffstat=`git config --bool guilt.diffstat`
 [ -z "$diffstat" ] && diffstat=$DIFFSTAT_DEFAULT
 
+# reuse Git branch?
+reuse_branch=`git config --bool guilt.reusebranch`
+[ -z "$reuse_branch" ] && reuse_branch=$REUSE_BRANCH_DEFAULT
+
 #
 # The following gets run every time this file is source'd
 #
@@ -911,13 +918,25 @@ else
die "Unsupported operating system: $UNAME_S"
 fi
 
-if [ "$branch" = "$raw_git_branch" ] && [ -n "`get_top 2>/dev/null`" ]
+if [ -n "`get_top 2>/dev/null`" ]
 then
-# This is for compat with old repositories that still have a
-# pushed patch without the new-style branch prefix.
-old_style_prefix=true
+   # If there is at least one pushed patch, we set
+   # old_style_prefix according to how it was pushed.  It is only
+   # possible to change the prefix style while no patches are
+   # applied.
+   if [ "$branch" = "$raw_git_branch" ]
+   then
+   old_style_prefix=true
+   else
+   old_style_prefix=false
+   fi
 else
-old_style_prefix=false
+   if $reuse_branch
+   then
+   old_style_prefix=true
+   else
+   old_style_prefix=false
+   fi
 fi
 
 _main "$@"
diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..acddb07 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -88,6 +88,7 @@ function setup_git_repo
git config log.date default
git config log.decorate no
git config guilt.diffstat false
+   git config guilt.reusebranch false
 }
 
 function setup_guilt_repo
diff --git a/regression/t-062.out b/regression/t-062.out
new file mode 100644
index 000..d00b3f6
--- /dev/null
+++ b/regression/t-062.out
@@ -0,0 +1,457 @@
+% setup_repo
+% git config guilt.reusebranch true
+% guilt push -a
+Applying patch..modify
+Patch applied.
+Applying patch..add
+Patch applied.
+Applying patch..remove
+Patch applied.
+Applying patch..mode
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8  .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d  .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac  .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba  .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9  .git/refs/patches/master/remove
+% git for-each-ref
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master
+37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode
+33633e7a1aa31972f125878baf7807be57b1672d commit
refs/patches/master/modify
+ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
+% git update-ref refs/heads/master refs/heads/guilt/master
+fatal: refs/heads/guilt/master: not a valid SHA1
+% git symbolic-ref HEAD refs/heads/master
+% git update-ref -d refs/heads/guilt/master
+% git for-each-ref
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master
+3

Re: [PATCH -v2] guilt: force the use of bare branches

2013-05-22 Thread Per Cederqvist

On 05/22/13 14:11, Theodore Ts'o wrote:

To make it harder to accidentally do "git push" with a guilt patch
applied, "guilt push" changes branch from e.g. "master" to
"guilt/master" starting with commit 67d3af63f422.  This is a feature
which I use for ext4 development; I actually *do* want to be able to
push patches to the dev branch, which is a rewindable branch much like
git's "pu" branch.

Allow the use of the environment variable GUILT_FORCE_BARE_BRANCH
which disables the new behavior introduced by commit 67d3af63f422.

Signed-off-by: "Theodore Ts'o" 
Cc: Per Cederqvist 


I just posted an alternative patch that solves the same issue.
I forgot to add in-reply-to headers, and did change the subject
to "Added guilt.reusebranch configuration option". Sorry if I've
caused any confusion.

/ceder


---
  guilt | 17 +
  1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/guilt b/guilt
index e9b2aab..35a84dc 100755
--- a/guilt
+++ b/guilt
@@ -914,13 +914,22 @@ else
die "Unsupported operating system: $UNAME_S"
  fi

-if [ "$branch" = "$raw_git_branch" ] && [ -n "`get_top 2>/dev/null`" ]
-then
-# This is for compat with old repositories that still have a
-# pushed patch without the new-style branch prefix.
+if [ -n "`get_top 2>/dev/null`" ]; then
+  #
+  # If we have repositories patches pushed, then use whatever scheme
+  # is currently in use
+  #
+  if [ "$branch" = "$raw_git_branch" ]; then
  old_style_prefix=true
+  else
+old_style_prefix=false
+  fi
  else
+  if [ "$(git config --bool --get guilt.bareBranch)" = "true" ]; then
+old_style_prefix=true
+  else
  old_style_prefix=false
+  fi
  fi

  _main "$@"



--
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: document --no-verify

2013-05-22 Thread Michael S. Tsirkin
On Wed, May 22, 2013 at 02:12:21PM +0200, Thomas Rast wrote:
> "Michael S. Tsirkin"  writes:
> 
> > commit ec9f937727bcb0fa8a3dfe6af68c188e968a added
> > --no-verify flag to git push, but didn't document it.
> > It's a useful flag when using pre-push hooks so
> > add the documentation.
> >
> > Suggested-by: Thomas Rast 
> > Cc: Aaron Schrab 
> > Signed-off-by: Michael S. Tsirkin 
> [...]
> > +-n::
> > +--no-verify::
> > +   This option bypasses the pre-commit and commit-msg hooks.
> > +   See also linkgit:githooks[5].
> > +
> 
> Umm, half of that is not correct :-)
> 
> Push doesn't have the -n short form that git-commit does,

Hmm true - in fact -n means dry-run.

> and the hook
> names are wrong.
> 
> I also ended up writing a patch myself; sorry for not telling you on
> IRC:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/225141

Great, thanks.

> -- 
> Thomas Rast
> trast@{inf,student}.ethz.ch
--
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: document --no-verify

2013-05-22 Thread Thomas Rast
"Michael S. Tsirkin"  writes:

> commit ec9f937727bcb0fa8a3dfe6af68c188e968a added
> --no-verify flag to git push, but didn't document it.
> It's a useful flag when using pre-push hooks so
> add the documentation.
>
> Suggested-by: Thomas Rast 
> Cc: Aaron Schrab 
> Signed-off-by: Michael S. Tsirkin 
[...]
> +-n::
> +--no-verify::
> + This option bypasses the pre-commit and commit-msg hooks.
> + See also linkgit:githooks[5].
> +

Umm, half of that is not correct :-)

Push doesn't have the -n short form that git-commit does, and the hook
names are wrong.

I also ended up writing a patch myself; sorry for not telling you on
IRC:

  http://thread.gmane.org/gmane.comp.version-control.git/225141

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH -v2] guilt: force the use of bare branches

2013-05-22 Thread Theodore Ts'o
To make it harder to accidentally do "git push" with a guilt patch
applied, "guilt push" changes branch from e.g. "master" to
"guilt/master" starting with commit 67d3af63f422.  This is a feature
which I use for ext4 development; I actually *do* want to be able to
push patches to the dev branch, which is a rewindable branch much like
git's "pu" branch.

Allow the use of the environment variable GUILT_FORCE_BARE_BRANCH
which disables the new behavior introduced by commit 67d3af63f422.

Signed-off-by: "Theodore Ts'o" 
Cc: Per Cederqvist 
---
 guilt | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/guilt b/guilt
index e9b2aab..35a84dc 100755
--- a/guilt
+++ b/guilt
@@ -914,13 +914,22 @@ else
die "Unsupported operating system: $UNAME_S"
 fi
 
-if [ "$branch" = "$raw_git_branch" ] && [ -n "`get_top 2>/dev/null`" ]
-then
-# This is for compat with old repositories that still have a
-# pushed patch without the new-style branch prefix.
+if [ -n "`get_top 2>/dev/null`" ]; then
+  #
+  # If we have repositories patches pushed, then use whatever scheme
+  # is currently in use
+  #
+  if [ "$branch" = "$raw_git_branch" ]; then
 old_style_prefix=true
+  else
+old_style_prefix=false
+  fi
 else
+  if [ "$(git config --bool --get guilt.bareBranch)" = "true" ]; then
+old_style_prefix=true
+  else
 old_style_prefix=false
+  fi
 fi
 
 _main "$@"
-- 
1.7.12.rc0.22.gcdd159b

--
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] guilt: fix date parsing

2013-05-22 Thread Theodore Ts'o
On Tue, May 21, 2013 at 11:39:21PM -0400, Josef 'Jeff' Sipek wrote:
> I applied this one and the "guilt: skip empty line after..." patch.

Thanks!  BTW, it looks like you are not using "git am -s" to apply
these patches?  The reason why I ask is that whatever you're using
isn't removing the [XXX] subject prefix (e.g., [PATCH] or [PATCH -v2]
which is useful for mailing lists, but less useful in the git commit
descriptions.

If you're using guilt, do you have some script that preformats a Unix
mbox into guilt-friendly files?  If so, maybe it would be good to
modify it to strip out the [PATCH] annotations.  If not, let me know,
since I've been thinking about writing a script to take a Unix mbox,
and bursts it into a separate patch-per-file with a series file
suitable for use by guilt, removing mail headers and doing other
appropriate pre-parsing --- basically, a "guilt am" which works much
like "git am".  But if someone else has done this already, no point
duplicating effort.  :-)

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


first parent, commit graph layout, and pull merge direction

2013-05-22 Thread Andreas Krey
Hi everyone,

I'm just looking into better displays of the commit graph (as
displayed with gitk, smartgit, fisheye) - they tend to quickly
dissolve into a heap of spaghetti.

We had the idea that treating the first parent specially would
have some advantage here - including graphically indicating which
one of the parents of a commit is the first parent. (For instance,
by letting that line leave the commit node at the top/bottom,
and the other(s) to the side.)

A short trial showed that representing first parent chains as
straight lines in the graph does actually improve understandability,
as feature branches clearly stand out as separate lines even when
they no longer carry a branch name.

Does any GUI already do that (treat first parent specially),
or does anybody think of doing such? I don't quite dare to
jump into the gitk code yet.

Also, there is an implication with 'git pull': You'd expect the
master branch to be a first parent line, but when I do a small
thing directly on master and need to pull before pushing back,
then origin/master is merged into my branch, and thus my side
branch becomes the first parent line.

So, feature discussion request: Invert the parent ordering
when doing git pull from upstream? Configurably so?

We actually thought about putting a restriction into our blessed
repo that it not only restricts to fast-forward pushed, but further
to only allow pushing new things that have the old branch head in
the first parent chain.

What do you think?

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800
--
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] push: document --no-verify

2013-05-22 Thread Michael S. Tsirkin
commit ec9f937727bcb0fa8a3dfe6af68c188e968a added
--no-verify flag to git push, but didn't document it.
It's a useful flag when using pre-push hooks so
add the documentation.

Suggested-by: Thomas Rast 
Cc: Aaron Schrab 
Signed-off-by: Michael S. Tsirkin 
---
 Documentation/git-push.txt | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index d514813..346b28a 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] 
[--receive-pack=]
   [--repo=] [-f | --force] [--prune] [-v | --verbose] [-u 
| --set-upstream]
-  [ [...]]
+  [--no-verify]  [ [...]]
 
 DESCRIPTION
 ---
@@ -162,6 +162,11 @@ useful if you write an alias or script around 'git push'.
linkgit:git-pull[1] and other commands. For more information,
see 'branch..merge' in linkgit:git-config[1].
 
+-n::
+--no-verify::
+   This option bypasses the pre-commit and commit-msg hooks.
+   See also linkgit:githooks[5].
+
 --[no-]thin::
These options are passed to linkgit:git-send-pack[1]. A thin transfer
significantly reduces the amount of sent data when the sender and
-- 
MST
--
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/2] sha1_name: fix error message for @{u}

2013-05-22 Thread Ramkumar Ramachandra
Currently, when no (valid) upstream is configured for a branch, you get
an error like:

  $ git show @{u}
  error: No upstream configured for branch 'upstream-error'
  error: No upstream configured for branch 'upstream-error'
  fatal: ambiguous argument '@{u}': unknown revision or path not in the working 
tree.
  Use '--' to separate paths from revisions, like this:
  'git  [...] -- [...]'

The "error: " line actually appears twice, and the rest of the error
message is useless.  In sha1_name.c:interpret_branch_name(), there is
really no point in processing further if @{u} couldn't be resolved, and
we might as well die() instead of returning an error().  After making
this change, you get:

  $ git show @{u}
  fatal: No upstream configured for branch 'upstream-error'

Also tweak a few tests in t1507 to expect this output.

To justify that this change is safe, consider that all callers of
interpret_branch_name() have to fall in two categories:

1. Direct end-user facing applications like [rev-parse, show] calling in
   with end-user data (in which case the data can contain "@{u}").
   Failing immediately is the right thing to do: the only difference is
   that the die() happens in interpret_branch_name() instead of
   die_verify_filename(), and this is desirable.

2. Callers calling in with programmatic data, and expecting the function
   to return and not die().  In this case, why would anyone ever
   construct a string containing "@{u}" programmatically in the first
   place?  So, these callers can never hit the codepath touched by this
   patch, and the change does not affect them.

Signed-off-by: Ramkumar Ramachandra 
---
 sha1_name.c   | 13 +++--
 t/t1507-rev-parse-upstream.sh | 15 +--
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..416a673 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1033,14 +1033,15 @@ int interpret_branch_name(const char *name, struct 
strbuf *buf)
 * points to something different than a branch.
 */
if (!upstream)
-   return error(_("HEAD does not point to a branch"));
+   die(_("HEAD does not point to a branch"));
if (!upstream->merge || !upstream->merge[0]->dst) {
if (!ref_exists(upstream->refname))
-   return error(_("No such branch: '%s'"), cp);
-   if (!upstream->merge)
-   return error(_("No upstream configured for branch 
'%s'"),
-upstream->name);
-   return error(
+   die(_("No such branch: '%s'"), cp);
+   if (!upstream->merge) {
+   die(_("No upstream configured for branch '%s'"),
+   upstream->name);
+   }
+   die(
_("Upstream branch '%s' not stored as a remote-tracking 
branch"),
upstream->merge[0]->src);
}
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index b27a720..2a19e79 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -129,8 +129,7 @@ test_expect_success 'branch@{u} works when tracking a local 
branch' '
 
 test_expect_success 'branch@{u} error message when no upstream' '
cat >expect <<-EOF &&
-   error: No upstream configured for branch ${sq}non-tracking${sq}
-   fatal: Needed a single revision
+   fatal: No upstream configured for branch ${sq}non-tracking${sq}
EOF
error_message non-tracking@{u} 2>actual &&
test_i18ncmp expect actual
@@ -138,8 +137,7 @@ test_expect_success 'branch@{u} error message when no 
upstream' '
 
 test_expect_success '@{u} error message when no upstream' '
cat >expect <<-EOF &&
-   error: No upstream configured for branch ${sq}master${sq}
-   fatal: Needed a single revision
+   fatal: No upstream configured for branch ${sq}master${sq}
EOF
test_must_fail git rev-parse --verify @{u} 2>actual &&
test_i18ncmp expect actual
@@ -147,8 +145,7 @@ test_expect_success '@{u} error message when no upstream' '
 
 test_expect_success 'branch@{u} error message with misspelt branch' '
cat >expect <<-EOF &&
-   error: No such branch: ${sq}no-such-branch${sq}
-   fatal: Needed a single revision
+   fatal: No such branch: ${sq}no-such-branch${sq}
EOF
error_message no-such-branch@{u} 2>actual &&
test_i18ncmp expect actual
@@ -156,8 +153,7 @@ test_expect_success 'branch@{u} error message with misspelt 
branch' '
 
 test_expect_success '@{u} error message when not on a branch' '
cat >expect <<-EOF &&
-   error: HEAD does not point to a branch
-   fatal: Needed a single revision
+   fatal: HEAD does not point to a branch
EOF
git checkout HEAD^0 &&
test_must_fail git rev-parse --verify @{u} 2>actual &&
@@ -166,8

[PATCH 2/2] sha1_name: fix error message for @{}, @{}

2013-05-22 Thread Ramkumar Ramachandra
Currently, when we try to resolve @{} or @{} when the reflog
doesn't go back far enough, we get errors like:

  # on branch master
  $ git show @{1}
  fatal: Log for '' only has 7 entries.

  $ git show @{1.days.ago}
  warning: Log for '' only goes back to Tue, 21 May 2013 14:14:45 +0530.
  ...

  # detached HEAD case
  $ git show @{1}
  fatal: Log for '' only has 2005 entries.

  $ git show master@{1}
  fatal: Log for 'master' only has 7 entries.

The empty string '' is ugly, inconsistent, and failing to convey
information about whose logs we are inspecting.  Change this so that we
get:

  # on branch master
  $ git show @{1}
  fatal: Log for 'master' only has 7 entries.

  $ git show @{1.days.ago}
  warning: Log for 'master' only goes back to Tue, 21 May 2013 14:14:45 +0530.
  ...

  # detached HEAD case
  $ git show @{1}
  fatal: Log for 'HEAD' only has 2005 entries.

  $ git show master@{1}
  fatal: Log for 'master' only has 7 entries.

Simple, consistent, and informative; suitable for output even from
plumbing commands like rev-parse.

Signed-off-by: Ramkumar Ramachandra 
---
 sha1_name.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/sha1_name.c b/sha1_name.c
index 416a673..b7e008a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -517,6 +517,16 @@ static int get_sha1_basic(const char *str, int len, 
unsigned char *sha1)
}
if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
&co_time, &co_tz, &co_cnt)) {
+   if (!len) {
+   if (!prefixcmp(real_ref, "refs/heads/")) {
+   str = real_ref + 11;
+   len = strlen(real_ref + 11);
+   } else {
+   /* detached HEAD */
+   str = "HEAD";
+   len = 4;
+   }
+   }
if (at_time)
warning("Log for '%.*s' only goes "
"back to %s.", len, str,
-- 
1.8.3.rc3.10.g6f8d616

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


[PATCH v2 0/2] Fix invalid revision error messages

2013-05-22 Thread Ramkumar Ramachandra
As Junio pointed out in [0/2], this is not for 1.8.3; it's just a
regular "enhacement".

In [1/2], I've extended the commit message with the justification I
wrote out for Junio.

In [2/2], I've made sure to print the "correct" error message
everytime: I missed the detached HEAD case last time.  I'm not in
favor of anything "prettier", as I already explained in my email.

Thanks.

Ramkumar Ramachandra (2):
  sha1_name: fix error message for @{u}
  sha1_name: fix error message for @{}, @{}

 sha1_name.c   | 23 +--
 t/t1507-rev-parse-upstream.sh | 15 +--
 2 files changed, 22 insertions(+), 16 deletions(-)

-- 
1.8.3.rc3.10.g6f8d616

--
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] Document push --no-verify

2013-05-22 Thread Thomas Rast
ec9 (push: Add support for pre-push hooks, 2013-01-13) forgot to
add a note to git-push(1) about the new --no-verify option.

Signed-off-by: Thomas Rast 
---

The insertion spot is at the end, because the existing ordering is
indistinguishable from random.  This should also be fixed, but is a
much bigger change and probably not suitable for an -rc period.


 Documentation/git-push.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index d514813..426b3d2 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] 
[--receive-pack=]
   [--repo=] [-f | --force] [--prune] [-v | --verbose] [-u 
| --set-upstream]
-  [ [...]]
+  [--no-verify] [ [...]]
 
 DESCRIPTION
 ---
@@ -195,6 +195,9 @@ useful if you write an alias or script around 'git push'.
be pushed. If on-demand was not able to push all necessary
revisions it will also be aborted and exit with non-zero status.
 
+--no-verify::
+   Bypass the pre-push hook (see linkgit:githooks[5]).
+
 
 include::urls-remotes.txt[]
 
-- 
1.8.3.rc3.490.g2f233b5

--
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-submodule nested subrepo bug (Segmentation fault)

2013-05-22 Thread John Keeping
I'm guessing Kirill meant to send this to the list and not just to me.

It looks to me like the segfault is in MSys's mkdir.exe and not a Git
process.

- Forwarded message from Kirill Berezin  -

From: Kirill Berezin 
To: John Keeping 
Date: Wed, 22 May 2013 09:54:47 +0400
Message-ID: 
Subject: Re: git-submodule nested subrepo bug (Segmentation fault)

Ok, version is: 1.8.1.msysgit.1
Segmentation fault at the git clone --recursive 
http://github.com/Exsul/al_server
0 [main] mkdir 6596 open_stackdumpfile: Dumping stack trace to 
mkdir.exe.stackdump
C:\Users\\libexec\git-core\git-submodule: line 181: 6596
Segmentation fault (core dumped) mkdir -p "$ditdir_base"
fatal: Could not switch to 's:/USER//al_server/.git/modules/': No such file 
or directory
Clone of 'https://.../Exsul/chat.git' into submodule path 'chat' failed

PS so much mails per day...

2013/5/20 Kirill Berezin :
> This is standart github shell(Windows 7). Right now i cant access my
> home platform, will tell you tommorow.
>
> 2013/5/20 John Keeping :
>> On Mon, May 20, 2013 at 09:32:21AM +0400, Kirill Berezin wrote:
>>> When you trying to add submodule, that already has submodule, it craches.
>>> For example you could try: git clone --recursive
>>> http://github.com/Exsul/al_server
>>
>> Which version of Git were you using?  I was not able to reproduce this
>> with 1.8.3-rc3.



- End forwarded message -
--
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 v13 02/15] path.c: refactor relative_path(), not only strip prefix

2013-05-22 Thread Michael Haggerty
Sorry for coming late to the party.

On 05/22/2013 03:40 AM, Jiang Xin wrote:
> Original design of relative_path() is simple, just strip the prefix
> (*base) from the absolute path (*abs). In most cases, we need a real
> relative path, such as: ../foo, ../../bar. That's why there is another
> reimplementation (path_relative()) in quote.c.
> 
> Refactor relative_path() in path.c to return real relative path, so
> that user can reuse this function without reimplement his/her own.
> I will use this method for interactive git-clean later. Some of the
> implementations are borrowed from path_relative() in quote.c.
> 
> Different results for relative_path() before and after this refactor:
> 
> abs path  base path  relative (original)  relative (refactor)
>   =  ===  ===
> /a/b/c/   /a/b   c/   c/
> /a/b//c/  //a///b/   c/   c/
> /a/b  /a/b   ../
> /a/b/ /a/b   ../
> /a/a/b/  /a   ../
> / /a/b/  /../../
> /a/c  /a/b/  /a/c ../c
> /a/b  (empty)/a/b /a/b
> /a/b  (null) /a/b /a/b
> (empty)   /a/b   (empty)  ./
> (null)(empty)(null)   ./
> (null)/a/b   (segfault)   ./

The old and new versions both seem to be (differently) inconsistent
about when the output has a trailing slash.  What is the rule?

> diff --git a/path.c b/path.c
> index 04ff..0174d 100644
> --- a/path.c
> +++ b/path.c
> @@ -441,42 +441,104 @@ int adjust_shared_perm(const char *path)
>   return 0;
>  }
>  
> -const char *relative_path(const char *abs, const char *base)
> +/*
> + * Give path as relative to prefix.
> + *
> + * The strbuf may or may not be used, so do not assume it contains the
> + * returned path.
> + */
> +const char *relative_path(const char *abs, const char *base,
> +   struct strbuf *sb)

Thanks for adding documentation.  But I think it could be improved:

* The comment refers to "path" and "prefix" but the function parameters
are "abs" and "base".  I suggest making them agree.

* Who owns the memory pointed to by the return value?

* The comment says that "the strbuf may or may not be used".  So why is
it part of the interface?  (I suppose it is because the strbuf might be
given ownership of the returned memory if it has to be allocated.)
Would it be more straightforward to *always* return the result in the
strbuf?

* Please document when the return value contains a trailing slash and
also that superfluous internal slashes are (apparently) normalized away.

* Leading double-slashes have a special meaning on some operating
systems.  The old and new versions of this function both seem to ignore
differences between initial slashes.  Perhaps somebody who knows the
rules better could say whether this is an issue but I guess the problem
would rarely be encountered in practice.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] prompt: fix show upstream with svn and zsh

2013-05-22 Thread Felipe Contreras
On Wed, May 22, 2013 at 2:40 AM, Thomas Gummerer  wrote:
> Currently the __git_ps1 git prompt gives the following error with a
> repository converted by git-svn, when used with zsh:
>
>__git_ps1_show_upstream:19: bad pattern: svn_remote[
>__git_ps1_show_upstream:45: bad substitution
>
> To reproduce the problem, the __git_ps1_show_upstream function can be
> executed in a repository converted with git-svn.  Both those errors are
> triggered by spaces after the '['.
>
> Zsh also doesn't support initializing an array with `local var=(...)`.
> This triggers the following error:
>
>__git_ps1_show_upstream:41: bad pattern: svn_upstream=(commit
>
> Use
>local -a
>var=(...)
> instead to make is compatible.
>
> This was introduced by 6d158cba (bash completion: Support "divergence
> from upstream" messages in __git_ps1), when the script was for bash
> only.
>
> Signed-off-by: Thomas Gummerer 

I think there's no need to be so verbose in the commit message, but
hey, that's just an opinion.

Acknowledged-by: Felipe Contreras 

-- 
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 (May 2013, #05; Mon, 20)

2013-05-22 Thread Johan Herland
On Tue, May 21, 2013 at 5:35 PM, Junio C Hamano  wrote:
> Johan Herland  writes:
>> On Tue, May 21, 2013 at 2:15 AM, Junio C Hamano  wrote:
>>> * jh/shorten-refname (2013-05-07) 4 commits
>>>  - t1514: refname shortening is done after dereferencing symbolic refs
>>>  - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to 
>>> origin
>>>  - t1514: Demonstrate failure to correctly shorten 
>>> "refs/remotes/origin/HEAD"
>>>  - t1514: Add tests of shortening refnames in strict/loose mode
>>>
>>>  When remotes/origin/HEAD is not a symbolic ref, "rev-parse
>>>  --abbrev-ref remotes/origin/HEAD" ought to show "origin", not
>>>  "origin/HEAD", which is fixed with this series (if it is a symbolic
>>>  ref that points at remotes/origin/something, then it should show
>>>  "origin/something" and it already does).
>>>
>>>  I think this is being rerolled using strbuf_expand().
>>
>> Actually, that was not on my TODO. Sure, my other series ([PATCHv2
>> 00/10] Prepare for alternative remote-tracking branch location) builds
>> on top of this one, and changes a lot of the same code, but I
>> considered jh/shorten-refname a good set of changes in its own right,
>> and I didn't want it to be held up by the much longer (and probably
>> much longer-running) series.
>
> On the other hand, this itself is fixing the case nobody encounters
> in real life, and in that sense it is not urgent at all even though
> it may be a correct fix, no?  When was the last time you saw a
> refs/remotes/*/HEAD that is not a symbolic ref?
>
> If it makes it is easier for you to work on the follow-on series to
> have this shorter one already cast in stone, I do not mind merging
> this early post 1.8.3 cycle at all.  If on the other hand you are
> hit by a realization that this part could be done in a different and
> a better way (I am not saying that that is the likely outcome) when
> you will look at redoing the follow-on series using strbuf_expand
> post 1.8.3, we would regret it if we cast this part in stone too
> early.  I think we can go either way, and the above "I think this is
> being rerolld" was primarily keeping the options open.

You're right. No point in setting things prematurely in stone. I'll
fold jh/shorten-refname into the ongoing series.


...Johan

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


[PATCH v2] prompt: fix show upstream with svn and zsh

2013-05-22 Thread Thomas Gummerer
Currently the __git_ps1 git prompt gives the following error with a
repository converted by git-svn, when used with zsh:

   __git_ps1_show_upstream:19: bad pattern: svn_remote[
   __git_ps1_show_upstream:45: bad substitution

To reproduce the problem, the __git_ps1_show_upstream function can be
executed in a repository converted with git-svn.  Both those errors are
triggered by spaces after the '['.

Zsh also doesn't support initializing an array with `local var=(...)`.
This triggers the following error:

   __git_ps1_show_upstream:41: bad pattern: svn_upstream=(commit

Use
   local -a
   var=(...)
instead to make is compatible.

This was introduced by 6d158cba (bash completion: Support "divergence
from upstream" messages in __git_ps1), when the script was for bash
only.

Signed-off-by: Thomas Gummerer 
---
 contrib/completion/git-prompt.sh | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index eaf5c36..b6b1534 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -124,7 +124,7 @@ __git_ps1_show_upstream ()
fi
;;
svn-remote.*.url)
-   svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
+   svn_remote[$((${#svn_remote[@]} + 1))]="$value"
svn_url_pattern+="\\|$value"
upstream=svn+git # default upstream is SVN if 
available, else git
;;
@@ -146,10 +146,11 @@ __git_ps1_show_upstream ()
svn*)
# get the upstream from the "git-svn-id: ..." in a commit 
message
# (git-svn uses essentially the same procedure internally)
-   local svn_upstream=($(git log --first-parent -1 \
+   local -a svn_upstream
+   svn_upstream=($(git log --first-parent -1 \
--grep="^git-svn-id: 
\(${svn_url_pattern#??}\)" 2>/dev/null))
if [[ 0 -ne ${#svn_upstream[@]} ]]; then
-   svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]}
+   svn_upstream=${svn_upstream[${#svn_upstream[@]} - 2]}
svn_upstream=${svn_upstream%@*}
local n_stop="${#svn_remote[@]}"
for ((n=1; n <= n_stop; n++)); do
-- 
1.8.3.rc2.359.g2fb82f5

--
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 (May 2013, #05; Mon, 20)

2013-05-22 Thread Michael J Gruber
BTW, I love our rev-list machinery:

log --graph --abbrev-commit --pretty=oneline --decorate --cherry-mark
--left-right mjg/grep-textconv...origin/next

>   701cdb7 Merge branch 'mg/more-textconv' into next
|\
| = afa15f3 (gitster/mg/more-textconv) grep: honor --textconv for the
case rev:path
| = 335ec3b grep: allow to use textconv filters
| = 97f6a9c t7008: demonstrate behavior of grep with textconv
| = 3ac2161 cat-file: do not die on --textconv without textconv filters
| = 083b993 show: honor --textconv for blobs
| = 6c37400 diff_opt: track whether flags have been set explicitly
| = 4bd52d0 t4030: demonstrate behavior of show with textconv

:)

So, at least there were no bits missing that I would have missed to send
out. I'll recheck the pertaining thread.

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


Re: [PATCH] prompt: fix show upstream with svn and zsh

2013-05-22 Thread Thomas Gummerer
Felipe Contreras  writes:

> On Tue, May 21, 2013 at 3:54 PM, Thomas Gummerer  wrote:
>> Currently the __git_ps1 git prompt gives the following error with a
>> repository converted by git-svn, when used with zsh:
>>
>>__git_ps1_show_upstream:19: bad pattern: svn_remote[
>>
>> This was introduced by 6d158cba (bash completion: Support "divergence
>> from upstream" messages in __git_ps1), when the script was for bash
>> only.  Make it compatible with zsh.
>>
>> Signed-off-by: Thomas Gummerer 
>
> This patch is fine by me. I would like to see an example of how to
> trigger the issue with a standalone command in the commit message, but
> it's not necessary. It would also make sense to address the comment
> from Szeder that does raise questions about other places in the code
> where 'array[ $foo ]' is used, maybe there's a caveat we are not
> considering, or maybe your use-case did not execute that code.

Yes, the code was not executed, it will be fixed in the re-roll.

> And finally, I don't recall seen 'set -a' used elsewhere in the code.
> If memory serves well, we have replaced 'local -a foo=value' with
> 'local -a foo\nfoo=value' before to fix zsh issues, I think that would
> be safer.

Yes, thanks for the suggestion, that works.

> But this patch is needed regardless, that, or the patch that broke
> things should be reverted for v1.8.3.

I don't think anything should or can be reverted, as this code was
introduced in 2010, but nobody triggered it until now.

> Thomas, if you don't have time, let me know and I can take a look.
>
> Cheers.
>
> --
> Felipe Contreras

Thanks,
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] prompt: fix show upstream with svn and zsh

2013-05-22 Thread Thomas Gummerer
SZEDER Gábor  writes:

> Hi,
>
>
> On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
>> Currently the __git_ps1 git prompt gives the following error with a
>> repository converted by git-svn, when used with zsh:
>>
>> __git_ps1_show_upstream:19: bad pattern: svn_remote[
>>
>> This was introduced by 6d158cba (bash completion: Support "divergence
>> from upstream" messages in __git_ps1), when the script was for bash
>> only.  Make it compatible with zsh.
>
> What is the actual cause of this problem/incompatibility and how/why do
> these changes fix it?
>
>> -svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
>> +svn_remote[$((${#svn_remote[@]} + 1))]="$value"
>
> I mean, did zsh really complained because of the space after the '[' ?!

Yes, removing the spaces after the '[' fixes the problem.  I'm not very
proficient in shell scripting, so I can't tell if there is another
cause.

>> @@ -146,8 +146,8 @@ __git_ps1_show_upstream ()
>>  svn*)
>>  # get the upstream from the "git-svn-id: ..." in a commit 
>> message
>>  # (git-svn uses essentially the same procedure internally)
>> -local svn_upstream=($(git log --first-parent -1 \
>> ---grep="^git-svn-id: 
>> \(${svn_url_pattern#??}\)" 2>/dev/null))
>> +set -a svn_upstream "$(git log --first-parent -1 \
>> +--grep="^git-svn-id: 
>> \(${svn_url_pattern#??}\)" 2>/dev/null)"
>>  if [[ 0 -ne ${#svn_upstream[@]} ]]; then
>>  svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]}
>
> If so, then what about this one?

You're right, this line gives an error too, the code just wasn't
following that path before.  I'll fix it in the re-roll.  Other than
those two I couldn't spot any other occurrence of this pattern.

> Best,
> Gábor

Thanks,
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: What's cooking in git.git (May 2013, #05; Mon, 20)

2013-05-22 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 21.05.2013 02:15:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
> 
> The final version of 1.8.3 is expected to be tagged late this week.
> While applying a few regression hot-fix patches, a couple of benign
> doc updates have also been merged to 'master'.
> 
> 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
> 
> --
> [Graduated to "master"]
> 
> * dw/asciidoc-sources-are-dot-txt-files (2013-05-10) 1 commit
>  - CodingGuidelines: Documentation/*.txt are the sources
> 
> 
> * fc/doc-style (2013-05-09) 1 commit
>  - documentation: trivial style cleanups
> 
> --
> [New Topics]
> 
> * fc/remote-bzr (2013-05-16) 6 commits
>  - remote-bzr: trivial cleanups
>  - remote-bzr: change global repo
>  - remote-bzr: delay cloning/pulling
>  - remote-bzr: simplify get_remote_branch()
>  - remote-bzr: fix for files with spaces
>  - remote-bzr: recover from failed clones
> 
>  The ones near the tip conflicted with the hotfix for 1.8.3 so I
>  discarded them for now.
> 
> 
> * jx/clean-interactive (2013-05-20) 15 commits
>  - test: add t7301 for git-clean--interactive
>  - git-clean: add documentation for interactive git-clean
>  - git-clean: add ask each interactive action
>  - git-clean: add select by numbers interactive action
>  - git-clean: add filter by pattern interactive action
>  - git-clean: use a git-add-interactive compatible UI
>  - git-clean: add colors to interactive git-clean
>  - git-clean: show items of del_list in columns
>  - git-clean: add support for -i/--interactive
>  - git-clean: refactor git-clean into two phases
>  - Refactor write_name_quoted_relative, remove unused params
>  - Refactor quote_path_relative, remove unused params
>  - quote.c: remove path_relative, use relative_path instead
>  - path.c: refactor relative_path(), not only strip prefix
>  - test: add test cases for relative_path
> 
> 
> * tr/test-v-and-v-subtest-only (2013-05-16) 6 commits
>  - test-lib: support running tests under valgrind in parallel
>  - test-lib: allow prefixing a custom string before "ok N" etc.
>  - test-lib: valgrind for only tests matching a pattern
>  - test-lib: verbose mode for only tests matching a pattern
>  - test-lib: refactor $GIT_SKIP_TESTS matching
>  - test-lib: enable MALLOC_* for the actual tests
> 
>  Allows N instances of tests run in parallel, each running 1/N parts
>  of the test suite under Valgrind, to speed things up.
> 
>  The tip one may be useful in practice but is a tad ugly ;-)
>  
> 
> * rh/merge-options-doc-fix (2013-05-16) 1 commit
>  - Documentation/merge-options.txt: restore `-e` option
> 
>  Even though it is not all that urgent, this can be merged to
>  'master' before the final,
> 
> 
> * rr/zsh-color-prompt (2013-05-17) 3 commits
>  - prompt: colorize ZSH prompt
>  - prompt: factor out gitstring coloring logic
>  - prompt: introduce GIT_PS1_STATESEPARATOR
> 
> 
> * an/diff-index-doc (2013-05-20) 1 commit
>  - Documentation/diff-index: mention two modes of operation
> 
> 
> * fc/contrib-related (2013-05-20) 1 commit
>  - Add new git-related helper to contrib
> 
> 
> * mc/describe-first-parent (2013-05-20) 1 commit
>  - describe: Add --first-parent option
> 
> 
> * rs/tar-tests (2013-05-20) 6 commits
>  - t5000: test long filenames
>  - t5000: simplify tar-tree tests
>  - t5000: use check_tar for prefix test
>  - t5000: factor out check_tar
>  - t5000, t5003: create directories for extracted files lazily
>  - t5000: integrate export-subst tests into regular tests
> 
> --
> [Stalled]
> 
> * mh/multimail (2013-04-21) 1 commit
>  - git-multimail: a replacement for post-receive-email
> 
>  Waiting for the initial history to pull from.
>  $gmane/223564
> 
> 
> * jc/format-patch (2013-04-22) 2 commits
>  - format-patch: --inline-single
>  - format-patch: rename "no_inline" field
> 
>  A new option to send a single patch to the standard output to be
>  appended at the bottom of a message.  I personally have no need for
>  this, but it was easy enough to cobble together.  Tests, docs and
>  stripping out more MIMEy stuff are left as exercises to interested
>  parties.
> 
>  Not ready for inclusion.
> 
> 
> * jk/gitweb-utf8 (2013-04-08) 4 commits
>  - gitweb: Fix broken blob action parameters on blob/commitdiff pages
>  - gitweb: Don't append ';js=(0|1)' to external links
>  - gitweb: Make feed title valid utf8
>  - gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, 
> commitdiff_plain, and patch
> 
>  Various fixes to gitweb.
> 
>  Waiting for a reroll after a review.
> 
> 
> * jk/commit-info-slab (2013-04-19) 3 commits
>  - commit-slab: introduce a macro to define a slab