Git development
 help / color / mirror / Atom feed
* Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
From: Junio C Hamano @ 2008-07-22 23:55 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <32541b130807221522r2a43c49cl6400f00dbe7451a0@mail.gmail.com>

"Avery Pennarun" <apenwarr@gmail.com> writes:

> On 7/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>  If the user called "rebase -i", marked a commit as "edit", "rebase
>>  --continue" would automatically amend the commit when there were
>>  staged changes.
>>
>>  However, this is actively wrong when the current commit is not the
>>  one marked with "edit".  So guard against this.
>
> This patch is perhaps a symptom of something I've been meaning to ask
> about for a while.
>
> Why doesn't "edit" just stage the commit and not auto-commit it at
> all?  That way an amend would *never* be necessary, and rebase
> --continue would always do a commit -a (if there was anything left to
> commit).  The special case fixed by this patch would thus not be
> needed.

That would actually be in-line with the way how "rebase --skip" does the
resetting without asking the user to do so, wouldn't it?

^ permalink raw reply

* Re: [PATCH] Documentation/git-filter-branch: Remove Useless Use of Plumbing
From: Junio C Hamano @ 2008-07-23  0:05 UTC (permalink / raw)
  To: Petr Baudis; +Cc: gitster, git
In-Reply-To: <20080722222418.15372.62190.stgit@localhost>

Petr Baudis <pasky@suse.cz> writes:

> The example to remove file using index-filter uses git update-index
>  --remove where git rm --cached works as well.

I am not sure "works as well" is a good enough justification to choose a
Porcelain command over a plumbing command in this particular context.
After all, filter-branch is a scripting enviornment, isn't it?

There also is another subtle difference.  "git rm" takes pathspecs while
"update-index" takes paths.

So after running one of these commands:

	$ git rm --cached 'Makefil?'
	$ git update-index --force-remove 'Makefil?'

output from:

	$ git diff --cached --stat

would be different.

^ permalink raw reply

* Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories
From: Junio C Hamano @ 2008-07-23  0:09 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, gitster, git
In-Reply-To: <20080722233652.GT10151@machine.or.cz>

Petr Baudis <pasky@suse.cz> writes:

>   um, oops. I actually never got to know these by heart since I learnt
> to expliciply group the expressions early on. I guess my only excuse is
> that I've stumbled at 0bdf93cbf earlier and understood it the _wrong_
> way around since I'm getting really sleepy. ;-)
>
>   I still think my change improves the code readibility so it could be
> kept, but I'm fairly neutral on this.

I cannot be neutral when a patch introduces unnecessary fork.

The quality of shell scripts in git.git seems to have deteriorated over
time but I do not think we would want to spend too much maintainer time to
go back and fix all of them.  Please don't make things worse, at least.

^ permalink raw reply

* [PATCH] am --abort: Add to bash-completion and mention in git-rerere documentation
From: Stephan Beyer @ 2008-07-23  0:10 UTC (permalink / raw)
  To: Olivier Marin
  Cc: Junio C Hamano, Theodore Tso, Nanako Shiraishi, git,
	Stephan Beyer
In-Reply-To: <4882350B.6020003@free.fr>

The git-rerere documentation talks about commands that invoke
"git rerere clear" automatically. git am --abort is added and
a typo is fixed additionally.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 Documentation/git-rerere.txt           |    2 +-
 contrib/completion/git-completion.bash |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index beebd53..89f321b 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -37,7 +37,7 @@ its working state.
 'clear'::
 
 This resets the metadata used by rerere if a merge resolution is to be
-is aborted.  Calling 'git-am --skip' or 'git-rebase [--skip|--abort]'
+aborted.  Calling 'git-am [--skip|--abort]' or 'git-rebase [--skip|--abort]'
 will automatically invoke this command.
 
 'diff'::
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2edb341..8fc9145 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -489,7 +489,7 @@ _git_am ()
 {
 	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
 	if [ -d "$dir"/rebase-apply ]; then
-		__gitcomp "--skip --resolved"
+		__gitcomp "--skip --resolved --abort"
 		return
 	fi
 	case "$cur" in
-- 
1.5.6.4.459.gfa44d

^ permalink raw reply related

* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Junio C Hamano @ 2008-07-23  0:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807222255450.8986@racer>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When a file's crlf attribute is explicitely set, it does not make sense
> to ignore it, just because the config variable core.autocrlf has not
> been set.

I am not sure if I agree with that reasoning.

Attribute defines what each path is.  Is it a text file, is it a binary?
The nature of the contents does not change between people on POSIX and
Windows, and that is why it is described in .gitattributes and cloned
across repositories.

On the other hand, the configuration defines what to do with contents with
various attributes in this particular repository.  Do I want to see a text
file checked out with CRLF endings, or LF?

So it is perfectly valid and normal for a cross-platform minded project to
use the crlf atttribute to say "These files are text" and expect them to
be checked out with LF endings on POSIX while making sure they are checked
out with CRLF on Windows.  Adding CR at the end of line for such files on
POSIX systems is positively a wrong thing to do in such a case.

Projects like the kernel that originate from LF side of the world may not
bother marking things as such, though.

^ permalink raw reply

* Re: [PATCH] Build configuration to skip ctime for modification test
From: Junio C Hamano @ 2008-07-23  0:12 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20080722203128.GB5113@blimp.local>

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

> Johannes Schindelin, Tue, Jul 22, 2008 22:17:21 +0200:
>> Hi,
>> 
>> On Tue, 22 Jul 2008, Alex Riesen wrote:
>> 
>> > +#ifndef NO_TRUSTABLE_FILEMODE
>> >  	if (ce->ce_ctime != (unsigned int) st->st_ctime)
>> >  		changed |= CTIME_CHANGED;
>> > +#endif
>> 
>> Surely you meant trust_executable_bit instead, right?
>
> No. Just what I said: we don't have filemode (like "at all") - so no
> ctime as well. But maybe you're right, and trust_executable_bit is
> more flexible. Or maybe both (the #ifdef _and_ trust_executable_bit)
> and must be used...
>
>> Otherwise, if you really want to tell at compile time,I think for clarity 
>> you have to introduce another #define, since NO_TRUSTABLE_FILEMODE 
>> definitely says something different than CTIME_IS_USELESS.
>
> I had that at first (NO_DEPENDABLE_CTIME, than IGNORE_CTIME), than
> deemed it excessive.

Why is it excessive?  My initial reaction was "what does trustable
filemode nor trust_executable_bit has anything to do with ctime".  Please
explain.

^ permalink raw reply

* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Dmitry Potapov @ 2008-07-23  0:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0807230019480.8986@racer>

On Wed, Jul 23, 2008 at 12:23:27AM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 23 Jul 2008, Dmitry Potapov wrote:
> 
> > On Tue, Jul 22, 2008 at 10:56:04PM +0100, Johannes Schindelin wrote:
> > > 
> > > When a file's crlf attribute is explicitely set, it does not make 
> > > sense to ignore it, just because the config variable core.autocrlf has 
> > > not been set.
> > 
> > Hmm... About a week ago, I was about to propose the same change, but 
> > after reading documentation and some thinking I was not able to convince 
> > myself that this change would be the right thing to do.
> 
> Well, I have a shared repository, where I set the attribute.  Now, every 
> once in a while, people check in text _with_ CR/LF.  Yes, that is right, I 
> marked it explicitely as crlf, yet I am on the whim of the people choosing 
> to set the config variable or not.
> 
> And I could not care less what the documentation says: if it does not make 
> sense, it does not make sense.

If you think that the current documentation does not make sense, why
don't you write something that will make sense? Really, the current
behavior may not be the best one, but at least it is consistent with
documentation, while your change may confuse users, because they may
expect one behavior, but git will act differently.

If I understand your change correctly, you take into account the crlf
attribute unconditionally only in worktree-to-git conversion, while you
still ignore it if core.autocrlf=false on checkout. Is it correct?  If
so, I think your patch does make sense, and it should not break anything
too badly, because you still respect core.autocrlf on checkout, but you
should have said that in your commit message.

Dmitry

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Junio C Hamano @ 2008-07-23  0:16 UTC (permalink / raw)
  To: Olivier Marin
  Cc: Theodore Tso, Nanako Shiraishi, Johannes Schindelin,
	René Scharfe, Stephan Beyer, Joe Fiorini, git, Jari Aalto
In-Reply-To: <4884917A.1060005@free.fr>

Olivier Marin <dkr+ml.git@free.fr> writes:

> The last thing that still annoy me is the --skip that refuse to skip in 3-way
> merge. Perhaps we can use the "git read-tree --reset -u" thing for skip too.

Hmm...

We traditionally left that as something the user deliberately should do to
signal --skip that the user knows he is dropping that change (by the way,
so did "git-rebase").  But with fb6e4e1 (Do git reset --hard HEAD when
using git rebase --skip, 2007-11-08), we run the reset upon rebase --skip,
so it probably is a good idea to match it here as well.

> diff --git a/git-am.sh b/git-am.sh
> index 60aaa4a..864c77e 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -202,8 +202,15 @@ then
>  	die "previous rebase directory $dotest still exists but mbox given."
>  	resume=yes
>  
> -	case "$abort" in
> -	t)
> +	case "$skip,$abort" in
> +	t,)
> +		git rerere clear
> +		git read-tree --reset -u HEAD HEAD
> +		orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
> +		git reset HEAD
> +		git update-ref ORIG_HEAD $orig_head
> +		;;

Sorry, I do not quite understand what this reset after the read-tree dance
is trying to do; you have already reset the index to the tree in HEAD when
you cleared the change involved in the patch application with that
two-tree form of read-tree.

^ permalink raw reply

* Re: [PATCH] bring description of git diff --cc up to date
From: Junio C Hamano @ 2008-07-23  0:36 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, David Greaves
In-Reply-To: <Pine.GSO.4.62.0807221812470.25746@harper.uchicago.edu>

Jonathan Nieder <jrnieder@uchicago.edu> writes:

> +	This flag implies the '-c' option and makes the patch output
> +	even more compact by omitting uninteresting hunks.  A hunk is
> +	considered uninteresting if the person merging had two versions
> +	to choose between among all of the parents and the result shows

Hmm, I am not a native speaker, but the above makes me confused into
thinking that even if there are 47 parent versions, it is Ok if I looked
at only two versions and picked from one of them -- the description does
not seem to make it clear that it is required that the other 45 agree with
one of the two I looked at and picked from.

    ... if the contents in the parents had only two variants and the merge
    result picked one of them without modification.

would be succinct enough, perhaps?

If we want to further elaborate, we could say something like:

    In a two-parent merge, by definition, there can only be two variants
    to choose from.  Even in a merge with more than two parents, if some
    parents share the same content, and all the other parents share
    another, same content, there are only two variants to choose from.

but I think that is too verbose...

^ permalink raw reply

* Re: regression in  92392b4
From: Shawn O. Pearce @ 2008-07-23  0:41 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Pierre Habouzit, Git ML, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0807230033000.8986@racer>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 23 Jul 2008, Pierre Habouzit wrote:
> 
> >   Hi, here is a manual painful down-secting (opposed to a bisect ;P) I
> > did, since git in next cannot fetch on a regular basis for me. The
> > culprit seems to be commit  92392b4:
> > 
> >     ┌─(1:11)──<~/dev/scm/git 92392b4...>──
> >     └[artemis] git fetch
> >     remote: Counting objects: 461, done.
> >     remote: Compressing objects: 100% (141/141), done.
> >     remote: Total 263 (delta 227), reused 155 (delta 121)
> >     Receiving objects: 100% (263/263), 95.55 KiB, done.
> >     fatal: Out of memory, malloc failed
> >     fatal: index-pack failed
> >     [2]    16674 abort (core dumped)  git fetch
...
> 
> Just a guess:
...
> diff --git a/index-pack.c b/index-pack.c
> index ac20a46..19c39e5 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -257,6 +257,7 @@ static void unlink_base_data(struct base_data *c)
>  		base_cache = NULL;
>  	if (c->data) {
>  		free(c->data);
> +		c->data = NULL;
>  		base_cache_used -= c->size;
>  	}
>  }

Oh.  This is a pointless assignment.  If you look at any call sites
for unlink_base_data() you will find that the struct passed in as
"c" here is going out of scope after unlink_base_data() returns.  In
no such case does the value of c->data get tested once this free is
complete.

We need the if (c->data) guard because we only want to decrement
base_cache_used if the memory is still allocated.  It may have been
released earlier, in which case base_cache_used has already been
decreased and we don't want to double-decrement it.

This patch makes the code more obvious, so Ack I guess, but it is
not a solution to Pierre's woes.  Something else is wrong.

Reading above shows we got a "fatal: Out of memory, malloc failed"
right before the segfault.  What's odd is we segfaulted after we
ran out of memory and should have die'd.

There's at least two bugs in the above output:

a) index-pack ran out of memory on a small pull (95 KiB).
b) fetch segfaulted when index-pack failed.

And this patch will unfortunately address neither of them.  :-|

I've had a long past couple of days, and another one tomorrow.
I'm not going to be able to debug this myself until perhaps Thursday
or Friday.  Sorry.  If nobody beats me to it, I will put this on
the top of the pile and try to fix it once I get back online at my
new home.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Junio C Hamano @ 2008-07-23  0:48 UTC (permalink / raw)
  To: Stephan Beyer
  Cc: Olivier Marin, Theodore Tso, Nanako Shiraishi,
	Johannes Schindelin, René Scharfe, Joe Fiorini, git,
	Jari Aalto
In-Reply-To: <20080722234703.GD5904@leksak.fem-net>

Stephan Beyer <s-beyer@gmx.net> writes:

> Hi,
>
> Junio C Hamano wrote:
>> Olivier Marin <dkr+ml.git@free.fr> writes:
>> > @@ -203,9 +204,10 @@ then
>> >  
>> >  	case "$abort" in
>> >  	t)
>> > -		rm -fr "$dotest" &&
>> > +		git rerere clear &&
>> >  		git read-tree -m -u ORIG_HEAD &&
> [...]
>> diff --git a/git-am.sh b/git-am.sh
>> index a44bd7a..5cbf8f4 100755
>> --- a/git-am.sh
>> +++ b/git-am.sh
>> @@ -203,9 +203,9 @@ then
>>  
>>  	case "$abort" in
>>  	t)
>> -		rm -fr "$dotest" &&
>> -		git read-tree -m -u ORIG_HEAD &&
>> -		git reset ORIG_HEAD && :
>> +		git rerere clear
>> +		git read-tree --reset -u HEAD ORIG_HEAD
>
> Perhaps I am confused, but ...
> Why is there "HEAD" and "ORIG_HEAD" and not only "ORIG_HEAD"?

Just being a bit defensive -- in this case I think it might be Ok to say
"read-tree --reset -u ORIG_HEAD", but I haven't checked in a conflicted
case.

If some path was added between ORIG_HEAD (that is where we started from)
and HEAD (that is where we are and we decide we do not want it), and that
path is conflicted in the index, a single tree form "read-tree --reset -u
HEAD" would leave it behind in the working tree, wouldn't it?

^ permalink raw reply

* [PATCH 1/2] Add test to show that show-branch misses out the 8th column
From: Johannes Schindelin @ 2008-07-23  0:50 UTC (permalink / raw)
  To: pasky, git, gitster


Noticed by Pasky.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3202-show-branch-octopus.sh |   59 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100755 t/t3202-show-branch-octopus.sh

diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh
new file mode 100755
index 0000000..8d50c23
--- /dev/null
+++ b/t/t3202-show-branch-octopus.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test_description='test show-branch with more than 8 heads'
+
+. ./test-lib.sh
+
+numbers="1 2 3 4 5 6 7 8 9 10"
+
+test_expect_success 'setup' '
+
+	> file &&
+	git add file &&
+	test_tick &&
+	git commit -m initial &&
+
+	for i in $numbers
+	do
+		git checkout -b branch$i master &&
+		> file$i &&
+		git add file$i &&
+		test_tick &&
+		git commit -m branch$i || break
+	done
+
+'
+
+cat > expect << EOF
+! [branch1] branch1
+ ! [branch2] branch2
+  ! [branch3] branch3
+   ! [branch4] branch4
+    ! [branch5] branch5
+     ! [branch6] branch6
+      ! [branch7] branch7
+       ! [branch8] branch8
+        ! [branch9] branch9
+         * [branch10] branch10
+----------
+         * [branch10] branch10
+        +  [branch9] branch9
+       +   [branch8] branch8
+      +    [branch7] branch7
+     +     [branch6] branch6
+    +      [branch5] branch5
+   +       [branch4] branch4
+  +        [branch3] branch3
+ +         [branch2] branch2
++          [branch1] branch1
++++++++++* [branch10^] initial
+EOF
+
+test_expect_failure 'show-branch with more than 8 branches' '
+
+	git show-branch $(for i in $numbers; do echo branch$i; done) > out &&
+	test_cmp expect out
+
+'
+
+test_done
-- 
1.6.0.rc0.22.gf2096d.dirty

^ permalink raw reply related

* [PATCH 2/2] sort_in_topological_order(): avoid setting a commit flag
From: Johannes Schindelin @ 2008-07-23  0:51 UTC (permalink / raw)
  To: pasky, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807230148130.8986@racer>


We used to set the TOPOSORT flag of commits during the topological
sorting, but we can just as well use the member "indegree" for it:
indegree is now incremented by 1 in the cases where the commit used
to have the TOPOSORT flag.

This is the same behavior as before, since indegree could not be
non-zero when TOPOSORT was unset.

Incidentally, this fixes the bug in show-branch where the 8th column
was not shown: show-branch sorts the commits in topological order,
assuming that all the commit flags are available for show-branch's
private matters.

But this was not true: TOPOSORT was identical to the flag corresponding
to the 8th ref.  So the flags for the 8th column were unset by the
topological sorting.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This is another late-night patch done by yours-truly.  However,
	I tried extra hard to make sure that every occurrence of
	indegree was properly changed, and I am pretty certain that
	the reasoning with the unset TOPOSORT is correct.

	But please check (I know, not necessary to ask for extra review
	for my patches,	but nevertheless).

 commit.c                       |   13 ++++++-------
 revision.h                     |    3 +--
 t/t3202-show-branch-octopus.sh |    2 +-
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/commit.c b/commit.c
index 5148ec5..9dacfb8 100644
--- a/commit.c
+++ b/commit.c
@@ -436,8 +436,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 	/* Mark them and clear the indegree */
 	for (next = orig; next; next = next->next) {
 		struct commit *commit = next->item;
-		commit->object.flags |= TOPOSORT;
-		commit->indegree = 0;
+		commit->indegree = 1;
 	}
 
 	/* update the indegree */
@@ -446,7 +445,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 		while (parents) {
 			struct commit *parent = parents->item;
 
-			if (parent->object.flags & TOPOSORT)
+			if (parent->indegree)
 				parent->indegree++;
 			parents = parents->next;
 		}
@@ -464,7 +463,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 	for (next = orig; next; next = next->next) {
 		struct commit *commit = next->item;
 
-		if (!commit->indegree)
+		if (commit->indegree == 1)
 			insert = &commit_list_insert(commit, insert)->next;
 	}
 
@@ -486,7 +485,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 		for (parents = commit->parents; parents ; parents = parents->next) {
 			struct commit *parent=parents->item;
 
-			if (!(parent->object.flags & TOPOSORT))
+			if (!parent->indegree)
 				continue;
 
 			/*
@@ -494,7 +493,8 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 			 * when all their children have been emitted thereby
 			 * guaranteeing topological order.
 			 */
-			if (!--parent->indegree) {
+			if (--parent->indegree == 1) {
+				parent->indegree = 0;
 				if (!lifo)
 					insert_by_date(parent, &work);
 				else
@@ -505,7 +505,6 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 		 * work_item is a commit all of whose children
 		 * have already been emitted. we can emit it now.
 		 */
-		commit->object.flags &= ~TOPOSORT;
 		*pptr = work_item;
 		pptr = &work_item->next;
 	}
diff --git a/revision.h b/revision.h
index fa68c65..f64e8ce 100644
--- a/revision.h
+++ b/revision.h
@@ -12,8 +12,7 @@
 #define CHILD_SHOWN	(1u<<6)
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
 #define SYMMETRIC_LEFT	(1u<<8)
-#define TOPOSORT	(1u<<9)	/* In the active toposort list.. */
-#define ALL_REV_FLAGS	((1u<<10)-1)
+#define ALL_REV_FLAGS	((1u<<9)-1)
 
 struct rev_info;
 struct log_info;
diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh
index 8d50c23..7fe4a6e 100755
--- a/t/t3202-show-branch-octopus.sh
+++ b/t/t3202-show-branch-octopus.sh
@@ -49,7 +49,7 @@ cat > expect << EOF
 +++++++++* [branch10^] initial
 EOF
 
-test_expect_failure 'show-branch with more than 8 branches' '
+test_expect_success 'show-branch with more than 8 branches' '
 
 	git show-branch $(for i in $numbers; do echo branch$i; done) > out &&
 	test_cmp expect out
-- 
1.6.0.rc0.22.gf2096d.dirty

^ permalink raw reply related

* Re: regression in  92392b4
From: Johannes Schindelin @ 2008-07-23  0:58 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Pierre Habouzit, Git ML, Junio C Hamano
In-Reply-To: <20080723004108.GB14668@spearce.org>

Hi,

On Tue, 22 Jul 2008, Shawn O. Pearce wrote:

> Reading above shows we got a "fatal: Out of memory, malloc failed" right 
> before the segfault.  What's odd is we segfaulted after we ran out of 
> memory and should have die'd.
> 
> There's at least two bugs in the above output:
> 
> a) index-pack ran out of memory on a small pull (95 KiB).
> b) fetch segfaulted when index-pack failed.
> 
> And this patch will unfortunately address neither of them.  :-|

Yeah, I thought I had something, but no matter what I tried, I could not 
find a breakage.  Not even valgrind complains.

But as I said: it was just a guess.

Maybe it will be easier to reproduce by patching the code to malloc() 
everything that is available first, and then continuing.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] sort_in_topological_order(): avoid setting a commit flag
From: Junio C Hamano @ 2008-07-23  1:01 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: pasky, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807230150480.8986@racer>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This is the same behavior as before, since indegree could not be
> non-zero when TOPOSORT was unset.
>
> Incidentally, this fixes the bug in show-branch where the 8th column
> was not shown: show-branch sorts the commits in topological order,
> assuming that all the commit flags are available for show-branch's
> private matters.

Do people still actively use show-branch as a G/CUI, especially after that
"log --graph" thing was introduced?

If that is the case, it might also make sense to stop using the object
flags but allocate necessary number of bits (not restricted to 25 or so)
pointed at by commit->util field to remove its limitation.

Hint, hint...

^ permalink raw reply

* [PATCH] git-checkout: fix argument parsing to detect ambiguous arguments.
From: Pierre Habouzit @ 2008-07-23  1:02 UTC (permalink / raw)
  To: git; +Cc: gitster, Pierre Habouzit

Note that it also fix a bug, git checkout -- <path> would be badly
understood as git checkout <branch> if <path> is ambiguous with a branch.

Testcases included.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  A user on #git happened to have issues that made me realize that
  builtin-checkout is badly broken wrt argument parseing.

  This clearly needs to be applied to master, probably to maint too.

  The patch is against next though, but should probably apply to other
  branches just fine.


 builtin-checkout.c            |    9 +++++++--
 t/t2010-checkout-ambiguous.sh |   27 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100755 t/t2010-checkout-ambiguous.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index fbd5105..1490e8e 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -438,9 +438,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 	opts.track = git_branch_track;
 
-	argc = parse_options(argc, argv, options, checkout_usage, 0);
-	if (argc) {
+	argc = parse_options(argc, argv, options, checkout_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc && strcmp(argv[0], "--")) {
 		arg = argv[0];
+
+		if (argc == 1 || strcmp(argv[1], "--"))
+			verify_non_filename(NULL, arg);
 		if (get_sha1(arg, rev))
 			;
 		else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
new file mode 100755
index 0000000..c1a86a2
--- /dev/null
+++ b/t/t2010-checkout-ambiguous.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='checkout and pathspecs/refspecs ambiguities'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	git branch world
+'
+
+test_expect_success 'branch switching' '
+	git checkout world --
+'
+
+test_expect_success 'checkout world from the index' '
+	git checkout -- world
+'
+
+test_expect_success 'ambiguous call' '
+	test_must_fail git checkout world
+'
+
+test_done
-- 
1.6.0.rc0.154.g60644

^ permalink raw reply related

* Re: regression in  92392b4
From: Pierre Habouzit @ 2008-07-23  1:09 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Schindelin, Git ML, Junio C Hamano
In-Reply-To: <20080723004108.GB14668@spearce.org>

[-- Attachment #1: Type: text/plain, Size: 3351 bytes --]

On Wed, Jul 23, 2008 at 12:41:08AM +0000, Shawn O. Pearce wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Wed, 23 Jul 2008, Pierre Habouzit wrote:
> > 
> > >   Hi, here is a manual painful down-secting (opposed to a bisect ;P) I
> > > did, since git in next cannot fetch on a regular basis for me. The
> > > culprit seems to be commit  92392b4:
> > > 
> > >     ┌─(1:11)──<~/dev/scm/git 92392b4....>──
> > >     └[artemis] git fetch
> > >     remote: Counting objects: 461, done.
> > >     remote: Compressing objects: 100% (141/141), done.
> > >     remote: Total 263 (delta 227), reused 155 (delta 121)
> > >     Receiving objects: 100% (263/263), 95.55 KiB, done.
> > >     fatal: Out of memory, malloc failed
> > >     fatal: index-pack failed
> > >     [2]    16674 abort (core dumped)  git fetch
> ....
> > 
> > Just a guess:
> ....
> > diff --git a/index-pack.c b/index-pack.c
> > index ac20a46..19c39e5 100644
> > --- a/index-pack.c
> > +++ b/index-pack.c
> > @@ -257,6 +257,7 @@ static void unlink_base_data(struct base_data *c)
> >  		base_cache = NULL;
> >  	if (c->data) {
> >  		free(c->data);
> > +		c->data = NULL;
> >  		base_cache_used -= c->size;
> >  	}
> >  }
> 
> Oh.  This is a pointless assignment.  If you look at any call sites
> for unlink_base_data() you will find that the struct passed in as
> "c" here is going out of scope after unlink_base_data() returns.  In
> no such case does the value of c->data get tested once this free is
> complete.
> 
> We need the if (c->data) guard because we only want to decrement
> base_cache_used if the memory is still allocated.  It may have been
> released earlier, in which case base_cache_used has already been
> decreased and we don't want to double-decrement it.
> 
> This patch makes the code more obvious, so Ack I guess, but it is
> not a solution to Pierre's woes.  Something else is wrong.
> 
> Reading above shows we got a "fatal: Out of memory, malloc failed"
> right before the segfault.  What's odd is we segfaulted after we
> ran out of memory and should have die'd.
> 
> There's at least two bugs in the above output:
> 
> a) index-pack ran out of memory on a small pull (95 KiB).
> b) fetch segfaulted when index-pack failed.
> 
> And this patch will unfortunately address neither of them.  :-|
> 
> I've had a long past couple of days, and another one tomorrow.
> I'm not going to be able to debug this myself until perhaps Thursday
> or Friday.  Sorry.  If nobody beats me to it, I will put this on
> the top of the pile and try to fix it once I get back online at my
> new home.

  Like I said, I had a core that I stupidly lost, but I remember that
the broken malloc was:


    static void *get_data_from_pack(struct object_entry *obj)
    {
	    off_t from = obj[0].idx.offset + obj[0].hdr_size;
	    unsigned long len = obj[1].idx.offset - from;
	    unsigned long rdy = 0;
	    unsigned char *src, *data;
	    z_stream stream;
	    int st;

	    src = xmalloc(len);
            ^^^^^^^^^^^^^^^^^^

  len was horribly big, and outputing obj[1].idx showed that `sha1` had
text in it. I mean something like "could not\r\n     han" IIRC.

  I don't remember the rest of the backtrace, and have stupidly not kept
any ways of reproducing it.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Johannes Schindelin @ 2008-07-23  1:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy73tihl6.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 22 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When a file's crlf attribute is explicitely set, it does not make 
> > sense to ignore it, just because the config variable core.autocrlf has 
> > not been set.
> 
> I am not sure if I agree with that reasoning.
> 
> Attribute defines what each path is.  Is it a text file, is it a binary? 
> The nature of the contents does not change between people on POSIX and 
> Windows, and that is why it is described in .gitattributes and cloned 
> across repositories.
> 
> On the other hand, the configuration defines what to do with contents with
> various attributes in this particular repository.  Do I want to see a text
> file checked out with CRLF endings, or LF?

Actually, I now see that I expressed myself badly.  Extremely badly.

The whole issue is about _check in_, as can be seen from the test case I 
provided.

And I think it is even a bug in crlf handling, as gitattributes.txt has 
this to say about crlf=input:

        This is similar to setting the attribute to `true`, but
        also forces git to act as if `core.autocrlf` is set to
        `input` for the path.

It suggests to this coder that core.autocrlf is not even looked at when 
crlf=input.

> So it is perfectly valid and normal for a cross-platform minded project 
> to use the crlf atttribute to say "These files are text" and expect them 
> to be checked out with LF endings on POSIX while making sure they are 
> checked out with CRLF on Windows.  Adding CR at the end of line for such 
> files on POSIX systems is positively a wrong thing to do in such a case.
> 
> Projects like the kernel that originate from LF side of the world may not
> bother marking things as such, though.

I fully agree.

However, if you want to avoid CRs to _enter_ the repository, when you have 
a lot of binary files tracked, you _do_ want to force all repositories to 
crlf=input.

Now, if you look at the patch, you will see that it _only_ touches 
crlf_to_git(), but not only for crlf=input, but also for crlf=true.

I maintain that this respects the law of least surprise, namely that if 
you set the attribute crlf=true, and some person forgets to set 
core.autocrlf=true, at _check in_ CRs are stripped, but at _check out_, no 
CR is added (as the person did not ask for core.autocrlf, but that should 
not punish all the others who do not want to have CRs in the repository).

But yes, the commit message, and the oneline in particular are severely 
lacking.

Tomorrow,
Dscho

^ permalink raw reply

* Re: [PATCH] git-checkout: fix argument parsing to detect ambiguous arguments.
From: Pierre Habouzit @ 2008-07-23  1:13 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <1216774940-4955-1-git-send-email-madcoder@debian.org>

[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]

On Wed, Jul 23, 2008 at 01:02:20AM +0000, Pierre Habouzit wrote:
> Note that it also fix a bug, git checkout -- <path> would be badly
> understood as git checkout <branch> if <path> is ambiguous with a branch.
> 
> Testcases included.
> 
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> 
>   A user on #git happened to have issues that made me realize that
>   builtin-checkout is badly broken wrt argument parseing.
> 
>   This clearly needs to be applied to master, probably to maint too.
> 
>   The patch is against next though, but should probably apply to other
>   branches just fine.
> 
> 
>  builtin-checkout.c            |    9 +++++++--
>  t/t2010-checkout-ambiguous.sh |   27 +++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 2 deletions(-)
>  create mode 100755 t/t2010-checkout-ambiguous.sh
> 
> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index fbd5105..1490e8e 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -438,9 +438,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  
>  	opts.track = git_branch_track;
>  
> -	argc = parse_options(argc, argv, options, checkout_usage, 0);
> -	if (argc) {
> +	argc = parse_options(argc, argv, options, checkout_usage,
> +			     PARSE_OPT_KEEP_DASHDASH);
> +
> +	if (argc && strcmp(argv[0], "--")) {
>  		arg = argv[0];
> +
> +		if (argc == 1 || strcmp(argv[1], "--"))
> +			verify_non_filename(NULL, arg);
>  		if (get_sha1(arg, rev))
>  			;
>  		else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
> diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
> new file mode 100755
> index 0000000..c1a86a2
> --- /dev/null
> +++ b/t/t2010-checkout-ambiguous.sh
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +
> +test_description='checkout and pathspecs/refspecs ambiguities'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	echo hello >world &&
> +	git add world &&
> +	git commit -m initial &&
> +	git branch world
> +'
> +
> +test_expect_success 'branch switching' '
> +	git checkout world --
> +'
> +
> +test_expect_success 'checkout world from the index' '
> +	git checkout -- world
> +'

  Okay those two tests are stupid in the sense that they don't check
git-checkout does what it's supposed to do. One should check the first
one outputs 'Switched to branch "world"'

  and the second should rather be:

'
  echo "bye bye" > world &&
  git checkout -- world &&
  git diff --quiet --exit-code
'


[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Stephan Beyer @ 2008-07-23  1:13 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Olivier Marin, Theodore Tso, Nanako Shiraishi,
	Johannes Schindelin, René Scharfe, Joe Fiorini, git,
	Jari Aalto
In-Reply-To: <7vbq0pifwq.fsf@gitster.siamese.dyndns.org>

Hi,

Junio C Hamano wrote:
> Stephan Beyer <s-beyer@gmx.net> writes:
> > Junio C Hamano wrote:
> >> Olivier Marin <dkr+ml.git@free.fr> writes:
> >> > @@ -203,9 +204,10 @@ then
> >> >  
> >> >  	case "$abort" in
> >> >  	t)
> >> > -		rm -fr "$dotest" &&
> >> > +		git rerere clear &&
> >> >  		git read-tree -m -u ORIG_HEAD &&
> > [...]
> >> diff --git a/git-am.sh b/git-am.sh
> >> index a44bd7a..5cbf8f4 100755
> >> --- a/git-am.sh
> >> +++ b/git-am.sh
> >> @@ -203,9 +203,9 @@ then
> >>  
> >>  	case "$abort" in
> >>  	t)
> >> -		rm -fr "$dotest" &&
> >> -		git read-tree -m -u ORIG_HEAD &&
> >> -		git reset ORIG_HEAD && :
> >> +		git rerere clear
> >> +		git read-tree --reset -u HEAD ORIG_HEAD
> >
> > Perhaps I am confused, but ...
> > Why is there "HEAD" and "ORIG_HEAD" and not only "ORIG_HEAD"?
> 
> Just being a bit defensive -- in this case I think it might be Ok to say
> "read-tree --reset -u ORIG_HEAD", but I haven't checked in a conflicted
> case.

Well, the test suite fails:
* FAIL 4: am --abort goes back after failed am

                        git-am --abort &&
                        git rev-parse HEAD >actual &&
                        git rev-parse initial >expect &&
                        test_cmp expect actual &&
  here>                 test_cmp file-2-expect file-2 &&
  ...                   git diff-index --exit-code --cached HEAD &&
                        test ! -f .git/rr-cache/MERGE_RR

* FAIL 7: am --abort goes back after failed am -3

                        git-am --abort &&
                        git rev-parse HEAD >actual &&
                        git rev-parse initial >expect &&
                        test_cmp expect actual &&
 and here>              test_cmp file-2-expect file-2 &&
                        git diff-index --exit-code --cached HEAD &&
                        test ! -f .git/rr-cache/MERGE_RR

So no reason to be defensive ;)

> If some path was added between ORIG_HEAD (that is where we started from)
> and HEAD (that is where we are and we decide we do not want it), and that
> path is conflicted in the index, a single tree form "read-tree --reset -u
> HEAD" would leave it behind in the working tree, wouldn't it?

Seems so.

The reason of my question was that I *blindly* incorporated the change into
sequencer to make it able to work on a dirty working tree and thus to be
able to migrate am onto it without losing the ability to apply patches
on a dirty working tree....
All am tests applied afterwards, but the sequencer and the rebase-i
test suite failed in a place where I didn't expect it. I *then* had
a deeper look at the read-tree line and I was wondering what the "HEAD"
should achieve.
I removed it and all tests passed. (I didn't have t4151 in my branch
at that point.)

Now, because t4151 does not pass, I am wondering what's the best thing
I could do...

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* Re: [PATCH] git-checkout: fix argument parsing to detect ambiguous arguments.
From: Johannes Schindelin @ 2008-07-23  1:17 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git, gitster
In-Reply-To: <1216774940-4955-1-git-send-email-madcoder@debian.org>

Hi,

On Wed, 23 Jul 2008, Pierre Habouzit wrote:

> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index fbd5105..1490e8e 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -438,9 +438,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  
>  	opts.track = git_branch_track;
>  
> -	argc = parse_options(argc, argv, options, checkout_usage, 0);
> -	if (argc) {
> +	argc = parse_options(argc, argv, options, checkout_usage,
> +			     PARSE_OPT_KEEP_DASHDASH);
> +
> +	if (argc && strcmp(argv[0], "--")) {
>  		arg = argv[0];
> +
> +		if (argc == 1 || strcmp(argv[1], "--"))
> +			verify_non_filename(NULL, arg);

Why "argc == 1"?  Should "git checkout <path>" really fail?  That would be 
a change in behavior that _would_ hit me.

However, you may want to verify_non_filename() when argc == 1 _and_ 
get_sha1() succeeded.  Because then, <path> is ambiguous.

Ciao,
Dscho

^ permalink raw reply

* Re: regression in  92392b4
From: Johannes Schindelin @ 2008-07-23  1:20 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Shawn O. Pearce, Git ML, Junio C Hamano
In-Reply-To: <20080723010928.GG11831@artemis.madism.org>

Hi,

On Wed, 23 Jul 2008, Pierre Habouzit wrote:

> I had a core that I stupidly lost, but I remember that the broken malloc 
> was:
> 
> 
>     static void *get_data_from_pack(struct object_entry *obj)
>     {
> 	    off_t from = obj[0].idx.offset + obj[0].hdr_size;
> 	    unsigned long len = obj[1].idx.offset - from;
> 	    unsigned long rdy = 0;
> 	    unsigned char *src, *data;
> 	    z_stream stream;
> 	    int st;
> 
> 	    src = xmalloc(len);
>             ^^^^^^^^^^^^^^^^^^
> 
>   len was horribly big, and outputing obj[1].idx showed that `sha1` had
> text in it. I mean something like "could not\r\n     han" IIRC.
> 
>   I don't remember the rest of the backtrace, and have stupidly not kept
> any ways of reproducing it.

That would not have helped.  The memory corruption almost _certainly_ took 
place way before that.

Ciao,
Dscho

^ permalink raw reply

* [RFC] Git User's Survey 2008
From: Jakub Narebski @ 2008-07-23  1:25 UTC (permalink / raw)
  To: git; +Cc: Stephan Beyer

It's been around a year since last Git User's Survey.  It would be
interesting to find what changed since then.  For example to see if 
there were visible improvements in easing learning curve and in 
usability.  Therefore the idea to have another survey.

(If there is no suck^W volunteer to create survey, announce it, and 
finally summarize results and publish summary on Git Wiki, I would do 
the 2008 survey.)


First there is a question about the form of survey. Should we use web
based survey, as the survey before (http://www.survey.net.nz), sending
emails with link to this survey, or perhaps do email based survey,
with email Reply-To: address put for this survey alone?  Should we use 
the same web survey service as before (found by Paolo Ciarrocchi for 
first, 2006 survey, and used also for 2007 survey), or is there one 
better (it would better be free, and without limitations on the number 
of responses; in 2006 there were around 117 responses, in 2007 there 
were 683 individual responses).

Second, what questions should be put in the survey, and in the case of
single choice and ultiple choice questions what possible answers
should be?  I'd rather avoid free-form questions, even if they are more 
interesting, as they are PITA to analyse and summarize, especially to 
create some kind of histogram from free-form replies data (some of 2007 
free-form responses are not fully summarized even now).  Below are 
slightly extended questions from the last survey.  Please comment on 
it.

Third, where to send survey to / where to publish information about the 
survey?  Last year the announcement was send to git mailing list, to
LKML (Linux kernel mailing list), and mailing list for git projects 
found on GitProjects page on GIT wiki.  Now that the number of projects 
using Git as version control system has grown, I don't think it would 
be good idea to "spam" all those mailing list; and if we don't send 
notice to all other projects I'm not sure if we should include LKML.
Last year survey announcement was put on Git Homepage (thanks Pasky), 
and on front page of Git Wiki; info about survey was also put on two 
git hosting sites: kernel.org and repo.or.cz.  It would be nice if it 
was possible to put announcement about Git User's Survey 2008 at front 
pages of other Git hosting sites, like GitHub (one of most popular, I 
think), Gitorious, Freedesktop.org.  If you know some other popular Git 
hosting sites, and even better if you know who to contact about putting 
survey announcement, please write.  Is there some channel that I have 
forgot about?  Should info/announcement about Git User's Survey 2008 be 
sent also to one of on-line magazines like LinuxToday or LWN, or asked 
to put on some blog?  I'll add it as journal entry for #git on Ohloh, 
and try to make it so it would appear in "News" section for Ohloh 
project page for Git: http://www.ohloh.net/projects/git.

Fourth, how long should the survey last?  When sending announcement we 
should say where notice about Git User's Survey 2008 should be taken 
down.  Last year the survey was meant to take three weeks, but was up 
longer.


Below there is initial version of announcement email (I should probably 
come up also with boilerplate announcement for web pages), and initial 
version of this year round of questions.  Comments are prefixed by '+',
and does not mean to be included in the survey text.

----
Hi all,

We would like to ask you a few questions about your use of the Git
version control system. This survey is mainly to understand who is
using Git, how and why.

The results will be published to the Git wiki and discussed on the git
mailing list.

We'll close the survey in <duration> starting from today, <date>.

Please devote a few minutes of your time to fill this simple
questionnaire, it will help a lot the git community to understand your
needs, what you like of git, and of course what you don't like  of it.

The survey can be found here:
  <survey URL>
----
About you

   01. What country are you in?
   02. What is your preferred non-programming language?
  (or) What is the language you want computer communicate with you?
   03. How old are you (in years)?
       (free form, integer)
   04. Which programming languages you are proficient with?
       (The choices include programming languages used by git)
       (zero or more: multiple choice)
     - C, shell, Perl, Python, Tcl/Tk
     + (should we include other languages, like C++, Java, PHP,
        Ruby,...?)


Getting started with GIT

   05. How did you hear about Git?
       (single choice?, in 2007 it was free-form)
     - Linux kernel news (LKML, LWN, KernelTrap, KernelTraffic,...),
       news site or magazine, blog entry, some project uses it,
       presentation or seminar (real life, not on-line), SCM research,
       IRC, mailing list, other Internet, other off-line, other(*)
     + the problem is with having not very long list (not too many
       choices), but spanning all possibilities.
     + is this question interesting/important to have in survey?
   06. Did you find GIT easy to learn?
     - very easy/easy/reasonably/hard/very hard
   07. What helped you most in learning to use it?
       (free form question)
   08. What did you find hardest in learning Git?
       What did you find harderst in using Git?
       (free form question)
   09. When did you start using git? From which version?
     - pre 1.0, 1.0, 1.1, 1.2, 1.3, 1.4, 1.5
     + might be important when checking "what did you find hardest" etc.
     + perhaps we should ask in addition to this question, or in place
       of this question (replacing it) what git version one uses; it
       should be multiple choice, and allow 'master', 'next', 'pu',
       'dirty (with own modifications)' versions in addition.


Other SCMs (shortened compared with 2007 survey)

   10. What other SCM did or do you use?
       (zero or more: multiple choice)
     - CVS, Subversion, GNU Arch or arch clone (ArX, tla, ...),
       Bazaar-NG, Darcs, Mercurial, Monotone, SVK, AccuRev, Perforce,
       BitKeeper, ClearCase, MS Visual Source Safe, MS Visual Studio
       Team System, custom, other(*)
   10b.If you selected other above, what SCM it was?
       (free form)
   11. Why did you choose Git? (if you use Git)
       What do you like about using Git?
       (free form, not to be tabulated)
   12. Why did you choose other SCMs? (if you use other SCMs)
       What do you like about using other SCMs?
       Note: please write name of SCMs you are talking about.
       (free form, not to be tabulated).


How you use Git

   13. Do you use Git for work, unpaid projects, or both?
       (single choice)
     - work/unpaid projects/both
   14. How do you obtain Git?
     - binary package/source package or source script(*)/
       source tarball/pull from main repository
       (*) this includes source based distributions like Gentoo
     + added new option: source package or source script
     + should this be multiple choice?
   15. What operating system do you use Git on?
       (one or more: multiple choice, as one can use more than one OS)
     - Linux, *BSD (FreeBSD, OpenBSD, etc.), MS Windows/Cygwin,
       MS Windows/msysGit, MacOS X, other UNIX, other
     + "What hardware platforms do you use GIT on?" question was
       removed; should it stay?
   15b.If you selected "other UNIX", or "other", what operating system
       or systems it was/were?
       (free form)
   16. Which porcelains / interfaces / implementations do you use?
       (zero or more: multiple choice)
     - core-git, Cogito (deprecated), StGIT, Guilt, pg (deprecated),
       Pyrite, Easy Git, IsiSetup, jgit, my own scripts, other
   16b.If you selected "other porcelain", what is its name?
       (free form)
   17. Which git GUI (commit tool or history viewer, or both) do you use
       (zero or more: multiple choice)
     - CLI, gitk, git-gui, qgit, gitview, giggle, tig, instaweb,
       (h)gct, qct, KGit, git-cola / ugit, GitNub, Pyrite, git.el, other
   17b.If you selected "other GUI", what is its name?
       (free form)
   18. Which (main) git web interface do you use for your projects?
       (zero or more: multiple choice)
     - gitweb, cgit, wit (Ruby), git-php, viewgit (PHP), other
     + should there be a question about web server (Apache, IIS, ...)
       used to host git web interface?
   18b.If you selected "other web interface", what it was?
       (free form)
   19. How do you publish/propagate your changes?
       (zero or more: multiple choice)
     - push, pull request, format-patch + email, bundle, other
   19b.If the way you publish your changes is not mentioned above, how
       do you publish your changes?
       (free form)
   20. Does git.git repository include code produced by you?
     - yes/no


What you think of Git

   21. Overall, how happy are you with Git?
     - unhappy/not so happy/happy/very happy/completely ecstatic
   22. How does Git compare to other SCM tools you have used?
     - worse/equal (or comparable)/better
   23. What would you most like to see improved about Git?
       (features, bugs, plug-ins, documentation, ...)
   24. If you want to see Git more widely used, what do you
       think we could do to make this happen?
     + Is this question necessary/useful?  Do we need wider adoption?


Changes in Git (since year ago, or since you started using it)

   25. Did you participate in previous Git User's Surveys?
       (zero or more, multiple choice)
     - 2006, 2007
   26. How do you compare current version with version from year ago?
     - current version is: better/worse/no changes
   27. Which of the following features do you use?
       (zero or more: multiple choice)
     - git-gui or other commit tool, gitk or other history viewer, patch
       management interface (e.g. StGIT), bundle, eol conversion,
       gitattributes, submodules, separate worktree, reflog, stash,
       shallow clone, detaching HEAD, mergetool, interactive rebase,
       add --interactive or other partial commit helper, commit
       templates, bisect, other (not mentioned here)
     + should probably be sorted in some resemblance of order
     + are there any new features which should be listed here?
   28. If you use some important Git features not mentioned above,
       what are it?
       (free form)


Documentation

   29. Do you use the Git wiki?
    -  yes/no
   30. Do you find Git wiki useful?
    -  yes/no/somewhat
   31. Do you contribute to Git wiki?
    -  yes/no/only corrections or spam removal
   32. Do you find Git's on-line help (homepage, documentation) useful?
    -  yes/no/somewhat
   33. Do you find help distributed with Git useful
       (manpages, manual, tutorial, HOWTO, release notes)?
    -  yes/no/somewhat
   34. What could be improved on the Git homepage?
       (free form)
   35. What could be improved in Git documentation?
       (free form)


Getting help, staying in touch

   36. Have you tried to get Git help from other people?
    -  yes/no
   37. What channel did you use to request help?
       (zero or more: multiple choice)
    -  git mailing list, git users group, IRC, blog post, 
       other
    +  this is one question which doesn't need, I think, explanation
       of "other" choice
   38. If yes, did you get these problems resolved quickly
       and to your liking?
    -  yes/no
   39. Would commerical (paid) support from a support vendor
       be of interest to you/your organization?
    -  yes/no/not applicable
   40. Do you read the mailing list?
    -  yes/no
   41. If yes, do you find it useful?
    -  yes/no (optional)
   42. Do you find traffic levels on Git mailing list OK.
    -  yes/no? (optional)
   43. Do you use the IRC channel (#git on irc.freenode.net)?
    -  yes/no
   44. If yes, do you find IRC channel useful?
    -  yes/no (optional)
   45. Did you have problems getting GIT help on mailing list or
       on IRC channel? What were it? What could be improved?
       (free form)

Open forum

   46. What other comments or suggestions do you have that are not
       covered by the questions above?
       (free form)

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [RESEND PATCH] git-checkout: fix argument parsing to detect ambiguous arguments.
From: Pierre Habouzit @ 2008-07-23  1:27 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <1216774940-4955-1-git-send-email-madcoder@debian.org>

[-- Attachment #1: Type: text/plain, Size: 3041 bytes --]

Note that it also fix a bug, git checkout -- <path> would be badly
understood as git checkout <branch> if <path> is ambiguous with a branch.

Testcases included.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  This is a resend with proper patches that made me realize that my
previous patch had silly mistakes and wasn't testing thigns properly.

 builtin-checkout.c            |   23 +++++++++++++++++------
 t/t2010-checkout-ambiguous.sh |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100755 t/t2010-checkout-ambiguous.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index fbd5105..97321e6 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -438,12 +438,17 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 	opts.track = git_branch_track;
 
-	argc = parse_options(argc, argv, options, checkout_usage, 0);
-	if (argc) {
+	argc = parse_options(argc, argv, options, checkout_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc && strcmp(argv[0], "--")) {
+		int may_be_ambiguous = argc == 1 || strcmp(argv[1], "--");
+
 		arg = argv[0];
-		if (get_sha1(arg, rev))
-			;
-		else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
+		if (get_sha1(arg, rev)) {
+			if (may_be_ambiguous)
+				verify_filename(NULL, arg);
+		} else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
 			new.name = arg;
 			setup_branch_path(&new);
 			if (resolve_ref(new.path, rev, 1, NULL))
@@ -454,10 +459,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			source_tree = new.commit->tree;
 			argv++;
 			argc--;
+			if (may_be_ambiguous)
+				verify_non_filename(NULL, arg);
 		} else if ((source_tree = parse_tree_indirect(rev))) {
 			argv++;
 			argc--;
-		}
+			if (may_be_ambiguous)
+				verify_non_filename(NULL, arg);
+		} else
+			if (may_be_ambiguous)
+				verify_filename(NULL, arg);
 	}
 
 	if (argc && !strcmp(argv[0], "--")) {
diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
new file mode 100755
index 0000000..389ba8c
--- /dev/null
+++ b/t/t2010-checkout-ambiguous.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='checkout and pathspecs/refspecs ambiguities'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	echo hello >all &&
+	git add all world &&
+	git commit -m initial &&
+	git branch world
+'
+
+test_expect_success 'branch switching' '
+	test "refs/heads/master" = "$(git symbolic-ref HEAD)" &&
+	git checkout world -- &&
+	test "refs/heads/world" = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success 'checkout world from the index' '
+	echo bye > world &&
+	git checkout -- world &&
+	git diff --exit-code --quiet
+'
+
+test_expect_success 'non ambiguous call' '
+	git checkout all
+'
+
+test_expect_success 'ambiguous call' '
+	test_must_fail git checkout world
+'
+
+test_done
-- 
1.6.0.rc0.154.ge2a39.dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply related

* [PATCH] Respect crlf attribute in "git add" even if core.autocrlf has not been set
From: Johannes Schindelin @ 2008-07-23  1:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Dmitry Potapov
In-Reply-To: <alpine.DEB.1.00.0807230203350.8986@racer>


When a file's crlf attribute is explicitely set, it does not make sense
to ignore it when adding the file, just because the config variable
core.autocrlf has not been set.

The alternative would be risking to get CR/LF files into the repository
just because one user forgot to set core.autocrlf.

This patch does not affect the case when the crlf attribute is unset,
or when checking files out.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Okay, so I lied and did not go to sleep (but that part comes 
	now).  Only the wording in the commit message has changed.

 convert.c       |    2 +-
 t/t0020-crlf.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/convert.c b/convert.c
index 78efed8..d038d2f 100644
--- a/convert.c
+++ b/convert.c
@@ -126,7 +126,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 	struct text_stat stats;
 	char *dst;
 
-	if ((action == CRLF_BINARY) || !auto_crlf || !len)
+	if ((action == CRLF_BINARY) || (!auto_crlf && action < 0) || !len)
 		return 0;
 
 	gather_stats(src, len, &stats);
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 1be7446..0bb3e6f 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -436,4 +436,14 @@ test_expect_success 'invalid .gitattributes (must not crash)' '
 
 '
 
+test_expect_success 'attribute crlf is heeded even without core.autocrlf' '
+
+	echo "allcrlf crlf=input" > .gitattributes &&
+	git config --unset core.autocrlf &&
+	git add allcrlf &&
+	git show :allcrlf | append_cr > expect &&
+	test_cmp allcrlf expect
+
+'
+
 test_done
-- 
1.6.0.rc0.22.gf2096d.dirty

^ 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