Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Miles Bader @ 2007-09-24 13:45 UTC (permalink / raw)
  To: Eygene Ryabinkin; +Cc: Johannes Schindelin, Pierre Habouzit, git
In-Reply-To: <20070924113556.GI8111@void.codelabs.ru>

Eygene Ryabinkin <rea-git@codelabs.ru> writes:
>> The comment "... holds only for a shell where [ is a builtin" doesn't
>> make any sense to me
>
> The 'while case ...' construct does not invoke any external commands.
> The 'while test ...' too, but only when 'test' is builtin.  When
> 'test' is the external binary you get one additional fork/exec per
> each cycle.

In practice that's not an issue though -- every reasonable shell has
test as a builtin these days, so the "works when test is not a builtin"
criteria is really important only for robustness.

> I believe that this trick comes from the old days where people were
> generally much more eager to save CPU cycles than now ;))

Yes.  I still occasionally find myself using "case" where if+test might
be more natural, but I think it's basically an anachronism these days,
and causes more harm by reducing readability than good.

-Miles
-- 
The car has become... an article of dress without which we feel uncertain,
unclad, and incomplete.  [Marshall McLuhan, Understanding Media, 1964]

^ permalink raw reply

* Re: behaviour of git diff, GIT_DIR & checked out tree
From: David Tweed @ 2007-09-24 13:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0709241400410.28395@racer.site>

On 9/24/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Yes, this is fully expected.
>
> The @{time} notation accesses the _reflogs_, which are purely local
> beasts.  They are not transmitted when cloning.

Thanks of your reply; however I suspect this isn't the problem
here since I just tried with just master and I get the same thing.
To be clear, I'm doing this on the same machine with the same
user account as the git directory, just from a directory outside
the git tree, eg, the git tracked tree is in $HOME/V and I'm
in $HOME/A running

env GIT_DIR=$HOME/V/.git git diff master

As a guess without looking at the code, what I imagine
is happening is that git diff limits showing changes to the directory
tree below $PWD, and when $PWD isn't actually within the git
dir that ends up somehow as an empty tree.

-- 
cheers, dave tweed__________________________
david.tweed@gmail.com
Rm 124, School of Systems Engineering, University of Reading.
"we had no idea that when we added templates we were adding a Turing-
complete compile-time language." -- C++ standardisation committee

^ permalink raw reply

* My stash wants to delete all my files
From: Jonathan del Strother @ 2007-09-24 13:12 UTC (permalink / raw)
  To: Git Mailing List

Heya,
I've run into a problem with git stash.  I suspect that I've already  
lost the work that I tried to stash, but would like to know where I  
went wrong.

I had some work in progress, wanted to check something from my last  
commit, and typed "git stash".  I forgot to "git stash apply"  
immediately afterward, and carried on doing some more work related to  
the stashed code.  I realized that I'd got a stash that I wanted to  
apply, and tried to restore it.
While trying to restore it, I typed "git stash --apply", which  
actually created a new stash named "--apply".  Ooops.  Anyway, the  
real problem is :

$ git stash list
stash@{0}: On master: --apply
stash@{1}: WIP on master: 09e3c30... Better handling of cell sizes in  
the grid

$ git stash show stash@{1}
  .gitignore                            |    3 -
  AppController.h                       |   10 -
  AppController.m                       |   30 -
  English.lproj/InfoPlist.strings       |  Bin 202 -> 0 bytes
  English.lproj/MainMenu.xib            | 2560  
---------------------------------
  GLScene.h                             |   21 -
  GLScene.m                             |  287 ----
  Info.plist                            |   28 -
  LayoutManager.h                       |   19 -
  LayoutManager.m                       |  118 --
  OpenGLTests.xcodeproj/project.pbxproj |  323 -----
  OpenGLTests_Prefix.pch                |   10 -
  TheWall.h                             |   28 -
  TheWall.m                             |   99 --
  UserImage.h                           |   17 -
  UserImage.m                           |  111 --
  Utilities.h                           |   38 -
  Utilities.m                           |   89 --
  main.m                                |   14 -
  19 files changed, 0 insertions(+), 3805 deletions(-)


Hmm.  Looks like it's trying to delete all of my versioned files.   
"git stash apply stash@{1}" confirms this.   Obviously that's not what  
I tried to stash - my WIP when I stashed was a few additions, a couple  
of new unversioned files, and moving a few lines of code from one file  
to another.

Any ideas what happened there?  I can't seem to reproduce the problem  
in a test repository.

Cheers,
Jon

^ permalink raw reply

* Re: behaviour of git diff, GIT_DIR & checked out tree
From: Johannes Schindelin @ 2007-09-24 13:03 UTC (permalink / raw)
  To: David Tweed; +Cc: Git Mailing List
In-Reply-To: <e1dab3980709240545o32eeefcdkd4bc67abab0e5343@mail.gmail.com>

Hi,

On Mon, 24 Sep 2007, David Tweed wrote:

> In a git tracked tree rooted at $HOME/V with
> git-dir of $HOME/V/.git, if I'm in $HOME/V then
> 
> git diff master@{midnight}
> 
> tells me the difference between the current modified files
> in the tree being tracked in V and the specified commit. In
> a different directory, OUTSIDE of $HOME/V, I tried
> 
> env GIT_DIR=$HOME/V/.git git diff master@{midnight}
> 
> to get the same effect but, whilst I do get a diff output, it
> looks like a diff of the commit against an empty tree.

Yes, this is fully expected.

The @{time} notation accesses the _reflogs_, which are purely local 
beasts.  They are not transmitted when cloning.

The rationale: in a distributed environment, you cannot trust others' 
timestamps.  Therefore we don't.

We can only (to a certain extent, at least) trust our _own_ timestamp.  
That is why we have timestamp access to the reflogs (which are purely 
local, as I mentioned above), but we have no way to reference commits by 
timestamp otherwise.

Hth,
Dscho

^ permalink raw reply

* behaviour of git diff, GIT_DIR & checked out tree
From: David Tweed @ 2007-09-24 12:45 UTC (permalink / raw)
  To: Git Mailing List

I'm doing something "sensible people don't do"
-- I know this -- but I'm getting results I don't
understand, so I'd appreciate any insight.

In a git tracked tree rooted at $HOME/V with
git-dir of $HOME/V/.git, if I'm in $HOME/V then

git diff master@{midnight}

tells me the difference between the current modified files
in the tree being tracked in V and the specified commit. In
a different directory, OUTSIDE of $HOME/V, I tried

env GIT_DIR=$HOME/V/.git git diff master@{midnight}

to get the same effect but, whilst I do get a diff output, it
looks like a diff of the commit against an empty tree. (Using

env GIT_DIR=$HOME/V/.git git diff HEAD master@{midnight}

works ok.) I suppose I could alias it to

cd $HOME/V && command && cd -

but that seems a bit clunky; is there another way to explicitly
say "working tree for repo with this GIT_DIR" to git diff?

[Why I'm doing this: I actually work on files in the $HOME/V
via various other directories containing symlinks to the basic
files (with tracked files being pointed to by multiple symlinks).
So I'm almost never actually "in" the tracked tree. This
usage makes more sense for my tasks than being within
the work tree.]

-- 
cheers, dave tweed__________________________
david.tweed@gmail.com
Rm 124, School of Systems Engineering, University of Reading.
"we had no idea that when we added templates we were adding a Turing-
complete compile-time language." -- C++ standardisation committee

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Kastrup @ 2007-09-24 11:39 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0709241128460.28395@racer.site>

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

> The reason we used "case" is that this has always been a builtin
> (has to be, because it changes workflow).
>
> Therefore I am somewhat uneasy that the patch went in so easily,

It didn't yet.

> especially given a message that flies in the face of our endeavours
> to make git less dependent on any given shell (as long as it is not
> broken to begin with).

"test" is not actually a shell dependency since it is available as an
external when not available as builtin.  And if you really want to
prefer "case" over "test" because the latter is not a built-in in a
small number of shells, then it should be done consistently everywhere
and not just in code I touch.

-- 
David Kastrup

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Eygene Ryabinkin @ 2007-09-24 11:35 UTC (permalink / raw)
  To: Miles Bader; +Cc: Johannes Schindelin, Pierre Habouzit, git
In-Reply-To: <87ps08s3zt.fsf@catnip.gol.com>

Miles,

Mon, Sep 24, 2007 at 08:21:42PM +0900, Miles Bader wrote:
> > The reason we used "case" is that this has always been a builtin (has to 
> > be, because it changes workflow).
> >
> > Therefore I am somewhat uneasy that the patch went in so easily, 
> > especially given a message that flies in the face of our endeavours to 
> > make git less dependent on any given shell (as long as it is not broken to 
> > begin with).
> 
> The comment "... holds only for a shell where [ is a builtin" doesn't
> make any sense to me

The 'while case ...' construct does not invoke any external commands.
The 'while test ...' too, but only when 'test' is builtin.  When
'test' is the external binary you get one additional fork/exec per
each cycle.

I believe that this trick comes from the old days where people were
generally much more eager to save CPU cycles than now ;))  But if
you tried to run Cygwin on the moderately new machines (like PIII),
you still can notice that commands like 'man' are a bit slow just
because they require a bunch of preprocessors (and processes in
pipe) to run.  So, "save the process, kill the fork/exec" ;))

> though:  "-ne" is a standard operator even if "["
> isn't a builtin.  It's useful if you are testing numbers in potentially
> non-canonical form, e.g., with leading zeroes or something, and AFAIK is
> quite portable.

In general -- yes, it is portable and good for leading zeros.  But
in the case of $#, there will hardly be leading zeros or something
else.  And using '!=' typically saves you one atoi() + some other
checks, since it typically translates to a bare strcmp().
-- 
Eygene

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Miles Bader @ 2007-09-24 11:21 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Pierre Habouzit, git
In-Reply-To: <Pine.LNX.4.64.0709241128460.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> >   $# != 0 would yield sth like (strcmp(sprintf("%d", argc), "0"))
>> >   $# -ne 0 would yield sth like (argc != atoi("0")).
>> 
>>   Of course this holds only for shell where test/[ is a builtin, which
>> is the at least the case for zsh, bash, and dash (but not posh).
>
> The reason we used "case" is that this has always been a builtin (has to 
> be, because it changes workflow).
>
> Therefore I am somewhat uneasy that the patch went in so easily, 
> especially given a message that flies in the face of our endeavours to 
> make git less dependent on any given shell (as long as it is not broken to 
> begin with).

The comment "... holds only for a shell where [ is a builtin" doesn't
make any sense to me though:  "-ne" is a standard operator even if "["
isn't a builtin.  It's useful if you are testing numbers in potentially
non-canonical form, e.g., with leading zeroes or something, and AFAIK is
quite portable.

-Miles
-- 
Everywhere is walking distance if you have the time.  -- Steven Wright

^ permalink raw reply

* Re: git-send-email is omitting author and date lines
From: Johannes Schindelin @ 2007-09-24 11:10 UTC (permalink / raw)
  To: martin f krafft; +Cc: Junio C Hamano, Hanspeter Kunz, git
In-Reply-To: <20070924110817.GA15797@lapse.madduck.net>

Hi,

On Mon, 24 Sep 2007, martin f krafft wrote:

> But information *does* get lost, namely the date. If I send a patch
> by email and it gets merged by the recipient, the date of the commit
> will be the date of the email, not the date of original commit, or
> when it was merged.

And that is perfectly okay, since as far as the public is concerned, this 
is the date of the patch.

Ciao,
Dscho

^ permalink raw reply

* Re: git-send-email is omitting author and date lines
From: martin f krafft @ 2007-09-24 11:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Hanspeter Kunz, git
In-Reply-To: <7v4phlc668.fsf@gitster.siamese.dyndns.org>

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

also sprach Junio C Hamano <gitster@pobox.com> [2007.09.24.0029 +0100]:
> Was the commit authored by yourself?

Sorry for barking in here, but Hanspeter tried to submit a patch to
me and that's when this issue arose.

Yes, he is the author of the commit, as in: user.name and user.email
are the same for the commit and for the email, and this seems to be
the reason the commit header is left out of the email, causing
git-am to use the RFC822 From header instead:

  - patch by Jane, email sent From Jane: no separate From header
  - patch by Jane, email sent From Joe: separate From Jane header 

But information *does* get lost, namely the date. If I send a patch
by email and it gets merged by the recipient, the date of the commit
will be the date of the email, not the date of original commit, or
when it was merged.

To be honest, I have no real preference on this, but it just seems
strange that out of three dates:

  - original commit
  - email sent
  - commit merged

git-send-email/git-am cause the second to be used, which is the
one that makes the *least* sense IMHO.

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
"i dislike arguments of any kind. they are always vulgar, and often
 convincing."
                                                        -- oscar wilde
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] resend: really plug memory leaks in git-svnimport
From: Stefan Sperling @ 2007-09-24 10:57 UTC (permalink / raw)
  To: git

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

Junio asked me to resend this patch to the mailing list.

This version of the patch is adjusted to apply cleanly
to current HEAD.

@Junio: I'm not resending the multiple branch/tag dirs patch
just yet, because I want to polish it first -- I've got another
idea how to improve it.

Log message:

Fix pool handling in git-svnimport to avoid memory leaks.

- Create an explicit one-and-only root pool.
- Closely follow examples in SVN::Core man page.
  Before calling a subversion function, create a subpool of our
  root pool and make it the new default pool. 
- Create a subpool for looping over svn revisions and clear
  this subpool (i.e. it mark for reuse, don't decallocate it)
  at the start of the loop instead of allocating new memory
  with each iteration.

See http://marc.info/?l=git&m=118554191513822&w=2 for a detailed
explanation of the issue.

Signed-off-by: Stefan Sperling <stsp@elego.de>

diff --git a/git-svnimport.perl b/git-svnimport.perl
index aa5b3b2..ea8c1b2 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -54,6 +54,7 @@ my $branch_name = $opt_b || "branches";
 my $project_name = $opt_P || "";
 $project_name = "/" . $project_name if ($project_name);
 my $repack_after = $opt_R || 1000;
+my $root_pool = SVN::Pool->new_default;
 
 @ARGV == 1 or @ARGV == 2 or usage();
 
@@ -132,7 +133,7 @@ sub conn {
 	my $auth = SVN::Core::auth_open ([SVN::Client::get_simple_provider,
 			  SVN::Client::get_ssl_server_trust_file_provider,
 			  SVN::Client::get_username_provider]);
-	my $s = SVN::Ra->new(url => $repo, auth => $auth);
+	my $s = SVN::Ra->new(url => $repo, auth => $auth, pool => $root_pool);
 	die "SVN connection to $repo: $!\n" unless defined $s;
 	$self->{'svn'} = $s;
 	$self->{'repo'} = $repo;
@@ -147,11 +148,10 @@ sub file {
 
 	print "... $rev $path ...\n" if $opt_v;
 	my (undef, $properties);
-	my $pool = SVN::Pool->new();
 	$path =~ s#^/*##;
+	my $subpool = SVN::Pool::new_default_sub;
 	eval { (undef, $properties)
-		   = $self->{'svn'}->get_file($path,$rev,$fh,$pool); };
-	$pool->clear;
+		   = $self->{'svn'}->get_file($path,$rev,$fh); };
 	if($@) {
 		return undef if $@ =~ /Attempted to get checksum/;
 		die $@;
@@ -185,6 +185,7 @@ sub ignore {
 
 	print "... $rev $path ...\n" if $opt_v;
 	$path =~ s#^/*##;
+	my $subpool = SVN::Pool::new_default_sub;
 	my (undef,undef,$properties)
 	    = $self->{'svn'}->get_dir($path,$rev,undef);
 	if (exists $properties->{'svn:ignore'}) {
@@ -202,6 +203,7 @@ sub ignore {
 sub dir_list {
 	my($self,$path,$rev) = @_;
 	$path =~ s#^/*##;
+	my $subpool = SVN::Pool::new_default_sub;
 	my ($dirents,undef,$properties)
 	    = $self->{'svn'}->get_dir($path,$rev,undef);
 	return $dirents;
@@ -358,10 +360,9 @@ open BRANCHES,">>", "$git_dir/svn2git";
 
 sub node_kind($$) {
 	my ($svnpath, $revision) = @_;
-	my $pool=SVN::Pool->new;
 	$svnpath =~ s#^/*##;
-	my $kind = $svn->{'svn'}->check_path($svnpath,$revision,$pool);
-	$pool->clear;
+	my $subpool = SVN::Pool::new_default_sub;
+	my $kind = $svn->{'svn'}->check_path($svnpath,$revision);
 	return $kind;
 }
 
@@ -889,7 +890,7 @@ sub commit_all {
 	# Recursive use of the SVN connection does not work
 	local $svn = $svn2;
 
-	my ($changed_paths, $revision, $author, $date, $message, $pool) = @_;
+	my ($changed_paths, $revision, $author, $date, $message) = @_;
 	my %p;
 	while(my($path,$action) = each %$changed_paths) {
 		$p{$path} = [ $action->action,$action->copyfrom_path, $action->copyfrom_rev, $path ];
@@ -925,14 +926,14 @@ print "Processing from $current_rev to $opt_l ...\n" if $opt_v;
 my $from_rev;
 my $to_rev = $current_rev - 1;
 
+my $subpool = SVN::Pool::new_default_sub;
 while ($to_rev < $opt_l) {
+	$subpool->clear;
 	$from_rev = $to_rev + 1;
 	$to_rev = $from_rev + $repack_after;
 	$to_rev = $opt_l if $opt_l < $to_rev;
 	print "Fetching from $from_rev to $to_rev ...\n" if $opt_v;
-	my $pool=SVN::Pool->new;
-	$svn->{'svn'}->get_log("/",$from_rev,$to_rev,0,1,1,\&commit_all,$pool);
-	$pool->clear;
+	$svn->{'svn'}->get_log("/",$from_rev,$to_rev,0,1,1,\&commit_all);
 	my $pid = fork();
 	die "Fork: $!\n" unless defined $pid;
 	unless($pid) {

-- 
Stefan Sperling <stsp@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner

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

^ permalink raw reply related

* Re: [OT] Re: C++ *for Git*
From: Dmitry Potapov @ 2007-09-24 10:46 UTC (permalink / raw)
  To: Reece Dunn
  Cc: Linus Torvalds, Marco Costalba, Pierre Habouzit, David Kastrup,
	Frank Lichtenheld, Alex Unleashed, Kyle Rose, Miles Bader,
	Dmitry Kakurin, Git
In-Reply-To: <3f4fd2640709231525q52a9865alc834ca46b85998fe@mail.gmail.com>

On Sun, Sep 23, 2007 at 11:25:01PM +0100, Reece Dunn wrote:
> The next version of C++ is going to have garbage collection that the
> user can enable, disable or remain neutral about. However, this is
> program-wide and has many traps that you could fall into.

Sure. C++ has not been design to be garbage collection friendly, in
fact, even now, you can use some GC with C++, but it can be painful.
I don't think that the new standard will change much in this respect.

> > > But other parts of C++ are just nasty. The whole OO layer seems designed
> > > to do a lot of things implicitly and in the wrong way.
> >
> > It could do a lot of things implicitly, but it does not force you,
> > except calling destructor when the control leaves the scope of
> > declaration, but I hardly can consider it as implicit.
> 
> You have to add the explicit keyword to any constructor to prevent an
> automatic conversion. Therefore, the constructors that are called are
> implicit by default.

Yes, I would prefer if it were opposite by default, but it was an
initial mistake in design, and you cannot change it without breaking
a lot of people code.

> If you have a conversion operator, this is always
> implicitly called when there is a match by the compiler.

Conversation operator should be written only if you do want an implicit
conversation, and that may be useful sometimes, albeit very rarely.

> 
> I agree with Linus here, there are a lot of things that happen implicily.
> 
> > > I also disagree with exception handling,
> >
> > Perhaps, you look at it from the kernel point of view. Otherwise, I
> > would like to hear your arguments against it. In fact, I don't think
> > it is possible to write generic algorithms without exceptions. Of
> > course, if you write a program that can print an error to stderr and
> > exit, there is no much need for them. So, it may depend on the task.
> 
> There are many issues with exceptions.
> 
> Firstly, there is throwing an exception from a destructor, which is
> warned against in any good C++ book, but does not prevent you from
> doing so (even if it is inadvertantly)!

In general, the compiler does not have all information to know whether
a destructor can or cannot throw an exception, and even less it knows
about your real intentions. There are many ways to write something
that will not work. You can create an infinite recursion, but I don't
think it is a good argument against recursion.

> 
> More importantly though, is the loss of contextual information.

Do you think that an error code contains much more contextual
information?

> Consider throwing the same exception on all calls to API that return
> the same error code type. 

I did not mean that all error codes should be returned as an exception.
Exception in C++ is something that should not normally happen, like
failure to allocate memory. So, you usually do not want to handle
this situation immediate.

> The code that processes this may be anywhere
> in the system. This makes it impossible to do any sensible recovery
> (if possible), or error reporting. The exception can be rethrown or
> translated to another exception, making it impossible to find the
> originator of the exception. This makes it harder, if not impossible,
> to track the exception back to the source when you are at a breakpoint
> in the exception handler.

In gdb, you can catch all exception when thrown using "catch throw".
And again, I don't see how it is better when a program returns an
error code, especially if this error code is recoded couple times
in the process of returning. So the problem is not with exceptions,
but usually with bad design.

> Then there is dealing with caller boundaries. That is, when a callback
> or interface function in the application will return to the operating
> system (e.g. when handling a draw request from X11), or another
> language such as Python. Also, because different compiler vendors and
> versions handle exceptions differently, if you want to support
> different compilers (and you have resolved the name mangling
> incompatibilities), you need to handle exceptions correctly in these
> cases, or risk having major problems that would be impossible to
> trace.

You named some interoperability issues with C++ (and there are many
of them), but it is not an argument against exceptions per se.

> Not to mention that anywhere new, dynamic_cast and other
> language features are used may throw exceptions.

dynamic_cast throws an exception only for references, but not for
pointers, and there is a good reason for that -- references should
not be NULL; and if you want to avoid bad_alloc exception, you can
use "T* p = new (std::nothrow) T;" but it is rarely needed.


Dmitry Potapov

^ permalink raw reply

* Re: [PATCH 2/2] Make builtin-rerere use of strbuf nicer and more efficient.
From: Johannes Schindelin @ 2007-09-24 10:38 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <1190625904-22808-3-git-send-email-madcoder@debian.org>

Hi,

both patches appear to be obviously correct to me.  (However, I was lazy 
enough not to compile and test, but I'd not expect any breakage there, 
given your previous patch serieses.  ;-)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Johannes Schindelin @ 2007-09-24 10:33 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070924080436.GB9112@artemis.corp>

Hi,

On Mon, 24 Sep 2007, Pierre Habouzit wrote:

> On Mon, Sep 24, 2007 at 08:01:34AM +0000, Pierre Habouzit wrote:
> > On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:
> > > "David Symonds" <dsymonds@gmail.com> writes:
> > > 
> > > > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
> > > >> Mike Hommey <mh@glandium.org> writes:
> > > >>
> > > >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
> > > >> >> -while case $# in 0) break ;; esac
> > > >> >> +while test $# != 0
> > > >> >
> > > >> > Wouldn't -ne be better ?
> > > >>
> > > >> Why?
> > > >
> > > > Because -ne does a numeric comparison, != does a string comparison,
> > > > and it's a numeric comparison happening, semantically speaking.
> > > 
> > > I don't see the point in converting $# and 0 into numbers before
> > > comparing them.  "!=" is quite more readable, and the old code also
> > > compared the strings.
> > 
> >   Fwiw $# already is a number. Hence test $# -ne 0 is definitely a
> > better test.
> > 
> >   $# != 0 would yield sth like (strcmp(sprintf("%d", argc), "0"))
> >   $# -ne 0 would yield sth like (argc != atoi("0")).
> 
>   Of course this holds only for shell where test/[ is a builtin, which
> is the at least the case for zsh, bash, and dash (but not posh).

The reason we used "case" is that this has always been a builtin (has to 
be, because it changes workflow).

Therefore I am somewhat uneasy that the patch went in so easily, 
especially given a message that flies in the face of our endeavours to 
make git less dependent on any given shell (as long as it is not broken to 
begin with).

Ciao,
Dscho

^ permalink raw reply

* Re: [EGIT PATCH] Change to simplified icon.
From: Johannes Schindelin @ 2007-09-24 10:26 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Robin Rosenberg, Ben Konrath, git, David Watson
In-Reply-To: <20070924041723.GF3099@spearce.org>

Hi,

On Mon, 24 Sep 2007, Shawn O. Pearce wrote:

> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > onsdag 19 september 2007 skrev Robin Rosenberg:
> > > onsdag 19 september 2007 skrev Robin Rosenberg:
> > > > I'll leave this one in my patch tree for now. The Egit icon contest is stil 
> > > open to all.
> > > 
> > > Here's another icon. The cup inspired the drug used to code the plugin. The 
> > > +/-'s from the git logo, the blue color to look better with the other icons 
> > > in Eclipse's about box and the rest comes form the fact that I cannot draw.
> > > 
> > And the subject matter :)
> 
> Very cute.  I actually like it.  Maybe I'll take a stab at creating
> a git-gui icon, because like you I also cannot draw.  A feather
> and some pluses and minuses can't be that hard, can it?  :)

You might want to look at http://git.or.cz/gitwiki/GitRelatedLogos (I 
especially like Henrik Nyh's, we use it in git-gui).  Might be a good 
starting point.

Ciao,
Dscho

^ permalink raw reply

* mini-refactor in rerere.c
From: Pierre Habouzit @ 2007-09-24  9:25 UTC (permalink / raw)
  To: gitster; +Cc: git

  Here is a smallish series in builtin-rerere.c. Nothing very exciting
though.

^ permalink raw reply

* Re: [PATCH] rebase -i: commit when continuing after "edit"
From: Dmitry Potapov @ 2007-09-24  9:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709240121080.28395@racer.site>

On Mon, Sep 24, 2007 at 01:29:30AM +0100, Johannes Schindelin wrote:
> 	On Mon, 24 Sep 2007, Dmitry Potapov wrote:
> 
> 	> I have tried to use git-rebase --interactive today, and run into 
> 	> a strange error message saying:
> 	> 
> 	> /usr/bin/git-rebase--interactive: \
> 	>	line 333: $GIT_DIR/.dotest-merge/author-script: \
> 	>		No such file or directory
> 
> 	Could you please apply this patch and try if the issue is gone?

I have tested it only on a couple cases, but everything works fine now.

Thank you,
Dmitry

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Kastrup @ 2007-09-24  8:29 UTC (permalink / raw)
  To: git
In-Reply-To: <20070924080134.GA9112@artemis.corp>

Pierre Habouzit <madcoder@debian.org> writes:

> On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:
>> "David Symonds" <dsymonds@gmail.com> writes:
>> 
>> > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
>> >> Mike Hommey <mh@glandium.org> writes:
>> >>
>> >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
>> >> >> -while case $# in 0) break ;; esac
>> >> >> +while test $# != 0
>> >> >
>> >> > Wouldn't -ne be better ?
>> >>
>> >> Why?
>> >
>> > Because -ne does a numeric comparison, != does a string comparison,
>> > and it's a numeric comparison happening, semantically speaking.
>> 
>> I don't see the point in converting $# and 0 into numbers before
>> comparing them.  "!=" is quite more readable, and the old code also
>> compared the strings.
>
>   Fwiw $# already is a number.

It isn't.

> Hence test $# -ne 0 is definitely a better test.

/* TEST/[ builtin. */
int
test_builtin (list)
     WORD_LIST *list;
{
  char **argv;
  int argc, result;

  /* We let Matthew Bradburn and Kevin Braunsdorf's code do the
     actual test command.  So turn the list of args into an array
     of strings, since that is what their code wants. */
  if (list == 0)
    {
      if (this_command_name[0] == '[' && !this_command_name[1])
	{
	  builtin_error ("missing `]'");
	  return (EX_BADUSAGE);
	}

      return (EXECUTION_FAILURE);
    }

  argv = make_builtin_argv  (list, &argc);
  result = test_command (argc, argv);
  free ((char *)argv);

  return (result);
}

>   $# != 0 would yield sth like (strcmp(sprintf("%d", argc), "0"))
>   $# -ne 0 would yield sth like (argc != atoi("0")).
>
>   Not that it matters much, but the latter looks better to me.

Not to me.  The code does not support your argument, and all $x
expansions certainly are strings, according to manual and usage.  I
will rework the patch this evening in order to get a commit message
more placable to Junio, and I will at his request remove all of the
(redundant) quoting.  But removing quoting from $# does not turn it
into a number: it remains the same string '0'.  If someone else feels
he should replace all "=" and "!=" tests for "numeric" comparisons
with the unreadable numeric tests, he can go ahead proposing a
separate patch that should not just cover $#.

You can have bash declare numeric variables, but even they are strings
(they just auto-evaluate on assignment):

declare -i nonsense
nonsense="3+$(echo 4)"
echo "$nonsense"

gives 7, even though everything has been "strings" here.

-- 
David Kastrup

^ permalink raw reply

* Re: [rfc] git submodules howto
From: Miklos Vajna @ 2007-09-24  8:30 UTC (permalink / raw)
  To: Uwe Kleine-König, git
In-Reply-To: <20070924071135.GB3278@informatik.uni-freiburg.de>

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

On Mon, Sep 24, 2007 at 09:11:35AM +0200, Uwe Kleine-König <ukleinek@informatik.uni-freiburg.de> wrote:
> I get 
> 
> 	bash: dg: command not found
> 
> :-)  Probably s/dg/git/

yes, thanks for the correction

- VMiklos

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

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Pierre Habouzit @ 2007-09-24  8:04 UTC (permalink / raw)
  To: David Kastrup, git
In-Reply-To: <20070924080134.GA9112@artemis.corp>

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

On Mon, Sep 24, 2007 at 08:01:34AM +0000, Pierre Habouzit wrote:
> On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:
> > "David Symonds" <dsymonds@gmail.com> writes:
> > 
> > > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
> > >> Mike Hommey <mh@glandium.org> writes:
> > >>
> > >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
> > >> >> -while case $# in 0) break ;; esac
> > >> >> +while test $# != 0
> > >> >
> > >> > Wouldn't -ne be better ?
> > >>
> > >> Why?
> > >
> > > Because -ne does a numeric comparison, != does a string comparison,
> > > and it's a numeric comparison happening, semantically speaking.
> > 
> > I don't see the point in converting $# and 0 into numbers before
> > comparing them.  "!=" is quite more readable, and the old code also
> > compared the strings.
> 
>   Fwiw $# already is a number. Hence test $# -ne 0 is definitely a
> better test.
> 
>   $# != 0 would yield sth like (strcmp(sprintf("%d", argc), "0"))
>   $# -ne 0 would yield sth like (argc != atoi("0")).

  Of course this holds only for shell where test/[ is a builtin, which
is the at least the case for zsh, bash, and dash (but not posh).

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: Pierre Habouzit @ 2007-09-24  8:01 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <86ejgowl5g.fsf@lola.quinscape.zz>

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

On Mon, Sep 24, 2007 at 07:57:31AM +0000, David Kastrup wrote:
> "David Symonds" <dsymonds@gmail.com> writes:
> 
> > On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
> >> Mike Hommey <mh@glandium.org> writes:
> >>
> >> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
> >> >> -while case $# in 0) break ;; esac
> >> >> +while test $# != 0
> >> >
> >> > Wouldn't -ne be better ?
> >>
> >> Why?
> >
> > Because -ne does a numeric comparison, != does a string comparison,
> > and it's a numeric comparison happening, semantically speaking.
> 
> I don't see the point in converting $# and 0 into numbers before
> comparing them.  "!=" is quite more readable, and the old code also
> compared the strings.

  Fwiw $# already is a number. Hence test $# -ne 0 is definitely a
better test.

  $# != 0 would yield sth like (strcmp(sprintf("%d", argc), "0"))
  $# -ne 0 would yield sth like (argc != atoi("0")).

  Not that it matters much, but the latter looks better to me.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Kastrup @ 2007-09-24  7:57 UTC (permalink / raw)
  To: git
In-Reply-To: <ee77f5c20709232330n7b47d9e9v38677678dbf197da@mail.gmail.com>

"David Symonds" <dsymonds@gmail.com> writes:

> On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
>> Mike Hommey <mh@glandium.org> writes:
>>
>> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
>> >> -while case $# in 0) break ;; esac
>> >> +while test $# != 0
>> >
>> > Wouldn't -ne be better ?
>>
>> Why?
>
> Because -ne does a numeric comparison, != does a string comparison,
> and it's a numeric comparison happening, semantically speaking.

I don't see the point in converting $# and 0 into numbers before
comparing them.  "!=" is quite more readable, and the old code also
compared the strings.

-- 
David Kastrup

^ permalink raw reply

* Re: git-send-email is omitting author and date lines
From: Hanspeter Kunz @ 2007-09-24  7:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709240129450.28395@racer.site>

On Mon, 2007-09-24 at 01:30 +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 24 Sep 2007, Hanspeter Kunz wrote:
> 
> > On Sun, 2007-09-23 at 16:29 -0700, Junio C Hamano wrote:
> > > Hanspeter Kunz <hp@edelkunz.ch> writes:
> > > 
> > > > When sending a patch to myself using `git-send-email` I realized that
> > > > the lines containing the author and the date (lines 5 and 6 in the patch
> > > > file) were not in the sent email.
> > > 
> > > Was the commit authored by yourself?
> > 
> > yes. does this make a difference?
> 
> Yes, it does.  The author is usually inferred from the sender of the mail, 
> and therefore git-send-email strips that information from the message 
> (IIRC).

Ah, I see. And this applies also to the date, I guess.

Many thanks,
Hp.

^ permalink raw reply

* Re: [rfc] git submodules howto
From: Uwe Kleine-König @ 2007-09-24  7:11 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <20070918105538.GL19019@genesis.frugalware.org>

Hello,

Miklos Vajna wrote:
> $ mkdir lib
> $ cd lib
> $ git init
> Initialized empty Git repository in .git/
> $ echo "libmakefile" > Makefile
> $ dg add Makefile
>
> [...]
>
> 1) is this correct? :) i use it and it seem to do what i except, but
> maybe it's not correct

I get 

	bash: dg: command not found

:-)  Probably s/dg/git/

Best regards
Uwe

-- 
Uwe Kleine-König

http://www.google.com/search?q=half+a+cup+in+teaspoons

^ permalink raw reply

* Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
From: David Symonds @ 2007-09-24  6:30 UTC (permalink / raw)
  To: David Kastrup; +Cc: Mike Hommey, git
In-Reply-To: <85k5qgk295.fsf@lola.goethe.zz>

On 24/09/2007, David Kastrup <dak@gnu.org> wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > On Sun, Sep 23, 2007 at 10:42:08PM +0200, David Kastrup wrote:
> >> -while case $# in 0) break ;; esac
> >> +while test $# != 0
> >
> > Wouldn't -ne be better ?
>
> Why?

Because -ne does a numeric comparison, != does a string comparison,
and it's a numeric comparison happening, semantically speaking.


Dave.

^ 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