Git development
 help / color / mirror / Atom feed
* Re: [PATCH Cogito] Make use of external editor work like CVS
From: Petr Baudis @ 2005-05-08 17:12 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: GIT Mailing List
In-Reply-To: <1115568937.9031.129.camel@pegasus>

Dear diary, on Sun, May 08, 2005 at 06:15:36PM CEST, I got a letter
where Marcel Holtmann <marcel@holtmann.org> told me that...
> Hi Petr,

Hi,

> What do you think about a special flag for automatic merging (which
> makes the commit message say "Automatic merge") and a .cogitorc file
> like .cvsrc where you can choose the default method.
> 
> I am using a lot of temporary trees where I pull a lot of kernel
> subsystems together and I don't need that "feature" there.

No problem with that per se, but please keep the configfile
infrastructure and the automerge switch as separate patches from this
one.

> > > This is only cosmetic. Using vim it displays the name of the temporary
> > > file and confusing the user with gitci2.XXXX instead of gitci.XXX is
> > > weird. Even using gitci as basename looks not good to me, but I left it
> > > for now.
> > 
> > It boosts the patch size unnecessarily. It shouldn't be called gitci2
> > anyway... :-) Feel free to change the mktemp templates instead.
> 
> I will check what I can do, but I don't really care that much about the
> patch size ;)

But I do. :-)

> > The gitci name comes all the way from the times where this command was
> > usually triggered by 'git ci'.
> 
> I thought so. Is using cogito.XXXXXX and cogito.temp.XXXXX fine with
> you?

No. I think it's useful (and doesn't cost us anything) to have the
"owner" of the file denoted in the filename.

> > > Index: cg-commit
> > > ===================================================================
> > > --- f00d7589973e8ea65d2264f5fbac82e1b217dc8f/cg-commit  (mode:100755)
> > > +++ cb61efa8a01400150162af9b0f3773f21d502fe9/cg-commit  (mode:100755)
> > > @@ -94,30 +78,55 @@
> > >  		echo "$uri" >>$LOGMSG
> > >  		[ "$msgs" ] && echo "$uri"
> > >  	done
> > > -	echo >>$LOGMSG
> > > +else
> > > +	first=1
> > >  fi
> > > -first=1
> > > +
> > >  for msg in "${msgs[@]}"; do
> > >  	if [ "$first" ]; then
> > >  		first=
> > >  	else
> > >  		echo >>$LOGMSG
> > >  	fi
> > > -	echo $msg | fmt >>$LOGMSG
> > > +	echo $msg | fmt -s -w 74 >>$LOGMSG
> > >  done
> > > +
> > > +if [ "$first" ]; then
> > > +	echo >>$LOGMSG
> > > +fi
> > 
> > This mess is still here.
> 
> That is not mess. Think about it. If we have messages provided by -m we
> want an empty line between the merge message and the the first commit
> message. And we don't wanna have an extra empty line at the top if you
> provide a commit messages via -m.

But, that's the current behaviour, isn't it?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: Broken adding of cache entries
From: Petr Baudis @ 2005-05-08 16:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kay Sievers, git
In-Reply-To: <7vll6q70mg.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sun, May 08, 2005 at 07:22:31AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> >>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes:
> 
> JCH> I am not quite sure the semantics is quite right, so I am
> JCH> holding it off from putting it in git-jc repository for now, but
> JCH> please review it, give it a try and tell me what you think.
> 
> Ok, I have two updates pushed out at git-jc archive at
> http://members.cox.net/junkio/git-jc.git/

I wanted to merge with you, but I'd like to make some (mostly minor)
nitpicks first.

Sorry for totally garbled whitespace and everything, this is just gpm.

--- e19665d8d42877246ac7e98a7c671d11adbe8b56/read-cache.c  (mode:100644)
+++ uncommitted/read-cache.c  (mode:100644)
+               char *ep = strchr(cp, '/');
+               if (ep == 0)

Please use NULL.

+static struct alternate_object_database
+{
+       char *base;
+       char *name;
+} *alt_odb;

The commonly accepted style is to have the { bracket on the same line as the
struct identifier.

Sticking a brief explanation to prepare_alt_odb(), like "pass 0
allocates the array and pass 1 fills it" couldn't hurt, it took me a
while of staring at it to figure out. There's also unused @buf variable.
But I personally think all this alt_odb code is quite creepy. ;-)

--- e19665d8d42877246ac7e98a7c671d11adbe8b56/write-tree.c  (mode:100644)
+++ uncommitted/write-tree.c  (mode:100644)
-                       if (++unmerged > 10) {
+                       if (10 < ++funny) {

Do those changes make any sense? The former version is certainly much
easier to read for me. I can live with it in the new code, but changing
old code to it...

With the current update-cache protections, how can you legally achieve a
cache with duplicate entries, so that you need to check for that in
write-tree?

Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: [PATCH Cogito] Make use of external editor work like CVS
From: Marcel Holtmann @ 2005-05-08 16:15 UTC (permalink / raw)
  To: Petr Baudis; +Cc: GIT Mailing List
In-Reply-To: <20050508155656.GV9495@pasky.ji.cz>

Hi Petr,

> > > What is so special about 74 columns? Why not 75 (fmt default), or 72
> > > (emails)?
> > 
> > I ended up with 74, because "CG" has only two letters instead of "CVS"
> > which has three. And cg-log uses a prefix of four whitespaces. This
> > leaves two free characters at the end of a line if your terminal uses a
> > width of 80 characters. The decision was of cosmetic nature.
> 
> Isn't one free character enough?  I'll just stay with 75. :-)

I think it looks a little bit squeezed, but I don't mind at all. Maybe
using 72 is a good idea. However it is only cosmetic and I can change it
to use the fmt default.

> > > Also, I'd prefer the empty line to be always there in front of the CG:
> > > stuff (two empty lines in case of merge - I want to encourage people to
> > > keep possible details w.r.t. the merge separated by an empty line from
> > > the merge information), and when reading it back cg-commit should strip
> > > any trailing empty lines.
> > 
> > I think we should differentiate between the merges. There is no need for
> > additional information if it is an automatic merge (no conflicts) and in
> > general it makes no sense to open the editor (until forced). I wanted to
> > address this later. And yes in case of a manual merge it is a good idea
> > to add two extra empty lines at the top.
> 
> Not so. I frequently write a brief summary of what I'm actually merging.
> I'm not forcing you to do so too, but I personally think it's a good
> idea, and want to do it in the future too. :-)

What do you think about a special flag for automatic merging (which
makes the commit message say "Automatic merge") and a .cogitorc file
like .cvsrc where you can choose the default method.

I am using a lot of temporary trees where I pull a lot of kernel
subsystems together and I don't need that "feature" there.

> > This is only cosmetic. Using vim it displays the name of the temporary
> > file and confusing the user with gitci2.XXXX instead of gitci.XXX is
> > weird. Even using gitci as basename looks not good to me, but I left it
> > for now.
> 
> It boosts the patch size unnecessarily. It shouldn't be called gitci2
> anyway... :-) Feel free to change the mktemp templates instead.

I will check what I can do, but I don't really care that much about the
patch size ;)

> The gitci name comes all the way from the times where this command was
> usually triggered by 'git ci'.

I thought so. Is using cogito.XXXXXX and cogito.temp.XXXXX fine with
you?

> > Index: cg-commit
> > ===================================================================
> > --- f00d7589973e8ea65d2264f5fbac82e1b217dc8f/cg-commit  (mode:100755)
> > +++ cb61efa8a01400150162af9b0f3773f21d502fe9/cg-commit  (mode:100755)
> > @@ -94,30 +78,55 @@
> >  		echo "$uri" >>$LOGMSG
> >  		[ "$msgs" ] && echo "$uri"
> >  	done
> > -	echo >>$LOGMSG
> > +else
> > +	first=1
> >  fi
> > -first=1
> > +
> >  for msg in "${msgs[@]}"; do
> >  	if [ "$first" ]; then
> >  		first=
> >  	else
> >  		echo >>$LOGMSG
> >  	fi
> > -	echo $msg | fmt >>$LOGMSG
> > +	echo $msg | fmt -s -w 74 >>$LOGMSG
> >  done
> > +
> > +if [ "$first" ]; then
> > +	echo >>$LOGMSG
> > +fi
> 
> This mess is still here.

That is not mess. Think about it. If we have messages provided by -m we
want an empty line between the merge message and the the first commit
message. And we don't wanna have an extra empty line at the top if you
provide a commit messages via -m.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH Cogito] Make use of external editor work like CVS
From: Petr Baudis @ 2005-05-08 15:56 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: GIT Mailing List
In-Reply-To: <1115566990.9031.108.camel@pegasus>

Dear diary, on Sun, May 08, 2005 at 05:43:10PM CEST, I got a letter
where Marcel Holtmann <marcel@holtmann.org> told me that...
> Hi Petr,

Hi,

> > What is so special about 74 columns? Why not 75 (fmt default), or 72
> > (emails)?
> 
> I ended up with 74, because "CG" has only two letters instead of "CVS"
> which has three. And cg-log uses a prefix of four whitespaces. This
> leaves two free characters at the end of a line if your terminal uses a
> width of 80 characters. The decision was of cosmetic nature.

Isn't one free character enough?  I'll just stay with 75. :-)

> > Also, I'd prefer the empty line to be always there in front of the CG:
> > stuff (two empty lines in case of merge - I want to encourage people to
> > keep possible details w.r.t. the merge separated by an empty line from
> > the merge information), and when reading it back cg-commit should strip
> > any trailing empty lines.
> 
> I think we should differentiate between the merges. There is no need for
> additional information if it is an automatic merge (no conflicts) and in
> general it makes no sense to open the editor (until forced). I wanted to
> address this later. And yes in case of a manual merge it is a good idea
> to add two extra empty lines at the top.

Not so. I frequently write a brief summary of what I'm actually merging.
I'm not forcing you to do so too, but I personally think it's a good
idea, and want to do it in the future too. :-)

> This is only cosmetic. Using vim it displays the name of the temporary
> file and confusing the user with gitci2.XXXX instead of gitci.XXX is
> weird. Even using gitci as basename looks not good to me, but I left it
> for now.

It boosts the patch size unnecessarily. It shouldn't be called gitci2
anyway... :-) Feel free to change the mktemp templates instead.

The gitci name comes all the way from the times where this command was
usually triggered by 'git ci'.

> Index: cg-commit
> ===================================================================
> --- f00d7589973e8ea65d2264f5fbac82e1b217dc8f/cg-commit  (mode:100755)
> +++ cb61efa8a01400150162af9b0f3773f21d502fe9/cg-commit  (mode:100755)
> @@ -94,30 +78,55 @@
>  		echo "$uri" >>$LOGMSG
>  		[ "$msgs" ] && echo "$uri"
>  	done
> -	echo >>$LOGMSG
> +else
> +	first=1
>  fi
> -first=1
> +
>  for msg in "${msgs[@]}"; do
>  	if [ "$first" ]; then
>  		first=
>  	else
>  		echo >>$LOGMSG
>  	fi
> -	echo $msg | fmt >>$LOGMSG
> +	echo $msg | fmt -s -w 74 >>$LOGMSG
>  done
> +
> +if [ "$first" ]; then
> +	echo >>$LOGMSG
> +fi

This mess is still here.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: [PATCH Cogito] Make use of external editor work like CVS
From: Marcel Holtmann @ 2005-05-08 15:43 UTC (permalink / raw)
  To: Petr Baudis; +Cc: GIT Mailing List
In-Reply-To: <20050508152529.GU9495@pasky.ji.cz>

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

Hi Petr,

> > this is a modified version of my patch that integrates the your latest
> > modifications to cg-commit and also fixes the cleanup of the temporary
> > files when we abort the operation.
> 
> ...and I've just pushed more modifications. I'm so evil.

attached is another version of the patch.

> > [PATCH] Make use of external editor work like CVS
> > 
> > The lines starting with `CG:' should be a trailer and not at the top
> > of the message presented in the editor. Also extend the number of `-'
> > up to 74 characters so that people know when they should start a new
> > line. If it's not a merge and no commit text is given as parameter
> > then add an extra empty line at the top. And don't forget to take
> > care of the temporary files when a commit is unneeded or canceled.
> > 
> > Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> 
> What is so special about 74 columns? Why not 75 (fmt default), or 72
> (emails)?

I ended up with 74, because "CG" has only two letters instead of "CVS"
which has three. And cg-log uses a prefix of four whitespaces. This
leaves two free characters at the end of a line if your terminal uses a
width of 80 characters. The decision was of cosmetic nature.

> Also, I'd prefer the empty line to be always there in front of the CG:
> stuff (two empty lines in case of merge - I want to encourage people to
> keep possible details w.r.t. the merge separated by an empty line from
> the merge information), and when reading it back cg-commit should strip
> any trailing empty lines.

I think we should differentiate between the merges. There is no need for
additional information if it is an automatic merge (no conflicts) and in
general it makes no sense to open the editor (until forced). I wanted to
address this later. And yes in case of a manual merge it is a good idea
to add two extra empty lines at the top.

Another good idea is maybe to remove empty lines at the top and bottom
before doing the commit.

> > Index: cg-commit
> > ===================================================================
> > --- 8bb38f8bfdc7411460c300c811da1987173f412f/cg-commit  (mode:100755)
> > +++ be440e169fa3b5ec5450fa9574cd8789b0e3ab20/cg-commit  (mode:100755)
> >  if [ "$merging" ]; then
> > -	echo -n 'Merge with ' >>$LOGMSG
> > -	[ "$msgs" ] && echo -n 'Merge with '
> > +	echo -n "Merge with " >>$LOGMSG
> > +	[ "$msgs" ] && echo -n "Merge with "
> 
> We aren't too consistent about this anyway now, so you might as well
> let it not clutter your patch. ;-)

I was moving parts of the code so I addressed it ;)

> >  cp $LOGMSG $LOGMSG2
> >  if tty -s; then
> >  	if ! [ "$msgs" ]; then
> > -		${EDITOR:-vi} $LOGMSG2
> > -		[ $LOGMSG2 -nt $LOGMSG ] || die 'Commit message not modified, commit aborted'
> > +		${EDITOR:-vi} $LOGMSG
> > +		if [ ! $LOGMSG -nt $LOGMSG2 ]; then
> > +			rm $LOGMSG $LOGMSG2
> > +			die 'Commit message not modified, commit aborted.'
> > +		fi
> >  	fi
> >  else
> > -	cat >>$LOGMSG2
> > +	cat >>$LOGMSG
> >  fi
> > -grep -v ^CG: $LOGMSG2 >$LOGMSG
> > -rm $LOGMSG2
> > +grep -v ^CG: $LOGMSG >$LOGMSG2
> > +mv $LOGMSG2 $LOGMSG
> 
> Why are you messing with the $LOGMSG variables here?

This is only cosmetic. Using vim it displays the name of the temporary
file and confusing the user with gitci2.XXXX instead of gitci.XXX is
weird. Even using gitci as basename looks not good to me, but I left it
for now.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3782 bytes --]

[PATCH] Make use of external editor work like CVS

The lines starting with `CG:' should be a trailer and not at the top
of the message presented in the editor. Also extend the number of `-'
up to 74 characters so that people know when they should start a new
line. If it's not a merge and no commit text is given as parameter
then add an extra empty line at the top. And don't forget to take
care of the temporary files when a commit is unneeded or canceled.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit a2c4e793a4dfb21f43494ad90c7b887df10e1be2
tree cb61efa8a01400150162af9b0f3773f21d502fe9
parent 79f86b0174159f016540734ac18560566389b823
author Marcel Holtmann <marcel@holtmann.org> Sun, 08 May 2005 17:30:01 +0200
committer Marcel Holtmann <marcel@holtmann.org> Sun, 08 May 2005 17:30:01 +0200

 cg-commit |   61 +++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 35 insertions(+), 26 deletions(-)

Index: cg-commit
===================================================================
--- f00d7589973e8ea65d2264f5fbac82e1b217dc8f/cg-commit  (mode:100755)
+++ cb61efa8a01400150162af9b0f3773f21d502fe9/cg-commit  (mode:100755)
@@ -67,26 +67,10 @@
 
 LOGMSG=$(mktemp -t gitci.XXXXXX)
 LOGMSG2=$(mktemp -t gitci2.XXXXXX)
-echo CG: ---------------------------------------------------------- >>$LOGMSG
-echo CG: Lines beggining with CG: will be automatically removed     >>$LOGMSG
-echo CG:                                                            >>$LOGMSG
-if [ ! "$ignorecache" ]; then
-	if [ ! "${commitfiles[*]}" ]; then
-		echo 'Nothing to commit.' >&2
-		exit 2
-	fi
-	for file in "${commitfiles[@]}"; do
-		# Prepend a letter describing whether it's addition,
-		# removal or update. Or call git status on those files.
-		echo CG: $file >>$LOGMSG
-		[ "$msgs" ] && echo $file
-	done
-	echo CG: >>$LOGMSG
-fi
 
 if [ "$merging" ]; then
-	echo -n 'Merge with ' >>$LOGMSG
-	[ "$msgs" ] && echo -n 'Merge with '
+	echo -n "Merge with " >>$LOGMSG
+	[ "$msgs" ] && echo -n "Merge with "
 	[ -s .git/merging-sym ] || cp .git/merging .git/merging-sym
 	for sym in $(cat .git/merging-sym); do
 		uri=$(cat .git/branches/$sym)
@@ -94,30 +78,55 @@
 		echo "$uri" >>$LOGMSG
 		[ "$msgs" ] && echo "$uri"
 	done
-	echo >>$LOGMSG
+else
+	first=1
 fi
-first=1
+
 for msg in "${msgs[@]}"; do
 	if [ "$first" ]; then
 		first=
 	else
 		echo >>$LOGMSG
 	fi
-	echo $msg | fmt >>$LOGMSG
+	echo $msg | fmt -s -w 74 >>$LOGMSG
 done
+
+if [ "$first" ]; then
+	echo >>$LOGMSG
+fi
+
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+echo "CG: Enter Log.  Lines beginning with \`CG:' are removed automatically"      >>$LOGMSG
+if [ ! "$ignorecache" ]; then
+	if [ ! "${commitfiles[*]}" ]; then
+		rm $LOGMSG $LOGMSG2
+		die 'Nothing to commit.'
+	fi
+	echo "CG: " >>$LOGMSG
+	echo "CG: Modified Files:" >>$LOGMSG
+	for file in "${commitfiles[@]}"; do
+		# Prepend a letter describing whether it's addition,
+		# removal or update. Or call git status on those files.
+		echo "CG:    $file" >>$LOGMSG
+		[ "$msgs" ] && echo $file
+	done
+fi
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+
 cp $LOGMSG $LOGMSG2
 if tty -s; then
 	if ! [ "$msgs" ] || [ "$forceeditor" ]; then
-		${EDITOR:-vi} $LOGMSG2
+		${EDITOR:-vi} $LOGMSG
 	fi
-	if ! [ "$msgs" ] && ! [ $LOGMSG2 -nt $LOGMSG ]; then
-		die 'Commit message not modified, commit aborted'
+	if ! [ "$msgs" ] && ! [ $LOGMSG -nt $LOGMSG2 ]; then
+		rm $LOGMSG $LOGMSG2
+		die 'Commit message not modified, commit aborted.'
 	fi
 else
 	cat >>$LOGMSG2
 fi
-grep -v ^CG: $LOGMSG2 >$LOGMSG
-rm $LOGMSG2
+grep -v ^CG: $LOGMSG >$LOGMSG2
+mv $LOGMSG2 $LOGMSG
 
 if [ ! "$ignorecache" ]; then
 	if [ "$customfiles" ]; then

^ permalink raw reply

* Re: [PATCH Cogito] Make use of external editor work like CVS
From: Petr Baudis @ 2005-05-08 15:25 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: GIT Mailing List
In-Reply-To: <1115564550.9031.96.camel@pegasus>

Dear diary, on Sun, May 08, 2005 at 05:02:30PM CEST, I got a letter
where Marcel Holtmann <marcel@holtmann.org> told me that...
> Hi Petr,

Hi,

> this is a modified version of my patch that integrates the your latest
> modifications to cg-commit and also fixes the cleanup of the temporary
> files when we abort the operation.

...and I've just pushed more modifications. I'm so evil.

> [PATCH] Make use of external editor work like CVS
> 
> The lines starting with `CG:' should be a trailer and not at the top
> of the message presented in the editor. Also extend the number of `-'
> up to 74 characters so that people know when they should start a new
> line. If it's not a merge and no commit text is given as parameter
> then add an extra empty line at the top. And don't forget to take
> care of the temporary files when a commit is unneeded or canceled.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

What is so special about 74 columns? Why not 75 (fmt default), or 72
(emails)?

Also, I'd prefer the empty line to be always there in front of the CG:
stuff (two empty lines in case of merge - I want to encourage people to
keep possible details w.r.t. the merge separated by an empty line from
the merge information), and when reading it back cg-commit should strip
any trailing empty lines.

> Index: cg-commit
> ===================================================================
> --- 8bb38f8bfdc7411460c300c811da1987173f412f/cg-commit  (mode:100755)
> +++ be440e169fa3b5ec5450fa9574cd8789b0e3ab20/cg-commit  (mode:100755)
>  if [ "$merging" ]; then
> -	echo -n 'Merge with ' >>$LOGMSG
> -	[ "$msgs" ] && echo -n 'Merge with '
> +	echo -n "Merge with " >>$LOGMSG
> +	[ "$msgs" ] && echo -n "Merge with "

We aren't too consistent about this anyway now, so you might as well
let it not clutter your patch. ;-)

>  cp $LOGMSG $LOGMSG2
>  if tty -s; then
>  	if ! [ "$msgs" ]; then
> -		${EDITOR:-vi} $LOGMSG2
> -		[ $LOGMSG2 -nt $LOGMSG ] || die 'Commit message not modified, commit aborted'
> +		${EDITOR:-vi} $LOGMSG
> +		if [ ! $LOGMSG -nt $LOGMSG2 ]; then
> +			rm $LOGMSG $LOGMSG2
> +			die 'Commit message not modified, commit aborted.'
> +		fi
>  	fi
>  else
> -	cat >>$LOGMSG2
> +	cat >>$LOGMSG
>  fi
> -grep -v ^CG: $LOGMSG2 >$LOGMSG
> -rm $LOGMSG2
> +grep -v ^CG: $LOGMSG >$LOGMSG2
> +mv $LOGMSG2 $LOGMSG

Why are you messing with the $LOGMSG variables here?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: [PATCH Cogito] Make use of external editor work like CVS
From: Sean @ 2005-05-08 15:24 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Petr Baudis, GIT Mailing List
In-Reply-To: <1115564550.9031.96.camel@pegasus>

On Sun, May 8, 2005 11:02 am, Marcel Holtmann said:
> Hi Petr,
>
> this is a modified version of my patch that integrates the your latest
> modifications to cg-commit and also fixes the cleanup of the temporary
> files when we abort the operation.
>

Hi Marcel,

What do you think about providing a per-repository commit template?  So,
if say ".git/commit.form" exists, use it instead of the default?

At a minimum, it would be nice to include a reminder about adding a
"Signed-off-by:" line.

Sean



^ permalink raw reply

* [PATCH Cogito] Make use of external editor work like CVS
From: Marcel Holtmann @ 2005-05-08 15:02 UTC (permalink / raw)
  To: Petr Baudis; +Cc: GIT Mailing List

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

Hi Petr,

this is a modified version of my patch that integrates the your latest
modifications to cg-commit and also fixes the cleanup of the temporary
files when we abort the operation.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3739 bytes --]

[PATCH] Make use of external editor work like CVS

The lines starting with `CG:' should be a trailer and not at the top
of the message presented in the editor. Also extend the number of `-'
up to 74 characters so that people know when they should start a new
line. If it's not a merge and no commit text is given as parameter
then add an extra empty line at the top. And don't forget to take
care of the temporary files when a commit is unneeded or canceled.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit 895bcd02ecc96bed1d35275def6dca1ca6f20e5f
tree be440e169fa3b5ec5450fa9574cd8789b0e3ab20
parent 64142a39e7a6701e69654a930de86a9fe296f8a0
author Marcel Holtmann <marcel@holtmann.org> Sun, 08 May 2005 16:58:50 +0200
committer Marcel Holtmann <marcel@holtmann.org> Sun, 08 May 2005 16:58:50 +0200

 cg-commit |   63 ++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 37 insertions(+), 26 deletions(-)

Index: cg-commit
===================================================================
--- 8bb38f8bfdc7411460c300c811da1987173f412f/cg-commit  (mode:100755)
+++ be440e169fa3b5ec5450fa9574cd8789b0e3ab20/cg-commit  (mode:100755)
@@ -61,26 +61,10 @@
 
 LOGMSG=$(mktemp -t gitci.XXXXXX)
 LOGMSG2=$(mktemp -t gitci2.XXXXXX)
-echo CG: ---------------------------------------------------------- >>$LOGMSG
-echo CG: Lines beggining with CG: will be automatically removed     >>$LOGMSG
-echo CG:                                                            >>$LOGMSG
-if [ ! "$ignorecache" ]; then
-	if [ ! "${commitfiles[*]}" ]; then
-		echo 'Nothing to commit.' >&2
-		exit 2
-	fi
-	for file in "${commitfiles[@]}"; do
-		# Prepend a letter describing whether it's addition,
-		# removal or update. Or call git status on those files.
-		echo CG: $file >>$LOGMSG
-		[ "$msgs" ] && echo $file
-	done
-	echo CG: >>$LOGMSG
-fi
 
 if [ "$merging" ]; then
-	echo -n 'Merge with ' >>$LOGMSG
-	[ "$msgs" ] && echo -n 'Merge with '
+	echo -n "Merge with " >>$LOGMSG
+	[ "$msgs" ] && echo -n "Merge with "
 	[ -s .git/merging-sym ] || cp .git/merging .git/merging-sym
 	for sym in $(cat .git/merging-sym); do
 		uri=$(cat .git/branches/$sym)
@@ -88,28 +72,55 @@
 		echo "$uri" >>$LOGMSG
 		[ "$msgs" ] && echo "$uri"
 	done
-	echo >>$LOGMSG
+else
+	first=1
 fi
-first=1
+
 for msg in "${msgs[@]}"; do
 	if [ "$first" ]; then
 		first=
 	else
 		echo >>$LOGMSG
 	fi
-	echo $msg | fmt >>$LOGMSG
+	echo $msg | fmt -s -w 74 >>$LOGMSG
 done
+
+if [ "$first" ]; then
+	echo >>$LOGMSG
+fi
+
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+echo "CG: Enter Log.  Lines beginning with \`CG:' are removed automatically"      >>$LOGMSG
+if [ ! "$ignorecache" ]; then
+	if [ ! "${commitfiles[*]}" ]; then
+		rm $LOGMSG $LOGMSG2
+		die 'Nothing to commit.'
+	fi
+	echo "CG: " >>$LOGMSG
+	echo "CG: Modified Files:" >>$LOGMSG
+	for file in "${commitfiles[@]}"; do
+		# Prepend a letter describing whether it's addition,
+		# removal or update. Or call git status on those files.
+		echo "CG:    $file" >>$LOGMSG
+		[ "$msgs" ] && echo "$file"
+	done
+fi
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+
 cp $LOGMSG $LOGMSG2
 if tty -s; then
 	if ! [ "$msgs" ]; then
-		${EDITOR:-vi} $LOGMSG2
-		[ $LOGMSG2 -nt $LOGMSG ] || die 'Commit message not modified, commit aborted'
+		${EDITOR:-vi} $LOGMSG
+		if [ ! $LOGMSG -nt $LOGMSG2 ]; then
+			rm $LOGMSG $LOGMSG2
+			die 'Commit message not modified, commit aborted.'
+		fi
 	fi
 else
-	cat >>$LOGMSG2
+	cat >>$LOGMSG
 fi
-grep -v ^CG: $LOGMSG2 >$LOGMSG
-rm $LOGMSG2
+grep -v ^CG: $LOGMSG >$LOGMSG2
+mv $LOGMSG2 $LOGMSG
 
 if [ ! "$ignorecache" ]; then
 	if [ "$customfiles" ]; then

^ permalink raw reply

* Re: [PATCH] Really *do* nothing in while loop
From: Michael Tokarev @ 2005-05-08 11:40 UTC (permalink / raw)
  To: jdow; +Cc: James Purser, Thomas Glanzmann, LKML, GIT
In-Reply-To: <12e801c553c1$c454ea20$1225a8c0@kittycat>

jdow wrote:
> From: "James Purser" <purserj@ksit.dynalias.com>
> 
   while (deflate(&stream, 0) == Z_OK)
-  /* nothing */
+  /* nothing */;
  stream.next_in = buf;
> 
> You guys REALLY do not see the changed semantics here? You are
> changing:
>   while (deflate(&stream, 0) == Z_OK)
>       stream.next_in = buf;
> 
> into
> 
>   while (deflate(&stream, 0) == Z_OK)
>     ;
>   /* Then the data itself.. */
>   stream.next_in = buf;
> 
> I suspect the results of that tiny bit of code would be slightly
> different, especially if "stream.next_in" is volatile, "buf"
> is volatile, or if the assignment to next_in has an effect on
> the "deflate" operation.

As I already said, deflate() in this case does only ONE iteration.
stream.avail_in is NOT changed in the loop (except of the deflate()
itself, where it will be set to 0 - provided out buffer have enouth
room).  So the whole while loop does only ONE iteration, returning
Z_NEED_DATA or something the next one.  So no, the semantics here
(actual semantics) does NOT change.

/mjt

^ permalink raw reply

* Re: [PATCH] Really *do* nothing in while loop
From: jdow @ 2005-05-08 11:33 UTC (permalink / raw)
  To: James Purser, Michael Tokarev; +Cc: Thomas Glanzmann, LKML, GIT
In-Reply-To: <1115551204.3085.0.camel@kryten>

From: "James Purser" <purserj@ksit.dynalias.com>

> On Sun, 2005-05-08 at 19:48, Michael Tokarev wrote:
> > Thomas Glanzmann wrote:
> > > [PATCH] Really *do* nothing in while loop
> > > 
> > > Signed-Off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>
> > > 
> > > --- a/sha1_file.c
> > > +++ b/sha1_file.c
> > > @@ -335,7 +335,7 @@
> > >  stream.next_in = hdr;
> > >  stream.avail_in = hdrlen;
> > >  while (deflate(&stream, 0) == Z_OK)
> > > - /* nothing */
> > > + /* nothing */;
> > >  
> > >  /* Then the data itself.. */
> > >  stream.next_in = buf;
> > 
> > Well, the lack of semicolon is wrong really (and funny).

You guys REALLY do not see the changed semantics here? You are
changing:
  while (deflate(&stream, 0) == Z_OK)
      stream.next_in = buf;

into

  while (deflate(&stream, 0) == Z_OK)
    ;
  /* Then the data itself.. */
  stream.next_in = buf;

I suspect the results of that tiny bit of code would be slightly
different, especially if "stream.next_in" is volatile, "buf"
is volatile, or if the assignment to next_in has an effect on
the "deflate" operation.

{^_^}




^ permalink raw reply

* Re: [PATCH] Really *do* nothing in while loop
From: James Purser @ 2005-05-08 11:20 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: Thomas Glanzmann, LKML, GIT
In-Reply-To: <427DE086.40307@tls.msk.ru>

On Sun, 2005-05-08 at 19:48, Michael Tokarev wrote:
> Thomas Glanzmann wrote:
> > [PATCH] Really *do* nothing in while loop
> > 
> > Signed-Off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>
> > 
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -335,7 +335,7 @@
> >  	stream.next_in = hdr;
> >  	stream.avail_in = hdrlen;
> >  	while (deflate(&stream, 0) == Z_OK)
> > -		/* nothing */
> > +		/* nothing */;
> >  
> >  	/* Then the data itself.. */
> >  	stream.next_in = buf;
> 
> Well, the lack of semicolon is wrong really (and funny).
> 
> But is the whole while loop needed at all?  deflate()
> consumes as much input as it can, producing as much output
> as it can.  So without the loop, and without updating the
> buffer pointers ({next,avail}_{in,out}) it will do just
> fine without the loop, and will return something != Z_OK
> on next iteration.  If this is to mean to flush output,
> it should be deflate(&stream, Z_FLUSH) or something.
> 
> /mjt
> 
> P.S.  What's git@vger.kernel.org for ?
Its the mailing list for git development.
> -
> 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
-- 
James Purser
http://ksit.dynalias.com


^ permalink raw reply

* Re: [PATCH] Really *do* nothing in while loop
From: Thomas Glanzmann @ 2005-05-08 11:18 UTC (permalink / raw)
  To: LKML, GIT
In-Reply-To: <427DE086.40307@tls.msk.ru>

Hello,

> >-		/* nothing */
> >+		/* nothing */;

> Well, the lack of semicolon is wrong really (and funny).

yes, it is but harmless in this envrionment.

> But is the whole while loop needed at all?  deflate()
> consumes as much input as it can, producing as much output
> as it can.  So without the loop, and without updating the
> buffer pointers ({next,avail}_{in,out}) it will do just
> fine without the loop, and will return something != Z_OK
> on next iteration.  If this is to mean to flush output,
> it should be deflate(&stream, Z_FLUSH) or something.

I have no idea.

> P.S.  What's git@vger.kernel.org for ?

It is the list which handles GIT related discussions. Frontend/backend
and isn't kernel related.

	Thomas

^ permalink raw reply

* Re: [PATCH] Really *do* nothing in while loop
From: Michael Tokarev @ 2005-05-08  9:48 UTC (permalink / raw)
  To: Thomas Glanzmann; +Cc: LKML, GIT
In-Reply-To: <20050508093440.GA9873@cip.informatik.uni-erlangen.de>

Thomas Glanzmann wrote:
> [PATCH] Really *do* nothing in while loop
> 
> Signed-Off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>
> 
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -335,7 +335,7 @@
>  	stream.next_in = hdr;
>  	stream.avail_in = hdrlen;
>  	while (deflate(&stream, 0) == Z_OK)
> -		/* nothing */
> +		/* nothing */;
>  
>  	/* Then the data itself.. */
>  	stream.next_in = buf;

Well, the lack of semicolon is wrong really (and funny).

But is the whole while loop needed at all?  deflate()
consumes as much input as it can, producing as much output
as it can.  So without the loop, and without updating the
buffer pointers ({next,avail}_{in,out}) it will do just
fine without the loop, and will return something != Z_OK
on next iteration.  If this is to mean to flush output,
it should be deflate(&stream, Z_FLUSH) or something.

/mjt

P.S.  What's git@vger.kernel.org for ?

^ permalink raw reply

* [PATCH] Really *do* nothing in while loop
From: Thomas Glanzmann @ 2005-05-08  9:34 UTC (permalink / raw)
  To: LKML, GIT

[PATCH] Really *do* nothing in while loop

Signed-Off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>

--- a/sha1_file.c
+++ b/sha1_file.c
@@ -335,7 +335,7 @@
 	stream.next_in = hdr;
 	stream.avail_in = hdrlen;
 	while (deflate(&stream, 0) == Z_OK)
-		/* nothing */
+		/* nothing */;
 
 	/* Then the data itself.. */
 	stream.next_in = buf;

^ permalink raw reply

* Re: [patch] add simple git documentation
From: David Greaves @ 2005-05-08  7:35 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Andrew Morton, Pavel Machek, git
In-Reply-To: <20050507113618.GA9495@pasky.ji.cz>

Petr Baudis wrote:

>Dear diary, on Sat, May 07, 2005 at 10:37:12AM CEST, I got a letter
>where David Greaves <david@dgreaves.com> told me that...
>  
>
>>Andrew Morton wrote:
>>    
>>
>>>Pavel Machek <pavel@ucw.cz> wrote:
>>>      
>>>
>>>>This adds short intro to git aimed at kernel hackers.
>>>>        
>>>>
>>>OK, but I'm hoping that shortly we'll have something more complete than
>>>this, and your patch might not be a suitable starting point for that. 
>>>(Large hint-dropping sounds).
>>>      
>>>
>>What are you looking for.
>>
>>For man-page-a-like, have you seen Documentation/core-git.txt in Linus'
>>repository?
>>
>>I'll be working more on that this weekend.
>>    
>>
>Are you planning to split it to individual files at some point?
>  
>
Yes. I'll see how using asciidoc affects that.
And with Linus vacationing it looks like there'll be time this week.

>(That's what I'd like to do with Documentation/cogito/, speaking of
>manpageness...)
>  
>
OK
BTW, whatever happened to perlification - the initial stab I sent you
used the perl/pod approach.
The reason I ask is that that means the docs should/could be initially
written as here-documents within the scripts rather than external manpages.

David

^ permalink raw reply

* Re: Broken adding of cache entries
From: Junio C Hamano @ 2005-05-08  5:22 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Kay Sievers, git
In-Reply-To: <7vbr7m8p05.fsf@assigned-by-dhcp.cox.net>

>>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes:

JCH> I am not quite sure the semantics is quite right, so I am
JCH> holding it off from putting it in git-jc repository for now, but
JCH> please review it, give it a try and tell me what you think.

Ok, I have two updates pushed out at git-jc archive at
http://members.cox.net/junkio/git-jc.git/

Here is the summary of what the changes do.

The first commit is to simply prevent Kay's situation from
happening from git-update-cache.

By default, git-update-cache refuses to:

 - add "path" when "path/file" exists.  That is, you cannot
   replace a directory with a file (or a symlink).

 - add "path/file" when "path" exists.  That is, you cannot
   replace a file (or a symlink) with a directory.

And the second commit introduces --replace option to the
command.  When --replace is in effect, instead of refusing, the
existing path that conflicts with whatever is being added is
dropped with a warning message.  That means you would lose an
entire subdirectory from the index when you rename it and
replace it with a file and then call git-update-cache on it
(which is probably fine and what the user expects anyway).

Note that this is not enough to prevent nonsensical tree from
happening.  Three-way read-tree can be fed with an ancestor tree
that does not have anything interesting, one side adding "path"
(as a file) and the other side adding "path/file".  Just after
read-tree finishes reading three trees, you would have:

    100644 1 not-interesting
    100644 2 not-interesting
    100644 3 not-interesting
    100644 2 path
    100644 3 path/file

And all of these paths are candidate for trivial "collapsing to
stage 0" operation.  You would end up with path vs path/file
conflicts this way, so the previous change to write-tree to
prevent it from writing such a result is still needed.

Ideally the "collapsing to stage 0" logic could also be tweaked
to detect and prevent this from happening, but I'd rather keep
it simple for now (the tool should not be too clever).  The user
cannot write such a tree, and when this kind of conflict
happens, essentially both heads being merged needs to be
examined and manual resolution is required anyway.



^ permalink raw reply

* [ANNOUNCE] Cogito-0.9
From: Petr Baudis @ 2005-05-08  2:59 UTC (permalink / raw)
  To: git

  Hello,

  so Cogito version 0.9, my SCM-like layer over Linus' GIT versioned
tree storage system, is out. Get it at

	http://www.kernel.org/pub/scm/cogito/

  or just cg-update your tree. The highlights is cg-pulling over HTTP,
ssh, and from local trees, sheer amount of bugfixes, regular merging
with Linus, and few usage changes; most importantly, cg-log and
cg-mkpatch now take the revision specifiers as -r arguments to make it
possible to narrow that down to individual files (otherwise, I consider
the Cogito CLI basically stabilized, except for some cg-cancel vs
cg-update cleanups; but that shouldn't affect anything day-to-day,
except making the day-to-day stuff possibly easier again).

  This is mostly so that I can satisfy people hungry to upgrade to next
Cogito release. I know about the unmerged patches (well, at least some
of them), and will go through them soon; I have some of own agenda I
want to work on too.

  Have fun,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: Broken adding of cache entries
From: Junio C Hamano @ 2005-05-08  1:50 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Kay Sievers, git
In-Reply-To: <7vll6q8s4o.fsf@assigned-by-dhcp.cox.net>

This patch makes git-update-cache --add refuse to create an
index where one entry is a file and another entry needs to have
that entry as its leading directory.

I am not quite sure the semantics is quite right, so I am
holding it off from putting it in git-jc repository for now, but
please review it, give it a try and tell me what you think.

Here is what I did.

    $ ls -a
    ./  ../
    $ git-init-db
    defaulting to local storage area
    $ date >path
    $ git-update-cache --add path
    $ rm -f path; mkdir path; date >path/file
    $ git-update-cache --add path/file
    fatal: Unable to add path/file to database
    $ git-update-cache --remove path           ;# *****
    fatal: Unable to add path to database
    $ git-update-cache --force-remove path
    $ git-update-cache --add path/file
    $ git-ls-files --stage
    100644 3e3f5e43a8e7baa7f0b1f2d7b99ed25fcbe582d2 0 path/file
    $ rm -fr path
    $ git-update-cache --remove path/file
    $ mkdir path; date >path/file
    $ git-update-cache --add path/file
    $ git-ls-files --stage
    100644 1b4e067c82b90419c2d2ce80e72079f926a49cc3 0 path/file
    $ rm -fr path; date >path
    $ git-update-cache --add path
    fatal: Unable to add path to database
    $ git-update-cache --remove path/file
    $ git-update-cache --add path
    $ git-ls-files --stage
    100644 b4509b7a4fed200befc45746061eb1d32ad6cbc9 0 path

The one marked with ***** is what I am debating myself about,
but that one would probably be an independent fix.

Not-signed-off-yet-by: Junio C Hamano <junkio@cox.net>
---
jit-diff 0:7 read-cache.c
# - HEAD: Notice tree objects with duplicate entries.
# + 7: Refuse to create file/dir conflict cache entry.
--- a/read-cache.c
+++ b/read-cache.c
@@ -123,6 +123,64 @@ int same_name(struct cache_entry *a, str
 	return ce_namelen(b) == len && !memcmp(a->name, b->name, len);
 }
 
+/* We may be in a situation where we already have
+ * path/file and path is being added, or we already
+ * have path and path/file is being added.  Either one
+ * would result in a nonsense tree that has path twice
+ * when git-write-tree tries to write it out.  Prevent it.
+ */
+static int check_file_directory_conflict(const char *path)
+{
+	int pos;
+	int namelen = strlen(path);
+	char *pathbuf = xmalloc(namelen + 1);
+	char *cp;
+
+	memcpy(pathbuf, path, namelen + 1);
+
+	cp = pathbuf;
+	while (1) {
+		char *ep = strchr(cp, '/');
+		if (ep == 0)
+			break;
+		*ep = 0;
+		pos = cache_name_pos(pathbuf,
+				     htons(create_ce_flags(ep-cp, 0)));
+		if (0 <= pos) {
+			/* Our leading path component is registered as a file,
+			 * and we are trying to make it a directory.
+			 */
+			free(pathbuf);
+			return -1;
+		}
+		*ep = '/';
+		cp = ep + 1;
+	}
+	free(pathbuf);
+
+	/* Do we have an entry in the cache that makes our path a prefix
+	 * of it?  That is, are we creating a file where they expect a
+	 * directory there?
+	 */
+	pos = cache_name_pos(path,
+			     htons(create_ce_flags(namelen, 0)));
+
+	/* (0 <= pos) cannot happen because add_cache_entry()
+	 * should have taken care of that case.
+	 */
+	pos = -pos-1;
+
+	/* pos would point at an existing entry that would come immediately
+	 * after our path.  Does it have our path as a leading path prefix?
+	 */
+	if ((pos < active_nr) &&
+	    !strncmp(active_cache[pos]->name, path, namelen) &&
+	    active_cache[pos]->name[namelen] == '/')
+		return -1;
+
+	return 0;
+}
+
 int add_cache_entry(struct cache_entry *ce, int ok_to_add)
 {
 	int pos;
@@ -152,6 +210,9 @@ int add_cache_entry(struct cache_entry *
 	if (!ok_to_add)
 		return -1;
 
+	if (check_file_directory_conflict(ce->name))
+		return -1;
+
 	/* Make sure the array is big enough .. */
 	if (active_nr == active_alloc) {
 		active_alloc = alloc_nr(active_alloc);



^ permalink raw reply

* Re: [PATCH] Add exclude file support to cg-status
From: Petr Baudis @ 2005-05-08  1:50 UTC (permalink / raw)
  To: Matt Porter; +Cc: git
In-Reply-To: <20050502171042.A24299@cox.net>

Dear diary, on Tue, May 03, 2005 at 02:10:42AM CEST, I got a letter
where Matt Porter <mporter@kernel.crashing.org> told me that...
> Adds a trivial per-repository exclude file implementation for
> cg-status on top of the new git-ls-files option.
> 
> Signed-off-by: Matt Porter <mporter@kernel.crashing.org>

Thanks, applied. I decided to go with this version instead of the
.git/info/exclude one since I *don't* think you want to share this. The
very fact it is in the .git/ directory implies here that these are your
local exclude patterns which are likely temporary and more importantly
you don't want them under version control. Therefore they aren't
really interesting for anyone else and should be kept per-tree. That's
this file's point as I view it. If you want something more persistent,
get it to git's sight.

The fact that we have no support for version-tracked exclude file can't
stop me! ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* [PATCH Cogito] Make use of external editor work like CVS
From: Marcel Holtmann @ 2005-05-08  1:10 UTC (permalink / raw)
  To: Petr Baudis; +Cc: GIT Mailing List

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

Hi Petr,

I like to extend the work from Pavel and make the cg-commit really work
like we know it from CVS. Please consider applying the attached patch.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3416 bytes --]

Make use of external editor work like CVS

The lines starting with `CG:' should be a trailer and not at the top
of the message presented in the editor. Also extend the number of `-'
up to 74 characters so that people know when they should start a new
line. If it's not a merge and no commit text is given via the command
line add an extra empty line at the top.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit ad5cabd7130fc180aed0c364d4014abb49f374b0
tree ca866565063dcd89204e747e8aa0fcb483d14841
parent 09f6584daa060eda641a8e727698f935e0960e9c
author Marcel Holtmann <marcel@holtmann.org> Sun, 08 May 2005 03:03:53 +0200
committer Marcel Holtmann <marcel@holtmann.org> Sun, 08 May 2005 03:03:53 +0200

 cg-commit |   55 +++++++++++++++++++++++++++++++------------------------
 1 files changed, 31 insertions(+), 24 deletions(-)

Index: cg-commit
===================================================================
--- 5d7e8450d626a4e99f8222378a6818d03a797864/cg-commit  (mode:100755)
+++ ca866565063dcd89204e747e8aa0fcb483d14841/cg-commit  (mode:100755)
@@ -61,24 +61,9 @@
 
 LOGMSG=$(mktemp -t gitci.XXXXXX)
 LOGMSG2=$(mktemp -t gitci2.XXXXXX)
-echo CG: ---------------------------------------------------------- >>$LOGMSG
-echo CG: Lines beggining with CG: will be automatically removed     >>$LOGMSG
-echo CG:                                                            >>$LOGMSG
-if [ ! "$ignorecache" ]; then
-	if [ ! "${commitfiles[*]}" ]; then
-		echo 'Nothing to commit.' >&2
-		exit 2
-	fi
-	for file in "${commitfiles[@]}"; do
-		# Prepend a letter describing whether it's addition,
-		# removal or update. Or call git status on those files.
-		echo CG: $file >>$LOGMSG
-	done
-	echo CG: >>$LOGMSG
-fi
 
 if [ "$merging" ]; then
-	echo -n 'Merge with ' >>$LOGMSG
+	echo -n "Merge with " >>$LOGMSG
 	[ -s .git/merging-sym ] || cp .git/merging .git/merging-sym
 	for sym in $(cat .git/merging-sym); do
 		uri=$(cat .git/branches/$sym)
@@ -86,28 +71,50 @@
 		echo "$uri" >>$LOGMSG
 		echo "$uri"
 	done
-	echo >>$LOGMSG
+else
+	first=1
 fi
-first=1
+
 for msg in "${msgs[@]}"; do
 	if [ "$first" ]; then
 		first=
 	else
 		echo >>$LOGMSG
 	fi
-	echo $msg | fmt >>$LOGMSG
+	echo $msg | fmt -s -w 74 >>$LOGMSG
 done
+
+if [ "$first" ]; then
+	echo >>$LOGMSG
+fi
+
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+echo "CG: Enter Log.  Lines beginning with \`CG:' are removed automatically"      >>$LOGMSG
+if [ ! "$ignorecache" ]; then
+	if [ ! "${commitfiles[*]}" ]; then
+		die 'Nothing to commit.'
+	fi
+	echo "CG: " >>$LOGMSG
+	echo "CG: Modified Files:" >>$LOGMSG
+	for file in "${commitfiles[@]}"; do
+		# Prepend a letter describing whether it's addition,
+		# removal or update. Or call git status on those files.
+		echo "CG:    $file" >>$LOGMSG
+	done
+fi
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+
 cp $LOGMSG $LOGMSG2
 if tty -s; then
 	if ! [ "$msgs" ]; then
-		${EDITOR:-vi} $LOGMSG2
-		[ $LOGMSG2 -nt $LOGMSG ] || die 'Commit message not modified, commit aborted'
+		${EDITOR:-vi} $LOGMSG
+		[ $LOGMSG -nt $LOGMSG2 ] || die 'Commit message not modified, commit aborted.'
 	fi
 else
-	cat >>$LOGMSG2
+	cat >>$LOGMSG
 fi
-grep -v ^CG: $LOGMSG2 >$LOGMSG
-rm $LOGMSG2
+grep -v ^CG: $LOGMSG >$LOGMSG2
+mv $LOGMSG2 $LOGMSG
 
 if [ ! "$ignorecache" ]; then
 	if [ "$customfiles" ]; then

^ permalink raw reply

* Re: Broken adding of cache entries
From: Junio C Hamano @ 2005-05-08  0:43 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Kay Sievers, git
In-Reply-To: <20050507224116.GF9495@pasky.ji.cz>

>>>>> "PB" == Petr Baudis <pasky@ucw.cz> writes:

PB> What about
PB> ... suggested changes removed.
PB> then? (Completely untested and everything.)

It is clear that it is untested.  In the sequence Kay had
trouble with:

    $ rm -fr path; date >path
    $ git-update-cache --add path
    $ rm -f path; mkdir path; date >path/file
    $ git-update-cache --add path/file

The cache_name_compare function you are looking at is called
with "path" and "path/file", and compares the name strings (up
to the length of the shorter one), and when they are the same,
then compares the lengths of them.  At that point, the if()
statements have already answered that "path" comes before
"path/file" and your changes will _not_ change the behavior of
that function.

That said, I understand the behaviour you would want to see, and
I tend to agree that this should be prevented from happening in
the first place when git-update-cache happens.  Let me think a
bit more about it.

PB> I'd still prefer add_cache_entry() to just replace the original entry
PB> (as it does otherwise).

I do not think it should just silently replace; instead it
should make the user take notice, because this is an unusual
situation.  In other words, it should refuse until the user
makes a conscious decision to tell it that the user is removing
it and then placing something completely different.

This implies (I am thinking aloud here):

    $ find fs -type f -print | xargs git-update-cache --add
    $ rm -fr fs
    $ date >fs
    $ git-update-cache --add fs

would refuse to add "fs" because it notices it has some entry
whose "name" field starts with "fs/".  Likewise, for the
opposite sequence:

    $ date >fs
    $ git-update-cache --add fs
    $ rm -f fs; mkdir fs; date >fs/file    
    $ find fs -type f -print | xargs git-update-cache --add

Telling git-update-cache about anything that contains a '/' in
its name would first check if an entry whose name is exactly one
of the leading path prefix, and refuses the operation if it
finds one.

The above logic should work and it would also work if we decide
to just replace without refusing as well.  I'll code it up and
see which one I like; maybe it will end up as an option.



^ permalink raw reply

* Re: [PATCH] Use backticks instead of $(command) to maintain /bin/sh compatibility
From: Thomas Glanzmann @ 2005-05-07 23:15 UTC (permalink / raw)
  To: GIT
In-Reply-To: <20050507172429.GJ3562@admingilde.org>

Hello Coworker,

> huh? which broken shell does not understand $()?

/bin/sh under Solaris 9 for example. That is where I hit it initial.

	Thomas

^ permalink raw reply

* Re: [PATCH] Use backticks instead of $(command) to maintain /bin/sh compatibility
From: Thomas Glanzmann @ 2005-05-07 23:15 UTC (permalink / raw)
  To: GIT
In-Reply-To: <7vy8aqanlh.fsf@assigned-by-dhcp.cox.net>

Hello,

> If that is the case then I think the patch you posted to force
> bash is backwards.  How about changing it to use backticks?

agreed. Already did that see previous eMail.

	Thomas

^ permalink raw reply

* Re: Broken adding of cache entries
From: Petr Baudis @ 2005-05-07 22:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kay Sievers, git
In-Reply-To: <7vhdhealjm.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, May 07, 2005 at 09:22:21PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Kay Sievers noticed that you can have both path and path/file in
> the cache and write-tree happily creates a tree object from such
> a state.  Since a merge can result in such situation and the
> user should be able to see the situation by looking at the
> cache, rather than forbidding add_cache_entry() to create such
> conflicts, fix it by making write-tree refuse to write such an
> nonsensical tree.

I'd still prefer add_cache_entry() to just replace the original entry
(as it does otherwise). Only make it to care about which stage it is
working in, to make merges to work. IOW, I think you are solving this at
the wrong workflow point. It is too "late" to know at that point, and
(a huge) PITA for the higher levels to deal with it then - that all when
it shouldn't fail _at all_ in the first place.

What about

--- b7ae63ab415e556c2f0f0ad2803f701b4a6d6956/read-cache.c  (mode:100644)
+++ uncommitted/read-cache.c  (mode:100644)
@@ -68,9 +68,9 @@
                return -1;
        if (len1 > len2)
                return 1;
-       if (flags1 < flags2)
+       if (flags1 & CE_STAGEMASK < flags2 & CE_STAGEMASK)
                return -1;
-       if (flags1 > flags2)
+       if (flags1 & CE_STAGEMASK > flags2 & CE_STAGEMASK)
                return 1;
        return 0;
 }

then? (Completely untested and everything.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: [PATCH] Use backticks instead of $(command) to maintain /bin/sh compatibility
From: Morten Welinder @ 2005-05-07 20:22 UTC (permalink / raw)
  To: Martin Waitz; +Cc: GIT, Junio C Hamano
In-Reply-To: <20050507172429.GJ3562@admingilde.org>

On 5/7/05, Martin Waitz <tali@admingilde.org> wrote:
> hoi :)
> 
> On Sat, May 07, 2005 at 11:05:43AM +0200, Thomas Glanzmann wrote:
> > * Junio C Hamano <junkio@cox.net> [050507 10:54]:
> > > A quick question.  Which construct in this bashism?
> > > Not using backtick but saying $(command)?
> >
> > Exactly:
> 
> huh? which broken shell does not understand $()?

Solaris' /bin/sh

I thought everything we were relying on bash anyway.  It'll take it.

Morten

^ permalink raw reply


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