Git development
 help / color / mirror / Atom feed
* Re: obnoxious CLI complaints
From: Junio C Hamano @ 2009-09-11  0:37 UTC (permalink / raw)
  To: demerphq
  Cc: John Tapsell, Jakub Narebski, Wincent Colaiuta, Brendan Miller,
	git
In-Reply-To: <9b18b3110909101619n6904a75dm10dd0b5717fb0d76@mail.gmail.com>

demerphq <demerphq@gmail.com> writes:

> 2009/9/11 Junio C Hamano <gitster@pobox.com>:
>> John Tapsell <johnflux@gmail.com> writes:
>>
>>> 2009/9/10 Jakub Narebski <jnareb@gmail.com>:
>>>> Dnia czwartek 10. września 2009 21:46, John Tapsell napisał:
>>>>> 2009/9/10 Jakub Narebski <jnareb@gmail.com>:
>>>>
>>>>> > First, it would be consistent with how ordinary archivers such as tar
>>>>> > or zip are used, where you have to specify list of files to archive
>>>>> > (in our case this list is HEAD).  Second, I'd rather not accidentally
>>>>> > dump binary to terminal: "git archive [HEAD]" dumps archive to standard
>>>>> > output.
>>>>>
>>>>> That could be fixed by outputting to a file.  git format-patch outputs
>>>>> to a file, so why wouldn't git achieve?
>>>>
>>>> "git format-patch" outputs to files because it generates _multiple_
>>>> files; generating single patch is special case.  Also git-format-patch
>>>> can generate file names from patch (commit) subject; it is not the case
>>>> for "git archive" (what name should it use?).
>>>
>>> What if it used the current (or topleve) directory name?  Wouldn't
>>> that work in most cases?
>>
>> Following along the same line of reasoning, it would work in most cases if
>> the output is literally named "archive.tar".  If it is not the name the
>> user wants, the user can "mv" afterwards, or give an explicit filename.
>
> Why not $sha1.tar?

Why not $(basename $(dirname $(pwd)))-$(date).tar instead?

See?  archive.tar is as good a compromise (so is a.out from cc).

> Is it portable to assume that piping is always in binmode? From a
> portability POV i could imagine piping being a problem in this
> respect, and might be why tar provides a way to output to a file and
> not just to a handle. For example ISTR that on windows piping is by
> default in text mode. I think its not a showstopper there as you can
> change it, but still, from a portability point of view you might not
> want to depend on piping.

Windows is not a showstopper to me ;-).

But seriously, I am glad that you brought up about a potential issue with
pipe.  There is one fairly important reason that it is better to say

	GZIP=-9 tar zcf here.tar.gz .

than to say

	tar cf - . | gzip -9 >here.tar.gz

but it has nothing to do with binmode.  The reason is error detection.

For exactly the same reason, if we can say

	git archive -9 --output-file=here.tar.gz HEAD

it is much better than having to say

	git archive HEAD | gzip -9 >here.tar.gz

In other words, I am not opposed to supporting a "--output-file here.tar"
at all.  I just do not want it to be mandatory.  I think that it is an
ugly kludge to force people to work it around with "-f /dev/stdout".

Oh wait.

That is exactly what we have, so what's the point of continuing this
discussion any further?  Can we just _really_ stop this time, please?

^ permalink raw reply

* Re: obnoxious CLI complaints
From: Björn Steinbrink @ 2009-09-11  3:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, Wincent Colaiuta, Brendan Miller, git
In-Reply-To: <7vbpliaaxo.fsf@alter.siamese.dyndns.org>

On 2009.09.10 11:53:23 -0700, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > First, it would be consistent with how ordinary archivers such as tar
> > or zip are used, where you have to specify list of files to archive
> > (in our case this list is HEAD).  Second, I'd rather not accidentally
> > dump binary to terminal: "git archive [HEAD]" dumps archive to standard
> > output.
> 
> So does "cat".  I do not agree with your second point.

"cat $some_binary" does, not just "cat". I guess Jakub's point was that
a command without arguments shouldn't just put some binary crap onto
your screen. Of course, "git archive HEAD" still does that, but I kind
of he where he's coming from, being one of those that tends to run
"$some_command" without arguments, just to see if it shows me some sort
of short help.

Björn

^ permalink raw reply

* Re: tracking branch for a rebase
From: Junio C Hamano @ 2009-09-11  4:54 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jeff King, Björn Steinbrink, Michael J Gruber, Pete Wyckoff,
	git
In-Reply-To: <alpine.DEB.1.00.0909100941330.8306@pacific.mpi-cbg.de>

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

> How about ^^branch? *ducks*

You'd better duck.

Just like %branch, it is a selfish and short-sighted "good enough for this
particular case but there is no room for extending it" hack that I have
serious problem with.  It closes the door for people who come later, and
such an attitude is okay only if this is _the last great invention_ and
there is no more great feature that deserves short-and-sweet notation to
come.  It might be the _latest_ great invention, but chances are that it
won't be the last.

> Seriously again, I think that ^{tracking} (with shorthand ^t, maybe) is 
> not too shabby an option.  The point is: if we make this unattractive 
> enough by requiring a lot of typing, we will never get to the point where 
> it is popular enough to make a shorthand: it just will not be used at all.

I actually think it is the other way around.

If it is so useful to be able to specify the ref your branch is based on
by applying a magic to the name of your branch, the users will use it even
if it is rather long to type, as long as the feature is easy to discover
and remember, and then they will demand a short-hand.

If on the other hand users say "Hey, I know can say 'git log X@{upstream}'
but why bother?  I always build my branch X on top of origin/X anyway, so
I'd forego that feature and type 'git log origin/X'.  It's not worth my
time to type that long magic," then the feature is not as useful as you
hoped.  And there is no point in coming up with a short-hand syntax for
it.

I personally suspect that users love to use the feature _despite_ the
initial lack of short-hand, and we would end up adding some short-hand,
and that would be a far better proof that the feature itself is useful
than "it is used just %X happens to be shorter than origin/X".

But before that happens, I'd rather not waste short-hand notations, such
as @{t} or @{u}, that will be in short-supply in the longer term.

^ permalink raw reply

* [RFC/PATCH v2] gitweb: check given hash before trying to create snapshot
From: Mark Rada @ 2009-09-11  5:01 UTC (permalink / raw)
  To: git, Jakub Narebski, Junio C Hamano

I squashed the two commits together and I also used Jakub's idea to
create a wrapper for git_get_head_hash.

--
Mark Rada (ferrous26)
marada@uwaterloo.ca


--->8---
Make things nicer in cases when you hand craft the snapshot URL but
make a typo in defining the hash variable (e.g. netx instead of next);
you will now get an error message instead of a broken tarball.

To maintain backwards compatibility, git_get_head_hash is now a wrapper
for git_get_full_hash, as suggested by Jakub Narebski.

Tests for t9501 are included to demonstrate changed functionality.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
 gitweb/gitweb.perl                       |   13 ++++++++++---
 t/t9501-gitweb-standalone-http-status.sh |   29 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b2193..a40a50d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,11 +1983,17 @@ sub quote_command {
 
 # get HEAD ref of given project as hash
 sub git_get_head_hash {
+	return git_get_full_hash(shift, 'HEAD');
+}
+
+# given a project and tree-ish, returns full hash
+sub git_get_full_hash {
 	my $project = shift;
+	my $hash = shift;
 	my $o_git_dir = $git_dir;
 	my $retval = undef;
 	$git_dir = "$projectroot/$project";
-	if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
+	if (open my $fd, '-|', git_cmd(), 'rev-parse', '--verify', $hash) {
 		my $head = <$fd>;
 		close $fd;
 		if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
@@ -5196,8 +5202,9 @@ sub git_snapshot {
 		die_error(403, "Unsupported snapshot format");
 	}
 
-	if (!defined $hash) {
-		$hash = git_get_head_hash($project);
+	my $snapshot = git_get_full_hash($project, $hash);
+	if (!$snapshot) {
+		die_error(400, "Not a valid hash id: $hash");
 	}
 
 	my $name = $project;
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index d0ff21d..81abfae 100644
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -75,4 +75,33 @@ test_expect_success \
 test_debug 'cat gitweb.output'
 
 
+# ----------------------------------------------------------------------
+# snapshot hash ids
+
+test_expect_success \
+	'snapshots: bad treeish id' \
+	'gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
+	grep "400 - Not a valid hash id:" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+	'snapshots: good treeish id' \
+	'gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+	'snapshots: bad object id' \
+	'gitweb_run "p=.git;a=snapshot;h=abcdef01234;sf=tgz" &&
+	grep "400 - Not a valid hash id:" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+	'snapshots: good object id' \
+	'ID=`git rev-parse --verify HEAD` &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
 test_done
-- 
1.6.4.2

^ permalink raw reply related

* Re: [PATCH 4/4] reset: add test cases for "--merge-dirty" option
From: Christian Couder @ 2009-09-11  5:05 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
	Jakub Narebski, Linus Torvalds
In-Reply-To: <alpine.LNX.2.00.0909101757530.28290@iabervon.org>

On Friday 11 September 2009, Daniel Barkalow wrote:
> On Thu, 10 Sep 2009, Christian Couder wrote:
>
>     This shows that with the "--merge-dirty" option,
>
>   changes that are both in the work tree and the index are kept
>
>     in the work tree after the reset (but discarded in the index). As
> with the "--merge" option,
>
>   changes that are in both the work tree and the index are discarded
>
>     after the reset.
>
> I'm lost here.
>
> If you have:
>
>          working index HEAD target
> version     B      B     A     A
>
> You get:
>
>          working index HEAD target
> --m-d       B      A     A     A
> --merge     A      A     A     A
>
> ?

Yes, files that are not different between HEAD and target are changed like 
that. Thanks for explaining it better than I could!

And also when files are different between HEAD and target (and when the 
index is the same as the working tree but different than HEAD), the result 
from -m-d and --merge are different:

If you have:

          working index HEAD target
 version     B      B     A     C

You get:

          working index HEAD target
 --m-d       B      B     A     C   : --m-d fails (so no change)
 --merge     C      C     C     C   : success but changes B discarded

In t/t7110-reset-merge.sh these differences are shown by test cases 3, 4, 5 
and 6:

--merge: discard changes added to index 1
--merge: discard changes added to index 2
--merge-dirty: not ok with touched changes in index
--merge-dirty: keep untouched changes

> > ---
> >  t/t7110-reset-merge.sh |   54
> > +++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49
> > insertions(+), 5 deletions(-)
> >
> > diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
> > index 45714ae..1e6d634 100755
> > --- a/t/t7110-reset-merge.sh
> > +++ b/t/t7110-reset-merge.sh
> > @@ -19,7 +19,7 @@ test_expect_success 'creating initial files' '
> >       git commit -m "Initial commit"
> >  '
> >
> > -test_expect_success 'ok with changes in file not changed by reset' '
> > +test_expect_success '--merge: ok if changes in file not touched by
> > reset' '
>
> Should probably have the "--merge: " from the beginning, since you're
> adding the test in this series anyway. That would make the diff come out
> clearer.

Yeah, but I am not sure that patches 3/4 and 4/4 will get merged in the end.
If they are not merged it will be better if there is no "--merge: ". 

If they get merged, the option name '--merge-dirty" will probably be changed 
to something else like "--merge-safe" so I will have to change some patches 
anyway.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCH 2/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Junio C Hamano @ 2009-09-11  5:20 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski, Linus Torvalds
In-Reply-To: <20090910202333.3722.58409.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> From: Stephan Beyer <s-beyer@gmx.net>
>
> This patch makes "reset_index_file()" call "unpack_trees()" directly
> instead of forking and execing "git read-tree".

And the reason why it is a good idea is...?

^ permalink raw reply

* Re: [PATCH 1/4] reset: add a few tests for "git reset --merge"
From: Christian Couder @ 2009-09-11  5:22 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Stephan Beyer, Daniel Barkalow
In-Reply-To: <200909102259.16469.jnareb@gmail.com>

On Thursday 10 September 2009, Jakub Narebski wrote:
> Christian Couder wrote:
> > diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
> > new file mode 100755
> > index 0000000..45714ae
> > --- /dev/null
> > +++ b/t/t7110-reset-merge.sh
> > @@ -0,0 +1,70 @@
> > +#!/bin/sh
> > +#
> > +# Copyright (c) 2009 Christian Couder
> > +#
> > +
> > +test_description='Tests for "git reset --merge"'
> > +
> > +exec </dev/null
>
> What does this do?

Nothing! Yeah, this is a mistake. 

Thanks for spoting it,
Christian.

^ permalink raw reply

* Re: [PATCH 3/4] reset: add option "--merge-dirty" to "git reset"
From: Christian Couder @ 2009-09-11  5:29 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski
In-Reply-To: <alpine.LFD.2.01.0909101617280.3654@localhost.localdomain>

On Friday 11 September 2009, Linus Torvalds wrote:
> On Thu, 10 Sep 2009, Christian Couder wrote:
> > In fact with "--merge-dirty", changes that are both in
> > the work tree and the index are kept in the work tree
> > after the reset (but discarded in the index). As with
> > "--merge", changes that are in both the work tree and
> > the index are discarded after the reset.
>
> Can you explain that again?

Yeah, I tried to explain it better in my reply to Daniel.

> That paragraph makes no sense, because it 
> boils down to saying two different thing for "changes that are in both
> the work tree and the index". First it says such changes are kept in the
> work tree (but discarded in the index), and then it says that the changes
> are discarded.
>
> Which seems very odd.
>
> Or are my reading comprehension skills lacking?

No, I realize it was my writing skills that were lacking again. I will 
rework the commit message using some tables like in Daniel's message.

> The patch also seems to imply that it's always about HEAD. True?

Yes.

Best regards,
Christian.

^ permalink raw reply

* Re: [PATCH 2/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2009-09-11  5:34 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski, Linus Torvalds
In-Reply-To: <7v1vme2h2f.fsf@alter.siamese.dyndns.org>

On Friday 11 September 2009, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > From: Stephan Beyer <s-beyer@gmx.net>
> >
> > This patch makes "reset_index_file()" call "unpack_trees()" directly
> > instead of forking and execing "git read-tree".
>
> And the reason why it is a good idea is...?

...that it should be faster.

Ok, I will add that to the commit message in the next version.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCH 3/4] reset: add option "--merge-dirty" to "git reset"
From: Junio C Hamano @ 2009-09-11  5:34 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski, Linus Torvalds
In-Reply-To: <20090910202333.3722.37592.chriscool@tuxfamily.org>

I have two comments (and I share Linus's puzzlement).

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

 - Trying to give credit to Stephan is nice, but comparing this with bits
   from the previous round, I am not sure if you need to keep calling this
   Stephan's code anymore.  Perhaps it would be enough to replace the last
   three lines in your commit log message with "Uses some code from
   Stephan's GSoC work in git://repo.or.cz/git/sbeyer.git repository".

   And I would actually prefer you take the authorship, especially if
   Stephan's series does not add an option to "reset".  Then we know whom
   to forward any bug reports to ;-)

   I did not compare this with the GSoC repository, so maybe this is what
   Stephan did after all, but still I thought it is worth to ask.

> This option is nearly like "--merge" except that it is a little bit
> safer as it seems that it tries to keep changes in the index. On the
> contrary "--merge", only keep changes in the work tree.

Nearly, a little bit, seems and tries?

These words do not build confidence that this new feature is robust and
behaves predictably.

Of course, you are not adding a random number generator, but a useful mode
with a bit more complex behaviour than "--merge that _always_ discards
changes in the index."  So as the salesman, you would need to do a bit
better job to explain what the new behaviour is, and why it is better to
have that new behaviour than the behaviour of --merge, to sell that new
feature.

^ permalink raw reply

* Re: [PATCH 4/4] reset: add test cases for "--merge-dirty" option
From: Daniel Barkalow @ 2009-09-11  5:36 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
	Jakub Narebski, Linus Torvalds
In-Reply-To: <200909110705.35083.chriscool@tuxfamily.org>

On Fri, 11 Sep 2009, Christian Couder wrote:

> On Friday 11 September 2009, Daniel Barkalow wrote:
> > On Thu, 10 Sep 2009, Christian Couder wrote:
> >
> >     This shows that with the "--merge-dirty" option,
> >
> >   changes that are both in the work tree and the index are kept
> >
> >     in the work tree after the reset (but discarded in the index). As
> > with the "--merge" option,
> >
> >   changes that are in both the work tree and the index are discarded
> >
> >     after the reset.
> >
> > I'm lost here.
> >
> > If you have:
> >
> >          working index HEAD target
> > version     B      B     A     A
> >
> > You get:
> >
> >          working index HEAD target
> > --m-d       B      A     A     A
> > --merge     A      A     A     A
> >
> > ?
> 
> Yes, files that are not different between HEAD and target are changed like 
> that. Thanks for explaining it better than I could!

I worked on the rules for merging way back when, so I've looked at tables 
of cases like that. If there are more cases to cover, it might work 
better to have a table like:

working index HEAD target         working index HEAD
   B      B     A     A   --m-d      B      A     A
                          --merge    A      A     A
   B      B     A     C   --m-d       (disallowed)
                          --merge    C      C     C

Are there other differences?

> > > ---
> > >  t/t7110-reset-merge.sh |   54
> > > +++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49
> > > insertions(+), 5 deletions(-)
> > >
> > > diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
> > > index 45714ae..1e6d634 100755
> > > --- a/t/t7110-reset-merge.sh
> > > +++ b/t/t7110-reset-merge.sh
> > > @@ -19,7 +19,7 @@ test_expect_success 'creating initial files' '
> > >       git commit -m "Initial commit"
> > >  '
> > >
> > > -test_expect_success 'ok with changes in file not changed by reset' '
> > > +test_expect_success '--merge: ok if changes in file not touched by
> > > reset' '
> >
> > Should probably have the "--merge: " from the beginning, since you're
> > adding the test in this series anyway. That would make the diff come out
> > clearer.
> 
> Yeah, but I am not sure that patches 3/4 and 4/4 will get merged in the end.
> If they are not merged it will be better if there is no "--merge: ". 

Maybe write those lines to mention "reset --merge" naturally? Like:

'ok with changes in file not changed by reset --merge'

'reset --merge discards changes added to index 1'

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH 2/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Junio C Hamano @ 2009-09-11  5:55 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Linus Torvalds
In-Reply-To: <200909110734.13903.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> On Friday 11 September 2009, Junio C Hamano wrote:
>> Christian Couder <chriscool@tuxfamily.org> writes:
>> > From: Stephan Beyer <s-beyer@gmx.net>
>> >
>> > This patch makes "reset_index_file()" call "unpack_trees()" directly
>> > instead of forking and execing "git read-tree".
>>
>> And the reason why it is a good idea is...?
>
> ...that it should be faster.
>
> Ok, I will add that to the commit message in the next version.

Please don't add "should be".  That is not a justification.

^ permalink raw reply

* Re: [PATCH 2/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Daniel Barkalow @ 2009-09-11  6:32 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
	Jakub Narebski, Linus Torvalds
In-Reply-To: <200909110734.13903.chriscool@tuxfamily.org>

On Fri, 11 Sep 2009, Christian Couder wrote:

> On Friday 11 September 2009, Junio C Hamano wrote:
> > Christian Couder <chriscool@tuxfamily.org> writes:
> > > From: Stephan Beyer <s-beyer@gmx.net>
> > >
> > > This patch makes "reset_index_file()" call "unpack_trees()" directly
> > > instead of forking and execing "git read-tree".
> >
> > And the reason why it is a good idea is...?
> 
> ...that it should be faster.
> 
> Ok, I will add that to the commit message in the next version.

There's also the benefit (IMHO, more significant) that git-read-tree's 
command line parsing is complicated, and using it from git-reset makes it 
hard to see exactly what each option of "git reset" does in terms of 
operations on the index and the working tree.

For example, it's not obvious when reading the code to run read-tree that 
all of the branches lead to the use of the "merge" flag, because some 
branches use "-m" and some use "--reset".

Actually, there's a behavior difference between the old version and the 
new version. The old version gives an error for "git reset --merge" with 
unmerged entries (unlike any other option to "git reset", AFAICT), and the 
new version does not. There's no way you'd know this without a careful 
reading of cmd_read_tree() and cross-reference with reset_index_file(), 
since the documentation doesn't mention it, the code in reset_index_file() 
only replaces "--reset" with "-m", and it seems to be doing that for the 
effect of not ignoring differences in the working tree.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Cannot clone redirecting stdout
From: Stefan Naewe @ 2009-09-11  6:39 UTC (permalink / raw)
  To: Aloisio; +Cc: git@vger.kernel.org
In-Reply-To: <fc8ab2ad0909101533l135c8003m80091cb40ec93f16@mail.gmail.com>

On 9/11/2009 12:33 AM, Aloisio wrote:
> Hi all,
> 
> I faced a problem when trying to clone git://gitorious.org/qt/qt.git
> 
> this works:
> git clone -n git://gitorious.org/qt/qt.git repo
> 
> this doesn't:
> git clone -n git://gitorious.org/qt/qt.git repo >log
> fatal: The remote end hung up unexpectedly
> fatal: early EOF
> fatal: index-pack failed
> 
> I reproduced the error in the following versions:
> git version 1.6.0.4
> git version 1.6.4.2
> git version 1.5.4.7
> 
> Any clues?
> Thanks

I'd say:

Take a look at /proc/<pid-of-git>/fd

fd 0 (stdout) is linked through a pipe to 'git index-pack'.
Redirecting this link breaks everything.

Regards,
Stefan

^ permalink raw reply

* Re: [PATCH v2] add documentation for mailinfo.scissors and '--no-scissors'
From: Junio C Hamano @ 2009-09-11  7:19 UTC (permalink / raw)
  To: Nicolas Sebrecht; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <f63e8402656c02fc2ecdcce8a37cc1d60a61aa1d.1252628643.git.nicolas.s.dev@gmx.fr>

Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:

> +---no-scissors::
> +	Do not obey to a scissors line (see linkgit:git-mailinfo[1]).
> +

obey is v.t. so "do not obey a scissors line" would be grammatical; I
think "ignore scissors lines" would be better.

> +--no-scissors::
> +	Do not obey to a scissors line. This is only useful if mailinfo.scissors is
> +	enabled (see --scissors).

Ditto; also it is useful in general if you do not know which way it is
configured.  Saying "_only_" is misleading.

	Ignore scissors lines; useful for overriding mailinfo.scissors
	settings.

> diff --git a/git-am.sh b/git-am.sh
> index 26ffe70..f242d1a 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -16,6 +16,7 @@ s,signoff       add a Signed-off-by line to the commit message
>  u,utf8          recode into utf8 (default)
>  k,keep          pass -k flag to git-mailinfo
>  c,scissors      strip everything before a scissors line
> +no-scissors     don't obey to a scissors line (default)
>  whitespace=     pass it through git-apply
>  ignore-space-change pass it through git-apply
>  ignore-whitespace pass it through git-apply

Do we want it to allow --no-no-scissors?  I do not think this hunk is
necessary at all.

^ permalink raw reply

* Re: Cannot clone redirecting stdout
From: Stefan Naewe @ 2009-09-11  7:27 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <4AA9F0A2.6050105@atlas-elektronik.com>

On 9/11/2009 8:39 AM, Stefan Naewe wrote:
> On 9/11/2009 12:33 AM, Aloisio wrote:
>> Hi all,
>>
>> I faced a problem when trying to clone git://gitorious.org/qt/qt.git
>>
>> this works:
>> git clone -n git://gitorious.org/qt/qt.git repo
>>
>> this doesn't:
>> git clone -n git://gitorious.org/qt/qt.git repo >log
>> fatal: The remote end hung up unexpectedly
>> fatal: early EOF
>> fatal: index-pack failed
>>
>> I reproduced the error in the following versions:
>> git version 1.6.0.4
>> git version 1.6.4.2
>> git version 1.5.4.7
>>
>> Any clues?
>> Thanks
> 
> I'd say:
> 
> Take a look at /proc/<pid-of-git>/fd
> 
> fd 0 (stdout) is linked through a pipe to 'git index-pack'.
> Redirecting this link breaks everything.

stdout is of course fd 1 ;-)

Regards,
Stefan
-- 
/dev/random says: Never trust a computer you can't lift. - Stan Masor

^ permalink raw reply

* Re: [RFC/PATCH 2/2] gitweb: check given hash before trying to create snapshot
From: Jakub Narebski @ 2009-09-11  7:52 UTC (permalink / raw)
  To: Mark Rada; +Cc: git, Junio C Hamano
In-Reply-To: <4AA96DAF.4070200@mailservices.uwaterloo.ca>

On Thu, 10 Sep 2009, Mark Rada wrote:

> This patch is dual purposed.
> 
> First, it makes things nicer in cases when you hand craft the snapshot
> URL but make a typo (e.g. netx instead of next); you will now get an
> error message instead of a broken tarball.

This is a very good idea.

> 
> Second, any given treeish will always be translated to the full length,
> unambiguous, hash id; this will be useful for things like creating
> unique names for snapshot caches.

But this is not a good idea, IMHO.

First, it introduces feature that nobody uses (at least yet); we can
introduce this feature when it is needed instead.

Second, I'd rather have better names for snapshots than using full SHA-1.
For snapshot of 'v1.5.0' of repository 'repo.git' I'd prefer for snapshot
to be named 'repo-v1.5.0', and for snapshot of 'next' branch of the same
project to be named for example 'repo-next-20090909', or perhaps
'repo-next-2009-09-10T09:16:18' or 'repo-next-20090909-g5f6b0ff',
or 'repo-v1.6.5-rc0-164-g5f6b0ff'.

I'm not sure what would be the best name of snapshot of given 
subdirectory...


In short: I'd rather not improve on bad design of using full SHA-1
in snapshot name.

> 
> This patch includes test for t9501 to demonstrate the changed
> functionality.
> 
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>
> ---
>  gitweb/gitweb.perl                       |    5 +++--
>  t/t9501-gitweb-standalone-http-status.sh |   26 ++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index d650188..4ae960c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5197,8 +5197,9 @@ sub git_snapshot {
>  		die_error(403, "Unsupported snapshot format");
>  	}
>  
> -	if (!defined $hash) {
> -		$hash = &git_get_hash($project);
> +	my $snapshot = &git_get_hash($project, $hash);

Same comment as for PATCH 1/2: don't use '&' subroutine call if it
is not required.


> +	if (!$snapshot) {
> +		die_error(400, "Not a valid hash id: $hash");

Note that we don't use user input in _any_ of other error messages;
you would probably need to sanitize $hash.

By the way, wouldn't 404 (Not Found) be a better error code?

>  	}
>  
>  	my $name = $project;
> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
> index d0ff21d..4f8f147 100644
> --- a/t/t9501-gitweb-standalone-http-status.sh
> +++ b/t/t9501-gitweb-standalone-http-status.sh
> @@ -75,4 +75,30 @@ test_expect_success \
>  test_debug 'cat gitweb.output'
>  
>  
> +test_expect_success \
> +	'snapshots: bad treeish id' \
> +	'gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
> +	grep "400 - Not a valid hash id:" gitweb.output'
> +test_debug 'cat gitweb.output'
> +
> +test_expect_success \
> +	'snapshots: good treeish id' \
> +	'gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
> +	grep "Status: 200 OK" gitweb.output'
> +test_debug 'cat gitweb.output'

Why you don't check for "Status: 400" too?

> +
> +test_expect_success \
> +	'snapshots: good object id' \
> +	'ID=`git rev-parse --verify HEAD` &&
> +	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
> +	grep "Status: 200 OK" gitweb.output'
> +test_debug 'cat gitweb.output'
> +
> +test_expect_success \
> +	'snapshots: bad object id' \
> +	'gitweb_run "p=.git;a=snapshot;h=abcdef01234;sf=tgz" &&
> +	grep "400 - Not a valid hash id:" gitweb.output'
> +test_debug 'cat gitweb.output'
> +
> +
>  test_done
> -- 
> 1.6.4.2
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: Cannot clone redirecting stdout
From: Jean-Luc Herren @ 2009-09-11  8:15 UTC (permalink / raw)
  To: Aloisio, git
In-Reply-To: <fc8ab2ad0909101533l135c8003m80091cb40ec93f16@mail.gmail.com>

Hi list!

Aloisio wrote:
> this doesn't:
> git clone -n git://gitorious.org/qt/qt.git repo >log
> fatal: The remote end hung up unexpectedly
> fatal: early EOF
> fatal: index-pack failed

What are you trying to store in that file?  The only line I
observe being printed to stdout while cloning is this one:

    Initialized empty Git repository in [...]/repo/.git/

Everything else (progress status) is printed to stderr and I
presume that any error messages would also go to stderr too, if
any.  So possibly you would be happy with this:

    git clone -n git://gitorious.org/qt/qt.git repo 2> log

which works fine here with git-1.6.4.2.  Though I agree that I
would expect redirecting stdout to work.

Cheers,
jlh

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Junio C Hamano @ 2009-09-11  8:54 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git, msysgit, Tom Preston-Werner, Jakub Narebski
In-Reply-To: <20090909203350.e3d7e5dc.rctay89@gmail.com>

Tay Ray Chuan <rctay89@gmail.com> writes:

>> I am in favour or removing this check, not just due to its
>> unreliability, but for the sake of consistency (we very rarely send a
>> HEAD request to poll data before doing a GET).

I think this is sensible.  The only case I think it could be a problem is
this scenario:

 - You clone from remote.

 - Remote works more and has two more packs, A and B. 

 - You fetch, but this does not need both of them (perhaps you fetched
   only one branch).  You get *.idx files for both, but *.pack file only
   for A.

 - Remote works a bit more and then repacks everything into a single pack
   C.

 - You fetch, and the walker walks the loose objects, and then finds one
   object that cannot be obtained as a loose object.  It tries to look up
   in the *.idx file and finds it in B.

   But the packfile B is long gone.

I didn't follow the codepath that uses http_get_info_packs() and then uses
repo->packs list to see what it does, but as long as the above does not
happen we should be Ok.

Thanks.

^ permalink raw reply

* Re: Cannot clone redirecting stdout
From: Jeff King @ 2009-09-11 10:23 UTC (permalink / raw)
  To: Aloisio; +Cc: git
In-Reply-To: <fc8ab2ad0909101533l135c8003m80091cb40ec93f16@mail.gmail.com>

On Thu, Sep 10, 2009 at 07:33:21PM -0300, Aloisio wrote:

> I faced a problem when trying to clone git://gitorious.org/qt/qt.git
> 
> this works:
> git clone -n git://gitorious.org/qt/qt.git repo
> 
> this doesn't:
> git clone -n git://gitorious.org/qt/qt.git repo >log
> fatal: The remote end hung up unexpectedly
> fatal: early EOF
> fatal: index-pack failed

I can reproduce the problem here. But after staring at the strace for a
long time, I don't think the problem is on the client side. The remote
end _does_ hang up unexpectedly.

Looking at what we send, the only difference between the redirected and
unredirected case I could find is that we send the "no-progress" flag to
the server, which then hangs up on us instead of sending us the pack.
Which makes no sense.

Confused,
-Peff

^ permalink raw reply

* git push stages original version on server
From: Roald de Vries @ 2009-09-11 10:40 UTC (permalink / raw)
  To: git

Hi all,

There's a thing I don't understand: I have a repository on a server  
from/to which I pull/push. But when I push to it, and do a checkout on  
the server, the pushed changes are not visible. It appears that the  
changed file is added to the index on the server automatically. Why is  
this? It seems dangerous. If I do a commit on the server, the changes  
from the client will be lost.

There's also a thing I'm not sure exists: I use a git server/client  
setup for developing a website. I would like git-pushes to appear  
immediately online (without having to ssh to the server and checking  
out manually). Is this possible?

My questions are:
- Why does git work like this?
- Can I change its behaviour (if I still want that after the answer to  
the previous question)?
- How can I have git automatically checkout pushed changes on the  
server?

Thanks in advance.

Kind regards,

Roald

^ permalink raw reply

* Re: git push stages original version on server
From: Paolo Bonzini @ 2009-09-11 11:09 UTC (permalink / raw)
  To: Roald de Vries; +Cc: git
In-Reply-To: <D7E5118C-AE88-4C4B-8D8A-5AF16ECF731D@roalddevries.nl>

Pushing to a non-bare repository is discouraged.

> - How can I have git automatically checkout pushed changes on the server?

I think you could in principle add a hook that does the "git checkout" 
or "git reset --hard $(git symbolic-ref HEAD)", but I suggest pushing to 
a bare repository and have the hook trigger a "git pull" elsewhere.

Paolo

^ permalink raw reply

* Re: git push stages original version on server
From: Mikael Magnusson @ 2009-09-11 11:11 UTC (permalink / raw)
  To: Roald de Vries; +Cc: git
In-Reply-To: <D7E5118C-AE88-4C4B-8D8A-5AF16ECF731D@roalddevries.nl>

2009/9/11 Roald de Vries <rdv@roalddevries.nl>:
> Hi all,
>
> There's a thing I don't understand: I have a repository on a server from/to
> which I pull/push. But when I push to it, and do a checkout on the server,
> the pushed changes are not visible. It appears that the changed file is
> added to the index on the server automatically. Why is this? It seems
> dangerous. If I do a commit on the server, the changes from the client will
> be lost.
>
> There's also a thing I'm not sure exists: I use a git server/client setup
> for developing a website. I would like git-pushes to appear immediately
> online (without having to ssh to the server and checking out manually). Is
> this possible?
>
> My questions are:
> - Why does git work like this?
> - Can I change its behaviour (if I still want that after the answer to the
> previous question)?
> - How can I have git automatically checkout pushed changes on the server?
>
> Thanks in advance.
>
> Kind regards,
>
> Roald

http://git.or.cz/gitwiki/GitFaq#Whywon.27tIseechangesintheremoterepoafter.22gitpush.22.3F

-- 
Mikael Magnusson

^ permalink raw reply

* previously committed file untracked prevent checking out an old  tag/branch
From: Daniele Segato @ 2009-09-11 11:26 UTC (permalink / raw)
  To: git

Hi,

I had this situation:

we used to have a properties file in our project that was used by ant
to build it.
That file was tracked since the beginning of the project (several months ago).

let's call it:
build.properties

(the project is on SVN and I'm the only one to use git)

Since everyone in the project has different directories paths I
decided to untrack the file
create a new template file and track it instead:

so I did something like this:

cp build.properties build.properties.template
vim build.properties.template
echo build.properties >> .git/info/exclude
git rm --cached build.properties
git add build.properties.template
git commit -m "finally we don't overwrite it each other anymore"

Now I had to switch back to a tag that still have that file
(build.properties) tracked to fix some
issue there but when I try I get:

error: Untracked working tree file 'build.properties' would be
overwritten by merge.

How would you handle a situation like this?

I only can think of doing a backup of my build.properties, delete it
and check out the tag or I just delete it and rebuild
it from the template later.

I did it but that made me think on it.

Actually what I really wanted here was the possibility to "locally
track" that file...

something like
git local <file>

that will have that file indexed by git so I can switch to branch that
have that file without loosing it when I go back.
furthermore that would allow me to keep versions of my local file even
if I don't want to commit it.
I could see if I accidentally modified it.

This could be useful, in my opinion.

Is there on git a feature to do something like that?
Do you think it could be useful or do you see any possible problem about that?

Regards,
Daniele

^ permalink raw reply

* Re: bash_completion outside repo
From: Michael J Gruber @ 2009-09-11 13:11 UTC (permalink / raw)
  To: james bardin; +Cc: git
In-Reply-To: <a3b675320909100813i3e70ab3at66408aebb9952c7d@mail.gmail.com>

james bardin venit, vidit, dixit 10.09.2009 17:13:
> Hi,
> 
> I'm making a git rpm for our site, and I'm getting an error with
> bash_completion on RHEL/CentOS 5.
> 
> When outside a repo, any completion causes git to spit out
>   fatal: error processing config file(s)
> 
> This is 1.6.4.2 using the contrib bash completion file
> 
> Thanks
> -jim

I can't reproduce this with git version 1.6.5.rc0.164.g5f6b0 on Fedora 11.

Which exact steps reproduce it for you, and which relevant settings (PS1
and GIT_PS1_...) do you use? Do you have a system or global .gitconfig?

Michael

^ 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