Git development
 help / color / mirror / Atom feed
* Re: Qgit performance and maintain CVS environment with GIT repository
From: Jan Wielemaker @ 2007-10-19 10:34 UTC (permalink / raw)
  To: pete
  Cc: Johannes Schindelin, Marco Costalba, Robin Rosenberg,
	piet.delaney, Linus Torvalds, VMiklos, free cycle, git,
	piet.delaney, Piet Delaney
In-Reply-To: <471871BD.7030608@bluelane.com>

On Friday 19 October 2007 10:58, Pete/Piet Delaney wrote:
> Jan Wielemaker wrote:
> > On Friday 19 October 2007 02:22, Pete/Piet Delaney wrote:
> >> We are definitely not fine with CVS, the branch merging isn't
> >> comfortable. I'm just wondering about maintaining the existing
> >> CVS browsers and the build scripts if it's not a big deal. I'll
> >> try the git-cvsserver path. If anyone has any war stories to share
> >> on the path this would be an ideal time to share them.
> >
> > As for web browsing the history, our project was quickly convinced
> > gitweb is a lot better than cvsweb.  We are starting to get use to
> > basic git.  One developer works on CVS.  This is a bit handicapped,
> > but workable after a few patches to git-shell and git-cvsserver.
>
> Could you tell me a bit more about those patches and the need for using
> git-shell (haven't even messed with that yet).

One patch concerned handling "cvs update -p", which was accepted and I
guess will end up in the stable version someday.  One concerned handling
"cvs diff -c", which I never submitted.  I first tried a more general
approach to get diff option processing complete, but I had to backtrack
on that.  Now I have a quite simple hack, but more complete coverage of
diff option processing requires a bit more perl knowledge than I have.

I submitted a patch for shell.c to make it call "git cvsserver server"
if a commandline "cvs server" was passed to it, so you can do CVS+SSH
compatible to normal CVS.  I got so many comments I decided to keep it
for myself for now.

> I don't think we need to have any developers continuing to use CVS;
> but I may be wrong. I think I read that there's a limitation to being
> on the main branch and unfortunately most of out tags are on a release
> branch.

No, you can checkout any GIT branch as it it were a CVS module.

	--- Jan

^ permalink raw reply

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Andreas Ericsson @ 2007-10-19 10:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: David Symonds, Jeff King, git
In-Reply-To: <20071019073938.GN14735@spearce.org>

Shawn O. Pearce wrote:
>  
> +static void determine_window_size(void)
> +{
> +	struct winsize ws;
> +	if (!ioctl(2, TIOCGWINSZ, &ws))
> +		ws_cols = ws.ws_col;
> +}
> +

I'd suggest re-using term_columns() from help.c instead. It's been in there
since the git wrapper was rewritten in C, so it's had a bit of testing and
seems to work fairly well.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: gitk patch collection pull request
From: Paul Mackerras @ 2007-10-19 11:05 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071019052823.GI14735@spearce.org>

Shawn O. Pearce writes:

> The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:
>   Paul Mackerras (1):
>         gitk: Fix bug causing undefined variable error when cherry-picking
> 
> are available in the git repository at:
> 
>   git://repo.or.cz:/git/spearce.git gitk

OK, but ...

> Jonathan del Strother (2):
>       gitk: Added support for OS X mouse wheel
>       Fixing gitk indentation

This one is bogus.  Firstly, it doesn't have "gitk:" at the start of
the headline (and "Fixing" should be "Fix").  Secondly, the actual
change itself is bogus.  It changes an initial tab to 8 spaces on each
of 4 lines.  I like it the way it is - and if he wanted to change it,
he should have changed it throughout the file, not just on 4 lines.
So that change is rejected.

The other changes are OK.  If you could re-do your tree without
0d6df4de (and possible change "Added" to "Add" in e1b5683c while
you're at it), I'll do the pull.

Paul.

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Wincent Colaiuta @ 2007-10-19 11:33 UTC (permalink / raw)
  To: Michael Witten; +Cc: Shawn O. Pearce, Jeff King, git
In-Reply-To: <A2491F60-A00E-412A-8389-1C9EB5EDFCEF@mit.edu>

El 19/10/2007, a las 4:29, Michael Witten escribió:

> Ah. Basically my 'pseudo-code' is correct, but redundant.

If I understood the original poster's proposal then I don't think  
your code does what he asked for:

> What you want to happen is the following:
> 	
> 	git show HEAD:A.txt > path/B.txt
> 	git add path/B.txt
> 	mv A.txt B.txt
> 	git rm A.txt
>
> Is this correct?

Here you're copying the content of A.txt as it was in the last (HEAD)  
commit, but from what the poster said he wants the content of A.txt  
as it is staged in the index (that is, there may be staged but  
uncomitted changes).

> Better:
>
>>  	mv A.txt path/B.txt
>> 	Point the index entry for A.txt to path/B.txt

Yes, that is basically what he was asking for, as I read it.

El 19/10/2007, a las 5:47, Jeff King escribió:

> Hrm. So you _do_ want to do an index-only move of A to B, in which  
> case
> the suggestion of a "git-mv --cached" seems sensible. Though I'm  
> curious
> why you want that.

I agree that git-stash can be used in this workflow but I can also  
imagine cases where the proposed "git-mv --cached" might be a bit  
nicer. I'm thinking of occasions where you just want to do something  
like:

   git mv --cached foo bar
   git add --interactive bar

I'm not sure the proposed "--cached" switch should ever be the  
default -- would need to ponder that one -- but I do think the switch  
would be a nice addition.

Cheers,
Wincent

^ permalink raw reply

* Re: Subversion developer: svn is for dumb people
From: Pierre Habouzit @ 2007-10-19 11:34 UTC (permalink / raw)
  To: Steven Grimm; +Cc: 'git'
In-Reply-To: <47176CE0.7030609@midwinter.com>

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

On Thu, Oct 18, 2007 at 02:25:36PM +0000, Steven Grimm wrote:
> Thought folks here might get a kick out of this:
> 
> http://blog.red-bean.com/sussman/?p=79
> 
> Okay, my summary is slightly facetious, but that's basically the gist of 
> what he's saying: you should choose Subversion rather than a DVCS because 
> most of your users won't be smart enough to use the better tool.
> 
> I can't say he's completely wrong, especially about the 20/80% idea 
> (though I think "20%" is generous), but some of his specific arguments 
> about DVCS are on the bogus side. "Centralized systems encourage code 
> reviews," for one -- I challenge him to find a project with a more 
> pervasive and effective code-reviewing culture than the git project.

  Your argument is also bogus.

  IMNSHO, peer reviewing has nothing to do with git, svn, or $SCM. It's
a social pattern. There are people that do it because they understand
it's a good and necessary sound thing to do, and there are the others.
Guess what, it has a lot to do with the 20%/80% line (that I would have
more described as the 2/98 but well…).

  Put git into the hands of fools, they won't proofread their code more
or less than with svn. And they will shoot themselves into their foots
twice as often as with svn.

  Though, for people that are able to deal with git and use it, git
allows way better code reviewing patterns than with svn, because you
can prepare a nice incremental branch that adds each new features you
worked on with small patches (see my parse-options series). With svn,
you can't do that, because there is no tool that allow you to record
those patches on your end, so you submit a big +4123/-2341 patch. That
makes peer reviewing really harder.

  Of course the git community is a perfect example of how code should be
reviewed. But it's not because we use git, it's because we definitely
are in the "20%".

-- 
·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: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Theodore Tso @ 2007-10-19 11:38 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <8aa486160710190303l4ce996daqf5c8025c857ea8@mail.gmail.com>

On Fri, Oct 19, 2007 at 12:03:24PM +0200, Santi Béjar wrote:
> This way it is slightly less terse than the other proposals but not
> that cryptic and it normally fits in one line without padding. And I
> really like to see what has changed explicitly with the old..new line.

Same here.

I find the old..new information occasionally useful, since it allows
me to do the git diff --- something for which ORIG_HEAD isn't enough
when you are pulling multiple heads, such as in git.  Can we keep that
optional via a config or an command-line option?

Hmm... how about this?

==> git://repo.or.cz/git/spearce.git
 * branch gitk -> spearce/gitk		(new)
 * branch maint -> spearce/maint	1aa3d01..e7187e4
 * branch master -> spearce/master	de61e42..7840ce6
 * branch next -> spearce/next		895be02..2fe5433
 + branch pu -> spearce/pu		89fa332...1e4c517
 * branch todo -> spearce/todo		(new)

If the branch is new, obviously old..new won't be useful.  The
non-fast forward branch is getting indicated twice, once with the "+"
sign, and once with the triple dot in the range.   

As far as the padding, it would be a pain to figure out how to make
the right hand column be padded so that it starts 3 spaces after the
longest "  * branch foo -> bar" line, but that would look the best.

Finally, one last question --- am I the only one who had to take a
second look at the whether the arrow should be <- or ->?  The question
is whether we are saying "gitk is moving to include all of
spearce/gitk"; but I could also see it stated that we are assigning
refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
arrow should be reversed.   Or maybe:

==> git://repo.or.cz/git/spearce.git
 * branch gitk := spearce/gitk		(new)
 * branch maint := spearce/maint	1aa3d01..e7187e4
 * branch master := spearce/master	de61e42..7840ce6
 * branch next := spearce/next		895be02..2fe5433
 + branch pu := spearce/pu		89fa332...1e4c517
 * branch todo := spearce/todo		(new)

(Or is that too Pascal-like?  :-)

      	       	    	       	      	  	   - Ted

^ permalink raw reply

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Wincent Colaiuta @ 2007-10-19 11:44 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Nicolas Pitre, Jeff King, git
In-Reply-To: <20071019034501.GG14735@spearce.org>

El 19/10/2007, a las 5:45, Shawn O. Pearce escribió:

> Nicolas Pitre <nico@cam.org> wrote:
>>
>> And imagine that you see the progress moving slowly because the  
>> remote
>> server is a NSLU2, but it says 80%.  Then you go for a coffee and the
>> progress says 20% when you return because it now has moved to a
>> different phase.  Rather counter intuitive.
>
> Yea, I didn't consider that.  That's where you need to show the
> number of steps and which one you are on, so the meter looks
> more like:
>
> 	Step 1/3: Counting objects: .... \r
> 	Step 2/4: Compressing objects: ... \r
> 	Step 3/3: Writing objects: .... \r
>
> only all smashed into one line of course, so only the most recent
> one is being displayed.

Looks good, although as Nicholas pointed out you might not know in  
advanced how many steps there are.

So I still think that Nicholas' original point is valid is here, and  
if you overwrite old progress bars with new ones then you may falsely  
give the impression of "going backwards".

Cheers,
Wincent

^ permalink raw reply

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 11:58 UTC (permalink / raw)
  To: Jeff King; +Cc: Linus Torvalds, Shawn O. Pearce, git
In-Reply-To: <20071019042930.GA16487@coredump.intra.peff.net>

On Fri, 19 Oct 2007, Jeff King wrote:

> On Thu, Oct 18, 2007 at 09:21:55PM -0700, Linus Torvalds wrote:
> 
> > I'd love it, but the way our current SHA1 parser works, I don't think it 
> > can really do it.
> > 
> > Basically, we currently assume that a SHA1 expression always expands to a 
> > *single* SHA1.
> 
> Ah, right. I hadn't thought of that. While it would be a nice
> convenience feature, it's probably not worth the deep internal hackery
> that would be required.

What about a preprocessor that could match <1>@{<2>..<3>} in the 
argument list and substitute that with <1>@{<2>}..<1>@{<3>} before it is 
actually parsed?


Nicolas

^ permalink raw reply

* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Dan McGee @ 2007-10-19 12:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019090400.GA8944@coredump.intra.peff.net>

On 10/19/07, Jeff King <peff@peff.net> wrote:
> When matching source and destination refs, we were failing
> to pull the 'force' parameter from wildcard respects (but
> not explicit ones) and attach it to the ref struct.
>
> This adds a test for explicit and wildcard refspecs; the
> latter fails without this patch.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  remote.c             |    2 ++
>  t/t5400-send-pack.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index b20e2be..170015a 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -762,6 +762,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
>                         hashcpy(dst_peer->new_sha1, src->new_sha1);
>                 }
>                 dst_peer->peer_ref = src;
> +               if (pat)
> +                       dst_peer->force = pat->force;
>         free_name:
>                 free(dst_name);
>         }
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 57c6397..2d0c07f 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -123,4 +123,52 @@ test_expect_success \
>         git-branch -a >branches && ! grep -q origin/master branches
>  '
>
> +rewound_push_setup() {
> +       rm -rf parent child &&
> +       mkdir parent && cd parent &&
> +       git-init && echo one >file && git-add file && git-commit -m one &&
> +       echo two >file && git-commit -a -m two &&
> +       cd .. &&
> +       git-clone parent child && cd child && git-reset --hard HEAD^
> +}
> +
> +rewound_push_succeeded() {
> +       cmp ../parent/.git/refs/heads/master .git/refs/heads/master
> +}
> +
> +rewound_push_failed() {
> +       if rewound_push_succeeded
> +       then
> +               false
> +       else
> +               true
> +       fi
> +}
> +
> +test_expect_success \
> +       'pushing explicit refspecs respects forcing' '
> +       rewound_push_setup &&
> +       if git-send-pack ../parent/.git refs/heads/master:refs/heads/master
> +       then
> +               false
> +       else
> +               true
> +       fi && rewound_push_failed &&
> +       git-send-pack ../parent/.git +refs/heads/master:refs/heads/master &&
> +       rewound_push_succeeded
> +'
> +
> +test_expect_success \
> +       'pushing wildcard refspecs respects forcing' '
> +       rewound_push_setup &&
> +       if git-send-pack ../parent/.git refs/heads/*:refs/heads/*
> +       then
> +               false
> +       else
> +               true
> +       fi && rewound_push_failed &&
> +       git-send-pack ../parent/.git +refs/heads/*:refs/heads/* &&
> +       rewound_push_succeeded
> +'
> +
>  test_done
> --
> 1.5.3.4.1254.gc1ca9-dirty
>

Hmm. For some reason this passes with your test case, but not with my
original bash test script[1]. Did you try it with this?

-Dan

[1] http://www.toofishes.net/uploads/

^ permalink raw reply

* Re: git on afs
From: Todd T. Fries @ 2007-10-19 12:19 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Linus Torvalds, Brandon Casey
In-Reply-To: <20071019060624.GK14735@spearce.org>

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

Shawn,

If DT_UNKNOWN exists, then we have to do a stat() of some form to
find out the right type.

This is difficult to fix properly to avoid the extra stat() since
inside the switch logic we do the recursion, but we might have
avoided it earlier because of the exclusion.

I'll send a separate diff for an updated link() vs rename() diff.

I've attached an updated diff that should address concerns of everyone
who gave me feedback on my dir.c changes.

Better?

Thanks,

On Friday 19 October 2007 01:06:24 Shawn O. Pearce wrote:
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Thu, 18 Oct 2007, Todd T. Fries wrote:
> > > 2) git presumes that DTYPE(de) != DT_DIR .. means the dirent is not a
> > > dir this is not true for afs
> >
> > That's a major bug, and has nothing to do with AFS. Oops.
> >
> > If you look just a bit lower, you'll see that just a few lines down, git
> > handles DT_UNKNOWN correctly, and just does a lstat() on it as required.
> > I guess that logic should be moved up, or alternatively the exclude logic
> > should be moved down.
> >
> > Your patch looks ok, but at the same time, I don't think it's really the
> > right thing to do, since it now does that lstat() twice.
>
> What about this instead?  It avoids the double lstat() of Todd's
> original patch but seems like it would fix the issue here.  Or did
> I misunderstand the problem?
[..]
> diff --git a/dir.c b/dir.c
> index eb6c3ab..d2597ff 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -487,9 +487,10 @@ static int read_directory_recursive(struct dir_struct
> *dir, const char *path, co && in_pathspec(fullname, baselen + len,
> simplify))
>  				dir_add_ignored(dir, fullname, baselen + len);
>  			if (exclude != dir->show_ignored) {
> -				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
> +				if (!dir->show_ignored)
> +					continue;
> +				else if (DTYPE(de) != DT_DIR && DTYPE(de) != DT_UNKNOWN)
>  					continue;
> -				}
>  			}
>
>  			switch (DTYPE(de)) {
> --
> 1.5.3.4.1249.g895be



-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting                      \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \          250797 (FWD)
|                                             \
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt






[-- Attachment #2: 0001-If-readdir-returns-a-DTYPE-of-DT_UNKNOWN-this-w.patch --]
[-- Type: text/x-diff, Size: 1848 bytes --]

From f7bca472645db70b52824e1be827ba99195198d2 Mon Sep 17 00:00:00 2001
From: Todd T. Fries <todd@fries.net>
Date: Fri, 19 Oct 2007 06:26:49 -0500
Subject: [PATCH] If readdir() returns a DTYPE() of DT_UNKNOWN, this warrants further
investigation, i.e. a stat() or lstat().  We have been presuming
anything not DT_DIR is not a dir, which is not the case for
all filesystems (e.g. afs).

Do the lstat() once, and use the results twice.

Signed-off-by: Todd T. Fries <todd@fries.net>
---
 dir.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/dir.c b/dir.c
index eb6c3ab..0ed4739 100644
--- a/dir.c
+++ b/dir.c
@@ -468,6 +468,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 		while ((de = readdir(fdir)) != NULL) {
 			int len;
 			int exclude;
+			struct stat st;
 
 			if ((de->d_name[0] == '.') &&
 			    (de->d_name[1] == 0 ||
@@ -486,19 +487,24 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (exclude && dir->collect_ignored
 			    && in_pathspec(fullname, baselen + len, simplify))
 				dir_add_ignored(dir, fullname, baselen + len);
+			if (DTYPE(de) == DT_UNKNOWN) {
+				if (lstat(fullname, &st))
+					continue;
+			}
 			if (exclude != dir->show_ignored) {
-				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
+				if (!dir->show_ignored))
 					continue;
-				}
+				if (DTYPE(de) == DT_UNKNOWN && !S_ISDIR(st.st_mode))
+					continue;
+				else
+					if (DTYPE(de) != DT_DIR)
+						continue;
 			}
 
 			switch (DTYPE(de)) {
-			struct stat st;
 			default:
 				continue;
 			case DT_UNKNOWN:
-				if (lstat(fullname, &st))
-					continue;
 				if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode))
 					break;
 				if (!S_ISDIR(st.st_mode))
-- 
1.5.2.5


^ permalink raw reply related

* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Jeff King @ 2007-10-19 12:27 UTC (permalink / raw)
  To: Dan McGee; +Cc: Shawn O. Pearce, git
In-Reply-To: <449c10960710190510y3af3ffa2ydb9ae4a01b5d480c@mail.gmail.com>

On Fri, Oct 19, 2007 at 07:10:42AM -0500, Dan McGee wrote:

> Hmm. For some reason this passes with your test case, but not with my
> original bash test script[1]. Did you try it with this?
>
> [1] http://www.toofishes.net/uploads/

[please trim quoted text; I had a hard time finding your message amidst
the patch]

I didn't try it until you sent your message, but your test seems to work
fine for me. My patch is on top of 'next', which is what I usually run.
I haven't looked into 'master' (I assumed since the bug was reproducible
in both, it would be the same in both, but that is perhaps not the
case).

-Peff

^ permalink raw reply

* Re: git on afs
From: Todd T. Fries @ 2007-10-19 12:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Brandon Casey
In-Reply-To: <20071019054814.GJ14735@spearce.org>

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

You're the second one to point out that I should check the errno of link() on
afs.

It should not matter, but I'm using arla's afs client on OpenBSD; the errno
is 17 (EEXIST), the very errno that bypasses the coda hack's rename():

  8484 git-index-pack CALL  mkdir(0xcfbdc750,0x1ff)
  8484 git-index-pack NAMI  ".git/objects/pack"
  8484 git-index-pack RET   mkdir -1 errno 17 File exists
  8484 git-index-pack CALL  link(0x829e7040,0xcfbdc750)
  8484 git-index-pack NAMI  ".git/objects/tmp_pack_BU8484"
  8484 git-index-pack 
NAMI  ".git/objects/pack/pack-db66c6d25d6e0044093252434b4aa2a7d67e386a.pack"
  8484 git-index-pack RET   link -1 errno 17 File exists

.. and does not go on to do a rename(), which based on the code makes sense.

I can assure you that the 2nd argument to link does not exist ;-)

So, I've made the following change and it seems to fix the problem, much
cleaner IMHO, and get the following change in behavior:

  5933 git-index-pack CALL  mkdir(0xcfbc0070,0x1ff)
  5933 git-index-pack NAMI  ".git/objects/pack"
  5933 git-index-pack RET   mkdir -1 errno 17 File exists
  5933 git-index-pack CALL  link(0x860c0040,0xcfbc0070)
  5933 git-index-pack NAMI  ".git/objects/tmp_pack_EV5933"
  5933 git-index-pack 
NAMI  ".git/objects/pack/pack-db66c6d25d6e0044093252434b4aa2a7d67e386a.pack"
  5933 git-index-pack RET   link -1 errno 17 File exists
[..]
  5933 git-index-pack CALL  rename(0x860c0040,0xcfbc0070)
  5933 git-index-pack NAMI  ".git/objects/tmp_pack_EV5933"
  5933 git-index-pack 
NAMI  ".git/objects/pack/pack-db66c6d25d6e0044093252434b4aa2a7d67e386a.pack"
  5933 git-index-pack RET   rename 0

The only downside is that either on coda or if the file already exists, it
will try a spurrous rename(), in which case it will fail with EEXIST again,
so I hope this is not a big negative.

If this is ok, an appropriate commit message might be something like:

    AFS inconveniently returns EEXIST from link() to say it
    doesn't like cross directory link()'s.  Do a little
    extra work to fix this by ignoring EEXIST and trying
    a rename() anyway.


Thanks,

On Friday 19 October 2007 00:48:14 Shawn O. Pearce wrote:
> There's two different issues here so I'm going to split the thread
> into two to simplify the discussion.  Well, for me anyway.  ;-)
>
> "Todd T. Fries" <todd@fries.net> wrote:
> > 1) git presumes that link() works fine across subdirs; in afs land,
> >    hardlinks do not succeed ever
>
> ...
>
> > diff --git a/sha1_file.c b/sha1_file.c
> > index 83a06a7..1b93322 100644
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -1961,7 +1961,7 @@ static int link_temp_to_file(const char *tmpfile,
> > const char *filename) int ret;
> >  	char *dir;
> >
> > -	if (!link(tmpfile, filename))
> > +	if (!rename(tmpfile, filename))
> >  		return 0;
> >
> >  	/*
> > @@ -1980,7 +1980,7 @@ static int link_temp_to_file(const char *tmpfile,
> > const char *filename) return -2;
> >  		}
> >  		*dir = '/';
> > -		if (!link(tmpfile, filename))
> > +		if (!rename(tmpfile, filename))
> >  			return 0;
> >  		ret = errno;
> >  	}
>
> These cases should already be handled lower, see l.1997-2012 of
> sha1_file.c:
>
>     /*
>      * Coda hack - coda doesn't like cross-directory links,
>      * so we fall back to a rename, which will mean that it
>      * won't be able to check collisions, but that's not a
>      * big deal.
>      *
>      * The same holds for FAT formatted media.
>      *
>      * When this succeeds, we just return 0. We have nothing
>      * left to unlink.
>      */
>     if (ret && ret != EEXIST) {
>         if (!rename(tmpfile, filename))
>
> > Brandon Casey <casey@nrlssc.navy.mil> wrote:
> >
> > On Thu, 18 Oct 2007, Todd T. Fries wrote:
> > > link() returns -1 errno 17 File exists on afs.
> > >
> > > To further muddy the waters, linking within the same dir is ok,
> > > linking outside the same dir is not:
> > >
> > > todd@ispdesk/p6 ~/tmp=A661$ mkdir dir
> > > todd@ispdesk/p6 ~/tmp=A662$ touch a
> > > todd@ispdesk/p6 ~/tmp=A663$ ln a b
> > > todd@ispdesk/p6 ~/tmp=A664$ ls -l a b
> > > -rw-r--r--  2 4  wheel  0 Oct 18 17:09 a
> > > -rw-r--r--  2 4  wheel  0 Oct 18 17:09 b
> > > todd@ispdesk/p6 ~/tmp=A665$ ls -li a b
> > > 2068032 -rw-r--r--  2 4  wheel  0 Oct 18 17:09 a
> > > 2068032 -rw-r--r--  2 4  wheel  0 Oct 18 17:09 b
> > > todd@ispdesk/p6 ~/tmp=A666$ ln a dir/b
> > > ln: dir/b: File exists
> > > todd@ispdesk/p6 ~/tmp=A667$ echo $?
> >
> > That error is just bogus on afs. If it returned a sane
> > error, things would just work.
> >
> > But, looks like afs only supports linking within the same
> > directory: http://www.angelfire.com/hi/plutonic/afs-faq.html
>
> So according to that error message from "ln" we really should have
> fallen into that Coda hack I just mentioned.  Did we instead get
> a different errno here and not use that hack?
>
>
> We probably could just use rename as you do above but then we would
> want to remove the unlink(tmpfile) call on l.2013 in sha1_file.c.
> Otherwise we're always incurring a syscall for no reason.  I'm not
> against a change here, I just want to make sure we make the right
> change for AFS.  :-)



-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting                      \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \          250797 (FWD)
|                                             \
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt



[-- Attachment #2: 0002-AFS-inconveniently-returns-EEXIST-from-link-to-say.patch --]
[-- Type: text/x-diff, Size: 1125 bytes --]

From e3fe1eae139dccb9738cf0c8f6818136be96657b Mon Sep 17 00:00:00 2001
From: Todd T. Fries <todd@fries.net>
Date: Fri, 19 Oct 2007 07:38:16 -0500
Subject: [PATCH] AFS inconveniently returns EEXIST from link() to say it
doesn't like cross directory link()'s.  Do a little
extra work to fix this by ignoring EEXIST and trying
a rename() anyway.

Signed-off-by: Todd T. Fries <todd@fries.net>
---
 sha1_file.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 83a06a7..9c7d74e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2004,8 +2004,13 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
 	 *
 	 * When this succeeds, we just return 0. We have nothing
 	 * left to unlink.
+	 *
+	 * AFS hack - afs is similar to coda, but inconveniently
+	 * set errno to EEXIST, so call rename() if the link()
+	 * above fails unconditionally.  Small bit of extra work
+	 * so afs functions properly.
 	 */
-	if (ret && ret != EEXIST) {
+	if (ret) {
 		if (!rename(tmpfile, filename))
 			return 0;
 		ret = errno;
-- 
1.5.2.5


^ permalink raw reply related

* Re: Subversion developer: svn is for dumb people
From: Andy Parkins @ 2007-10-19 12:10 UTC (permalink / raw)
  To: git
In-Reply-To: <47176CE0.7030609@midwinter.com>

Steven Grimm wrote:

> find code reviews *harder* in a centralized system because you end up

I think I'd argue that git actually reduces the problems of patchbombs
(which is a code-review problem).

Those who are going to patchbomb will do it whatever their VCS choice (and
I'm not entirely convinced that patchbombs are as bad as is made out in
that article).  With a centralised VCS the patch almost has to be one giant
diff - very hard to read and review.  Git, on the other hand, allows the
patchbomber to work alone crafting not only beautiful code, but a beautiful
patch series.  The bomb can be easy to review, easy to reject, and easy to
modify.  It becomes far more likely that the code can be used in some way.

I guarantee that the only reason I ever contributed anything to git itself
was because the bureaucracy line is so low - and it is low because of git.

Key features for a DVCS from a newbie point of view:
 - Making a fool of yourself can be done in private rather than in public.
   I have loads of branches in my git repository that I will never submit,
   because they were the beginnings of ideas that didn't pan out (and I'm 
   lazy)
 - You don't need _any_ permission to fork/branch.
 - You can work away on your feature without worrying too much that time
   has passed and the main project has moved on and you aren't keeping up
   to date.  I've got a set of patches for git (the REF_PATHS stuff) that I
   started a year ago.  I recently rebased it and brought it up to date.
   It's still not in mainline, but I'm not panicking, nor do I have to
   abandon it because it's so easy to catch up whenever I want to.
 - I can work on two features simultaneously.  If I have two ideas - crazy
   and safe, both independent, I can work on both without having to mix
   them for the day when I submit.  This makes it a lot easier to keep them
   on the go, and a lot easier for the maintainer to decide what goes in and
   what doesn't.
 - The ability to craft a log messages that are kept locally and easily
   edited before submission goes a long way to helping cross the
   brave-enough-to-submit barrier.  Presenting new code as an unknown
   developer on a project is (I think) quite a stressful thing.  Git let's
   you feel as prepared as you want before you take that leap into having
   your code ripped apart.

In short: those pro-centralised arguments are nonsense.  Even if they
weren't the advantages of DVCS heavily outweigh the advantages of CVCS.



Andy

-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

^ permalink raw reply

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Johannes Sixt @ 2007-10-19 12:31 UTC (permalink / raw)
  To: Theodore Tso
  Cc: Santi Béjar, Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <20071019113822.GB16726@thunk.org>

Theodore Tso schrieb:
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk		(new)
>  * branch maint -> spearce/maint	1aa3d01..e7187e4
>  * branch master -> spearce/master	de61e42..7840ce6
>  * branch next -> spearce/next		895be02..2fe5433
>  + branch pu -> spearce/pu		89fa332...1e4c517
>  * branch todo -> spearce/todo		(new)

> As far as the padding, it would be a pain to figure out how to make
> the right hand column be padded so that it starts 3 spaces after the
> longest "  * branch foo -> bar" line, but that would look the best.

But this way it wouldn't be difficult at all:

==> git://repo.or.cz/git/spearce.git
  * (new)              gitk -> spearce/gitk
  * 1aa3d01..e7187e4   maint -> spearce/maint
  * de61e42..7840ce6   master -> spearce/master
  * 895be02..2fe5433   next -> spearce/next
  + 89fa332...1e4c517  pu -> spearce/pu
  * (new)              todo -> spearce/todo

(I don't know where to put the label 'branch'.)

BTW, I like the ID ranges, too, and have used the information
occasionally.

-- Hannes

^ permalink raw reply

* [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Scott Parish @ 2007-10-19 13:04 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <47185DAF.7060809@viscovery.net>

I have a situation where software for a distribution is installed
into a fake "prefix" and then moved to one of several potential
places to be used by users. Given that the final location isn't
static, i can't depend on builtin_exec_path. I'd really like users
to be able to get started with git as easily as possible. With the
current setup, they would have to create and maintain either an
GIT_EXEC_PATH or an alias for including --exec-path, as well as a
MANPATH and PERL5LIB. This seem like an unnessisary burden.

I'd like to make it so that git works equally well when it is ran
via an absolute path (already partially works), relative path, or
from the PATH. (in saying "equally well" i'm including perl commands
and help commands)

To do this i've had to make the following changes:

 + check PATH for the location of git
 + the checking of argv[0] was restricted to absolute paths; remove
   that restriction so it also works when called with a relative
   path (eg ../../otheruser/usr/bin/git)
 + try to guess and set the env for the typical relative locations for
   MANPATH and PERL5LIB based off exec_path

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |   50 ++++++++++++++++++++++++++++++++++++++-
 git.c      |   76 +++++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..c6ecca9 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -13,19 +13,67 @@ void git_set_exec_path(const char *exec_path)
 }
 
 
+/* Return the first path in PATH that git is found in or NULL if not found */
+char *git_path_from_env(void)
+{
+	const char *env_paths = getenv("PATH");
+	const char *git = "/git";
+	int git_len = strlen(git);
+	char *paths, *path, *colon, *git_path;
+	int path_len;
+	struct stat st;
+
+	if (!env_paths)
+		return NULL;
+
+	path_len = strlen(env_paths);
+	path = paths = xmalloc(path_len + 1);
+	memcpy(paths, env_paths, path_len + 1);
+
+	while ((char *)1 != path) {
+		if ((colon = strchr(path, ':')))
+		    *colon = 0;
+
+		path_len = strlen(path);
+		git_path = xmalloc(path_len + git_len + 1);
+		memcpy(git_path, path, path_len);
+		memcpy(git_path + path_len, git, git_len + 1);
+
+		if (!stat(git_path, &st)) { /* found */
+			free(paths);
+			git_path[path_len] = 0;
+			return git_path;
+		}
+
+		free(git_path);
+		path = colon + 1;
+	}
+
+	free(paths);
+	return NULL;
+}
+
+
 /* Returns the highest-priority, location to look for git programs. */
 const char *git_exec_path(void)
 {
-	const char *env;
+	const char *env, *path;
 
 	if (current_exec_path)
 		return current_exec_path;
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
+		current_exec_path = env;
 		return env;
 	}
 
+	if ((path = git_path_from_env())) {
+		current_exec_path = path;
+		return path;
+	}
+
+	current_exec_path = builtin_exec_path;
 	return builtin_exec_path;
 }
 
diff --git a/git.c b/git.c
index 9eaca1d..252ee7c 100644
--- a/git.c
+++ b/git.c
@@ -6,26 +6,56 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
-static void prepend_to_path(const char *dir, int len)
+static void prepend_to_env(const char *env, const char *basedir,
+			   const char *subdir, const char *env_default)
 {
-	const char *old_path = getenv("PATH");
-	char *path;
-	int path_len = len;
-
-	if (!old_path)
-		old_path = "/usr/local/bin:/usr/bin:/bin";
-
-	path_len = len + strlen(old_path) + 1;
-
-	path = xmalloc(path_len + 1);
+	const char *old = getenv(env);
+	int basedir_len = strlen(basedir);
+	int subdir_len = strlen(subdir);
+	char *new;
+	int old_len;
+	
+	if (!old)
+		old = env_default;
+
+	old_len = strlen(old);
+
+	new = xmalloc(basedir_len + subdir_len + old_len + 1);
+	
+	memcpy(new, basedir, basedir_len);
+	memcpy(new + basedir_len, subdir, subdir_len);
+	memcpy(new + basedir_len + subdir_len, old, old_len + 1);
+	
+	if (setenv(env, new, 1))
+		fprintf(stderr, "Setenv failed: %s\n", strerror(errno));
+
+	free(new);
+}
 
-	memcpy(path, dir, len);
-	path[len] = ':';
-	memcpy(path + len + 1, old_path, path_len - len);
+static void prepend_to_envs(const char *dir, int len)
+{
+	char *slash;
+	char *basedir;
+
+	/* basedir is dir with "/bin" stripped off */
+	basedir = xmalloc(len + 1);
+	memcpy(basedir, dir, len + 1);
+	
+	if ((slash = strrchr(basedir, '/'))) {
+		*slash = 0;
+		while (slash == basedir + --len) /* found trailing slash */
+			if ((slash = strrchr(basedir, '/')))
+				*slash = 0;
+	}
 
-	setenv("PATH", path, 1);
+	prepend_to_env("PATH", basedir, "/bin:",
+		       "/usr/local/bin:/usr/bin:/bin");
+	prepend_to_env("MANPATH", basedir, "/share/man:",
+		       "/usr/local/share/man:/usr/share/man");
+	prepend_to_env("PERL5LIB", basedir, "/lib/perl5:",
+		       "/usr/lib/perl5");
 
-	free(path);
+	free(basedir);
 }
 
 static int handle_options(const char*** argv, int* argc, int* envchanged)
@@ -414,8 +444,7 @@ int main(int argc, const char **argv)
 	 */
 	if (slash) {
 		*slash++ = 0;
-		if (*cmd == '/')
-			exec_path = cmd;
+		exec_path = cmd;
 		cmd = slash;
 	}
 
@@ -453,14 +482,15 @@ int main(int argc, const char **argv)
 	/*
 	 * We execute external git command via execv_git_cmd(),
 	 * which looks at "--exec-path" option, GIT_EXEC_PATH
-	 * environment, and $(gitexecdir) in Makefile while built,
-	 * in this order.  For scripted commands, we prepend
-	 * the value of the exec_path variable to the PATH.
+	 * environment, PATH environment, and $(gitexecdir) in
+	 * Makefile while built, in this order.  For scripted
+	 * commands, we prepend the value of the exec_path
+	 * variable to the PATH.
 	 */
 	if (exec_path)
-		prepend_to_path(exec_path, strlen(exec_path));
+		prepend_to_envs(exec_path, strlen(exec_path));
 	exec_path = git_exec_path();
-	prepend_to_path(exec_path, strlen(exec_path));
+	prepend_to_envs(exec_path, strlen(exec_path));
 
 	while (1) {
 		/* See if it's an internal command */
-- 
1.5.3.4.206.g58ba4-dirty

^ permalink raw reply related

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 13:05 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: David Symonds, Jeff King, git
In-Reply-To: <20071019073938.GN14735@spearce.org>

On Fri, 19 Oct 2007, Shawn O. Pearce wrote:

> What about this on top of Jeff's patch?
> 
> $ git fetch jc
> ...
> ==> git://repo.or.cz/alt-git.git
>  * tag junio-gpg-pub ......................... (new)
>  * tag v1.5.0 .......................... (tag moved)
> 
> $ git fetch me
> ...
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> spearce/gitk ............... (new)
>  * branch maint -> spearce/maint
>  * branch master -> spearce/master
>  * branch next -> spearce/next
>  * branch pu -> spearce/pu ......... (forced update)
>  * branch todo -> spearce/todo ............... (new)
> 
> The width of the terminal is computed to produce the ... padding.
> I used a very narrow terminal to produce the above so it doesn't
> linewrap badly in email.  If we cannot get the terminal width then
> we just don't produce the padding.

I like it.

I would change the '*' to a '+' for a forced update (for similarity with 
the + notation in refspecs), and a '!' instead of a '-' for refused 
update which might be more indicative of a refusal.


Nicolas

^ permalink raw reply

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 13:15 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <8aa486160710190303l4ce996daqf5c8025c857ea8@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 993 bytes --]

On Fri, 19 Oct 2007, Santi Béjar wrote:

> Another possibility is with just some minor reductions from the
> current output, as:
> 
> $ git fetch spearce
> ...
> >From git://repo.or.cz/git/spearce
> * spearce/gitk: fast forward to branch 'gitk'
>   old..new: 0d6df4d..2b5afb7
> * spearce/maint: fast forward to branch 'maint'
>   old..new: 1aa3d01..e7187e4
> * spearce/master: fast forward to branch 'master'
>   old..new: de61e42..7840ce6
> * spearce/next: fast forward to branch 'next'
>   old..new: 895be02..2fe5433
> * spearce/pu: forcing update to non-fast forward branch 'pu'
>   old...new: 89fa332...1e4c517
> 
> This way it is slightly less terse than the other proposals but not
> that cryptic and it normally fits in one line without padding. And I
> really like to see what has changed explicitly with the old..new line.

I think the advantage of having only one line of output per branch 
really outweight the need for old..new notation.  Do you really benefit 
from it?


Nicolas

^ permalink raw reply

* Re: Subversion developer: svn is for dumb people
From: Johannes Schindelin @ 2007-10-19 13:17 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Steven Grimm, 'git'
In-Reply-To: <20071019113447.GC4404@artemis.corp>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1726 bytes --]

Hi,

On Fri, 19 Oct 2007, Pierre Habouzit wrote:

> On Thu, Oct 18, 2007 at 02:25:36PM +0000, Steven Grimm wrote:
> > Thought folks here might get a kick out of this:
> > 
> > http://blog.red-bean.com/sussman/?p=79
> > 
> > Okay, my summary is slightly facetious, but that's basically the gist 
> > of what he's saying: you should choose Subversion rather than a DVCS 
> > because most of your users won't be smart enough to use the better 
> > tool.
> > 
> > I can't say he's completely wrong, especially about the 20/80% idea 
> > (though I think "20%" is generous), but some of his specific arguments 
> > about DVCS are on the bogus side. "Centralized systems encourage code 
> > reviews," for one -- I challenge him to find a project with a more 
> > pervasive and effective code-reviewing culture than the git project.
> 
>   Your argument is also bogus.
> 
>   IMNSHO, peer reviewing has nothing to do with git, svn, or $SCM. It's 
> a social pattern. There are people that do it because they understand 
> it's a good and necessary sound thing to do, and there are the others. 
> Guess what, it has a lot to do with the 20%/80% line (that I would have 
> more described as the 2/98 but well…).

I tend to disagree.  Git at least _enables_ you to have the 
one-committer-per-repository scheme, it even _encourages_ it to a certain 
extent.

And once you go that route, it is easy to see that the committer says "I 
will not let that _crap_ enter my repository."  Bingo, peer review.

Compare that to a centralised repository, where more often than not, the 
administrator is not even part of the developer community!  It is much 
easier not to feel too responsible for the code you are committing there.

Ciao,
Dscho

^ permalink raw reply

* Re: Subversion developer: svn is for dumb people
From: Pierre Habouzit @ 2007-10-19 13:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Steven Grimm, 'git'
In-Reply-To: <Pine.LNX.4.64.0710191514190.16728@wbgn129.biozentrum.uni-wuerzburg.de>

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

On Fri, Oct 19, 2007 at 01:17:05PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 19 Oct 2007, Pierre Habouzit wrote:
> 
> > On Thu, Oct 18, 2007 at 02:25:36PM +0000, Steven Grimm wrote:
> > > Thought folks here might get a kick out of this:
> > > 
> > > http://blog.red-bean.com/sussman/?p=79
> > > 
> > > Okay, my summary is slightly facetious, but that's basically the gist 
> > > of what he's saying: you should choose Subversion rather than a DVCS 
> > > because most of your users won't be smart enough to use the better 
> > > tool.
> > > 
> > > I can't say he's completely wrong, especially about the 20/80% idea 
> > > (though I think "20%" is generous), but some of his specific arguments 
> > > about DVCS are on the bogus side. "Centralized systems encourage code 
> > > reviews," for one -- I challenge him to find a project with a more 
> > > pervasive and effective code-reviewing culture than the git project.
> > 
> >   Your argument is also bogus.
> > 
> >   IMNSHO, peer reviewing has nothing to do with git, svn, or $SCM. It's 
> > a social pattern. There are people that do it because they understand 
> > it's a good and necessary sound thing to do, and there are the others. 
> > Guess what, it has a lot to do with the 20%/80% line (that I would have 
> > more described as the 2/98 but well…).
> 
> I tend to disagree.  Git at least _enables_ you to have the 
> one-committer-per-repository scheme, it even _encourages_ it to a certain 
> extent.
> 
> And once you go that route, it is easy to see that the committer says "I 
> will not let that _crap_ enter my repository."  Bingo, peer review.
> 
> Compare that to a centralised repository, where more often than not, the 
> administrator is not even part of the developer community!  It is much 
> easier not to feel too responsible for the code you are committing there.

  I agree, that's why I said that git made it easier. WHat I pretend to
be wrong is to say that a SCM will make people review code or not. Git
merely help people that want to always review code to be completely sure
some review has happened before the code is merged. But nothing in git
forbids you to use the big fat <centralized repo with everyone having
push access to it>-mode.


-- 
·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: git stash apply usability issues
From: Karl Hasselström @ 2007-10-19 13:27 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <20071019013156.GU14735@spearce.org>

On 2007-10-18 21:31:56 -0400, Shawn O. Pearce wrote:

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

> > (2) when 'git stash apply' runs merge-recursive, it treats the
> > current state as 'ours' and the stash as 'theirs'. IMHO it should
> > be the other way round: I have stashed away changes to a binary
> > file. Then committed a different modification to it, and now want
> > to apply the stash. This results in a conflict that leaves the
> > current state in the working tree, but I had preferred that the
> > stashed binary file were in the working tree now.
> >
> > What do other git-stash users think about changing the order?
>
> The current order is the same order that git-rebase uses. I'm not
> saying its correct, just that its the same as rebase.

FWIW, StGit push works the same way. The idea being that the current
HEAD is our current state ("ours"), and the patch we're pushing is
some change we want to apply ("theirs"). I always felt that this was a
very natural order of things. But I guess the philosophy in the
"stash" case is subtly different, so maybe the change is warranted
there.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* stash clear, was Re: git: avoiding merges, rebasing
From: Johannes Schindelin @ 2007-10-19 13:37 UTC (permalink / raw)
  To: Eric Blake; +Cc: bug-gnulib, Bruno Haible, git
In-Reply-To: <47023699.3080606@byu.net>

Hi,

On Tue, 2 Oct 2007, Eric Blake wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> [adding the git list]
> 
> According to Bruno Haible on 10/2/2007 5:50 AM:
> > Hello Benoit,
> > 
> >>>     $ git stash
> >>>     $ git pull
> >>>     $ git stash apply
> >>>     $ git stash clean              ; typo!
> >>>     $ git stash clear              ; fatal correction to typo!
> >>>
> >>> and lost 20 modified files. Well, not really lost. Just took me a  
> >>> while to
> >> I don't really see how and why you "lost 20 modified files".
> > 
> > I lost modifications to 20 files. "git stash clean" moved these modifications
> > into a stash named "clean", and "git stash clear" killed it.
> 
> While we're at it, I wish 'git stash clear' would take an optional 
> argument that says which stash(es) to clear, rather than blindly 
> clearing the entire stash.

I'd rather avoid "enhancing" stash clear.  IMHO it is a little 
misdesigned, making it way too easy to hang yourself.

Instead, how about writing a stash pop?  "git stash pop [<stash>]".  It 
would literally just call git stash apply && git reflog delete.  Should 
not be too difficult, now that I provided "git reflog delete" ;-)

Maybe even deprecating "git stash clear", or doing away with it 
altogether.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Dan McGee @ 2007-10-19 13:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019122755.GA17002@coredump.intra.peff.net>

On 10/19/07, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 07:10:42AM -0500, Dan McGee wrote:
>
> > Hmm. For some reason this passes with your test case, but not with my
> > original bash test script[1]. Did you try it with this?
> >
> > [1] http://www.toofishes.net/uploads/
>
> I didn't try it until you sent your message, but your test seems to work
> fine for me. My patch is on top of 'next', which is what I usually run.
> I haven't looked into 'master' (I assumed since the bug was reproducible
> in both, it would be the same in both, but that is perhaps not the
> case).

Still getting this error:
error: remote 'refs/heads/working' is not a strict subset of local ref
'refs/heads/working'. maybe you are not up-to-date and need to pull
first?
error: failed to push to '/tmp/testpush'

I've tried applying the patch on the following commits, and maybe I'm
smoking something but I can't get it to pass my test script.

origin(junio)/master: 58ba4f6
origin(junio)/next: fe96ee67ec5840
spearce/master: 7840ce6cb24a9d
spearce/next: 2fe5433b416f0df

Can you let me know what commit you based the patch off of? I'm at
work for the next 8 hours or so, so I can't look in to this a whole
lot until tonight.

-Dan

^ permalink raw reply

* [PATCH-resent] gitk: fix in procedure drawcommits
From: Michele Ballabio @ 2007-10-19 13:44 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Paul Mackerras
In-Reply-To: <20071019052823.GI14735@spearce.org>

This patch indroduces a check before unsetting an array element.

Without this, gitk may complain with

	can't unset "prevlines(...)": no such element in array

when scrolling happens.

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---

There's an error that seems to occur in gitk only on
mutt's imported repo, but I don't know why. This is
hopefully the right fix.

An example of this error:

can't unset "prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)": no such element in array
can't unset "prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)": no such element in array
    while executing
"unset prevlines($lid)"
    (procedure "drawcommits" line 39)
    invoked from within
"drawcommits $row $endrow"
    (procedure "drawfrac" line 10)
    invoked from within
"drawfrac $f0 $f1"
    (procedure "scrollcanv" line 3)
    invoked from within
"scrollcanv .tf.histframe.csb 0.00672513 0.0087015"

 gitk |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 300fdce..527b716 100755
--- a/gitk
+++ b/gitk
@@ -3697,7 +3697,9 @@ proc drawcommits {row {endrow {}}} {
 
 	if {[info exists lineends($r)]} {
 	    foreach lid $lineends($r) {
-		unset prevlines($lid)
+		if {[info exists prevlines($lid)]} {
+		    unset prevlines($lid)
+		}
 	    }
 	}
 	set rowids [lindex $rowidlist $r]
-- 
1.5.3

^ permalink raw reply related

* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Jeff King @ 2007-10-19 13:43 UTC (permalink / raw)
  To: Dan McGee; +Cc: Shawn O. Pearce, git
In-Reply-To: <449c10960710190638j5823b19dl903ae369965e884e@mail.gmail.com>

On Fri, Oct 19, 2007 at 08:38:06AM -0500, Dan McGee wrote:

> origin(junio)/master: 58ba4f6
> origin(junio)/next: fe96ee67ec5840
> spearce/master: 7840ce6cb24a9d
> spearce/next: 2fe5433b416f0df
> 
> Can you let me know what commit you based the patch off of? I'm at
> work for the next 8 hours or so, so I can't look in to this a whole
> lot until tonight.

It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
something silly like executing an older version of git that is in your
PATH?

-Peff

^ permalink raw reply

* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Sixt @ 2007-10-19 13:21 UTC (permalink / raw)
  To: Scott Parish; +Cc: git
In-Reply-To: <20071019130402.GD1463@srparish.net>

Scott Parish schrieb:
> I have a situation where software for a distribution is installed
> into a fake "prefix" and then moved to one of several potential
> places to be used by users. Given that the final location isn't
> static, i can't depend on builtin_exec_path. I'd really like users
> to be able to get started with git as easily as possible. With the
> current setup, they would have to create and maintain either an
> GIT_EXEC_PATH or an alias for including --exec-path, as well as a
> MANPATH and PERL5LIB. This seem like an unnessisary burden.

Interesting. How does this compare to this 2-patch-series:

http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=e479ea2f911b8c70a269ba59372a4fef90f8907c
http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=00a4ff4f3f8ec7e6b3ac15456f00b22b03f438ae

which I had come up with to accomplish something very similar
(on Windows). Your approach looks superior, but I hadn't gone
into depths, yet.

-- Hannes

^ 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