Git development
 help / color / mirror / Atom feed
* Re: PPC SHA-1 Updates in "pu"
From: Linus Torvalds @ 2006-06-24 18:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwtb6yip5.fsf@assigned-by-dhcp.cox.net>



On Sat, 24 Jun 2006, Junio C Hamano wrote:
> 
> If somebody has time and inclination, please try updated PPC SHA-1
> from linux@horizon.com that is in "pu" (say make check-sha1) and
> report impressions.

"make check-sha1" passes.

Before:
	[torvalds@g5 linux]$ /usr/bin/time git-fsck-objects --full
	101.90user 4.66system 1:46.72elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
	0inputs+0outputs (0major+1787158minor)pagefaults 0swaps

After:
	[torvalds@g5 linux]$ /usr/bin/time ~/git/git-fsck-objects --full
	101.16user 4.32system 1:45.56elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
	0inputs+0outputs (0major+1787127minor)pagefaults 0swaps

which doesn't seem to really imply anything seriously changed (in fact, 
rerunning it made the numbers even closer).

This is on a G5 powerpc.

Also, "pu" in general is totally unusable. It doesn't even compile.

I think that "Git.xs" thing is fine for random hacks, but please please 
PLEASE don't make any central program depend on it.

		Linus

^ permalink raw reply

* Re: [PATCH] git-commit: filter out log message lines only when editor was run.
From: Yann Dirson @ 2006-06-24  9:42 UTC (permalink / raw)
  To: junkio, mcostalba; +Cc: GIT list

Junio wrote:
> I agree with this in principle but we would need to make sure
> that our own scripts do not expect that the message is cleaned
> up when feeding a commit log message via stdin, -m or -F, and if
> they do fix them before applying this patch.

The only tools in git.git I could identify as using git-commit rather
than commit-tree are:

git-revert.sh: OK
git-rebase.sh: only uses -C

cogito, stgit, and pg also use commit-tree.  Only qgit seems to be
using git-commit, and probably makes use of this (mis)feature.

I guess the easiest way to get the 1.4.0 behaviour is to change
git-commit calls to "git-commit -e" with EDITOR=true in the
environment.  I'm not sure it would be worth adding a new flag to
switch on/off log stripping.

Best regards,
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* Re: A series file for git?
From: Eric W. Biederman @ 2006-06-24 17:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <7vac83ylk6.fsf@assigned-by-dhcp.cox.net>

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

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> I was using:
>> 	git-rev-list $revargs | tac > list
>> 	for sha1 in $(cat list); do git-cherry-pick -r $sha1 ; done
>>...
>> - Keeping patches in git and just remembering their sha1 is nice
>>   but it has the downside that it can be really easy to loose
>>   the sha1, and thus the patch.  So some sort of history mechanism
>>   so you can get back to where you were would be nice.
>
> Actually, the $revargs above is composed of your branch names
> (e.g. "^master this-topic that-topic"), so as long as you do not
> lose these branches they are protected.

Right but typically I have something that looks like.
"git-rev-list --pretty=oneline 2.6.17-rc6-mm2..HEAD | tac > list"
and HEAD changes.

I could be careful and before I do each operation save a branch
name.

>> - This is a similar technique to topic branches.  However in some
>>   of the more interesting cases a topic branch can't be used because
>>   you have a whole series of little changes, that allow depend on
>>   each other.  So topic branches need not apply.
>
> Sorry I fail to see why.  A series of little changes that depend
> on each other would be a string of pearls on a topic branch in
> the simplest case, or a handful of topic branches that
> occasionally merge with each other if you want to get fancier.
> Cherry-picking from a DAG of the latter kind with your rev-list
> piped to tac is no different than cherry-picking from a simple
> straight line of the former kind, isn't it?

It is exactly a string of perls on a topic branch.  My point
is that when things are sufficiently interrelated that you
can't use more than one topic branch. 

With topic branches when you find a small fix to an existing
perl you just commit it to that branch, and it works
because each branch really is composed of a single pearl.
(At least that is my intuition).  

When you have multiple highly interrelated pearls and you
are testing you can easily put the fix for a pearl at
the end of the branch.  But before you push your changes
upstream that fix needs to be merged with the original
pearl that it fixes.  Which is fundamentally a history
editing kind of action.

>> - One of the places where we currently uses series files
>>   (patch-scripts && quilt), and heavy cherry picking is for patch
>>   aging.  That is letting patches sit in a testing branch for 
>>   a while so people have a chance to test and look at them.
>
> I agree that patch aging and updating does not mesh well with
> how git inherently works, as git regards commits immutable
> objects.  Even then, I use my "pu" branch (and topics that
> hasn't merged to "next" but in "pu") pretty much as patch aging
> area and I regularly do "git commit --amend" to update them.
> This however is cumbersome with core git tools alone, and I
> suspect is better done with StGIT.

I have a similar suspicion.  I am glad we pretty much see
the same problem.

>> If we create a meta data branch with just the series file
>> we can remove the risk of loosing things, as we always
>> have a path back to the old history if we want it.
>
> I am not sure about that.  What does the series file contain,
> and what other things the meta data branch contain?  If you are
> listing commit SHA1 in the series file, you _do_ have the risk
> of losing things -- git-fsck-objects needs to look at such blob
> object and consider that as the root of reachability chain; to
> me that seems too specialized hack.

When described that way I agree.   The best I can imagine
is to list those commits as ancestors of the current commit.
Hmm.  Or possibly I could collect up tag objects and work
with them.  In any case representing this in a non-hackish
way is my goal.

> I have a feeling that I really should study how well StGIT does
> things before talking about this further.  It may suit your
> needs perfectly.  What do you feel is lacking in StGIT that you
> need?

What I want and what I see lacking in the git and StGit is
the ability to record the history of editing the history
of a branch. 

A mundane example.  It would be nice if I rebased a branch if
I could record in some fashion what that branch was before
I rebased it.

So I am trying to figure out how to construct a meta branch
that records that kind of information about a branch.

Intuitively this feels like the history of a series file
to me.  Did I add commits?  Did I remove commits?  Did
I reorder commits?  Did I merge commits?

Andrew in his patch-scripts documentation recommends using
CVS to record this information (The documentation predates
git).  I think this is something that got lost with quilt when Andrews
patch-scripts were cleaned up and made more general.

I need to look some more but I don't think recording the history
of editing a branch is problem that has been solved in any of
the git flavors.

I will certainly look at StGit more and at the core git data
structures some more and see if I can come up with a creative
way of representing the history of branch edit operations
that will not break existing tools.

Eric

^ permalink raw reply

* [PATCH 7/7] Remove awkward compatibility warts
From: Timo Hirvonen @ 2006-06-24 17:28 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>


Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 builtin-diff-files.c |    7 -------
 builtin-diff.c       |    7 -------
 2 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index a655eea..3361898 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -47,12 +47,5 @@ int cmd_diff_files(int argc, const char 
 	if (rev.pending.nr ||
 	    rev.min_age != -1 || rev.max_age != -1)
 		usage(diff_files_usage);
-	/*
-	 * Backward compatibility wart - "diff-files -s" used to
-	 * defeat the common diff option "-s" which asked for
-	 * DIFF_FORMAT_NO_OUTPUT.
-	 */
-	if (rev.diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
-		rev.diffopt.output_format = DIFF_FORMAT_RAW;
 	return run_diff_files(&rev, silent);
 }
diff --git a/builtin-diff.c b/builtin-diff.c
index 47e0a37..076eb09 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -56,13 +56,6 @@ static int builtin_diff_files(struct rev
 	if (revs->max_count < 0 &&
 	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
 		revs->combine_merges = revs->dense_combined_merges = 1;
-	/*
-	 * Backward compatibility wart - "diff-files -s" used to
-	 * defeat the common diff option "-s" which asked for
-	 * DIFF_FORMAT_NO_OUTPUT.
-	 */
-	if (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
-		revs->diffopt.output_format = DIFF_FORMAT_RAW;
 	return run_diff_files(revs, silent);
 }
 
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 6/7] --name-only, --name-status, --check and -s are mutually exclusive
From: Timo Hirvonen @ 2006-06-24 17:26 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>


Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 diff.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 6be31e7..4b1b4eb 100644
--- a/diff.c
+++ b/diff.c
@@ -1366,6 +1366,19 @@ void diff_setup(struct diff_options *opt
 
 int diff_setup_done(struct diff_options *options)
 {
+	int count = 0;
+
+	if (options->output_format & DIFF_FORMAT_NAME)
+		count++;
+	if (options->output_format & DIFF_FORMAT_NAME_STATUS)
+		count++;
+	if (options->output_format & DIFF_FORMAT_CHECKDIFF)
+		count++;
+	if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
+		count++;
+	if (count > 1)
+		die("--name-only, --name-status, --check and -s are mutually exclusive");
+
 	if ((options->find_copies_harder &&
 	     options->detect_rename != DIFF_DETECT_COPY) ||
 	    (0 <= options->rename_limit && !options->detect_rename))
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 5/7] DIFF_FORMAT_RAW is not default anymore
From: Timo Hirvonen @ 2006-06-24 17:25 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>

diff_setup() used to initialize output_format to DIFF_FORMAT_RAW.  Now
the default is 0 (no output) so don't compare against DIFF_FORMAT_RAW to
see if any diff format command line flags were given.

Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 builtin-log.c |    5 +----
 revision.c    |    3 +--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 65f9527..f173070 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -24,11 +24,8 @@ static int cmd_log_wc(int argc, const ch
 	rev->verbose_header = 1;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 	if (rev->always_show_header) {
-		if (rev->diffopt.pickaxe || rev->diffopt.filter) {
+		if (rev->diffopt.pickaxe || rev->diffopt.filter)
 			rev->always_show_header = 0;
-			if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
-				rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
-		}
 	}
 
 	if (argc > 1)
diff --git a/revision.c b/revision.c
index b963f2a..ae4ca82 100644
--- a/revision.c
+++ b/revision.c
@@ -851,8 +851,7 @@ int setup_revisions(int argc, const char
 	}
 	if (revs->combine_merges) {
 		revs->ignore_merges = 0;
-		if (revs->dense_combined_merges &&
-		    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))
+		if (revs->dense_combined_merges && !revs->diffopt.output_format)
 			revs->diffopt.output_format = DIFF_FORMAT_PATCH;
 	}
 	revs->diffopt.abbrev = revs->abbrev;
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 1/7] Clean up diff.c
From: Timo Hirvonen @ 2006-06-24 17:20 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>


Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 diff.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/diff.c b/diff.c
index ad77543..f358546 100644
--- a/diff.c
+++ b/diff.c
@@ -203,7 +203,7 @@ static void emit_rewrite_diff(const char
 static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
 {
 	if (!DIFF_FILE_VALID(one)) {
-		mf->ptr = ""; /* does not matter */
+		mf->ptr = (char *)""; /* does not matter */
 		mf->size = 0;
 		return 0;
 	}
@@ -395,7 +395,7 @@ static void show_stats(struct diffstat_t
 	}
 
 	for (i = 0; i < data->nr; i++) {
-		char *prefix = "";
+		const char *prefix = "";
 		char *name = data->files[i]->name;
 		int added = data->files[i]->added;
 		int deleted = data->files[i]->deleted;
@@ -918,7 +918,7 @@ int diff_populate_filespec(struct diff_f
 			err_empty:
 				err = -1;
 			empty:
-				s->data = "";
+				s->data = (char *)"";
 				s->size = 0;
 				return err;
 			}
@@ -1409,7 +1409,7 @@ int diff_setup_done(struct diff_options 
 	return 0;
 }
 
-int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
+static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
 {
 	char c, *eq;
 	int len;
@@ -1725,16 +1725,12 @@ static void diff_flush_raw(struct diff_f
 		free((void*)path_two);
 }
 
-static void diff_flush_name(struct diff_filepair *p,
-			    int inter_name_termination,
-			    int line_termination)
+static void diff_flush_name(struct diff_filepair *p, int line_termination)
 {
 	char *path = p->two->path;
 
 	if (line_termination)
 		path = quote_one(p->two->path);
-	else
-		path = p->two->path;
 	printf("%s%c", path, line_termination);
 	if (p->two->path != path)
 		free(path);
@@ -1955,9 +1951,7 @@ static void flush_one_pair(struct diff_f
 				       options, diff_output_format);
 			break;
 		case DIFF_FORMAT_NAME:
-			diff_flush_name(p,
-					inter_name_termination,
-					line_termination);
+			diff_flush_name(p, line_termination);
 			break;
 		case DIFF_FORMAT_NO_OUTPUT:
 			break;
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 2/7] Merge with_raw, with_stat and summary variables to output_format
From: Timo Hirvonen @ 2006-06-24 17:21 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>

DIFF_FORMAT_* are now bit-flags instead of enumerated values.

Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 builtin-diff.c |    2 -
 builtin-log.c  |    4 -
 combine-diff.c |   55 +++++++----------
 diff.c         |  183 +++++++++++++++++++++++++++-----------------------------
 diff.h         |   27 +++++---
 log-tree.c     |    9 ++-
 6 files changed, 136 insertions(+), 144 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index 99a2f76..3b44296 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -56,7 +56,7 @@ static int builtin_diff_files(struct rev
 	    3 < revs->max_count)
 		usage(builtin_diff_usage);
 	if (revs->max_count < 0 &&
-	    (revs->diffopt.output_format == DIFF_FORMAT_PATCH))
+	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
 		revs->combine_merges = revs->dense_combined_merges = 1;
 	/*
 	 * Backward compatibility wart - "diff-files -s" used to
diff --git a/builtin-log.c b/builtin-log.c
index 5a8a50b..5c656bc 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -176,11 +176,9 @@ int cmd_format_patch(int argc, const cha
 	rev.commit_format = CMIT_FMT_EMAIL;
 	rev.verbose_header = 1;
 	rev.diff = 1;
-	rev.diffopt.with_raw = 0;
-	rev.diffopt.with_stat = 1;
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
-	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+	rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
 
 	git_config(git_format_config);
 	rev.extra_headers = extra_headers;
diff --git a/combine-diff.c b/combine-diff.c
index 64b20cc..3daa8cb 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -771,7 +771,7 @@ static void show_raw_diff(struct combine
 	if (rev->loginfo)
 		show_log(rev, rev->loginfo, "\n");
 
-	if (opt->output_format == DIFF_FORMAT_RAW) {
+	if (opt->output_format & DIFF_FORMAT_RAW) {
 		offset = strlen(COLONS) - num_parent;
 		if (offset < 0)
 			offset = 0;
@@ -791,8 +791,7 @@ static void show_raw_diff(struct combine
 		printf(" %s ", diff_unique_abbrev(p->sha1, opt->abbrev));
 	}
 
-	if (opt->output_format == DIFF_FORMAT_RAW ||
-	    opt->output_format == DIFF_FORMAT_NAME_STATUS) {
+	if (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) {
 		for (i = 0; i < num_parent; i++)
 			putchar(p->parent[i].status);
 		putchar(inter_name_termination);
@@ -818,17 +817,12 @@ void show_combined_diff(struct combine_d
 	struct diff_options *opt = &rev->diffopt;
 	if (!p->len)
 		return;
-	switch (opt->output_format) {
-	case DIFF_FORMAT_RAW:
-	case DIFF_FORMAT_NAME_STATUS:
-	case DIFF_FORMAT_NAME:
+	if (opt->output_format & (DIFF_FORMAT_RAW |
+				  DIFF_FORMAT_NAME |
+				  DIFF_FORMAT_NAME_STATUS)) {
 		show_raw_diff(p, num_parent, rev);
-		return;
-	case DIFF_FORMAT_PATCH:
+	} else if (opt->output_format & DIFF_FORMAT_PATCH) {
 		show_patch_diff(p, num_parent, dense, rev);
-		return;
-	default:
-		return;
 	}
 }
 
@@ -842,13 +836,9 @@ void diff_tree_combined(const unsigned c
 	struct diff_options diffopts;
 	struct combine_diff_path *p, *paths = NULL;
 	int i, num_paths;
-	int do_diffstat;
 
-	do_diffstat = (opt->output_format == DIFF_FORMAT_DIFFSTAT ||
-		       opt->with_stat);
 	diffopts = *opt;
-	diffopts.with_raw = 0;
-	diffopts.with_stat = 0;
+	diffopts.output_format &= ~(DIFF_FORMAT_RAW | DIFF_FORMAT_DIFFSTAT);
 	diffopts.recursive = 1;
 
 	/* find set of paths that everybody touches */
@@ -856,19 +846,18 @@ void diff_tree_combined(const unsigned c
 		/* show stat against the first parent even
 		 * when doing combined diff.
 		 */
-		if (i == 0 && do_diffstat)
-			diffopts.output_format = DIFF_FORMAT_DIFFSTAT;
+		if (i == 0 && opt->output_format & DIFF_FORMAT_DIFFSTAT)
+			diffopts.output_format |= DIFF_FORMAT_DIFFSTAT;
 		else
-			diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
+			diffopts.output_format |= DIFF_FORMAT_NO_OUTPUT;
 		diff_tree_sha1(parent[i], sha1, "", &diffopts);
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
 
-		if (do_diffstat && rev->loginfo)
-			show_log(rev, rev->loginfo,
-				 opt->with_stat ? "---\n" : "\n");
+		if (opt->output_format & DIFF_FORMAT_DIFFSTAT && rev->loginfo)
+			show_log(rev, rev->loginfo, "---\n");
 		diff_flush(&diffopts);
-		if (opt->with_stat)
+		if (opt->output_format & DIFF_FORMAT_DIFFSTAT)
 			putchar('\n');
 	}
 
@@ -878,17 +867,21 @@ void diff_tree_combined(const unsigned c
 			num_paths++;
 	}
 	if (num_paths) {
-		if (opt->with_raw) {
-			int saved_format = opt->output_format;
-			opt->output_format = DIFF_FORMAT_RAW;
+		if (opt->output_format & (DIFF_FORMAT_RAW |
+					  DIFF_FORMAT_NAME |
+					  DIFF_FORMAT_NAME_STATUS)) {
 			for (p = paths; p; p = p->next) {
-				show_combined_diff(p, num_parent, dense, rev);
+				if (p->len)
+					show_raw_diff(p, num_parent, rev);
 			}
-			opt->output_format = saved_format;
 			putchar(opt->line_termination);
 		}
-		for (p = paths; p; p = p->next) {
-			show_combined_diff(p, num_parent, dense, rev);
+		if (opt->output_format & DIFF_FORMAT_PATCH) {
+			for (p = paths; p; p = p->next) {
+				if (p->len)
+					show_patch_diff(p, num_parent, dense,
+							rev);
+			}
 		}
 	}
 
diff --git a/diff.c b/diff.c
index f358546..bfed79c 100644
--- a/diff.c
+++ b/diff.c
@@ -1372,23 +1372,27 @@ int diff_setup_done(struct diff_options 
 	    (0 <= options->rename_limit && !options->detect_rename))
 		return -1;
 
+	if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
+		options->output_format = 0;
+
+	if (options->output_format & (DIFF_FORMAT_NAME |
+				      DIFF_FORMAT_NAME_STATUS |
+				      DIFF_FORMAT_CHECKDIFF |
+				      DIFF_FORMAT_NO_OUTPUT))
+		options->output_format &= ~(DIFF_FORMAT_RAW |
+					    DIFF_FORMAT_DIFFSTAT |
+					    DIFF_FORMAT_SUMMARY |
+					    DIFF_FORMAT_PATCH);
+
 	/*
 	 * These cases always need recursive; we do not drop caller-supplied
 	 * recursive bits for other formats here.
 	 */
-	if ((options->output_format == DIFF_FORMAT_PATCH) ||
-	    (options->output_format == DIFF_FORMAT_DIFFSTAT) ||
-	    (options->output_format == DIFF_FORMAT_CHECKDIFF))
+	if (options->output_format & (DIFF_FORMAT_PATCH |
+				      DIFF_FORMAT_DIFFSTAT |
+				      DIFF_FORMAT_CHECKDIFF))
 		options->recursive = 1;
 
-	/*
-	 * These combinations do not make sense.
-	 */
-	if (options->output_format == DIFF_FORMAT_RAW)
-		options->with_raw = 0;
-	if (options->output_format == DIFF_FORMAT_DIFFSTAT)
-		options->with_stat  = 0;
-
 	if (options->detect_rename && options->rename_limit < 0)
 		options->rename_limit = diff_rename_limit_default;
 	if (options->setup & DIFF_SETUP_USE_CACHE) {
@@ -1460,22 +1464,20 @@ int diff_opt_parse(struct diff_options *
 {
 	const char *arg = av[0];
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
-		options->output_format = DIFF_FORMAT_PATCH;
+		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (opt_arg(arg, 'U', "unified", &options->context))
-		options->output_format = DIFF_FORMAT_PATCH;
+		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (!strcmp(arg, "--patch-with-raw")) {
-		options->output_format = DIFF_FORMAT_PATCH;
-		options->with_raw = 1;
+		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
 	}
 	else if (!strcmp(arg, "--stat"))
-		options->output_format = DIFF_FORMAT_DIFFSTAT;
+		options->output_format |= DIFF_FORMAT_DIFFSTAT;
 	else if (!strcmp(arg, "--check"))
-		options->output_format = DIFF_FORMAT_CHECKDIFF;
+		options->output_format |= DIFF_FORMAT_CHECKDIFF;
 	else if (!strcmp(arg, "--summary"))
-		options->summary = 1;
+		options->output_format |= DIFF_FORMAT_SUMMARY;
 	else if (!strcmp(arg, "--patch-with-stat")) {
-		options->output_format = DIFF_FORMAT_PATCH;
-		options->with_stat = 1;
+		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
 	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
@@ -1484,19 +1486,20 @@ int diff_opt_parse(struct diff_options *
 	else if (!strcmp(arg, "--full-index"))
 		options->full_index = 1;
 	else if (!strcmp(arg, "--binary")) {
-		options->output_format = DIFF_FORMAT_PATCH;
+		options->output_format |= DIFF_FORMAT_PATCH;
 		options->full_index = options->binary = 1;
 	}
 	else if (!strcmp(arg, "--name-only"))
-		options->output_format = DIFF_FORMAT_NAME;
+		options->output_format |= DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
-		options->output_format = DIFF_FORMAT_NAME_STATUS;
+		options->output_format |= DIFF_FORMAT_NAME_STATUS;
 	else if (!strcmp(arg, "-R"))
 		options->reverse_diff = 1;
 	else if (!strncmp(arg, "-S", 2))
 		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s"))
-		options->output_format = DIFF_FORMAT_NO_OUTPUT;
+	else if (!strcmp(arg, "-s")) {
+		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
+	}
 	else if (!strncmp(arg, "-O", 2))
 		options->orderfile = arg + 2;
 	else if (!strncmp(arg, "--diff-filter=", 14))
@@ -1671,15 +1674,17 @@ const char *diff_unique_abbrev(const uns
 }
 
 static void diff_flush_raw(struct diff_filepair *p,
-			   int line_termination,
-			   int inter_name_termination,
-			   struct diff_options *options,
-			   int output_format)
+			   struct diff_options *options)
 {
 	int two_paths;
 	char status[10];
 	int abbrev = options->abbrev;
 	const char *path_one, *path_two;
+	int inter_name_termination = '\t';
+	int line_termination = options->line_termination;
+
+	if (!line_termination)
+		inter_name_termination = 0;
 
 	path_one = p->one->path;
 	path_two = p->two->path;
@@ -1708,7 +1713,7 @@ static void diff_flush_raw(struct diff_f
 		two_paths = 0;
 		break;
 	}
-	if (output_format != DIFF_FORMAT_NAME_STATUS) {
+	if (!(options->output_format & DIFF_FORMAT_NAME_STATUS)) {
 		printf(":%06o %06o %s ",
 		       p->one->mode, p->two->mode,
 		       diff_unique_abbrev(p->one->sha1, abbrev));
@@ -1917,48 +1922,30 @@ static void diff_resolve_rename_copy(voi
 	diff_debug_queue("resolve-rename-copy done", q);
 }
 
-static void flush_one_pair(struct diff_filepair *p,
-			   int diff_output_format,
-			   struct diff_options *options,
-			   struct diffstat_t *diffstat)
+static int check_pair_status(struct diff_filepair *p)
 {
-	int inter_name_termination = '\t';
-	int line_termination = options->line_termination;
-	if (!line_termination)
-		inter_name_termination = 0;
-
 	switch (p->status) {
 	case DIFF_STATUS_UNKNOWN:
-		break;
+		return 0;
 	case 0:
 		die("internal error in diff-resolve-rename-copy");
-		break;
 	default:
-		switch (diff_output_format) {
-		case DIFF_FORMAT_DIFFSTAT:
-			diff_flush_stat(p, options, diffstat);
-			break;
-		case DIFF_FORMAT_CHECKDIFF:
-			diff_flush_checkdiff(p, options);
-			break;
-		case DIFF_FORMAT_PATCH:
-			diff_flush_patch(p, options);
-			break;
-		case DIFF_FORMAT_RAW:
-		case DIFF_FORMAT_NAME_STATUS:
-			diff_flush_raw(p, line_termination,
-				       inter_name_termination,
-				       options, diff_output_format);
-			break;
-		case DIFF_FORMAT_NAME:
-			diff_flush_name(p, line_termination);
-			break;
-		case DIFF_FORMAT_NO_OUTPUT:
-			break;
-		}
+		return 1;
 	}
 }
 
+static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
+{
+	int fmt = opt->output_format;
+
+	if (fmt & DIFF_FORMAT_CHECKDIFF)
+		diff_flush_checkdiff(p, opt);
+	else if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))
+		diff_flush_raw(p, opt);
+	else if (fmt & DIFF_FORMAT_NAME)
+		diff_flush_name(p, opt->line_termination);
+}
+
 static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)
 {
 	if (fs->mode)
@@ -2041,55 +2028,61 @@ static void diff_summary(struct diff_fil
 void diff_flush(struct diff_options *options)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
-	int i;
-	int diff_output_format = options->output_format;
-	struct diffstat_t *diffstat = NULL;
+	int i, output_format = options->output_format;
 
-	if (diff_output_format == DIFF_FORMAT_DIFFSTAT || options->with_stat) {
-		diffstat = xcalloc(sizeof (struct diffstat_t), 1);
-		diffstat->xm.consume = diffstat_consume;
-	}
+	/*
+	 * Order: raw, stat, summary, patch
+	 * or:    name/name-status/checkdiff (other bits clear)
+	 */
 
-	if (options->with_raw) {
+	if (output_format & (DIFF_FORMAT_RAW |
+			     DIFF_FORMAT_NAME |
+			     DIFF_FORMAT_NAME_STATUS |
+			     DIFF_FORMAT_CHECKDIFF)) {
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			flush_one_pair(p, DIFF_FORMAT_RAW, options, NULL);
+			if (check_pair_status(p))
+				flush_one_pair(p, options);
 		}
-		putchar(options->line_termination);
 	}
-	if (options->with_stat) {
+
+	if (output_format & DIFF_FORMAT_DIFFSTAT) {
+		struct diffstat_t *diffstat;
+
+		diffstat = xcalloc(sizeof (struct diffstat_t), 1);
+		diffstat->xm.consume = diffstat_consume;
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			flush_one_pair(p, DIFF_FORMAT_DIFFSTAT, options,
-				       diffstat);
+			if (check_pair_status(p))
+				diff_flush_stat(p, options, diffstat);
 		}
 		show_stats(diffstat);
 		free(diffstat);
-		diffstat = NULL;
-		if (options->summary)
-			for (i = 0; i < q->nr; i++)
-				diff_summary(q->queue[i]);
-		if (options->stat_sep)
-			fputs(options->stat_sep, stdout);
-		else
-			putchar(options->line_termination);
-	}
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		flush_one_pair(p, diff_output_format, options, diffstat);
 	}
 
-	if (diffstat) {
-		show_stats(diffstat);
-		free(diffstat);
+	if (output_format & DIFF_FORMAT_SUMMARY) {
+		for (i = 0; i < q->nr; i++)
+			diff_summary(q->queue[i]);
 	}
 
-	for (i = 0; i < q->nr; i++) {
-		if (diffstat && options->summary)
-			diff_summary(q->queue[i]);
-		diff_free_filepair(q->queue[i]);
+	if (output_format & DIFF_FORMAT_PATCH) {
+		if (output_format & (DIFF_FORMAT_DIFFSTAT |
+				     DIFF_FORMAT_SUMMARY)) {
+			if (options->stat_sep)
+				fputs(options->stat_sep, stdout);
+			else
+				putchar(options->line_termination);
+		}
+
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if (check_pair_status(p))
+				diff_flush_patch(p, options);
+		}
 	}
 
+	for (i = 0; i < q->nr; i++)
+		diff_free_filepair(q->queue[i]);
 	free(q->queue);
 	q->queue = NULL;
 	q->nr = q->alloc = 0;
diff --git a/diff.h b/diff.h
index b61fdc8..2b6dc0c 100644
--- a/diff.h
+++ b/diff.h
@@ -20,19 +20,31 @@ typedef void (*add_remove_fn_t)(struct d
 		    const unsigned char *sha1,
 		    const char *base, const char *path);
 
+#define DIFF_FORMAT_RAW		0x0001
+#define DIFF_FORMAT_DIFFSTAT	0x0002
+#define DIFF_FORMAT_SUMMARY	0x0004
+#define DIFF_FORMAT_PATCH	0x0008
+
+/* These override all above */
+#define DIFF_FORMAT_NAME	0x0010
+#define DIFF_FORMAT_NAME_STATUS	0x0020
+#define DIFF_FORMAT_CHECKDIFF	0x0040
+
+/* Same as output_format = 0 but we know that -s flag was given
+ * and we should not give default value to output_format.
+ */
+#define DIFF_FORMAT_NO_OUTPUT	0x0080
+
 struct diff_options {
 	const char *filter;
 	const char *orderfile;
 	const char *pickaxe;
 	unsigned recursive:1,
-		 with_raw:1,
-		 with_stat:1,
 		 tree_in_recursive:1,
 		 binary:1,
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1,
-		 summary:1,
 		 color_diff:1;
 	int context;
 	int break_opt;
@@ -151,15 +163,6 @@ #define COMMON_DIFF_OPTIONS_HELP \
 "                show all files diff when -S is used and hit is found.\n"
 
 extern int diff_queue_is_empty(void);
-
-#define DIFF_FORMAT_RAW		1
-#define DIFF_FORMAT_PATCH	2
-#define DIFF_FORMAT_NO_OUTPUT	3
-#define DIFF_FORMAT_NAME	4
-#define DIFF_FORMAT_NAME_STATUS	5
-#define DIFF_FORMAT_DIFFSTAT	6
-#define DIFF_FORMAT_CHECKDIFF	7
-
 extern void diff_flush(struct diff_options*);
 
 /* diff-raw status letters */
diff --git a/log-tree.c b/log-tree.c
index ebb49f2..7d4c51f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -163,8 +163,13 @@ int log_tree_diff_flush(struct rev_info 
 		return 0;
 	}
 
-	if (opt->loginfo && !opt->no_commit_id)
-		show_log(opt, opt->loginfo, opt->diffopt.with_stat ? "---\n" : "\n");
+	if (opt->loginfo && !opt->no_commit_id) {
+		if (opt->diffopt.output_format & DIFF_FORMAT_DIFFSTAT) {
+			show_log(opt, opt->loginfo,  "---\n");
+		} else {
+			show_log(opt, opt->loginfo,  "\n");
+		}
+	}
 	diff_flush(&opt->diffopt);
 	return 1;
 }
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 3/7] Make --raw option available for all diff commands
From: Timo Hirvonen @ 2006-06-24 17:23 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>


Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 builtin-diff.c |   48 ++++++++++++------------------------------------
 diff.c         |    2 ++
 2 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index 3b44296..91235a1 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -39,8 +39,6 @@ static int builtin_diff_files(struct rev
 			revs->max_count = 3;
 		else if (!strcmp(arg, "-q"))
 			silent = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
 		else
 			usage(builtin_diff_usage);
 		argv++; argc--;
@@ -107,14 +105,9 @@ static int builtin_diff_b_f(struct rev_i
 	/* Blob vs file in the working tree*/
 	struct stat st;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc > 1)
+		usage(builtin_diff_usage);
+
 	if (lstat(path, &st))
 		die("'%s': %s", path, strerror(errno));
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
@@ -137,14 +130,9 @@ static int builtin_diff_blobs(struct rev
 	 */
 	unsigned mode = canon_mode(S_IFREG | 0644);
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc > 1)
+		usage(builtin_diff_usage);
+
 	stuff_change(&revs->diffopt,
 		     mode, mode,
 		     blob[1].sha1, blob[0].sha1,
@@ -162,8 +150,6 @@ static int builtin_diff_index(struct rev
 		const char *arg = argv[1];
 		if (!strcmp(arg, "--cached"))
 			cached = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
 		else
 			usage(builtin_diff_usage);
 		argv++; argc--;
@@ -185,14 +171,9 @@ static int builtin_diff_tree(struct rev_
 {
 	const unsigned char *(sha1[2]);
 	int swap = 0;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+
+	if (argc > 1)
+		usage(builtin_diff_usage);
 
 	/* We saw two trees, ent[0] and ent[1].
 	 * if ent[1] is unintesting, they are swapped
@@ -214,14 +195,9 @@ static int builtin_diff_combined(struct 
 	const unsigned char (*parent)[20];
 	int i;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc > 1)
+		usage(builtin_diff_usage);
+
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	parent = xmalloc(ents * sizeof(*parent));
diff --git a/diff.c b/diff.c
index bfed79c..6e5ae77 100644
--- a/diff.c
+++ b/diff.c
@@ -1467,6 +1467,8 @@ int diff_opt_parse(struct diff_options *
 		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (opt_arg(arg, 'U', "unified", &options->context))
 		options->output_format |= DIFF_FORMAT_PATCH;
+	else if (!strcmp(arg, "--raw"))
+		options->output_format |= DIFF_FORMAT_RAW;
 	else if (!strcmp(arg, "--patch-with-raw")) {
 		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
 	}
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 4/7] Set default diff output format after parsing command line
From: Timo Hirvonen @ 2006-06-24 17:24 UTC (permalink / raw)
  To: junkio; +Cc: git
In-Reply-To: <20060624201843.a5b4f7b9.tihirvon@gmail.com>

Initialize output_format to 0 instead of DIFF_FORMAT_RAW so that we can see
later if any command line options changed it.  Default value is set only if
output format was not specified.

Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>
---
 builtin-diff-files.c  |    3 +++
 builtin-diff-index.c  |    3 +++
 builtin-diff-stages.c |    3 +++
 builtin-diff-tree.c   |    3 +++
 builtin-diff.c        |    4 +++-
 builtin-log.c         |    4 +++-
 diff.c                |    1 -
 7 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 5afc1d7..a655eea 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -36,6 +36,9 @@ int cmd_diff_files(int argc, const char 
 			usage(diff_files_usage);
 		argv++; argc--;
 	}
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_RAW;
+
 	/*
 	 * Make sure there are NO revision (i.e. pending object) parameter,
 	 * rev.max_count is reasonable (0 <= n <= 3),
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index c42ef9a..b37c9e8 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -28,6 +28,9 @@ int cmd_diff_index(int argc, const char 
 		else
 			usage(diff_cache_usage);
 	}
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_RAW;
+
 	/*
 	 * Make sure there is one revision (i.e. pending object),
 	 * and there is no revision filtering parameters.
diff --git a/builtin-diff-stages.c b/builtin-diff-stages.c
index 7c157ca..30931fe 100644
--- a/builtin-diff-stages.c
+++ b/builtin-diff-stages.c
@@ -85,6 +85,9 @@ int cmd_diff_stages(int ac, const char *
 		ac--; av++;
 	}
 
+	if (!diff_options.output_format)
+		diff_options.output_format = DIFF_FORMAT_RAW;
+
 	if (ac < 3 ||
 	    sscanf(av[1], "%d", &stage1) != 1 ||
 	    ! (0 <= stage1 && stage1 <= 3) ||
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 3409a39..ae1cde9 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -84,6 +84,9 @@ int cmd_diff_tree(int argc, const char *
 		usage(diff_tree_usage);
 	}
 
+	if (!opt->diffopt.output_format)
+		opt->diffopt.output_format = DIFF_FORMAT_RAW;
+
 	/*
 	 * NOTE! We expect "a ^b" to be equal to "a..b", so we
 	 * reverse the order of the objects if the second one
diff --git a/builtin-diff.c b/builtin-diff.c
index 91235a1..47e0a37 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -252,9 +252,11 @@ int cmd_diff(int argc, const char **argv
 
 	git_config(git_diff_config);
 	init_revisions(&rev);
-	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 
 	argc = setup_revisions(argc, argv, &rev, NULL);
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+
 	/* Do we have --cached and not have a pending object, then
 	 * default to HEAD by hand.  Eek.
 	 */
diff --git a/builtin-log.c b/builtin-log.c
index 5c656bc..65f9527 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -178,7 +178,6 @@ int cmd_format_patch(int argc, const cha
 	rev.diff = 1;
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
-	rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
 
 	git_config(git_format_config);
 	rev.extra_headers = extra_headers;
@@ -247,6 +246,9 @@ int cmd_format_patch(int argc, const cha
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
+
 	if (output_directory) {
 		if (use_stdout)
 			die("standard output, or directory, which one?");
diff --git a/diff.c b/diff.c
index 6e5ae77..6be31e7 100644
--- a/diff.c
+++ b/diff.c
@@ -1355,7 +1355,6 @@ static void run_checkdiff(struct diff_fi
 void diff_setup(struct diff_options *options)
 {
 	memset(options, 0, sizeof(*options));
-	options->output_format = DIFF_FORMAT_RAW;
 	options->line_termination = '\n';
 	options->break_opt = -1;
 	options->rename_limit = -1;
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 0/7] Rework diff options
From: Timo Hirvonen @ 2006-06-24 17:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This patch series cleans up diff output format options.

This makes it possible to use any combination of --raw, -p, --stat and
--summary options and they work as you would expect.

These patches passed all test and are for the next branch. Patches 6 and
7 are optional.

 b/builtin-diff-files.c  |   10 --
 b/builtin-diff-index.c  |    3 
 b/builtin-diff-stages.c |    3 
 b/builtin-diff-tree.c   |    3 
 b/builtin-diff.c        |   62 +++----------
 b/builtin-log.c         |   13 +-
 b/combine-diff.c        |   55 +++++------
 b/diff.c                |  221 +++++++++++++++++++++++-------------------------
 b/diff.h                |   27 +++--
 b/log-tree.c            |   10 +-
 b/revision.c            |    4 
 11 files changed, 189 insertions(+), 222 deletions(-)

-- 
http://onion.dynserv.net/~timo/

^ permalink raw reply

* Re: [RFC] GIT user survey
From: Paolo Ciarrocchi @ 2006-06-24 17:08 UTC (permalink / raw)
  To: Randal L. Schwartz; +Cc: Git Mailing List
In-Reply-To: <86ac82bi1u.fsf@blue.stonehenge.com>

On 24 Jun 2006 10:05:33 -0700, Randal L. Schwartz <merlyn@stonehenge.com> wrote:
> >>>>> "Paolo" == Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> writes:
>
> Paolo>     1. Do you use the GIT wiki?   If yes, do you find it useful?
>
> There's a git wiki??

Yup.
http://git.or.cz/gitwiki/FrontPage

Ciao,

-- 
Paolo
http://paolociarrocchi.googlepages.com
http://picasaweb.google.com/paolo.ciarrocchi

^ permalink raw reply

* Re: [RFC] GIT user survey
From: Randal L. Schwartz @ 2006-06-24 17:05 UTC (permalink / raw)
  To: Paolo Ciarrocchi; +Cc: Git Mailing List
In-Reply-To: <4d8e3fd30606240918m6b452314m6514b5e5fc86f147@mail.gmail.com>

>>>>> "Paolo" == Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> writes:

Paolo>     1. Do you use the GIT wiki?   If yes, do you find it useful?

There's a git wiki??

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [Patch] trap: exit: invalid signal specification
From: Matthias Lederhofer @ 2006-06-24 16:57 UTC (permalink / raw)
  To: S.Ça??lar Onur; +Cc: git
In-Reply-To: <200606241555.03147.caglar@pardus.org.tr>

> As an example bash (v. 3.1.17) permits lowercase signal names but it converts 
> this lowercase signal names into uppercase ones while interpreting the 
> script. But for our "Turkish has 4 letter "I"s" problem this convert to 
> uppercase one process fails but for bash invalid signal names not be 
> considered a syntax error and do not cause the shell to abort. 

It is ``trap <action> <signal>'' so for the signal part this may be
right that this is made uppercase. But action is not modified I guess.
So it should be exit because there is no built-in named EXIT in
bash:

$ type exit
exit is a shell builtin
$ type EXIT
bash: type: EXIT: not found

So for me it does not seem to work:

$ trap 'err=$?; echo trap; EXIT $?' 0
$ exit 5
exit
trap
bash: EXIT: command not found
[1]    12906 exit 5     bash

^ permalink raw reply

* [RFC] GIT user survey
From: Paolo Ciarrocchi @ 2006-06-24 16:18 UTC (permalink / raw)
  To: Git Mailing List

Hi all,
I was wondering whether it could be a good idea to have a kind of "GIT
users survey" when google pointed my eyes to this page:
http://www.selenic.com/pipermail/mercurial/2006-April/007513.html

So I modified the content of the survey and published a DRAF here:
http://paolo.ciarrocchi.googlepages.com/GITSurvey

Here is the content of the proposed survey:

About you

    1. What country are you in?
    2. What is your preferred language?
    3. What's your gender?

Getting started with GIT

    1. How did you hear about GIT?
    2. Did you find GIT easy to learn?
    3. What helped you most in learning to use it?

How you use GIT

    1. Do you use GIT for work, unpaid projects, or both?
    2. How do you obtain GIT?  Source tarball, binary package, or
       pull the main repository?
    3. What platforms (hardware, OS, version) do you use GIT on?
    4. How many people do you collaborate with using GIT?
    5. How big are the repositories that you work on? (e.g. how many
       files, how much disk space)
    6. How many different projects do you manage using GIT?
    7. Which extensions/plugins do you use?

What you think of GIT

    1. Overall, how happy are you with GIT?
    2. How does GIT compare to other SCM tools you have used?
    3. What do you like about using GIT?
    4. What would you most like to see improved about GIT?
       (features, bugs, plugins, documentation, ...)
    5. If you want to see GIT more widely used, what do you
       think we could do to make this happen?

Documentation

    1. Do you use the GIT wiki?   If yes, do you find it useful?
    2. Do you find GIT's online help useful?
    3. What is your favourite user documentation for any software
       projects or products you have used?

Getting help, staying in touch

    1. Have you tried to get GIT help from other people?
          * If yes, did you get these problems resolved quickly and to
            your liking?
    2. Do you subscribe to the mailing list?
          * If yes, do you find it useful, and traffic levels OK?
    3. Do you use the IRC channel (#git on irc.freenode.net)?
          * If no, did you know that all of the core developers use
            IRC, and that there's almost 24-hour help available?

Open forum

    1. What other comments or suggestions do you have that are not
       covered by the questions above?


What do people living in this ML think about this suvery?
Do you have any suggestion?
Do you think it worth the effort?

Thanks in advace.

Regards,

-- 
Paolo
http://paolociarrocchi.googlepages.com
http://picasaweb.google.com/paolo.ciarrocchi

^ permalink raw reply

* [PATCH 2/2] Convert git-send-email to use Git.pm
From: Petr Baudis @ 2006-06-24 14:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624143421.13849.21667.stgit@machine.or.cz>

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

 git-send-email.perl |   30 ++++++++----------------------
 1 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index c5d9e73..e794e44 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -21,6 +21,7 @@ use warnings;
 use Term::ReadLine;
 use Getopt::Long;
 use Data::Dumper;
+use Git;
 
 # most mail servers generate the Date: header, but not all...
 $ENV{LC_ALL} = 'C';
@@ -46,6 +47,8 @@ my $smtp_server;
 # Example reply to:
 #$initial_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
 
+my $repo = Git->repository();
+
 my $term = new Term::ReadLine 'git-send-email';
 
 # Begin by accumulating all the variables (defined above), that we will end up
@@ -81,23 +84,9 @@ foreach my $entry (@bcclist) {
 
 # Now, let's fill any that aren't set in with defaults:
 
-sub gitvar {
-    my ($var) = @_;
-    my $fh;
-    my $pid = open($fh, '-|');
-    die "$!" unless defined $pid;
-    if (!$pid) {
-	exec('git-var', $var) or die "$!";
-    }
-    my ($val) = <$fh>;
-    close $fh or die "$!";
-    chomp($val);
-    return $val;
-}
-
 sub gitvar_ident {
     my ($name) = @_;
-    my $val = gitvar($name);
+    my $val = $repo->command('var', $name);
     my @field = split(/\s+/, $val);
     return join(' ', @field[0...(@field-3)]);
 }
@@ -106,8 +95,8 @@ my ($author) = gitvar_ident('GIT_AUTHOR_
 my ($committer) = gitvar_ident('GIT_COMMITTER_IDENT');
 
 my %aliases;
-chomp(my @alias_files = `git-repo-config --get-all sendemail.aliasesfile`);
-chomp(my $aliasfiletype = `git-repo-config sendemail.aliasfiletype`);
+my @alias_files = $repo->config('sendemail.aliasesfile');
+my $aliasfiletype = $repo->config('sendemail.aliasfiletype');
 my %parse_alias = (
 	# multiline formats can be supported in the future
 	mutt => sub { my $fh = shift; while (<$fh>) {
@@ -132,7 +121,7 @@ my %parse_alias = (
 		}}}
 );
 
-if (@alias_files && defined $parse_alias{$aliasfiletype}) {
+if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
 	foreach my $file (@alias_files) {
 		open my $fh, '<', $file or die "opening $file: $!\n";
 		$parse_alias{$aliasfiletype}->($fh);
@@ -374,10 +363,7 @@ sub send_message
 	my $date = strftime('%a, %d %b %Y %H:%M:%S %z', localtime($time++));
 	my $gitversion = '@@GIT_VERSION@@';
 	if ($gitversion =~ m/..GIT_VERSION../) {
-	    $gitversion = `git --version`;
-	    chomp $gitversion;
-	    # keep only what's after the last space
-	    $gitversion =~ s/^.* //;
+	    $gitversion = Git::version();
 	}
 
 	my $header = "From: $from

^ permalink raw reply related

* [PATCH 1/2] Git.pm: Add config() method
From: Petr Baudis @ 2006-06-24 14:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This accessor will retrieve value(s) of the given configuration variable.

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

 Documentation/git-repo-config.txt |    3 ++-
 perl/Git.pm                       |   37 ++++++++++++++++++++++++++++++++++++-
 repo-config.c                     |    2 +-
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-repo-config.txt b/Documentation/git-repo-config.txt
index 803c0d5..cc72fa9 100644
--- a/Documentation/git-repo-config.txt
+++ b/Documentation/git-repo-config.txt
@@ -54,7 +54,8 @@ OPTIONS
 
 --get::
 	Get the value for a given key (optionally filtered by a regex
-	matching the value).
+	matching the value). Returns error code 1 if the key was not
+	found and error code 2 if multiple key values were found.
 
 --get-all::
 	Like get, but does not fail if the number of values for the key
diff --git a/perl/Git.pm b/perl/Git.pm
index 7bbb5be..2e1241b 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -472,7 +472,6 @@ and the directory must exist.
 
 sub wc_chdir {
 	my ($self, $subdir) = @_;
-
 	$self->wc_path()
 		or throw Error::Simple("bare repository");
 
@@ -485,6 +484,42 @@ sub wc_chdir {
 }
 
 
+=item config ( VARIABLE )
+
+Retrieve the configuration C<VARIABLE> in the same manner as C<repo-config>
+does. In scalar context requires the variable to be set only one time
+(exception is thrown otherwise), in array context returns allows the
+variable to be set multiple times and returns all the values.
+
+Must be called on a repository instance.
+
+This currently wraps command('repo-config') so it is not so fast.
+
+=cut
+
+sub config {
+	my ($self, $var) = @_;
+	$self->repo_path()
+		or throw Error::Simple("not a repository");
+
+	try {
+		if (wantarray) {
+			return $self->command('repo-config', '--get-all', $var);
+		} else {
+			return $self->command_oneline('repo-config', '--get', $var);
+		}
+	} catch Git::Error::Command with {
+		my $E = shift;
+		if ($E->value() == 1) {
+			# Key not found.
+			return undef;
+		} else {
+			throw $E;
+		}
+	};
+}
+
+
 =item hash_object ( FILENAME [, TYPE ] )
 
 =item hash_object ( FILEHANDLE [, TYPE ] )
diff --git a/repo-config.c b/repo-config.c
index ab8f1af..346fb14 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -121,7 +121,7 @@ static int get_value(const char* key_, c
 	if (do_all)
 		ret = !seen;
 	else
-		ret =  (seen == 1) ? 0 : 1;
+		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
 
 free_strings:
 	if (repo_config)

^ permalink raw reply related

* [PATCH] Rename safe_strncpy() to strlcpy().
From: Peter Eriksen @ 2006-06-24 14:01 UTC (permalink / raw)
  To: git

This cleans up the use of safe_strncpy() even more.  Since it has the
same semantics as strlcpy() use this name instead.  Also move the
definition from inside path.c to its own file compat/strlcpy.c, and use
it conditionally at compile time, since some platforms already has
strlcpy().  It's included in the same way as compat/setenv.c.

Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>
---

I've introduced a NO_STRLCPY variable in the Makefile.  What do
you think about this?

I've made a qualified guess as to what systems have strlcpy() in
their standard library, but I am probably not completely right.

These are the ones I've said have it:
OpenBSD, FreeBSD, NetBSD

These I've said don't:
Linux, Darwin, SunOS, Cygwin, AIX, IRIX64

Regards,

Peter

 Makefile           |   14 ++++++++++++++
 builtin-log.c      |    2 +-
 builtin-tar-tree.c |    4 ++--
 cache.h            |    1 -
 compat/strlcpy.c   |   13 +++++++++++++
 config.c           |    6 +++---
 git-compat-util.h  |    5 +++++
 http-fetch.c       |    6 +++---
 http-push.c        |    6 +++---
 ident.c            |    4 ++--
 path.c             |   15 +--------------
 sha1_name.c        |    2 +-
 12 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/Makefile b/Makefile
index e29e3fa..cde619c 100644
--- a/Makefile
+++ b/Makefile
@@ -26,6 +26,8 @@ # d_type in struct dirent (latest Cygwin
 #
 # Define NO_STRCASESTR if you don't have strcasestr.
 #
+# Define NO_STRLCPY if you don't have strlcpy.
+#
 # Define NO_SETENV if you don't have setenv in the C library.
 #
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
@@ -240,9 +242,13 @@ # We choose to avoid "if .. else if .. e
 # because maintaining the nesting to match is a pain.  If
 # we had "elif" things would have been much nicer...
 
+ifeq ($(uname_S),Linux)
+	NO_STRLCPY = YesPlease
+endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
+	NO_STRLCPY = YesPlease
 	## fink
 	ifeq ($(shell test -d /sw/lib && echo y),y)
 		ALL_CFLAGS += -I/sw/include
@@ -259,6 +265,7 @@ ifeq ($(uname_S),SunOS)
 	NEEDS_NSL = YesPlease
 	SHELL_PATH = /bin/bash
 	NO_STRCASESTR = YesPlease
+	NO_STRLCPY = YesPlease
 	ifeq ($(uname_R),5.8)
 		NEEDS_LIBICONV = YesPlease
 		NO_UNSETENV = YesPlease
@@ -276,6 +283,7 @@ ifeq ($(uname_O),Cygwin)
 	NO_D_TYPE_IN_DIRENT = YesPlease
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
+	NO_STRLCPY = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	# There are conflicting reports about this.
@@ -305,12 +313,14 @@ ifeq ($(uname_S),NetBSD)
 endif
 ifeq ($(uname_S),AIX)
 	NO_STRCASESTR=YesPlease
+	NO_STRLCPY = YesPlease
 	NEEDS_LIBICONV=YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
 	NO_IPV6=YesPlease
 	NO_SETENV=YesPlease
 	NO_STRCASESTR=YesPlease
+	NO_STRLCPY = YesPlease
 	NO_SOCKADDR_STORAGE=YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
 	ALL_CFLAGS += -DPATH_MAX=1024
@@ -403,6 +413,10 @@ ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
 	COMPAT_OBJS += compat/strcasestr.o
 endif
+ifdef NO_STRLCPY
+	COMPAT_CFLAGS += -DNO_STRLCPY
+	COMPAT_OBJS += compat/strlcpy.o
+endif
 ifdef NO_SETENV
 	COMPAT_CFLAGS += -DNO_SETENV
 	COMPAT_OBJS += compat/setenv.o
diff --git a/builtin-log.c b/builtin-log.c
index 5a8a50b..44d2d13 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -115,7 +115,7 @@ static void reopen_stdout(struct commit 
 	int len = 0;
 
 	if (output_directory) {
-		safe_strncpy(filename, output_directory, 1010);
+		strlcpy(filename, output_directory, 1010);
 		len = strlen(filename);
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 39a61b6..f2e48aa 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -233,8 +233,8 @@ static void write_entry(const unsigned c
 	/* XXX: should we provide more meaningful info here? */
 	sprintf(header.uid, "%07o", 0);
 	sprintf(header.gid, "%07o", 0);
-	safe_strncpy(header.uname, "git", sizeof(header.uname));
-	safe_strncpy(header.gname, "git", sizeof(header.gname));
+	strlcpy(header.uname, "git", sizeof(header.uname));
+	strlcpy(header.gname, "git", sizeof(header.gname));
 	sprintf(header.devmajor, "%07o", 0);
 	sprintf(header.devminor, "%07o", 0);
 
diff --git a/cache.h b/cache.h
index efeafea..14358a8 100644
--- a/cache.h
+++ b/cache.h
@@ -216,7 +216,6 @@ enum sharedrepo {
 int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
-size_t safe_strncpy(char *, const char *, size_t);
 char *enter_repo(char *path, int strict);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
diff --git a/compat/strlcpy.c b/compat/strlcpy.c
new file mode 100644
index 0000000..b66856a
--- /dev/null
+++ b/compat/strlcpy.c
@@ -0,0 +1,13 @@
+#include <string.h>
+
+size_t gitstrlcpy(char *dest, const char *src, size_t size)
+{
+	size_t ret = strlen(src);
+
+	if (size) {
+		size_t len = (ret >= size) ? size - 1 : ret;
+		memcpy(dest, src, len);
+		dest[len] = '\0';
+	}
+	return ret;
+}
diff --git a/config.c b/config.c
index 3e077d4..ec44827 100644
--- a/config.c
+++ b/config.c
@@ -280,17 +280,17 @@ int git_default_config(const char *var, 
 	}
 
 	if (!strcmp(var, "user.name")) {
-		safe_strncpy(git_default_name, value, sizeof(git_default_name));
+		strlcpy(git_default_name, value, sizeof(git_default_name));
 		return 0;
 	}
 
 	if (!strcmp(var, "user.email")) {
-		safe_strncpy(git_default_email, value, sizeof(git_default_email));
+		strlcpy(git_default_email, value, sizeof(git_default_email));
 		return 0;
 	}
 
 	if (!strcmp(var, "i18n.commitencoding")) {
-		safe_strncpy(git_commit_encoding, value, sizeof(git_commit_encoding));
+		strlcpy(git_commit_encoding, value, sizeof(git_commit_encoding));
 		return 0;
 	}
 
diff --git a/git-compat-util.h b/git-compat-util.h
index b3d4cf5..93f5580 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -79,6 +79,11 @@ #define strcasestr gitstrcasestr
 extern char *gitstrcasestr(const char *haystack, const char *needle);
 #endif
 
+#ifdef NO_STRLCPY
+#define strlcpy gitstrlcpy
+extern size_t gitstrlcpy(char *, const char *, size_t);
+#endif
+
 static inline void *xmalloc(size_t size)
 {
 	void *ret = malloc(size);
diff --git a/http-fetch.c b/http-fetch.c
index 2b63d89..44eba5f 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -584,8 +584,8 @@ static void process_alternates_response(
 			// skip 'objects' at end
 			if (okay) {
 				target = xmalloc(serverlen + posn - i - 6);
-				safe_strncpy(target, base, serverlen);
-				safe_strncpy(target + serverlen, data + i, posn - i - 6);
+				strlcpy(target, base, serverlen);
+				strlcpy(target + serverlen, data + i, posn - i - 6);
 				if (get_verbosely)
 					fprintf(stderr,
 						"Also look at %s\n", target);
@@ -727,7 +727,7 @@ xml_cdata(void *userData, const XML_Char
 	if (ctx->cdata)
 		free(ctx->cdata);
 	ctx->cdata = xmalloc(len + 1);
-	safe_strncpy(ctx->cdata, s, len + 1);
+	strlcpy(ctx->cdata, s, len + 1);
 }
 
 static int remote_ls(struct alt_base *repo, const char *path, int flags,
diff --git a/http-push.c b/http-push.c
index 8d472f0..3c89a17 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1271,7 +1271,7 @@ xml_cdata(void *userData, const XML_Char
 	if (ctx->cdata)
 		free(ctx->cdata);
 	ctx->cdata = xmalloc(len + 1);
-	safe_strncpy(ctx->cdata, s, len + 1);
+	strlcpy(ctx->cdata, s, len + 1);
 }
 
 static struct remote_lock *lock_remote(char *path, long timeout)
@@ -1473,7 +1473,7 @@ static void process_ls_object(struct rem
 		return;
 	path += 8;
 	obj_hex = xmalloc(strlen(path));
-	safe_strncpy(obj_hex, path, 3);
+	strlcpy(obj_hex, path, 3);
 	strcpy(obj_hex + 2, path + 3);
 	one_remote_object(obj_hex);
 	free(obj_hex);
@@ -2172,7 +2172,7 @@ static void fetch_symref(char *path, cha
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
 	if (!strncmp((char *)buffer.buffer, "ref: ", 5)) {
 		*symref = xmalloc(buffer.posn - 5);
-		safe_strncpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);
+		strlcpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);
 	} else {
 		get_sha1_hex(buffer.buffer, sha1);
 	}
diff --git a/ident.c b/ident.c
index 7b44cbd..efec97f 100644
--- a/ident.c
+++ b/ident.c
@@ -71,9 +71,9 @@ int setup_ident(void)
 		len = strlen(git_default_email);
 		git_default_email[len++] = '.';
 		if (he && (domainname = strchr(he->h_name, '.')))
-			safe_strncpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);
+			strlcpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);
 		else
-			safe_strncpy(git_default_email + len, "(none)", sizeof(git_default_email) - len);
+			strlcpy(git_default_email + len, "(none)", sizeof(git_default_email) - len);
 	}
 	/* And set the default date */
 	datestamp(git_default_date, sizeof(git_default_date));
diff --git a/path.c b/path.c
index 36972fd..db8905f 100644
--- a/path.c
+++ b/path.c
@@ -77,25 +77,12 @@ int git_mkstemp(char *path, size_t len, 
 		pch += n;
 	}
 
-	safe_strncpy(pch, template, len);
+	strlcpy(pch, template, len);
 
 	return mkstemp(path);
 }
 
 
-size_t safe_strncpy(char *dest, const char *src, size_t size)
-{
-	size_t ret = strlen(src);
-
-	if (size) {
-		size_t len = (ret >= size) ? size - 1 : ret;
-		memcpy(dest, src, len);
-		dest[len] = '\0';
-	}
-	return ret;
-}
-
-
 int validate_symref(const char *path)
 {
 	struct stat st;
diff --git a/sha1_name.c b/sha1_name.c
index cd85d1f..f2cbafa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -262,7 +262,7 @@ static int get_sha1_basic(const char *st
 		if (str[am] == '@' && str[am+1] == '{' && str[len-1] == '}') {
 			int date_len = len - am - 3;
 			char *date_spec = xmalloc(date_len + 1);
-			safe_strncpy(date_spec, str + am + 2, date_len + 1);
+			strlcpy(date_spec, str + am + 2, date_len + 1);
 			at_time = approxidate(date_spec);
 			free(date_spec);
 			len = am;
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* Re: [PATCH 07/12] Git.pm: Better error handling
From: Petr Baudis @ 2006-06-24 13:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmzc3ymnu.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 10:37:25AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > +int
> > +error_xs(const char *err, va_list params)
> > +{
> 
> You said in git-compat-util.h that set_error_routine takes a
> function that returns void, so this gives unnecessary type
> clash.
> 
> --------------------------------
> In file included from /usr/lib/perl/5.8/CORE/perl.h:756,
>                  from Git.xs:15:
> /usr/lib/perl/5.8/CORE/embed.h:4193:1: warning: "die" redefined
> Git.xs:11:1: warning: this is the location of the previous definition
> Git.xs: In function 'boot_Git':
> Git.xs:57: warning: passing argument 1 of 'set_error_routine' from incompatible pointer type
> Git.xs:58: warning: passing argument 1 of 'set_die_routine' makes qualified function pointer from unqualified
> --------------------------------

Oh, I forgot to fix it in the .xs. :-(

> Other troubles I saw with the v4 series while compiling:
> 
> --------------------------------
> usage.c:35: warning: initialization makes qualified function pointer from unqualified
> usage.c:36: warning: initialization makes qualified function pointer from unqualified
> 
> I'd fix it with this
...

Ah, so THAT's where you put the NORETURN thing. ;-)

> --------------------------------
> 
> (cd perl && /usr/bin/perl Makefile.PL \
>                 PREFIX="/home/junio/git-test" \
>                 DEFINE="-O2 -Wall -Wdeclaration-after-statement
>                 -g -DSHA1_HEADER='<openssl/sha.h>'
>                 -DGIT_VERSION=\\\"1.4.1.rc1.gab0df\\\"" \
>                 LIBS="libgit.a xdiff/lib.a -lz  -lcrypto")
> Unrecognized argument in LIBS ignored: 'libgit.a'
> Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
> 
> Do you need to pass LIBS, and if so maybe this is not a way
> Makefile.PL expects it to be passed perhaps?

It is harmless, but this should fix it:

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

diff --git a/Makefile b/Makefile
index d614f18..91bef4e 100644
--- a/Makefile
+++ b/Makefile
@@ -230,7 +230,8 @@ BUILTIN_OBJS = \
 	builtin-update-ref.o
 
 GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
-LIBS = $(GITLIBS) -lz
+
+EXTLIBS = -lz
 
 #
 # Platform specific tweaks
@@ -380,14 +381,14 @@ ifdef NEEDS_LIBICONV
 	else
 		ICONV_LINK =
 	endif
-	LIBS += $(ICONV_LINK) -liconv
+	EXTLIBS += $(ICONV_LINK) -liconv
 endif
 ifdef NEEDS_SOCKET
-	LIBS += -lsocket
+	EXTLIBS += -lsocket
 	SIMPLE_LIB += -lsocket
 endif
 ifdef NEEDS_NSL
-	LIBS += -lnsl
+	EXTLIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
 ifdef NO_D_TYPE_IN_DIRENT
@@ -446,7 +447,7 @@ ifdef MOZILLA_SHA1
 	LIB_OBJS += mozilla-sha1/sha1.o
 else
 	SHA1_HEADER = <openssl/sha.h>
-	LIBS += $(LIB_4_CRYPTO)
+	EXTLIBS += $(LIB_4_CRYPTO)
 endif
 endif
 endif
@@ -469,9 +470,13 @@ PERL_PATH_SQ = $(subst ','\'',$(PERL_PAT
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 GIT_PYTHON_DIR_SQ = $(subst ','\'',$(GIT_PYTHON_DIR))
 
+LIBS = $(GITLIBS) $(EXTLIBS)
+
 ALL_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' $(COMPAT_CFLAGS)
 LIB_OBJS += $(COMPAT_OBJS)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
+
+
 ### Build rules
 
 all: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk
@@ -601,7 +606,7 @@ perl/Makefile:	perl/Git.pm perl/Makefile
 	(cd perl && $(PERL_PATH) Makefile.PL \
 		PREFIX="$(prefix)" \
 		DEFINE="$(ALL_CFLAGS) -DGIT_VERSION=\\\"$(GIT_VERSION)\\\"" \
-		LIBS="$(LIBS)")
+		LIBS="$(EXTLIBS)")
 
 doc:
 	$(MAKE) -C Documentation all

> --------------------------------
> Makefile out-of-date with respect to Makefile.PL
> Cleaning current config before rebuilding Makefile...
> make -f Makefile.old clean > /dev/null 2>&1
> /usr/bin/perl Makefile.PL "PREFIX=/home/junio/git-test" "DEFINE=-O2 -Wall -Wdeclaration-after-statement -g -DSHA1_HEADER='<openssl/sha.h>'  -DGIT_VERSION=\"1.4.1.rc1.gab0df\"" "LIBS=libgit.a xdiff/lib.a -lz  -lcrypto"
> Unrecognized argument in LIBS ignored: 'libgit.a'
> Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
> Writing Makefile for Git
> ==> Your Makefile has been rebuilt. <==
> ==> Please rerun the make command.  <==
> false
> make[1]: *** [Makefile] Error 1
> --------------------------------
> 
> The latter is what Perl's build mechanism does so it is not
> strictly your fault, but it nevertheless is irritating that we
> have to say make clean twice.

What about just this?

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

diff --git a/Makefile b/Makefile
index 91bef4e..55c07b6 100644
--- a/Makefile
+++ b/Makefile
@@ -731,7 +731,8 @@ clean:
 	rm -f $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	rm -f $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
-	[ ! -e perl/Makefile ] || $(MAKE) -C perl/ clean
+	# Try twice in case the Makefile has been rebuilt
+	[ ! -e perl/Makefile ] || $(MAKE) -C perl/ clean || $(MAKE) -C perl/ clean
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

^ permalink raw reply related

* Re: From b65bc21e7d8dc8cafc70dfa6354cb66b8874b2d9 Mon Sep 17 00:00:00 2001 [PATCH] Makefile: add framework to verify and bench sha1 implementations.
From: linux @ 2006-06-24  9:29 UTC (permalink / raw)
  To: git, junkio; +Cc: linux
In-Reply-To: <7v3bdv0xyd.fsf_-_@assigned-by-dhcp.cox.net>

Nice work, but I might point out that the original PPC SHA bug was hashing
more than 0.5G of contiguous data in a *single* call to SHA1_Update,
while your test program works with 8K buffers.

^ permalink raw reply

* Re: [PATCH] cvsimport: setup indexes correctly for ancestors and   incremental imports
From: Sergey Vlasov @ 2006-06-24 13:06 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git, junkio, Johannes.Schindelin
In-Reply-To: <11511475882820-git-send-email-martin@catalyst.net.nz>

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

On Sat, 24 Jun 2006 23:13:08 +1200 Martin Langhoff wrote:

> Two bugs had slipped in the "keep one index per branch during import"
> patch. Both incremental imports and new branches would see an
> empty tree for their initial commit. Now we cover all the relevant
> cases, checking whether we actually need to setup the index before
> preparing the actual commit, and doing it.
> 
> Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
> 
> ---
>  git-cvsimport.perl |   19 +++++++++++++++++--
>  1 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> old mode 100644
> new mode 100755
> index d961b7b..1c1fd02
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -813,11 +813,26 @@ while(<CVS>) {
>  			unless ($index{$branch}) {
>  			    $index{$branch} = tmpnam();
>  			    $ENV{GIT_INDEX_FILE} = $index{$branch};
> -			    system("git-read-tree", $branch);
> +			}
> +			if ($ancestor) {
> +			    system("git-read-tree", $ancestor);
>  			    die "read-tree failed: $?\n" if $?;
>  			} else {
> +			    unless ($index{$branch}) {

This seems to be dead code - even if $index{$branch} was not set, it
will be set inside "unless ($index{$branch})" above.  Or there is
another bug here?

> +				$index{$branch} = tmpnam();
> +				$ENV{GIT_INDEX_FILE} = $index{$branch};
> +				system("git-read-tree", $branch);
> +				die "read-tree failed: $?\n" if $?;
> +			    }
> +			}    
> +		} else {
> +			# just in case
> +			unless ($index{$branch}) {
> +			    $index{$branch} = tmpnam();
>  			    $ENV{GIT_INDEX_FILE} = $index{$branch};
> -		        }
> +			    system("git-read-tree", $branch);
> +			    die "read-tree failed: $?\n" if $?;
> +			}
>  		}
>  		$last_branch = $branch if $branch ne $last_branch;
>  		$state = 9;

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

^ permalink raw reply

* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Petr Baudis @ 2006-06-24 13:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vac82wytw.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 01:57:31PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > (This is also why I was a bit confused by your make test patch - it does
> > not "fix" anything per se since no tests directly use Git.pm.)
> 
> You are right.
> 
> You do not want to be testing installed version, but the one
> freshly built, so the patch does not have any effect, except for
> one case: testing before installing Git.pm for the first time
> anywhere yet.  -I prepends the directory to the search path, so
> we are not testing the freshly built copy at all.
> 
> Is there a way from the environment to override this behaviour,
> so that we can run the tests properly?  I think PERL5LIB and
> PERLLIB are defeated by having -I there (that's why I said I
> liked what Fredrik did with his Python script, which appends the
> final installed location to the search path).  I think unshift
> into @INC by hand (i.e. without even using use lib "$path")
> would do what we want, but I feel that is a bit too ugly just 
> for the testing X-<.

PERL5LIB and use lib at the same time works for me. Anyway, with the
second patch I've sent things should work well even if you don't have
Git.pm installed anywhere yet.

> diff --git a/perl/Makefile.PL b/perl/Makefile.PL
> index 54e8b20..92c140d 100644
> --- a/perl/Makefile.PL
> +++ b/perl/Makefile.PL
> @@ -3,7 +3,7 @@ use ExtUtils::MakeMaker;
>  sub MY::postamble {
>  	return <<'MAKE_FRAG';
>  instlibdir:
> -	@echo $(INSTALLSITELIB)
> +	@echo $(INSTALLSITEARCH)
>  
>  MAKE_FRAG
>  }

Oh, yes; that line came from the time when we had no .xs yet. It is not
visible here since both arch-specific and non-arch-specific libraries
get installed to ~/lib.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

^ permalink raw reply

* Re: x86 asm SHA1 (draft)
From: linux @ 2006-06-24  9:20 UTC (permalink / raw)
  To: git, junkio; +Cc: linux
In-Reply-To: <7vfyhv11ej.fsf@assigned-by-dhcp.cox.net>

> OK.  I somehow got an impression that your two versions had
> quite different performance characteristics on G4 and G5 and
> there was a real choice.  If they are between a few per-cent,
> then I agree it is not worth doing at all.

My apologies for being unclear.

The place where a noticeable (if not disastrous) difference can appear
is x86, which has a lot more models with "interesting" performance
characteristics.  In particular, Intel is fond of building CPUs with a
very small "sweet spot".

The openssl SHA1 code had to be reworked to not suck on a P4, with the
resultant performance change:

#               compared with original  compared with Intel cc
#               assembler impl.         generated code
# Pentium       -16%                    +48%
# PIII/AMD      +8%                     +16%
# P4            +85%(!)                 +45%

The original code had the most popular round (what I call
ROUND_MIX(F2,...))) implemented as follows, with single-uop
instructions (no load+op) scheduled for the Pentium pipeline:
(A..E are working variables, S and T are temps)

	movl    16(%esp),S	U  \
        movl    24(%esp),T	 V  \
        xorl    S,T		U    \
        movl    48(%esp),S	 V    > "MIX", pentium-optimized
        xorl    S,T		U    /
        movl    4(%esp),S	 V  /
        xorl    S,T		U  /
        movl	B,S		 V
	roll	$1,T		U	Rotate of mix (SHA0 -> SHA1 fix)
	xor	C,S		 V
	mov	T,16(%esp)	U	Store back W[i]
	xor	D,S		 V	Finish computing F(B,C,D) = B^C^D
	lea	K(T,E),E	U	Add K and W[i] to E
	mov	A,T		 V
	roll	$5,T		UV
	rorl	$1,B		U
	add	S,E		 V
	rorl	$1,B		U
	add	T,E		 V

While the P4-optimized version goes:
	movl	B,S
	movl	16(%esp),T
	rorl	$2,B
	xorl	24(%esp),T
	xorl	C,S
	xorl	48(%esp),T
	xorl	D,S		This is F(B,C,D) = B^C^D
	xorl	4(%esp),T
	roll	$1,T		Rotate of mix (SHA0 -> SHA1 fix)
	addl	S,E
	movl	T,16(%esp)
	movl	A,S
	roll	$5,S
	lea	K(E,T),E
	add	S,E

(The original code actually rotates the working variables around 6
registers, not 5, but I've rearranged the last couple of instructions
to rotate around 5.)

^ permalink raw reply

* Re: [Patch] trap: exit: invalid signal specification
From: S.Çağlar Onur @ 2006-06-24 12:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejxf5ktc.fsf@assigned-by-dhcp.cox.net>

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

Cumartesi 24 Haziran 2006 05:50 tarihinde, Junio C Hamano şunları yazmıştı: 
> I am not quite sure what to make out this...  Do you mean your
> shell does not like the command "exit" spelled in lowercase
> under Turkic locale?

Sorry to not clear enough previously, for Turkic locales (tr_TR, az_AZ etc.) 
upper(i) != I. More detailed analysis can be found at 
http://www.i18nguy.com/unicode/turkish-i18n.html, "Why Applications Fail With 
The Turkish Language" section. This is the main reason of this problem. 

According to its man page signals defined with uppercase letters but also 
different trap implementations may permit lowercase signal names as an 
extension. 

As an example bash (v. 3.1.17) permits lowercase signal names but it converts 
this lowercase signal names into uppercase ones while interpreting the 
script. But for our "Turkish has 4 letter "I"s" problem this convert to 
uppercase one process fails but for bash invalid signal names not be 
considered a syntax error and do not cause the shell to abort. 

Yours
-- 
S.Çağlar Onur <caglar@pardus.org.tr>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

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

^ permalink raw reply

* Ocupacion temporal para un trabajo a tiempo parcial
From: Diann @ 2006-06-25  0:36 UTC (permalink / raw)
  To: git

Hola!.

Este correo electrónico le muestra una oferta de trabajo, que podría ser interesante a usted.

Gerente financiero situado en su país! Trabajo en Internet con buen sueldo!
GoldLeader Inc. busca a personas enérgicas y responsables para completar el
puesto de encargado de deudores de media jornada.
Como encargado de deudores, usted será el responsable de procesar y facilitar
las transferencias de fondos iniciadas por nuestros clientes bajo la supervisión del gerente regional.

 Ofrecemos:

- Ventajas buenas (más de 1000 $ por semana);

- Contrato legal;

 Se precisa puntualidad, capacidades directivas y responsabilidad.

Usted también recibirá instrucciones detalladas para acciones
subsecuentes de nuestro gerente, con información sobre como recibir/transferir el dinero.

 
1. Ser capaz de comprobar su correo electrónico varias veces por día

2. Ser capaz de responder a correos electrónico inmediatamente

3. Ser capaz de trabajar horas extra si es necesario

4. Ser responsable y trabajador

5. Hablar ingles

6. Tener más de 21 años;

7. Debería tener una cuenta bancaria personal

Para información adicionales y preguntas sobre el puesto de trabajo,
por favor envíe sus datos de contacto a  career@goldleader.biz

NO SON VENTAS!!! NO SON LLAMADAS!!!
USTED NO NECESITA DINERO PARA COMENZAR!!!
 
Gracias por su atencion.
Con respeto,
Departamento de personal Goldleader Inc.
http://www.goldleader.biz

^ 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