Git development
 help / color / mirror / Atom feed
* [PATCH] rebase-i-p: squashing and limiting todo
From: Stephen Haberman @ 2008-10-15  7:44 UTC (permalink / raw)
  To: gitster; +Cc: git, Stephen Haberman

This is v3, rebased on top of sp/maint with sp/master and sh/maint-rebase3.

So, it should apply cleanly to that. Junio's feedback aside, the only change
from v2 is that maint-rebase3's dropped commit check needs to be done before
Johannes's recent addition of:

    test -s "$TODO" || echo noop >> "$TODO"

Because now the todo may temporarily have picks in it while probing for parents
that could later be removed by the dropped/cherry-picked commit check.

Previously todo never had temporary entries, so it didn't matter when the
dropped commit check was done.

(Apologies for not seeing Junio's what's cooking first and using his
maint/master instead of Shawn's. Let me know if I need to redo this and I will
get even more practice at rebasing.)

(Gah, resending with the list cc'd this time. Dammit, sorry about that.)

Thanks,
Stephen

Stephen Haberman (7):
  rebase-i-p: test to exclude commits from todo based on its parents
  rebase-i-p: use HEAD for updating the ref instead of mapping OLDHEAD
  rebase-i-p: delay saving current-commit to REWRITTEN if squashing
  rebase-i-p: fix 'no squashing merges' tripping up non-merges
  rebase-i-p: only list commits that require rewriting in todo
  rebase-i-p: do not include non-first-parent commits touching UPSTREAM
  rebase-i-p: if todo was reordered use HEAD as the rewritten parent

 git-rebase--interactive.sh               |  131 ++++++++++++++++++-----------
 t/t3411-rebase-preserve-around-merges.sh |  136 ++++++++++++++++++++++++++++++
 2 files changed, 218 insertions(+), 49 deletions(-)
 create mode 100644 t/t3411-rebase-preserve-around-merges.sh

^ permalink raw reply

* Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL  interfere with the test
From: Johannes Sixt @ 2008-10-15  7:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matt McCutchen, git
In-Reply-To: <7vmyh64bgy.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> Junio C Hamano schrieb:
>> ...
>>>> +# Remove a default ACL from the test dir if possible.
>>>> +setfacl -k . 2>/dev/null
>>>> +
>>> Makes me wonder why this is _not_ inside test-lib.sh where it creates the
>>> test (trash) directory.  That way, you would cover future tests that wants
>>> to see a saner/simpler POSIX permission behaviour, wouldn't you?
>> But that would also paper over unanticipated bad interactions with strange
>> ACLs that people might set, wouldn't it? By not placing this into
>> test-lib.sh there is a higher chance that such an interaction is revealed,
>> and we can react on it (educate users or fix the code).
> 
> What do you exactly mean by "educate users or fix the code"?  For example,
> by not putting this setfacl in test-lib.sh, t1301 revealed that with a
> default ACL higher up, "git init --shared" would not work as expected.
> 
> Then what?
> 
>  - Do you mean, by "educate users", that we teach users not to play fun
>    games with ACL in a git controled working tree?

Correct. In the case of a shared repository we can educate users not to
play with ACLs.

>  - Do you mean, by "fix the code", that we teach adjust_shared_perm() to
>    deal with ACL?

Correct in principle, but we need not go this route in the case of shared
repositories because we better educate users.

-- Hannes

^ permalink raw reply

* [PATCH 2/2] git-check-attr(1): use 'verse' for multi-line synopsis sections
From: Jonas Fonseca @ 2008-10-15  7:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabd6686n.fsf@gitster.siamese.dyndns.org>

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 Documentation/git-check-attr.txt |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 59d5cd2..dcf7650 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -8,8 +8,9 @@ git-check-attr - Display gitattributes information.
 
 SYNOPSIS
 --------
+[verse]
 'git check-attr' attr... [--] pathname...
-'git check-attr' --stdin [-z] attr... < <list-of-paths
+'git check-attr' --stdin [-z] attr... < <list-of-paths>
 
 DESCRIPTION
 -----------
-- 
tg: (34458dd..) jf/man-git-check-attr-synopsis (depends on: jf/man-git-check-attr-output next)

-- 
Jonas Fonseca

^ permalink raw reply related

* [PATCH 1/2] git-check-attr(1): add output and example sections
From: Jonas Fonseca @ 2008-10-15  7:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabd6686n.fsf@gitster.siamese.dyndns.org>

Plumbing tools should document what output can be expected.

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 Documentation/git-check-attr.txt |   50 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

 Junio C Hamano <gitster@pobox.com> wrote Tue, Oct 14, 2008:
 > Should this really have to depend on next?

 This is just topgit listing the patch dependencies.

 > Could you split this into two patches, one for 'maint' to describe the
 > output format, and another for 'next' (or 'dp/checkattr') to update the
 > SYNOPSIS part?

 Done. The example has been updated to also show '!' in action.

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 2b821f2..8460a87 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -22,6 +22,56 @@ OPTIONS
 	arguments as path names. If not supplied, only the first argument will
 	be treated as an attribute.
 
+OUTPUT
+------
+
+The output is of the form:
+<path> COLON SP <attribute> COLON SP <info> LF
+
+Where <path> is the path of a file being queried, <attribute> is an attribute
+being queried and <info> can be either:
+
+'unspecified';; when the attribute is not defined for the path.
+'unset';;	when the attribute is defined to false.
+'set';;		when the attribute is defined to true.
+<value>;;	when a value has been assigned to the attribute.
+
+EXAMPLES
+--------
+
+In the examples, the following '.gitattributes' file is used:
+---------------
+*.java diff=java -crlf myAttr
+NoMyAttr.java !myAttr
+README caveat=unspecified
+---------------
+
+* Listing a single attribute:
+---------------
+$ git check-attr diff org/example/MyClass.java
+org/example/MyClass.java: diff: java
+---------------
+
+* Listing multiple attributes for a file:
+---------------
+$ git check-attr crlf diff myAttr -- org/example/MyClass.java
+org/example/MyClass.java: crlf: unset
+org/example/MyClass.java: diff: java
+org/example/MyClass.java: myAttr: set
+---------------
+
+* Listing attribute for multiple files:
+---------------
+$ git check-attr myAttr -- org/example/MyClass.java org/example/NoMyAttr.java
+org/example/MyClass.java: myAttr: set
+org/example/NoMyAttr.java: myAttr: unspecified
+---------------
+
+* Not all values are equally unambiguous:
+---------------
+$ git check-attr caveat README
+README: caveat: unspecified
+---------------
 
 SEE ALSO
 --------
-- 
tg: (6c16792..) jf/man-git-check-attr-output (depends on: maint)

-- 
Jonas Fonseca

^ permalink raw reply related

* Re: What's cooking in git.git (Oct 2008, #03; Tue, 14)
From: Stephen Haberman @ 2008-10-15  7:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wsq7rt5.fsf@gitster.siamese.dyndns.org>


> * sh/maint-rebase3 (Sun Oct 5 23:26:52 2008 -0500) 1 commit
>  . rebase--interactive: fix parent rewriting for dropped commits
> 
> Tentatively dropped from everwhere, waiting for a reroll together with
> sh/rebase-i-p.

Assuming sh/maint-rebase3 goes before sh/rebase-i-p, there are no
changes to it. I rebased it on top of sp/maint and the tests passed.

> * sh/rebase-i-p (Wed Oct 8 01:41:57 2008 -0500) 7 commits
>  - rebase-i-p: if todo was reordered use HEAD as the rewritten parent
>  - rebase-i-p: do not include non-first-parent commits touching
>    UPSTREAM
>  - rebase-i-p: only list commits that require rewriting in todo
>  - rebase-i-p: fix 'no squashing merges' tripping up non-merges
>  - rebase-i-p: delay saving current-commit to REWRITTEN if squashing
>  - rebase-i-p: use HEAD for updating the ref instead of mapping
>    OLDHEAD
>  - rebase-i-p: test to exclude commits from todo based on its parents
> 
> Changes the `rebase -i -p` behavior to behave like git sequencer's
> rewrite of `rebase -i` would behave.

Attempting to follow your previous advice, I merged sp/master onto my
local sh/maint-rebase3, and rebased sh/rebase-i-p on top that resulting
merge, and got the changes fixed to play nicely with sh/maint-rebase3
+master, and integrated your feedback.

I'll be sending the new sh/rebase-i-p series soon.

Let me know if I did the wrong thing--your comment of maint-rebase3
"waiting for a reroll" makes me think you wanted that one rebased on
top of rebase-i-p, when I just got done doing the opposite (I hadn't
checked the git list before hacking on it).

Thanks,
Stephen

^ permalink raw reply

* Re: [PATCH 3/3] git pull: Support --preserve-merges as a flag to rebase
From: Stephen Haberman @ 2008-10-15  6:59 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <48D9586C.4010102@op5.se>


> Now that "git rebase" supports non-interactive rebases
> preserving merges, this patch is the next logical step
> for those who wish to use such a workflow.
> 
> Since this patch makes the last test marked as expecting
> failure in t3409-rebase-preserve-merges, we now alter it
> to expect success.

Does anyone know the current status of this?

The first 1/3 and 2/3 of this series is in next now (which grant
non-interactive merge-preserving rebasing), but this additional
--preserve-merges flag to git pull didn't seem to make it.

(Correct me if I'm wrong, I cannot pin point exactly where 1/2
and 2/3 got merged in, I'm just cheating and looking at the files
as they exist in next's tip.)

My grandeur plan is to have this 3/3 go in as well and then follow it
up with my patch to add a branch.name.preservemerges config variable.

This will make our internal workflow of "always rebase local
changes/always preserve local merges" just work with "git pull".

Does this seem reasonable?

Thanks,
Stephen

^ permalink raw reply

* Re: The behaviour of git bisect skip
From: Christian Couder @ 2008-10-15  6:54 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Git Mailing List
In-Reply-To: <48F3DCEB.1060803@zytor.com>

Le mardi 14 octobre 2008, H. Peter Anvin a écrit :
> I recently had the unhappy experience of trying to bisect a tree with a
> large region of the history obscured by auxilliary bugs.  "git bisect
> skip" will stay in the central region, thus being largely useless.

Yeah, it tries to find the first non skipped commit among the best possible 
bisection points. And if you have a linear history the best bisection 
points are in the middle of the good and bad commits.

> I was thinking about how to possibly do it better.  This is something I
> came up with, and thought it might be interesing.
>
> a. we obviously cannot move the start and end (good and bad) markers,
> since they have not been shown one way or the other.
>
> b. however, the practice of testing the centermost point is merely the
> *optimal*, corresponding to 1 bit of information per iteration.  An
> off-center test is also possible (as long as the value depends on both
> endpoints, and isn't fixed from one of the endpoints; in that case we
> have a linear search), corresponding to a smaller amount of information
> - for example, sampling at the one-quarter point corresponds to
> 3/4*log2(3/4) + 1/4*log2(1/4) =~ 0.811 bits of information.
>
> I would suggest something based on the following algorithm:
>
> Given an interval with a certain number of skip points, subdivide the
> interval into subintervals each separated by a skip point.  Pick the
> centermost point of the *largest* of these intervals.  If there is more
> than one largest interval, choose the one centermost point that ends up
> being centermost in the overall interval.
>
> This algorithm obviously needs some adjustment (as does plain binary
> search) in order to deal with a branched history, but I thought it might
> be an interesting starting point.  It has the desirable property that it
> can make forward progress even in the presence of skip points, and that
> it avoids needlessly searching close to skip points.

It looks like a great alternative algorithm but as the current bisect skip 
is implemented in shell, I think that it might be too difficult or 
inefficient to implement something like that in shell.

I think it might be better to try a more simple alternative algorithm first 
like removing all skipped commits from the sorted list of possible 
bisection point (the list is sorted because best commits from a bisection 
point are first) and trying the commit at one third (or another fixed 
ration) of the best commit.

Regards,
Christian.
 

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

^ permalink raw reply

* Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL  interfere with the test
From: Junio C Hamano @ 2008-10-15  6:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Matt McCutchen, git
In-Reply-To: <48F589EC.6050307@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
> ...
>>> +# Remove a default ACL from the test dir if possible.
>>> +setfacl -k . 2>/dev/null
>>> +
>> 
>> Makes me wonder why this is _not_ inside test-lib.sh where it creates the
>> test (trash) directory.  That way, you would cover future tests that wants
>> to see a saner/simpler POSIX permission behaviour, wouldn't you?
>
> But that would also paper over unanticipated bad interactions with strange
> ACLs that people might set, wouldn't it? By not placing this into
> test-lib.sh there is a higher chance that such an interaction is revealed,
> and we can react on it (educate users or fix the code).

What do you exactly mean by "educate users or fix the code"?  For example,
by not putting this setfacl in test-lib.sh, t1301 revealed that with a
default ACL higher up, "git init --shared" would not work as expected.

Then what?

 - Do you mean, by "educate users", that we teach users not to play fun
   games with ACL in a git controled working tree?

 - Do you mean, by "fix the code", that we teach adjust_shared_perm() to
   deal with ACL?

^ permalink raw reply

* Re: builtin conversion between tabs and spaces
From: Johannes Sixt @ 2008-10-15  6:26 UTC (permalink / raw)
  To: Stefan Karpinski; +Cc: Git Mailing List
In-Reply-To: <d4bc1a2a0810141844x76223e76xf04e07ece834fc61@mail.gmail.com>

Stefan Karpinski schrieb:
> This led me to wonder if it wouldn't make sense to have this
> conversion ability built into git.

This wouldn't help your case a lot. It is still at the discretion of each
individual repository owner to enable the conversion. (You didn't mean to
make this conversion mandatory, did you?)

BTW, you don't need to change git code to achieve this. It's sufficient to
install a suitable "clean" filter:

echo "*.c filter=c-code" > .git/info/attributes
git config filter.c-code.clean tabs2spaces

where tabs2spaces is your utility that does the conversion.

-- Hannes

^ permalink raw reply

* Re: builtin conversion between tabs and spaces
From: Alex Riesen @ 2008-10-15  6:25 UTC (permalink / raw)
  To: Stefan Karpinski; +Cc: Git Mailing List
In-Reply-To: <d4bc1a2a0810141844x76223e76xf04e07ece834fc61@mail.gmail.com>

Stefan Karpinski, Wed, Oct 15, 2008 03:44:10 +0200:
> I find myself really wanting to enforce standards in the use of spaces
> versus tabs. I deal with some unruly programmers who refuse to set
> their editors to use soft tabs, but I *hate* tabs in the repo. And of
> course others feel equally strongly about keeping only tabs in the
> repo (e.g. the git repo).
> 
> This led me to wonder if it wouldn't make sense to have this
> conversion ability built into git. The following patch implements this
> functionality. It still needs work—it's not meant to be final, just to
> give an idea—but I just wanted to see if people on the git list
> thought this sort of thing would be worthwhile at all.

Is your conversion two-way? IOW, is it possible to convert the
converted file and get the original? (Because all the existing
conversions are reversible)

^ permalink raw reply

* Re: [PATCH] Fix mkpath abuse in dwim_ref/sha1_name.c
From: Alex Riesen @ 2008-10-15  6:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Shawn O. Pearce
In-Reply-To: <7viqru6a1r.fsf@gitster.siamese.dyndns.org>

Junio C Hamano, Wed, Oct 15, 2008 01:17:52 +0200:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
> >
> > Otherwise the function sometimes fail to resolve obviously correct refnames,
> > because the string data pointed to by "ref" argument were reused.
> >
> But your patch instead rewrites the computation of str2 by bypassing the
> call to "another_function_that_uses_get_pathname()" and duplicating its
> logic, which I do not think is a viable approach in the longer term.

There is not enough logic to bypass there. It's just a dumb sprintf!
I actually saved a useless call (two, if automatic inlining
optimization doesn't work).

> > diff --git a/sha1_name.c b/sha1_name.c
> > index 41b6809..b5b53bf 100644
> > --- a/sha1_name.c
> > +++ b/sha1_name.c
> > @@ -242,6 +242,7 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
> >  {
> >  	const char **p, *r;
> >  	int refs_found = 0;
> > +	char fullref[PATH_MAX];
> >  
> >  	*ref = NULL;
> >  	for (p = ref_rev_parse_rules; *p; p++) {
> > @@ -249,7 +250,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
> >  		unsigned char *this_result;
> >  
> >  		this_result = refs_found ? sha1_from_ref : sha1;
> > -		r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
> > +		snprintf(fullref, sizeof(fullref), *p, len, str);
> > +		r = resolve_ref(fullref, this_result, 1, NULL);
> >  		if (r) {
> >  			if (!refs_found++)
> >  				*ref = xstrdup(r);
> 
> I suspect that I am grossly misleading the code, but I wonder why this xstrdup()
> is not protecting us from the reusing of "the string data pointed to by
> "ref" argument".  Are you fixing the overwriting of the string pointed to
> by "str" argument instead?

Yes, I mistyped. xstrdup's called after the breakage happened.

> What specific call chain has this breakage you found?

That's up to this function from builtin_rev_parse and resolve_ref
we're about to call.

^ permalink raw reply

* Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL  interfere with the test
From: Johannes Sixt @ 2008-10-15  6:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matt McCutchen, git
In-Reply-To: <7vzll66c5u.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> Matt McCutchen <matt@mattmccutchen.net> writes:
>> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
>> index dc85e8b..2275caa 100755
>> --- a/t/t1301-shared-repo.sh
>> +++ b/t/t1301-shared-repo.sh
>> @@ -7,6 +7,9 @@ test_description='Test shared repository initialization'
>>  
>>  . ./test-lib.sh
>>  
>> +# Remove a default ACL from the test dir if possible.
>> +setfacl -k . 2>/dev/null
>> +
> 
> Makes me wonder why this is _not_ inside test-lib.sh where it creates the
> test (trash) directory.  That way, you would cover future tests that wants
> to see a saner/simpler POSIX permission behaviour, wouldn't you?

But that would also paper over unanticipated bad interactions with strange
ACLs that people might set, wouldn't it? By not placing this into
test-lib.sh there is a higher chance that such an interaction is revealed,
and we can react on it (educate users or fix the code).

-- Hannes

^ permalink raw reply

* Updating Submodules
From: benhill @ 2008-10-15  5:51 UTC (permalink / raw)
  To: git


Heyah Folks.

So I have been playing with this for a few hours, and I must be missing
something.

I have created a submodule in my super project with the standard commands:

git submodule add <repo> <path>

No problem.  Repo Loads.  I push it to origin (github) and the little arrow
designating it as a sub is there.

Then one of my colleagues checks out the super and runs:

git submodule init
git submodule add

...no problems thus far...the repo loads as planned.

The she makes some changes to the submodule, checks them in, pushes them to
origin (also github).

Now, if she tried to check in changes to the super...it is no longer just
the submodule ref that she is checking in...but the whole tree....like it
adds all the files in the submodule to the super...if she were to push to
github, the little arrow would be gone..and the whole tree is there.  

What am I doing wrong here?  How do you make updates to supers to point to
the head revision of the sub?

Thanks for any help.

Ben
-- 
View this message in context: http://www.nabble.com/Updating-Submodules-tp19987153p19987153.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: tip tree clone fail
From: H. Peter Anvin @ 2008-10-15  5:25 UTC (permalink / raw)
  To: Wang Chen
  Cc: Ingo Molnar, Petr Baudis, Thomas Gleixner, FNST-Lai Jiangshan,
	FJ-KOSAKI Motohiro, git, Junio C Hamano
In-Reply-To: <48F566C4.2050905@cn.fujitsu.com>

Wang Chen wrote:
> 
> Yes. My clone is successful. Thanks, Peter.
> But, after cloning, git-pull failed because of conflict.
> I think maybe because Ingo rebased his tree?
> ---
> Auto-merged init/main.c
> CONFLICT (content): Merge conflict in init/main.c
> Auto-merged scripts/bootgraph.pl
> CONFLICT (add/add): Merge conflict in scripts/bootgraph.pl
> Removed sound/soc/at91/eti_b1_wm8731.c
> Automatic merge failed; fix conflicts and then commit the result.
> ---

Yes, the master branch on tip is not rebase-free.

	-hpa

^ permalink raw reply

* [PATCH] Fix reading of cloud tags
From: Junio C Hamano @ 2008-10-15  4:27 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Shawn O. Pearce
In-Reply-To: <20081014000812.GZ4856@spearce.org>

The projectroot path could have SP in it, in which case iterating over
<$git_dir/ctags/*> does not correctly enumerate the cloud tags files at
all.

This can be observed by creating an empty t/trash directory and running
t9500 test.  The $projectroot ends with "trash directory.t9500-gitweb-/"
and <$glob> would give "trash", which can be opened and reading from it
immediately yields undef, which in turn gives an undef value warning to
the standard error stream upon attempt to chomp it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 gitweb/gitweb.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git c/gitweb/gitweb.perl w/gitweb/gitweb.perl
index 1116800..577c041 100755
--- c/gitweb/gitweb.perl
+++ w/gitweb/gitweb.perl
@@ -1805,7 +1805,8 @@ sub git_get_project_ctags {
 	my $ctags = {};
 
 	$git_dir = "$projectroot/$path";
-	foreach (<$git_dir/ctags/*>) {
+	opendir D, "$git_dir/ctags";
+	foreach (grep { -f $_ } map { "$git_dir/ctags/$_" } readdir(D)) {
 		open CT, $_ or next;
 		my $val = <CT>;
 		chomp $val;
@@ -1813,6 +1814,7 @@ sub git_get_project_ctags {
 		my $ctag = $_; $ctag =~ s#.*/##;
 		$ctags->{$ctag} = $val;
 	}
+	closedir D;
 	$ctags;
 }
 

^ permalink raw reply related

* Re: tip tree clone fail
From: Wang Chen @ 2008-10-15  3:43 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ingo Molnar, Petr Baudis, Thomas Gleixner, FNST-Lai Jiangshan,
	FJ-KOSAKI Motohiro, git, Junio C Hamano
In-Reply-To: <48F37073.1030808@zytor.com>

H. Peter Anvin said the following on 2008-10-13 23:59:
> Wang Chen wrote:
>>
>> Ingo, thank you for your work.
>> I can clone more, but error still occurs:
>>
>> Getting alternates list for
>> http://www.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
>> Also look at
>> http://www.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
>> Also look at
>> http://www.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git/
>> Getting pack list for
>> http://www.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
>> error: transfer closed with 8280 bytes remaining to read
>> Getting pack list for
>> http://www.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
>> Getting pack list for
>> http://www.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git/
>> error: Unable to find 95630fe2917f805a26f8d8beaafb80cd2f729eb5 under
>> http://www.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
>> Cannot obtain needed object 95630fe2917f805a26f8d8beaafb80cd2f729eb5
> 
> I cleaned it up and it should work now.
> 

Yes. My clone is successful. Thanks, Peter.
But, after cloning, git-pull failed because of conflict.
I think maybe because Ingo rebased his tree?
---
Auto-merged init/main.c
CONFLICT (content): Merge conflict in init/main.c
Auto-merged scripts/bootgraph.pl
CONFLICT (add/add): Merge conflict in scripts/bootgraph.pl
Removed sound/soc/at91/eti_b1_wm8731.c
Automatic merge failed; fix conflicts and then commit the result.
---

^ permalink raw reply

* Re: builtin conversion between tabs and spaces
From: Stefan Karpinski @ 2008-10-15  1:47 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <d4bc1a2a0810141844x76223e76xf04e07ece834fc61@mail.gmail.com>

Appologies for gmail mangling. I will use git send-email for real patches.

On Tue, Oct 14, 2008 at 6:44 PM, Stefan Karpinski
<stefan.karpinski@gmail.com> wrote:
> I find myself really wanting to enforce standards in the use of spaces
> versus tabs. I deal with some unruly programmers who refuse to set
> their editors to use soft tabs, but I *hate* tabs in the repo. And of
> course others feel equally strongly about keeping only tabs in the
> repo (e.g. the git repo).
>
> This led me to wonder if it wouldn't make sense to have this
> conversion ability built into git. The following patch implements this
> functionality. It still needs work—it's not meant to be final, just to
> give an idea—but I just wanted to see if people on the git list
> thought this sort of thing would be worthwhile at all.
>
> If people think it's worth having in git, then how should it be
> configured? I feel like a project should be able to define the
> expected tab size for binary file types. Moreover, the project should
> be able to define the default cannonicalization with resepect to
> whitespace for different files types. Then, if they so desire, each
> git user should be able to override the output format on a
> per-repository basis.
>
> Does this make any sense? Comments?
>
> ---
> diff --git a/convert.c b/convert.c
> index 1816e97..280f45b 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -18,7 +18,7 @@
>
>  struct text_stat {
>       /* NUL, CR, LF and CRLF counts */
> -       unsigned nul, cr, lf, crlf;
> +       unsigned nul, cr, lf, crlf, tab;
>
>       /* These are just approximations! */
>       unsigned printable, nonprintable;
> @@ -48,7 +48,10 @@
>  static void gather_stats(const char *buf, unsigned long size, struct
> text_stat *
>               else if (c < 32) {
>                       switch (c) {
>                               /* BS, HT, ESC and FF */
> -                       case '\b': case '\t': case '\033': case '\014':
> +                       case '\t':
> +                               stats->tab++;
> +                               /* fall through */
> +                       case '\b': case '\033': case '\014':
>                               stats->printable++;
>                               break;
>                       case 0:
> @@ -235,6 +238,105 @@
>  static int crlf_to_worktree(const char *path, const char *src, size_t len,
>       return 1;
>  }
>
> +static int tabs_to_spaces(const char *path, const char *src, size_t len,
> +
>                  struct strbuf *buf, int untabify)
> +{
> +       char *to_free = NULL;
> +       struct text_stat stats;
> +  static const unsigned tab_size = 4;
> +       char *spaces;
> +
> +       if (!untabify)
> +               return 0;
> +
> +  /* instead of calling twice, should cache these stats across calls */
> +       gather_stats(src, len, &stats);
> +
> +       if (!stats.tab)
> +    return 0;
> +
> +       /* are we "faking" in place editing ? */
> +       if (src == buf->buf)
> +               to_free = strbuf_detach(buf, NULL);
> +
> +  /* this growth may be excessive: not all tabs get tab_size spaces */
> +       strbuf_grow(buf, len + tab_size * stats.tab);
> +  spaces = (char *) xmalloc(tab_size);
> +  memset(spaces, ' ', tab_size);
> +       for (;;) {
> +               const char *line = src;
> +               const char *nl = memchr(src, '\n', len);
> +               char *tab;
> +    if (!nl)
> +      nl = src + len;
> +    while (src < nl && (tab = memchr(src, '\t', nl - src))) {
> +      strbuf_add(buf, src, tab - src);
> +      strbuf_add(buf, spaces, tab_size - ((tab - line) % tab_size));
> +      src = tab + 1;
> +    }
> +    if (src < nl)
> +      strbuf_add(buf, src, nl - src);
> +               if (nl < src + len)
> +                       strbuf_addch(buf, '\n');
> +               else
> +                       break;
> +               src = nl + 1;
> +               len -= src - line;
> +       }
> +
> +       free(to_free);
> +       free(spaces);
> +       return 1;
> +}
> +
> +static int spaces_to_tabs(const char *path, const char *src, size_t len,
> +
>                  struct strbuf *buf, int tabify)
> +{
> +  static const unsigned tab_size = 4;
> +
> +       if (!tabify)
> +               return 0;
> +
> +       /* only grow if not in place */
> +       if (strbuf_avail(buf) + buf->len < len)
> +               strbuf_grow(buf, len - buf->len);
> +
> +       for (;;) {
> +               int tabs = 0, spaces = 0;
> +               const char *line = src;
> +               const char *nl = memchr(src, '\n', len);
> +    if (!nl)
> +      nl = src + len;
> +               for (;; src++) {
> +                       if (*src == ' ') {
> +                               spaces++;
> +                               if (spaces == tab_size) {
> +                                       tabs++;
> +                                       spaces = 0;
> +                               }
> +                       } else if (*src == '\t') {
> +                               tabs++;
> +                               spaces = 0;
> +                       } else break;
> +               }
> +               if (line < src) {
> +                       memset(buf->buf + buf->len, '\t', tabs);
> +                       memset(buf->buf + buf->len + tabs, ' ', spaces);
> +                       strbuf_setlen(buf, buf->len + tabs + spaces);
> +               }
> +    if (src < nl)
> +      strbuf_add(buf, src, nl - src);
> +               if (nl < src + len)
> +                       strbuf_addch(buf, '\n');
> +               else
> +                       break;
> +               src = nl + 1;
> +               len -= src - line;
> +       }
> +
> +       return 1;
> +}
> +
>  struct filter_params {
>       const char *src;
>       unsigned long size;
> @@ -370,22 +472,29 @@
>  static int read_convert_config(const char *var, const char *value, void *cb)
>       return 0;
>  }
>
> -static void setup_convert_check(struct git_attr_check *check)
> +struct convert_checks {
> +  struct git_attr_check crlf, tabs, ident, filter;
> +};
> +
> +static void setup_convert_check(struct convert_checks *checks)
>  {
>       static struct git_attr *attr_crlf;
> +       static struct git_attr *attr_tabs;
>       static struct git_attr *attr_ident;
>       static struct git_attr *attr_filter;
>
>       if (!attr_crlf) {
>               attr_crlf = git_attr("crlf", 4);
> +               attr_tabs = git_attr("tabs", 4);
>               attr_ident = git_attr("ident", 5);
>               attr_filter = git_attr("filter", 6);
>               user_convert_tail = &user_convert;
>               git_config(read_convert_config, NULL);
>       }
> -       check[0].attr = attr_crlf;
> -       check[1].attr = attr_ident;
> -       check[2].attr = attr_filter;
> +       checks->crlf.attr = attr_crlf;
> +       checks->tabs.attr = attr_tabs;
> +       checks->ident.attr = attr_ident;
> +       checks->filter.attr = attr_filter;
>  }
>
>  static int count_ident(const char *cp, unsigned long size)
> @@ -566,20 +675,22 @@
>  static int git_path_check_ident(const char *path, struct git_attr_check *check)
>       return !!ATTR_TRUE(value);
>  }
>
> +#define CHECK_ARRAY_SIZE (sizeof(struct convert_checks)/sizeof(struct
> git_attr_check))
> +
>  int convert_to_git(const char *path, const char *src, size_t len,
>                   struct strbuf *dst, enum safe_crlf checksafe)
>  {
> -       struct git_attr_check check[3];
> +       struct convert_checks checks;
>       int crlf = CRLF_GUESS;
>       int ident = 0, ret = 0;
>       const char *filter = NULL;
>
> -       setup_convert_check(check);
> -       if (!git_checkattr(path, ARRAY_SIZE(check), check)) {
> +       setup_convert_check(&checks);
> +       if (!git_checkattr(path, CHECK_ARRAY_SIZE, (struct git_attr_check *)
> &checks)) {
>               struct convert_driver *drv;
> -               crlf = git_path_check_crlf(path, check + 0);
> -               ident = git_path_check_ident(path, check + 1);
> -               drv = git_path_check_convert(path, check + 2);
> +               crlf = git_path_check_crlf(path, &(checks.crlf));
> +               ident = git_path_check_ident(path, &(checks.ident));
> +               drv = git_path_check_convert(path, &(checks.filter));
>               if (drv && drv->clean)
>                       filter = drv->clean;
>       }
> @@ -589,6 +700,11 @@
>  int convert_to_git(const char *path, const char *src, size_t len,
>               src = dst->buf;
>               len = dst->len;
>       }
> +       ret |= tabs_to_spaces(path, src, len, dst, 1); // get real variable
> +       if (ret) {
> +               src = dst->buf;
> +               len = dst->len;
> +       }
>       ret |= crlf_to_git(path, src, len, dst, crlf, checksafe);
>       if (ret) {
>               src = dst->buf;
> @@ -599,17 +715,17 @@
>  int convert_to_git(const char *path, const char *src, size_t len,
>
>  int convert_to_working_tree(const char *path, const char *src, size_t
> len, struct strbuf *dst)
>  {
> -       struct git_attr_check check[3];
> +       struct convert_checks checks;
>       int crlf = CRLF_GUESS;
>       int ident = 0, ret = 0;
>       const char *filter = NULL;
>
> -       setup_convert_check(check);
> -       if (!git_checkattr(path, ARRAY_SIZE(check), check)) {
> +       setup_convert_check(&checks);
> +       if (!git_checkattr(path, CHECK_ARRAY_SIZE, (struct git_attr_check *)
> &checks)) {
>               struct convert_driver *drv;
> -               crlf = git_path_check_crlf(path, check + 0);
> -               ident = git_path_check_ident(path, check + 1);
> -               drv = git_path_check_convert(path, check + 2);
> +               crlf = git_path_check_crlf(path, &(checks.crlf));
> +               ident = git_path_check_ident(path, &(checks.ident));
> +               drv = git_path_check_convert(path, &(checks.filter));
>               if (drv && drv->smudge)
>                       filter = drv->smudge;
>       }
> @@ -624,5 +740,10 @@
>  int convert_to_working_tree(const char *path, const char *src, size_t
> len, struc
>               src = dst->buf;
>               len = dst->len;
>       }
> +       ret |= tabs_to_spaces(path, src, len, dst, 1); // get real variable
> +       if (ret) {
> +               src = dst->buf;
> +               len = dst->len;
> +       }
>       return ret | apply_filter(path, src, len, dst, filter);
>  }
>

^ permalink raw reply

* builtin conversion between tabs and spaces
From: Stefan Karpinski @ 2008-10-15  1:44 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <d4bc1a2a0810141842q1e50c85au7d813f2e5e37a84c@mail.gmail.com>

I find myself really wanting to enforce standards in the use of spaces
versus tabs. I deal with some unruly programmers who refuse to set
their editors to use soft tabs, but I *hate* tabs in the repo. And of
course others feel equally strongly about keeping only tabs in the
repo (e.g. the git repo).

This led me to wonder if it wouldn't make sense to have this
conversion ability built into git. The following patch implements this
functionality. It still needs work—it's not meant to be final, just to
give an idea—but I just wanted to see if people on the git list
thought this sort of thing would be worthwhile at all.

If people think it's worth having in git, then how should it be
configured? I feel like a project should be able to define the
expected tab size for binary file types. Moreover, the project should
be able to define the default cannonicalization with resepect to
whitespace for different files types. Then, if they so desire, each
git user should be able to override the output format on a
per-repository basis.

Does this make any sense? Comments?

---
diff --git a/convert.c b/convert.c
index 1816e97..280f45b 100644
--- a/convert.c
+++ b/convert.c
@@ -18,7 +18,7 @@

 struct text_stat {
       /* NUL, CR, LF and CRLF counts */
-       unsigned nul, cr, lf, crlf;
+       unsigned nul, cr, lf, crlf, tab;

       /* These are just approximations! */
       unsigned printable, nonprintable;
@@ -48,7 +48,10 @@
 static void gather_stats(const char *buf, unsigned long size, struct
text_stat *
               else if (c < 32) {
                       switch (c) {
                               /* BS, HT, ESC and FF */
-                       case '\b': case '\t': case '\033': case '\014':
+                       case '\t':
+                               stats->tab++;
+                               /* fall through */
+                       case '\b': case '\033': case '\014':
                               stats->printable++;
                               break;
                       case 0:
@@ -235,6 +238,105 @@
 static int crlf_to_worktree(const char *path, const char *src, size_t len,
       return 1;
 }

+static int tabs_to_spaces(const char *path, const char *src, size_t len,
+
                  struct strbuf *buf, int untabify)
+{
+       char *to_free = NULL;
+       struct text_stat stats;
+  static const unsigned tab_size = 4;
+       char *spaces;
+
+       if (!untabify)
+               return 0;
+
+  /* instead of calling twice, should cache these stats across calls */
+       gather_stats(src, len, &stats);
+
+       if (!stats.tab)
+    return 0;
+
+       /* are we "faking" in place editing ? */
+       if (src == buf->buf)
+               to_free = strbuf_detach(buf, NULL);
+
+  /* this growth may be excessive: not all tabs get tab_size spaces */
+       strbuf_grow(buf, len + tab_size * stats.tab);
+  spaces = (char *) xmalloc(tab_size);
+  memset(spaces, ' ', tab_size);
+       for (;;) {
+               const char *line = src;
+               const char *nl = memchr(src, '\n', len);
+               char *tab;
+    if (!nl)
+      nl = src + len;
+    while (src < nl && (tab = memchr(src, '\t', nl - src))) {
+      strbuf_add(buf, src, tab - src);
+      strbuf_add(buf, spaces, tab_size - ((tab - line) % tab_size));
+      src = tab + 1;
+    }
+    if (src < nl)
+      strbuf_add(buf, src, nl - src);
+               if (nl < src + len)
+                       strbuf_addch(buf, '\n');
+               else
+                       break;
+               src = nl + 1;
+               len -= src - line;
+       }
+
+       free(to_free);
+       free(spaces);
+       return 1;
+}
+
+static int spaces_to_tabs(const char *path, const char *src, size_t len,
+
                  struct strbuf *buf, int tabify)
+{
+  static const unsigned tab_size = 4;
+
+       if (!tabify)
+               return 0;
+
+       /* only grow if not in place */
+       if (strbuf_avail(buf) + buf->len < len)
+               strbuf_grow(buf, len - buf->len);
+
+       for (;;) {
+               int tabs = 0, spaces = 0;
+               const char *line = src;
+               const char *nl = memchr(src, '\n', len);
+    if (!nl)
+      nl = src + len;
+               for (;; src++) {
+                       if (*src == ' ') {
+                               spaces++;
+                               if (spaces == tab_size) {
+                                       tabs++;
+                                       spaces = 0;
+                               }
+                       } else if (*src == '\t') {
+                               tabs++;
+                               spaces = 0;
+                       } else break;
+               }
+               if (line < src) {
+                       memset(buf->buf + buf->len, '\t', tabs);
+                       memset(buf->buf + buf->len + tabs, ' ', spaces);
+                       strbuf_setlen(buf, buf->len + tabs + spaces);
+               }
+    if (src < nl)
+      strbuf_add(buf, src, nl - src);
+               if (nl < src + len)
+                       strbuf_addch(buf, '\n');
+               else
+                       break;
+               src = nl + 1;
+               len -= src - line;
+       }
+
+       return 1;
+}
+
 struct filter_params {
       const char *src;
       unsigned long size;
@@ -370,22 +472,29 @@
 static int read_convert_config(const char *var, const char *value, void *cb)
       return 0;
 }

-static void setup_convert_check(struct git_attr_check *check)
+struct convert_checks {
+  struct git_attr_check crlf, tabs, ident, filter;
+};
+
+static void setup_convert_check(struct convert_checks *checks)
 {
       static struct git_attr *attr_crlf;
+       static struct git_attr *attr_tabs;
       static struct git_attr *attr_ident;
       static struct git_attr *attr_filter;

       if (!attr_crlf) {
               attr_crlf = git_attr("crlf", 4);
+               attr_tabs = git_attr("tabs", 4);
               attr_ident = git_attr("ident", 5);
               attr_filter = git_attr("filter", 6);
               user_convert_tail = &user_convert;
               git_config(read_convert_config, NULL);
       }
-       check[0].attr = attr_crlf;
-       check[1].attr = attr_ident;
-       check[2].attr = attr_filter;
+       checks->crlf.attr = attr_crlf;
+       checks->tabs.attr = attr_tabs;
+       checks->ident.attr = attr_ident;
+       checks->filter.attr = attr_filter;
 }

 static int count_ident(const char *cp, unsigned long size)
@@ -566,20 +675,22 @@
 static int git_path_check_ident(const char *path, struct git_attr_check *check)
       return !!ATTR_TRUE(value);
 }

+#define CHECK_ARRAY_SIZE (sizeof(struct convert_checks)/sizeof(struct
git_attr_check))
+
 int convert_to_git(const char *path, const char *src, size_t len,
                   struct strbuf *dst, enum safe_crlf checksafe)
 {
-       struct git_attr_check check[3];
+       struct convert_checks checks;
       int crlf = CRLF_GUESS;
       int ident = 0, ret = 0;
       const char *filter = NULL;

-       setup_convert_check(check);
-       if (!git_checkattr(path, ARRAY_SIZE(check), check)) {
+       setup_convert_check(&checks);
+       if (!git_checkattr(path, CHECK_ARRAY_SIZE, (struct git_attr_check *)
&checks)) {
               struct convert_driver *drv;
-               crlf = git_path_check_crlf(path, check + 0);
-               ident = git_path_check_ident(path, check + 1);
-               drv = git_path_check_convert(path, check + 2);
+               crlf = git_path_check_crlf(path, &(checks.crlf));
+               ident = git_path_check_ident(path, &(checks.ident));
+               drv = git_path_check_convert(path, &(checks.filter));
               if (drv && drv->clean)
                       filter = drv->clean;
       }
@@ -589,6 +700,11 @@
 int convert_to_git(const char *path, const char *src, size_t len,
               src = dst->buf;
               len = dst->len;
       }
+       ret |= tabs_to_spaces(path, src, len, dst, 1); // get real variable
+       if (ret) {
+               src = dst->buf;
+               len = dst->len;
+       }
       ret |= crlf_to_git(path, src, len, dst, crlf, checksafe);
       if (ret) {
               src = dst->buf;
@@ -599,17 +715,17 @@
 int convert_to_git(const char *path, const char *src, size_t len,

 int convert_to_working_tree(const char *path, const char *src, size_t
len, struct strbuf *dst)
 {
-       struct git_attr_check check[3];
+       struct convert_checks checks;
       int crlf = CRLF_GUESS;
       int ident = 0, ret = 0;
       const char *filter = NULL;

-       setup_convert_check(check);
-       if (!git_checkattr(path, ARRAY_SIZE(check), check)) {
+       setup_convert_check(&checks);
+       if (!git_checkattr(path, CHECK_ARRAY_SIZE, (struct git_attr_check *)
&checks)) {
               struct convert_driver *drv;
-               crlf = git_path_check_crlf(path, check + 0);
-               ident = git_path_check_ident(path, check + 1);
-               drv = git_path_check_convert(path, check + 2);
+               crlf = git_path_check_crlf(path, &(checks.crlf));
+               ident = git_path_check_ident(path, &(checks.ident));
+               drv = git_path_check_convert(path, &(checks.filter));
               if (drv && drv->smudge)
                       filter = drv->smudge;
       }
@@ -624,5 +740,10 @@
 int convert_to_working_tree(const char *path, const char *src, size_t
len, struc
               src = dst->buf;
               len = dst->len;
       }
+       ret |= tabs_to_spaces(path, src, len, dst, 1); // get real variable
+       if (ret) {
+               src = dst->buf;
+               len = dst->len;
+       }
       return ret | apply_filter(path, src, len, dst, filter);
 }

^ permalink raw reply related

* Re: [PATCH] Improve and fix git-check-attr(1)
From: Junio C Hamano @ 2008-10-14 23:58 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20081013175705.GA9538@diku.dk>

Jonas Fonseca <fonseca@diku.dk> writes:

> Always use 'verse' for multi-line synopsis sections. Add output and
> example sections to document what output can be expected.
>
> Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
>
> ---
>  Documentation/git-check-attr.txt |   63 +++++++++++++++++++++++++++++++++++++-
>  1 files changed, 62 insertions(+), 1 deletions(-)
>
>  At least the first chunk should be applied. The last is a quick attempt
>  at documenting the expected output formally and with examples.
> ...
> tg: (340fcf4..) jf/man-git-check-attr (depends on: next)

Should this really have to depend on next?  I have a feeling that a large
part of this (sans --stdin/-z) is a 'maint' material.

Could you split this into two patches, one for 'maint' to describe the
output format, and another for 'next' (or 'dp/checkattr') to update the
SYNOPSIS part?

^ permalink raw reply

* Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL interfere with the test
From: Deskin Miller @ 2008-10-14 23:45 UTC (permalink / raw)
  To: Matt McCutchen; +Cc: Junio C Hamano, git
In-Reply-To: <1224025227.2699.13.camel@mattlaptop2.local>

On Tue, Oct 14, 2008 at 07:00:27PM -0400, Matt McCutchen wrote:
> On Tue, 2008-10-14 at 15:32 -0700, Junio C Hamano wrote:
> > Makes me wonder why this is _not_ inside test-lib.sh where it creates the
> > test (trash) directory.  That way, you would cover future tests that wants
> > to see a saner/simpler POSIX permission behaviour, wouldn't you?
> 
> Yes.  However, I don't anticipate there being any tests specifically
> about file permissions other than t1301-shared-repo.sh, and if the user
> has set a default ACL on the git source tree, we might want to let trash
> directories obey that setting except in the one case where it breaks the
> test.  What do you think?

I'll add a shameless plug for my patch: Fix testcase failure when extended
attributes are in use, available from Gmane at

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

It's orthogonal to this patch, I think: this patch deals with ACLs overriding
the umask testing we're doing, while my patch deals with parsing the
permissions that ls returns, and applies to instances where extended attributes
are in use which we can't get rid of, like SELinux.

Deskin Miller

^ permalink raw reply

* Re: [PATCH] Fix mkpath abuse in dwim_ref/sha1_name.c
From: Junio C Hamano @ 2008-10-14 23:17 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Git Mailing List, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <81b0412b0810140923x5cf58bb9x5acd1517a19e9847@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

> From: Alex Riesen <raa.lkml@gmail.com>
> Date: Tue, 14 Oct 2008 18:14:20 +0200
> Subject: [PATCH] Fix mkpath abuse in sha1_name.c
>
> Otherwise the function sometimes fail to resolve obviously correct refnames,
> because the string data pointed to by "ref" argument were reused.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

The get_pathname() interface has been handy but it indeed is a bug waiting
to happen, and we have fixed occasional breakages by fixing the earlier
callers to stash the returned value away to keep it from getting
overwritten.  I.e. when we have:

	str1 = function_that_uses_get_pathname();
        ...
        str2 = another_function_that_uses_get_pathname();
        some_processing(str1, str2);

and we have too many other calls to functions that use get_pathname() in
the ... section, str1 will become invalid when we try to feed it to
some_processing().  So far our strategy to fix this kind of breakages has
been to rewrite the above to:

	str1 = xstrdup(function_that_uses_get_pathname());
        ...
        str2 = another_function_that_uses_get_pathname();
        some_processing(str1, str2);
	free(str1);

But your patch instead rewrites the computation of str2 by bypassing the
call to "another_function_that_uses_get_pathname()" and duplicating its
logic, which I do not think is a viable approach in the longer term.

> diff --git a/sha1_name.c b/sha1_name.c
> index 41b6809..b5b53bf 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -242,6 +242,7 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
>  {
>  	const char **p, *r;
>  	int refs_found = 0;
> +	char fullref[PATH_MAX];
>  
>  	*ref = NULL;
>  	for (p = ref_rev_parse_rules; *p; p++) {
> @@ -249,7 +250,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
>  		unsigned char *this_result;
>  
>  		this_result = refs_found ? sha1_from_ref : sha1;
> -		r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
> +		snprintf(fullref, sizeof(fullref), *p, len, str);
> +		r = resolve_ref(fullref, this_result, 1, NULL);
>  		if (r) {
>  			if (!refs_found++)
>  				*ref = xstrdup(r);

I suspect that I am grossly misleading the code, but I wonder why this xstrdup()
is not protecting us from the reusing of "the string data pointed to by
"ref" argument".  Are you fixing the overwriting of the string pointed to
by "str" argument instead?

What specific call chain has this breakage you found?

^ permalink raw reply

* Re: Problem with CRLF line endings and colored diff
From: Jean-Luc Herren @ 2008-10-14 23:08 UTC (permalink / raw)
  To: Junio C Hamano, Git List
In-Reply-To: <7v4p3e7r9e.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Jean-Luc Herren <jlh@gmx.ch> writes:
>>     <GREEN FOREGROUND>+<RESET><GREEN FOREGROUND>
>>     line2<RESET><RED BACKGROUND><CR><RESET><LF>
> 
> This is expected and deliberate, as you haven't told about your desire to
> consider carriage-returns not as line termination letter.  By default CR
> is just one of the whitespace letters, and colored diff output is
> reminding you that you introduced a line with a trailing whitespace.

Ah, I see, I didn't know I have to tell it that I want to use
CRLF...  Works fine now.  Sorry for the noise.

jlh

^ permalink raw reply

* from symlinked directory: unable to chdir or not a git archive
From: Marcel M. Cary @ 2008-10-14 22:58 UTC (permalink / raw)
  To: git

I'm getting an error:

  fatal: 'origin': unable to chdir or not a git archive

when trying to pull even though pushing works just fine.  So I don't
think the problem is that the remote repository doesn't exist.  (I see
the entry in GitFaq for this error message, but I don't think that
answer quite explains this situation.)

I think it's important that, when pulling, I'm in a directory that is a
symlink into the Git working copy.  Having git operate correctly in that
environment is important to me because Git doesn't support checking out
a subdirectory of my project.

A shell transcript follows that illustrates a simplified problem
scenario using Git 1.6.0.1.  I see the same error message with 1.6.0.2
and 1.6.0.2.530.g67faa (master branch today).

Is there an easy way around this?  Right now I have to cd to the target
of the symlink to do the pull, which is annoying.

Thanks,
Marcel

~/git$ mkdir tmp-repo
~/git$ cd tmp-repo/
~/git/tmp-repo$ git init
Initialized empty Git repository in /home/marcel/git/tmp-repo/.git/
~/git/tmp-repo$ mkdir foo
~/git/tmp-repo$ touch foo/bar
~/git/tmp-repo$ git add foo/bar
~/git/tmp-repo$ git commit -m 'Add empty file'
Created initial commit 651b80a: Add empty file
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 foo/bar
~/git/tmp-repo$ cd ..
~/git$ git clone tmp-repo clone-repo
Initialized empty Git repository in /home/marcel/git/clone-repo/.git/
~/git$ ln -s clone-repo/foo/ foo-link
~/git$ cd foo-link/
~/git/foo-link$ echo hello >> bar
~/git/foo-link$ git commit -a -m 'Say hi'
Created commit 0a92c49: Say hi
 1 files changed, 1 insertions(+), 0 deletions(-)
~/git/foo-link$ git push origin master
Counting objects: 7, done.
Writing objects: 100% (4/4), 281 bytes, done.
Total 4 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (4/4), done.
To /home/marcel/git/tmp-repo/.git
   651b80a..0a92c49  master -> master
~/git/foo-link$ git pull
fatal: 'origin': unable to chdir or not a git archive
fatal: The remote end hung up unexpectedly
~/git/foo-link$ cd ~/git/clone-repo/foo
~/git/clone-repo/foo$ git pull
Already up-to-date.
~/git/clone-repo/foo$

on Linux 2.6.18-4-486, ext3

^ permalink raw reply

* Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL interfere with the test
From: Matt McCutchen @ 2008-10-14 23:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzll66c5u.fsf@gitster.siamese.dyndns.org>

On Tue, 2008-10-14 at 15:32 -0700, Junio C Hamano wrote:
> Matt McCutchen <matt@mattmccutchen.net> writes:
> > This test creates files with several different umasks and expects the files to
> > be permissioned according to the umasks, so a default ACL on the test dir causes
> 
> Is "to permission" a verb?

I thought it could be, but I'll reword that sentence.

> > the test to fail.  To avoid that, remove the default ACL if possible with
> > setfacl(1).  (Will work on many systems.)
> 
> It is not clear in the comment in parentheses what provision you have made
> not to harm people on systems without setfacl.
> 
> I think "if possible" which you already have is a good enough description
> (i.e. "if setfacl fails we do not barf and if you do not have the command
> you probably are not running with a funky default ACL to see this issue
> anyway"), so I'd rather drop the comment in parentheses.

Sure.

> > Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
> > ---
> > This time with a signoff.
> >
> >  t/t1301-shared-repo.sh |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> > index dc85e8b..2275caa 100755
> > --- a/t/t1301-shared-repo.sh
> > +++ b/t/t1301-shared-repo.sh
> > @@ -7,6 +7,9 @@ test_description='Test shared repository initialization'
> >  
> >  . ./test-lib.sh
> >  
> > +# Remove a default ACL from the test dir if possible.
> > +setfacl -k . 2>/dev/null
> > +
> 
> Makes me wonder why this is _not_ inside test-lib.sh where it creates the
> test (trash) directory.  That way, you would cover future tests that wants
> to see a saner/simpler POSIX permission behaviour, wouldn't you?

Yes.  However, I don't anticipate there being any tests specifically
about file permissions other than t1301-shared-repo.sh, and if the user
has set a default ACL on the git source tree, we might want to let trash
directories obey that setting except in the one case where it breaks the
test.  What do you think?

Matt

^ permalink raw reply

* [PATCH] pull: allow "git pull origin $something:$current_branch" into an unborn branch
From: Junio C Hamano @ 2008-10-14 22:53 UTC (permalink / raw)
  To: Johannes Schindelin, Daniel Barkalow; +Cc: git, spearce
In-Reply-To: <7vk5ca7rw3.fsf@gitster.siamese.dyndns.org>

Some misguided documents floating on the Net suggests this sequence:

    mkdir newdir && cd newdir
    git init
    git remote add origin $url
    git pull origin master:master

"git pull" has known about an misguided "pull" like this where the
underlying fetch updates the current branch for a long time, and it also
has known about another form of insanity "git pull origin master" into an
unborn branch, but these two workarounds were not aware of the existence
of each other and did not work well together.

This fixes it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This comes on top of js/maint-fetch-update-head.  Passes all the tests,
   and also its own.

 git-pull.sh     |    4 ++--
 t/t5520-pull.sh |   12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 75c3610..664fe34 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -124,7 +124,7 @@ orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
 git fetch --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
-if test "$curr_head" != "$orig_head"
+if test -n "$orig_head" && test "$curr_head" != "$orig_head"
 then
 	# The fetch involved updating the current branch.
 
@@ -172,7 +172,7 @@ esac
 
 if test -z "$orig_head"
 then
-	git update-ref -m "initial pull" HEAD $merge_head "" &&
+	git update-ref -m "initial pull" HEAD $merge_head "$curr_head" &&
 	git read-tree --reset -u HEAD || exit 1
 	exit
 fi
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 997b2db..725771f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -29,6 +29,18 @@ test_expect_success 'checking the results' '
 	diff file cloned/file
 '
 
+test_expect_success 'pulling into void using master:master' '
+	mkdir cloned-uho &&
+	(
+		cd cloned-uho &&
+		git init &&
+		git pull .. master:master
+	) &&
+	test -f file &&
+	test -f cloned-uho/file &&
+	test_cmp file cloned-uho/file
+'
+
 test_expect_success 'test . as a remote' '
 
 	git branch copy master &&
-- 
1.6.0.2.711.gf1ba4

^ permalink raw reply related


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