Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Git.pm: Avoid ppport.h
From: Johannes Schindelin @ 2006-07-02  9:52 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git
In-Reply-To: <20060701234832.GD29115@pasky.or.cz>

Hi,

On Sun, 2 Jul 2006, Petr Baudis wrote:

>   ow, Devel::PPPort might not be around all the way back to 5.6.0. What
> is your Perl version, BTW?

This is perl, v5.6.0 built for darwin

> This makes us not include ppport.h which seems not to give us anything real
> anyway; it is useful for checking for portability warts but since Devel::PPPort
> is a portability wart itself, we shouldn't require it for build.

Why do people introduce a "portability enhancer" like that? This is soo 
dumb.

Well, your patch helped. Now the error is somewhere else:

mkdir blib
mkdir blib/lib
mkdir blib/arch
mkdir blib/arch/auto
mkdir blib/arch/auto/Git
mkdir blib/lib/auto
mkdir blib/lib/auto/Git
mkdir blib/man3
cp private-Error.pm blib/lib/Error.pm
cp Git.pm blib/lib/Git.pm
/usr/bin/perl -I/System/Library/Perl/darwin -I/System/Library/Perl 
/System/Library/Perl/ExtUtils/xsubpp  -typemap 
/System/Library/Perl/ExtUtils/typemap Git.xs > Git.xsc && mv Git.xsc Git.c
Error: 'const char *' not in typemap in Git.xs, line 69
Error: 'const char *' not in typemap in Git.xs, line 79
make: *** [Git.c] Error 1

It seems like my typemap starts like this:

-- snip --
# $Header: /cvs/Darwin/src/live/perl/perl/lib/ExtUtils/typemap,v 1.2 
2002/03/14 
08:58:22 zarzycki Exp $ 
# basic C types
int                     T_IV
unsigned                T_UV
unsigned int            T_UV
long                    T_IV
unsigned long           T_UV
short                   T_IV
unsigned short          T_UV
char                    T_CHAR
unsigned char           T_U_CHAR
char *                  T_PV
unsigned char *         T_PV
caddr_t                 T_PV
wchar_t *               T_PV
wchar_t                 T_IV
bool_t                  T_IV
size_t                  T_IV
ssize_t                 T_IV
time_t                  T_NV
-- snap --

So, no "const char *". See next mail for a minimal patch. The warning 
(IIRC that was mentioned already on the list) still persists:

cc -c -I. -I.. -g -pipe -pipe -fno-common -no-cpp-precomp -flat_namespace 
-DHAS_TELLDIR_PROTOTYPE -fno-strict-aliasing -Os     -DVERSION=\"0.01\" 
-DXS_VERSION=\"0.01\"  -I/System/Library/Perl/darwin/CORE -I/sw/include 
-DSHA1_HEADER='<openssl/sha.h>' -DNO_STRCASESTR -DNO_STRLCPY 
-DGIT_VERSION='"1.4.1.g3b26"' Git.c
In file included from /System/Library/Perl/darwin/CORE/perl.h:500,
                 from Git.xs:15:
/System/Library/Perl/darwin/CORE/embed.h:156:1: warning: "die" redefined
Git.xs:11:1: warning: this is the location of the previous definition

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Git.xs: older perl do not know const char *
From: Johannes Schindelin @ 2006-07-02  9:53 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git


Both of these casts _should_ be safe, since you do not want to muck around 
with the version or the path anyway.

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

---

 perl/Git.xs |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/perl/Git.xs b/perl/Git.xs
index 51bfac3..c824210 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -59,7 +59,7 @@ BOOT:
 # /* TODO: xs_call_gate(). See Git.pm. */
 
 
-const char *
+char *
 xs_version()
 CODE:
 {
@@ -69,11 +69,11 @@ OUTPUT:
 	RETVAL
 
 
-const char *
+char *
 xs_exec_path()
 CODE:
 {
-	RETVAL = git_exec_path();
+	RETVAL = (char *)git_exec_path();
 }
 OUTPUT:
 	RETVAL

^ permalink raw reply related

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Johannes Schindelin @ 2006-07-02  9:56 UTC (permalink / raw)
  To: Rene Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <20060702094938.GA10944@lsrfire.ath.cx>

Hi,

On Sun, 2 Jul 2006, Rene Scharfe wrote:

> Due to popular request ;-), change get_merge_bases() to be able to
> clean up after itself if needed by adding a cleanup parameter.
> 
> We don't need to save the flags and restore them afterwards anymore;
> that was a leftover from before the flags were moved out of the
> range used in revision.c.  clear_commit_marks() sets them to zero,
> which is enough.

Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

Ciao,
Dscho

^ permalink raw reply

* Re: [POOL] Who likes running Git without make install?
From: Petr Baudis @ 2006-07-02 11:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkrcdg6d.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sun, Jul 02, 2006 at 02:05:46AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> That's fixed in ancient past in git timescale.  Why are you
> bringing it up again? ;-)

Because, it is, well, not fixed? ;-) (Apparently.)

Dear diary, on Sun, Jul 02, 2006 at 02:08:07AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Dear diary, on Mon, Jun 26, 2006 at 08:48:31AM CEST, I got a letter
> > where Junio C Hamano <junkio@cox.net> said that...
> >> I remember myself getting utterly discusted when I saw the
> >> inclusion of the build-time blib directory in the search path in
> >> some other Perl code outside git.
> >
> > Well, yes, it is ugly, but it was really cool that we could have used
> > Git without installing it anywhere.
> >
> > But perhaps that's just me refusing to break his old ways of doing
> > things. Does anyone else care about it? (And why?)
> 
> Well, for a quick test to see if I haven't broken anything, I
> use a new shell and do ". ./+denv" in my git repository where
> that file has something like this:
> 
>         $ cat ./+denv
>         :
> 
>         GIT_EXEC_PATH=`pwd`
>         PATH=`pwd`:/usr/bin:/bin
> 
>         export GIT_EXEC_PATH PATH
> 
> So to a certain degree, yes I do care.

But it is currently broken:

xpasky@machine[0:0]~/git-pb$ rm -rf /home/xpasky/lib/perl5/
xpasky@machine[0:0]~/git-pb$ ./git-mv
Can't locate Git.pm in @INC (...)

(And I can't see any change that could aim to fix it.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply

* Re: A note on merging conflicts..
From: Rene Scharfe @ 2006-07-02 11:31 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Linus Torvalds, J. Bruce Fields, Junio C Hamano, git,
	Johannes Schindelin
In-Reply-To: <Pine.LNX.4.64.0607011937190.9789@iabervon.org>

On Sat, Jul 01, 2006 at 07:45:33PM -0400, Daniel Barkalow wrote:
> That is: (this only has the logic portion, and it's against master, so it 
> isn't actually a really working patch or anything; also, it doesn't handle 
> "--not a...b" correctly, whatever that should mean)

[concept patch snipped]

You mean something like the patch below?  It seems to work, but in my
unscientific tests it's significant slower than the version based on
get_merge_bases() (0.17s vs 0.05s for
"git-rev-list 89719209...262a6ef7 66ae0c77...ced9456a").  Did I do
something wrong?

You had no mark_parents_left_right() in your patch.  I added it because
otherwise it wouldn't remove any common commits.  Was this supposed to
work some other way?

We still need an automatic test case, and a better benchmark.

diff --git a/revision.c b/revision.c
index ebee05a..8c494ee 100644
--- a/revision.c
+++ b/revision.c
@@ -339,6 +339,20 @@ static void try_to_simplify_commit(struc
 		commit->object.flags |= TREECHANGE;
 }
 
+static void mark_parents_left_right(struct commit *commit)
+{
+	unsigned int flags = commit->object.flags & (RIGHT_HALF | LEFT_HALF);
+	struct commit_list *parents = commit->parents;
+
+	while (parents) {
+		struct commit *p = parents->item;
+		p->object.flags |= flags;
+		if (p->parents)
+			mark_parents_left_right(p);
+		parents = parents->next;
+	}
+}
+
 static void add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)
 {
 	struct commit_list *parent = commit->parents;
@@ -347,6 +361,13 @@ static void add_parents_to_list(struct r
 		return;
 	commit->object.flags |= ADDED;
 
+	if (commit->object.flags & (RIGHT_HALF | LEFT_HALF)) {
+		if (commit->object.flags & RIGHT_HALF &&
+		    commit->object.flags & LEFT_HALF)
+			commit->object.flags |= UNINTERESTING;
+		mark_parents_left_right(commit);
+	}
+
 	/*
 	 * If the commit is uninteresting, don't try to
 	 * prune parents - we want the maximal uninteresting
@@ -772,7 +793,10 @@ int setup_revisions(int argc, const char
 			unsigned char from_sha1[20];
 			const char *next = dotdot + 2;
 			const char *this = arg;
+			int symmetric = *next == '.';
+
 			*dotdot = 0;
+			next += symmetric;
 			if (!*next)
 				next = "HEAD";
 			if (dotdot == arg)
@@ -782,8 +806,13 @@ int setup_revisions(int argc, const char
 				struct object *exclude;
 				struct object *include;
 
-				exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
-				include = get_reference(revs, next, sha1, flags);
+				if (symmetric) {
+					exclude = get_reference(revs, this, from_sha1, flags | LEFT_HALF);
+					include = get_reference(revs, next, sha1, flags | RIGHT_HALF);
+				} else {
+					exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
+					include = get_reference(revs, next, sha1, flags);
+				}
 				if (!exclude || !include)
 					die("Invalid revision range %s..%s", arg, next);
 
diff --git a/revision.h b/revision.h
index c010a08..0090232 100644
--- a/revision.h
+++ b/revision.h
@@ -9,6 +9,8 @@ #define TMP_MARK	(1u<<4) /* for isolated
 #define BOUNDARY	(1u<<5)
 #define BOUNDARY_SHOW	(1u<<6)
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
+#define RIGHT_HALF	(1u<<8)
+#define LEFT_HALF	(1u<<9)
 
 struct rev_info;
 struct log_info;

^ permalink raw reply related

* [PATCH] instaweb: fix unportable ';' usage in sed
From: Eric Wong @ 2006-07-02 11:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0607021109540.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Hint taken from Johannes.  I've tested this with sed --posix on
my system with GNU sed and it works fine with and also without
it.  Further portability testing/review would be good.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

 Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
 > 
 > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
 > ---
 > 	Re: What's in git.git
 > 
 > 	On Sun, 2 Jul 2006, Junio C Hamano wrote:
 > 
 > 	>  - instaweb by Eric Wong.
 > 
 > 	This breaks make on _all_ platforms I have.
 > 
 > 	First of all, it is _ugly_ (three invocations of sed, where one 
 > 	really is sufficient). Then, it uses the non-portable ';' 
 > 	operator, and then, the non-at-all-portable 'T'.
 > 
 > 	And worst: it is unnecessary.

 *Blushes*  Sorry about that, I've been meaning to brush up on my
 sed skills (coming from a Perl-heavy environment).

 git-instaweb.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 51067d9..69aef3c 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -194,9 +194,9 @@ EOF
 }
 
 script='
-s#^\(my\|our\) $projectroot =.*#\1 $projectroot = "'`dirname $fqgitdir`'";#;
-s#\(my\|our\) $gitbin =.*#\1 $gitbin = "'$GIT_EXEC_PATH'";#;
-s#\(my\|our\) $projects_list =.*#\1 $projects_list = $projectroot;#;
+s#^\(my\|our\) $projectroot =.*#\1 $projectroot = "'`dirname $fqgitdir`'";#
+s#\(my\|our\) $gitbin =.*#\1 $gitbin = "'$GIT_EXEC_PATH'";#
+s#\(my\|our\) $projects_list =.*#\1 $projects_list = $projectroot;#
 s#\(my\|our\) $git_temp =.*#\1 $git_temp = "'$fqgitdir/gitweb/tmp'";#'
 
 gitweb_cgi () {
-- 
1.4.1.rc2.gbc4f

^ permalink raw reply related

* [POSSIBLE REGRESSION] Spurious revs after patch "revision.c: --full-history fix"
From: Marco Costalba @ 2006-07-02 12:19 UTC (permalink / raw)
  To: junkio; +Cc: git

After patch "revision.c: --full-history fix" (6631c73)

I have this in git tree.

I have _manually_  ( --abbrev=nr does not apply in this case)
truncated to 7 chars the SHA to be more readable)

$ git-rev-list --topo-order --parents --remove-empty HEAD -- builtin-add.c
021b6e4 93872e0
93872e0 3c6a370
3c6a370 e8f990b
e8f990b f259339
f259339 0d78153
0d78153
b4189aa
283c8ee
$ git-rev-list --topo-order --remove-empty HEAD -- builtin-add.c
021b6e4
93872e0
3c6a370
e8f990b
f259339
0d78153
$

What it seems is that with --parents  option two more spurious revs
are printed out. This revs have nothing to do with builtin-add.c, the
file is newer then both of them.

I have tested with different files and always we have spurious
revisions in case of files that were added after initial commit, and
always the erroneous revisions are older then file creation one.

This is a possible regression that breaks things (I've found it thanks
to an assert in qgit).

   Marco

^ permalink raw reply

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Linus Torvalds @ 2006-07-02 16:43 UTC (permalink / raw)
  To: Rene Scharfe; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <20060702094938.GA10944@lsrfire.ath.cx>



On Sun, 2 Jul 2006, Rene Scharfe wrote:
>
> Due to popular request ;-), change get_merge_bases() to be able to
> clean up after itself if needed by adding a cleanup parameter.

Btw, I think the symmetric thing is still wrong.

Try this:

	git rev-list ^HEAD~1 HEAD...HEAD~2

which _should_ return just HEAD ("HEAD...HEAD~2" should basically expand 
into "HEAD HEAD~2 ^HEAD~2")

I haven't actually tested your patch, but I think it returns HEAD and 
HEAD~1.

The reason? You clear UNINTERESTING as part of clear_commit_marks(), so 
HEAD~1, which was marked thus by the user, gets cleared of its mark as 
part of the get_merge_bases() invocation.

I suspect the only way to fix that is to make "get_merge_bases()" not use 
UNINTERESTING at all, but instead just explicitly use something like

	(obj.flags & (LEFT | RIGHT)) == (LEFT | RIGHT)

instead.

			Linus

^ permalink raw reply

* Re: [POSSIBLE REGRESSION] Spurious revs after patch "revision.c: --full-history fix"
From: Linus Torvalds @ 2006-07-02 17:14 UTC (permalink / raw)
  To: Marco Costalba; +Cc: junkio, git
In-Reply-To: <e5bfff550607020519k6007f41bne34d10c0e919f3c8@mail.gmail.com>



On Sun, 2 Jul 2006, Marco Costalba wrote:
> 
> What it seems is that with --parents  option two more spurious revs
> are printed out. This revs have nothing to do with builtin-add.c, the
> file is newer then both of them.

Gaah. Does this trivial patch fix it for you?

It had the wrong test for whether a commit was a merge. What it did was to 
say that a non-merge has exactly one parent (which sounds almost right), 
but the fact is, initial trees have no parent at all, but they're 
obviously not merges.

So the test for non-merge should be "!parents || !parents->next", not 
"parents && !parents->next".

		Linus

----
diff --git a/revision.c b/revision.c
index 1cf6276..880fb7b 100644
--- a/revision.c
+++ b/revision.c
@@ -997,7 +997,7 @@ struct commit *get_revision(struct rev_i
 				if (!revs->parents)
 					continue;
 				/* non-merge - always ignore it */
-				if (commit->parents && !commit->parents->next)
+				if (!commit->parents || !commit->parents->next)
 					continue;
 			}
 			if (revs->parents)

^ permalink raw reply related

* Re: [POOL] Who likes running Git without make install?
From: Junio C Hamano @ 2006-07-02 17:19 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060702113057.GF29115@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Sun, Jul 02, 2006 at 02:05:46AM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> That's fixed in ancient past in git timescale.  Why are you
>> bringing it up again? ;-)
>
> Because, it is, well, not fixed? ;-) (Apparently.)

That response is about "Worse yet, the order is wrong -- ah you
are right" exchange, not about in-place execution.

>> Well, for a quick test to see if I haven't broken anything, I
>> use a new shell and do ". ./+denv" in my git repository where
>> that file has something like this:
>> 
>>         $ cat ./+denv
>>         :
>> 
>>         GIT_EXEC_PATH=`pwd`
>>         PATH=`pwd`:/usr/bin:/bin
>> 
>>         export GIT_EXEC_PATH PATH
>> 
>> So to a certain degree, yes I do care.
>
> But it is currently broken:

I know.  The scriptlet needs to muck with PERL5LIB too.

^ permalink raw reply

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Rene Scharfe @ 2006-07-02 17:40 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <Pine.LNX.4.64.0607020935460.12404@g5.osdl.org>

Linus Torvalds schrieb:
> 
> On Sun, 2 Jul 2006, Rene Scharfe wrote:
>> Due to popular request ;-), change get_merge_bases() to be able to
>> clean up after itself if needed by adding a cleanup parameter.
> 
> Btw, I think the symmetric thing is still wrong.
> 
> Try this:
> 
> 	git rev-list ^HEAD~1 HEAD...HEAD~2
> 
> which _should_ return just HEAD ("HEAD...HEAD~2" should basically expand 
> into "HEAD HEAD~2 ^HEAD~2")
> 
> I haven't actually tested your patch, but I think it returns HEAD and 
> HEAD~1.
> 
> The reason? You clear UNINTERESTING as part of clear_commit_marks(), so 
> HEAD~1, which was marked thus by the user, gets cleared of its mark as 
> part of the get_merge_bases() invocation.
> 
> I suspect the only way to fix that is to make "get_merge_bases()" not use 
> UNINTERESTING at all, but instead just explicitly use something like

No and yes.  Patch 1 in the 3+1 series changes the flags used in
commit.c to not conflict with the ones in revision.h[*].  So we have two
different UNINTERESTINGs, and get_merge_bases() doesn't mess up the
show/no-show markings.

René


[*] That fix was the one which reportedly made Dscho break a table.
Sorry for that by the way. :-P

^ permalink raw reply

* Re: [POSSIBLE REGRESSION] Spurious revs after patch "revision.c: --full-history fix"
From: Junio C Hamano @ 2006-07-02 17:48 UTC (permalink / raw)
  To: Linus Torvalds, Marco Costalba; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0607021012180.12404@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Gaah. Does this trivial patch fix it for you?
>
> It had the wrong test for whether a commit was a merge.

Gaah indeed -- I did not notice the logic error when I picked it
up either, sorry.

> diff --git a/revision.c b/revision.c
> index 1cf6276..880fb7b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -997,7 +997,7 @@ struct commit *get_revision(struct rev_i
>  				if (!revs->parents)
>  					continue;
>  				/* non-merge - always ignore it */
> -				if (commit->parents && !commit->parents->next)
> +				if (!commit->parents || !commit->parents->next)
>  					continue;
>  			}
>  			if (revs->parents)

For a casual reader who is curious, the reason it matters to
treat the "root" commit sanely in this example is because with
the --remove-empty option the commits that add the specified
paths are already made into "fake" root commits when the above
function sees them (done in try_to_simplify_commit()).

Thanks, Linus and Marco.

^ permalink raw reply

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Junio C Hamano @ 2006-07-02 18:28 UTC (permalink / raw)
  To: Rene Scharfe; +Cc: Linus Torvalds, git, Johannes Schindelin
In-Reply-To: <44A8051D.6040605@lsrfire.ath.cx>

Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

>> I suspect the only way to fix that is to make "get_merge_bases()" not use 
>> UNINTERESTING at all, but instead just explicitly use something like
>
> No and yes.  Patch 1 in the 3+1 series changes the flags used in
> commit.c to not conflict with the ones in revision.h[*].  So we have two
> different UNINTERESTINGs, and get_merge_bases() doesn't mess up the
> show/no-show markings.

Gaah.  commit.c defines its own UNINTERESTING and you rely on
not including revision.h which is ... gasp ... #$@#$!!!

Could we do something like this, pretty please?

---
diff --git a/commit.c b/commit.c
index 94c1d0e..a608faf 100644
--- a/commit.c
+++ b/commit.c
@@ -851,14 +851,14 @@ void sort_in_topological_order_fn(struct
 /* bits #0..7 in revision.h */
 #define PARENT1		(1u<< 8)
 #define PARENT2		(1u<< 9)
-#define UNINTERESTING	(1u<<10)
+#define STALE		(1u<<10)
 
 static struct commit *interesting(struct commit_list *list)
 {
 	while (list) {
 		struct commit *commit = list->item;
 		list = list->next;
-		if (commit->object.flags & UNINTERESTING)
+		if (commit->object.flags & STALE)
 			continue;
 		return commit;
 	}
@@ -920,17 +920,17 @@ static struct commit *interesting(struct
  *
  * Next, we pop B and something very interesting happens.  It has flags==3
  * so it is also placed on the result list, and its parents are marked
- * uninteresting, retroactively, and placed back on the list:
+ * stale, retroactively, and placed back on the list:
  *
  *    list=C(7), result=C(7) B(3)
  *
  * Now, list does not have any interesting commit.  So we find the newest
- * commit from the result list that is not marked uninteresting.  Which is
+ * commit from the result list that is not marked stale.  Which is
  * commit B.
  *
  *
  * Another pathological example how this thing used to fail to mark an
- * ancestor of a merge base as UNINTERESTING before we introduced the
+ * ancestor of a merge base as STALE before we introduced the
  * postprocessing phase (mark_reachable_commits).
  *
  *		  2
@@ -960,8 +960,8 @@ static struct commit *interesting(struct
  *	 C7			2 3 7 1 3 2 1 2
  *
  * At this point, unfortunately, everybody in the list is
- * uninteresting, so we fail to complete the following two
- * steps to fully marking uninteresting commits.
+ * stale, so we fail to complete the following two
+ * steps to fully marking stale commits.
  *
  *	 D7			2 3 7 7 3 2 1 2
  *	 E7			2 3 7 7 7 2 1 2
@@ -981,10 +981,10 @@ static void mark_reachable_commits(struc
 	 */
 	for (tmp = result; tmp; tmp = tmp->next) {
 		struct commit *c = tmp->item;
-		/* Reinject uninteresting ones to list,
+		/* Reinject stale ones to list,
 		 * so we can scan their parents.
 		 */
-		if (c->object.flags & UNINTERESTING)
+		if (c->object.flags & STALE)
 			commit_list_insert(c, &list);
 	}
 	while (list) {
@@ -995,8 +995,8 @@ static void mark_reachable_commits(struc
 		list = list->next;
 		free(tmp);
 
-		/* Anything taken out of the list is uninteresting, so
-		 * mark all its parents uninteresting.  We do not
+		/* Anything taken out of the list is stale, so
+		 * mark all its parents stale.  We do not
 		 * parse new ones (we already parsed all the relevant
 		 * ones).
 		 */
@@ -1004,8 +1004,8 @@ static void mark_reachable_commits(struc
 		while (parents) {
 			struct commit *p = parents->item;
 			parents = parents->next;
-			if (!(p->object.flags & UNINTERESTING)) {
-				p->object.flags |= UNINTERESTING;
+			if (!(p->object.flags & STALE)) {
+				p->object.flags |= STALE;
 				commit_list_insert(p, &list);
 			}
 		}
@@ -1034,7 +1034,7 @@ struct commit_list *get_merge_bases(stru
 		struct commit *commit = list->item;
 		struct commit_list *parents;
 		int flags = commit->object.flags
-			& (PARENT1 | PARENT2 | UNINTERESTING);
+			& (PARENT1 | PARENT2 | STALE);
 
 		tmp = list;
 		list = list->next;
@@ -1042,8 +1042,8 @@ struct commit_list *get_merge_bases(stru
 		if (flags == (PARENT1 | PARENT2)) {
 			insert_by_date(commit, &result);
 
-			/* Mark parents of a found merge uninteresting */
-			flags |= UNINTERESTING;
+			/* Mark parents of a found merge stale */
+			flags |= STALE;
 		}
 		parents = commit->parents;
 		while (parents) {
@@ -1067,7 +1067,7 @@ struct commit_list *get_merge_bases(stru
 	for (tmp = result, list = NULL; tmp; ) {
 		struct commit *commit = tmp->item;
 		struct commit_list *next = tmp->next;
-		if (commit->object.flags & UNINTERESTING) {
+		if (commit->object.flags & STALE) {
 			if (list != NULL)
 				list->next = next;
 			free(tmp);
@@ -1075,15 +1075,15 @@ struct commit_list *get_merge_bases(stru
 			if (list == NULL)
 				result = tmp;
 			list = tmp;
-			commit->object.flags |= UNINTERESTING;
+			commit->object.flags |= STALE;
 		}
 
 		tmp = next;
 	}
 
 	if (cleanup) {
-		clear_commit_marks(rev1, PARENT1 | PARENT2 | UNINTERESTING);
-		clear_commit_marks(rev2, PARENT1 | PARENT2 | UNINTERESTING);
+		clear_commit_marks(rev1, PARENT1 | PARENT2 | STALE);
+		clear_commit_marks(rev2, PARENT1 | PARENT2 | STALE);
 	}
 
 	return result;

^ permalink raw reply related

* Re: [PATCH] Makefile: replace ugly and unportable sed invocation
From: Junio C Hamano @ 2006-07-02 18:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0607021109540.29667@wbgn013.biozentrum.uni-wuerzburg.de>

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

> 	First of all, it is _ugly_ (three invocations of sed, where one 
> 	really is sufficient). Then, it uses the non-portable ';' 
> 	operator, and then, the non-at-all-portable 'T'.
>
> 	And worst: it is unnecessary.

Thanks -- I was not paying enough attention.

^ permalink raw reply

* qgit idea: interface for cherry-picking
From: Jakub Narebski @ 2006-07-02 19:01 UTC (permalink / raw)
  To: git

Currently in qgit one can git-format-patch a commit. It woul be nice if one
would be able to git-cherry-pick and git-cherry-pick -n a commit (denoting
the head, i.e. where cherry pick would be applied to). It would be very
usefull in reordering patches (cleaning up history).  

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] Git.pm: Avoid ppport.h
From: Junio C Hamano @ 2006-07-02 19:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Petr Baudis
In-Reply-To: <Pine.LNX.4.63.0607021141260.29667@wbgn013.biozentrum.uni-wuerzburg.de>

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

>> This makes us not include ppport.h which seems not to give us anything real
>> anyway; it is useful for checking for portability warts but since Devel::PPPort
>> is a portability wart itself, we shouldn't require it for build.
>
> Why do people introduce a "portability enhancer" like that? This is soo 
> dumb.

I think that statement is a bit too harsh.

My understanding is that it's more for use by developers working
with later version to produce portability headers, so used that
way it is very sane.  From its manual page:

       How to use ppport.h

       Don't direct the users of your module to download "Devel::PPPort".
       They are most probably no XS writers. Also, don't make ppport.h
       optional. Rather, just take the most recent copy of ppport.h that you
       can find (e.g. by generating it with the latest "Devel::PPPort" release
       from CPAN), copy it into your project, adjust your project to use it,
       and distribute the header along with your module.

But in the case of source distribution, and if the source
distribution wants to be compatible with older versions, the
above advice does not apply.

> Error: 'const char *' not in typemap in Git.xs, line 69
> Error: 'const char *' not in typemap in Git.xs, line 79
> make: *** [Git.c] Error 1
>
> It seems like my typemap starts like this:
>...
> So, no "const char *". See next mail for a minimal patch.

An alternative would be to carry our own typemap but I think
your fix is less intrusive and fine.  Pasky?

> The warning 
> (IIRC that was mentioned already on the list) still persists:
>
> cc -c -I. -I.. -g -pipe -pipe -fno-common -no-cpp-precomp -flat_namespace 
> -DHAS_TELLDIR_PROTOTYPE -fno-strict-aliasing -Os     -DVERSION=\"0.01\" 
> -DXS_VERSION=\"0.01\"  -I/System/Library/Perl/darwin/CORE -I/sw/include 
> -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRCASESTR -DNO_STRLCPY 
> -DGIT_VERSION='"1.4.1.g3b26"' Git.c
> In file included from /System/Library/Perl/darwin/CORE/perl.h:500,
>                  from Git.xs:15:
> /System/Library/Perl/darwin/CORE/embed.h:156:1: warning: "die" redefined
> Git.xs:11:1: warning: this is the location of the previous definition

I see the same here.

^ permalink raw reply

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Linus Torvalds @ 2006-07-02 20:44 UTC (permalink / raw)
  To: Rene Scharfe; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <44A8051D.6040605@lsrfire.ath.cx>



On Sun, 2 Jul 2006, Rene Scharfe wrote:
> 
> No and yes.  Patch 1 in the 3+1 series changes the flags used in
> commit.c to not conflict with the ones in revision.h[*].  So we have two
> different UNINTERESTINGs, and get_merge_bases() doesn't mess up the
> show/no-show markings.

Gaah. So UNINTERESTING in commit.c needs something else than everywhere 
else? That's a bug waiting to happen.

Please give it a name of its own.

		Linus

^ permalink raw reply

* Re: Quick merge status updates.
From: Petr Baudis @ 2006-07-02 20:49 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Junio C Hamano, git
In-Reply-To: <1151489103.28036.6.camel@dv>

Dear diary, on Wed, Jun 28, 2006 at 12:05:03PM CEST, I got a letter
where Pavel Roskin <proski@gnu.org> said that...
> I think the BEGIN block has priority over other statements.  My solution
> was to put the @INC change in the BEGIN block as well.
> 
> This patch is working for me:
> 
> diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl
> index e8fad02..1b23fa1 100755
> --- a/git-fmt-merge-msg.perl
> +++ b/git-fmt-merge-msg.perl
> @@ -5,7 +5,7 @@ #
>  # Read .git/FETCH_HEAD and make a human readable merge message
>  # by grouping branches and tags together to form a single line.
>  
> -unshift @INC, '@@INSTLIBDIR@@';
> +BEGIN { unshift @INC, '@@INSTLIBDIR@@'; }
>  use strict;
>  use Git;
>  use Error qw(:try);

I feel that it is time for another stupid question of mine - why can't
you just use lib?

	use lib ('@@INSTLIBDIR@@');

Looks a lot better than some @INC unshifting, and it should be
equivalent.

Let's pour in to the confusion:

The unshifting was introduced w/o BEGIN{} in

	From: Junio C Hamano <junkio@cox.net>
	Subject: Re: [PATCH 01/12] Introduce Git.pm (v4)
	Date:   Sat, 24 Jun 2006 04:57:31 -0700

but that patch is not in pu anymore while the description of the new
patch implicitly refers to it, which made it all a bit confusing.

So the purpose of the original patch was to make it play nicely with
$PERLLIB, but unshifting helps nothing, since:

	There's default @INC
	Perl spots PERLLIB and unshifts @INC
	We then unshift @INC too, taking precedence

So didn't the original patch rather want to do push?

	$ PERL5LIB=perl perl -le "BEGIN { unshift @INC, '/home/xpasky/lib/perl5/site_perl/5.8.8/i686-linux'; } use Git; print Git::hash_object('blob','Makefile');"
	17842a3657ae8e5b4fd3ddfeb69268a4b94cb97a
	$ PERL5LIB=perl perl -le "use Git; print Git::hash_object('blob','Makefile');"
	syntax error at perl/Git.pm line 44, near "h>"

(after inserting random junk to perl/Git.pm)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply

* [PATCH] Git.pm: Don't #define around die
From: Petr Baudis @ 2006-07-02 20:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v4pxz4yki.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sun, Jul 02, 2006 at 09:05:33PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > Error: 'const char *' not in typemap in Git.xs, line 69
> > Error: 'const char *' not in typemap in Git.xs, line 79
> > make: *** [Git.c] Error 1
> >
> > It seems like my typemap starts like this:
> >...
> > So, no "const char *". See next mail for a minimal patch.
> 
> An alternative would be to carry our own typemap but I think
> your fix is less intrusive and fine.  Pasky?

Yes, it should be fine.

> > The warning 
> > (IIRC that was mentioned already on the list) still persists:
> >
> > cc -c -I. -I.. -g -pipe -pipe -fno-common -no-cpp-precomp -flat_namespace 
> > -DHAS_TELLDIR_PROTOTYPE -fno-strict-aliasing -Os     -DVERSION=\"0.01\" 
> > -DXS_VERSION=\"0.01\"  -I/System/Library/Perl/darwin/CORE -I/sw/include 
> > -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRCASESTR -DNO_STRLCPY 
> > -DGIT_VERSION='"1.4.1.g3b26"' Git.c
> > In file included from /System/Library/Perl/darwin/CORE/perl.h:500,
> >                  from Git.xs:15:
> > /System/Library/Perl/darwin/CORE/embed.h:156:1: warning: "die" redefined
> > Git.xs:11:1: warning: this is the location of the previous definition
> 
> I see the same here.

-8<-

Back in the old days, we called Git's die() from the .xs code, but we had to
hijack Perl's die() for that. Now we don't call Git's die() so no need to do
the hijacking and it silences a compiler warning.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 perl/Git.xs |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/perl/Git.xs b/perl/Git.xs
index 51bfac3..1e6c1eb 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -8,15 +8,11 @@ #include <ctype.h>
 #include "../cache.h"
 #include "../exec_cmd.h"
 
-#define die perlyshadow_die__
-
 /* XS and Perl interface */
 #include "EXTERN.h"
 #include "perl.h"
 #include "XSUB.h"
 
-#undef die
-
 
 static char *
 report_xs(const char *prefix, const char *err, va_list params)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply related

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Rene Scharfe @ 2006-07-02 20:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git, Johannes Schindelin
In-Reply-To: <7vmzbr50b3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano schrieb:
> Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>>> I suspect the only way to fix that is to make "get_merge_bases()" not use 
>>> UNINTERESTING at all, but instead just explicitly use something like
>> No and yes.  Patch 1 in the 3+1 series changes the flags used in
>> commit.c to not conflict with the ones in revision.h[*].  So we have two
>> different UNINTERESTINGs, and get_merge_bases() doesn't mess up the
>> show/no-show markings.
> 
> Gaah.  commit.c defines its own UNINTERESTING and you rely on
> not including revision.h which is ... gasp ... #$@#$!!!

You mean crap?  Yes, it is.  That's why I said we should centralize the
commit flags.  There are several duplicate names for different commit
flags in different files.  I couldn't come up with a clean solution, though.

I have an idea.  Would it help to gather a list of all commit object
flags from the different files?  And then we would come up with unique
names?  And then move the definition of all of them to commit.h?

René

^ permalink raw reply

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Rene Scharfe @ 2006-07-02 21:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git, Johannes Schindelin
In-Reply-To: <44A8339B.7070608@lsrfire.ath.cx>

Rene Scharfe schrieb:
> Junio C Hamano schrieb:
>> Gaah.  commit.c defines its own UNINTERESTING and you rely on
>> not including revision.h which is ... gasp ... #$@#$!!!

> I have an idea.  Would it help to gather a list of all commit object
> flags from the different files?  And then we would come up with unique
> names?  And then move the definition of all of them to commit.h?

Err, I mean object flags (not only commit flags) and object.h instead of
commit.h.

René

^ permalink raw reply

* Re: [PATCH 4/3] Fold get_merge_bases_clean() into get_merge_bases()
From: Linus Torvalds @ 2006-07-02 21:17 UTC (permalink / raw)
  To: Rene Scharfe; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <44A8339B.7070608@lsrfire.ath.cx>



On Sun, 2 Jul 2006, Rene Scharfe wrote:
>
> I have an idea.  Would it help to gather a list of all commit object
> flags from the different files?  And then we would come up with unique
> names?  And then move the definition of all of them to commit.h?

We already did that once, for a subset of the programs. Except we put them 
in revision.h instead ;)

And yes, we should probably do it for most of the rest.

Some of them (git-show-branch) have really special flags usage, and don't 
want to pollute their magic flags with anybody else at all, but most of 
the rest (fetch.c etc) have just a few flags and could in fact mostly 
re-use the standard ones.

		Linus

^ permalink raw reply

* Re: [POSSIBLE REGRESSION] Spurious revs after patch "revision.c: --full-history fix"
From: Marco Costalba @ 2006-07-02 21:23 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: junkio, git
In-Reply-To: <Pine.LNX.4.64.0607021012180.12404@g5.osdl.org>

>
> Gaah. Does this trivial patch fix it for you?
>

Yes. It works.

Thanks
Marco

^ permalink raw reply

* Re: Quick merge status updates.
From: Junio C Hamano @ 2006-07-02 21:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060702204906.GG29115@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> So the purpose of the original patch was to make it play nicely with
> $PERLLIB, but unshifting helps nothing, since:
>
> 	There's default @INC
> 	Perl spots PERLLIB and unshifts @INC
> 	We then unshift @INC too, taking precedence

I tried this:

 0. check out the branch that has the Perly git ("pu").  build
    and install normally to have a perfectly working version
    accessible on your usual $PATH.

 1. apply the patch [1] below to make it use "use lib" instead of
    "unshift".

 2. break perl/Git.pm being built to pretend we introduced a bug
    in the work in progress by applying the patch [2] below.

 3. without installing the broken Git.pm, run "make test", and
    see a test that uses "git pull" and needs to create a true
    merge succeed.  It tells me that everything including
    perl/Git.pm is GOOD, and I'd find the breakage only after
    installing and running the test again.

If you make the freshly built script to say "use lib" to use
Git.pm and its friends from the installed location, it defeats
the attempt by test-lib.sh to override it to test what we
freshly built -- the use of "unshift @INC" is to work that
around and is parallel to the way Python stuff works around the
same problem.

[Footnotes]
*1* "use lib" patch.

diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl
index 1b23fa1..3a42950 100755
--- a/git-fmt-merge-msg.perl
+++ b/git-fmt-merge-msg.perl
@@ -5,7 +5,7 @@ #
 # Read .git/FETCH_HEAD and make a human readable merge message
 # by grouping branches and tags together to form a single line.
 
-BEGIN { unshift @INC, '@@INSTLIBDIR@@'; }
+use lib '@@INSTLIBDIR@@';
 use strict;
 use Git;
 use Error qw(:try);

*2* "break Git.pm" patch.

diff --git a/perl/Git.pm b/perl/Git.pm
index b4ee88b..34b4f12 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -6,6 +6,7 @@ Git - Perl interface to the Git version 
 
 
 package Git;
+syntax error -- kill me
 
 use strict;
 

^ permalink raw reply related

* Re: qgit idea: interface for cherry-picking
From: Marco Costalba @ 2006-07-02 21:33 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <e8954u$srh$1@sea.gmane.org>

On 7/2/06, Jakub Narebski <jnareb@gmail.com> wrote:
> Currently in qgit one can git-format-patch a commit. It woul be nice if one
> would be able to git-cherry-pick and git-cherry-pick -n a commit (denoting
> the head, i.e. where cherry pick would be applied to). It would be very
> usefull in reordering patches (cleaning up history).
>
> --

Currently in qgit you can git-format-patch a commit series and git-am
a given patch file series.
This can be done transparently with a drag & drop mechanic:

1) Open the source repository
2) Then open a new qgit instance (File->Open in a new window...)
3) Open the destination repository in the new qgit window
4) Drag & drop selected commits (multi selection in supported) from
source to destination.

I normally use this instead of git-cherry-pick  that, I admit, I don't
know very well, so please I need some more hints on how to upgrade
this behaviour introducing git.cherry-pick support.

    Marco

^ 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