Git development
 help / color / mirror / Atom feed
* Re: PPC SHA-1 Updates in "pu"
From: Junio C Hamano @ 2006-06-24 20:21 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Petr Baudis
In-Reply-To: <Pine.LNX.4.64.0606241147480.6483@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

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

Care to share problems with Pasky and I to take a look at,
please?

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

I agree.  I got really disgusted when I tentatively pulled the
current Git.xs into "next" and installed it for my own use to
notice that it broke git-fmt-merge-msg hence git-pull workflow.

Pasky, can we first iron out kinks in the build procedure and
installation before converting existing programs further?  The
things I worry about currently are:

 - the SITELIBARCH gotcha I sent you a message about (and you
   responded to it already -- was that an Acked-by?);

 - RPM target -- we probably acquired a new build-dependency in
   which case the .spec file needs to be updated;

 - Make sure Git.xs builds and installed result works fine on
   all platforms we care about, including Cygwin and other
   non-Linux boxes.

I'd even suggest we revert the changes to git-fmt-merge-msg to
keep it working for now, until the above worries are resolved.
Otherwise we cannot have it in "next" safely (and I REALLY _do_
want to have Git.pm infrastructure in "next" soonish).

We can start using Git.xs and friends in some _new_ ancillary
programs, once we solve building and installing problems for
everybody.  That way it would help us gain portability and
confidence without disrupting existing users.

^ permalink raw reply

* Re: PPC SHA-1 Updates in "pu"
From: Linus Torvalds @ 2006-06-24 20:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Petr Baudis
In-Reply-To: <7vhd2atid1.fsf@assigned-by-dhcp.cox.net>



On Sat, 24 Jun 2006, Junio C Hamano wrote:

> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > Also, "pu" in general is totally unusable. It doesn't even compile.
> 
> Care to share problems with Pasky and I to take a look at,
> please?

Don't everybody see them?

	In file included from Git.xs:8:
	../cache.h:6:10: error: #include expects "FILENAME" or <FILENAME>
	In file included from /usr/lib/perl5/5.8.8/ppc-linux-thread-multi/CORE/perl.h:756,
	                 from Git.xs:15:
	/usr/lib/perl5/5.8.8/ppc-linux-thread-multi/CORE/embed.h:4195: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
	make[1]: *** [Git.o] Error 1
	make[1]: Leaving directory `/home/torvalds/git/perl'
	make: *** [all] Error 2

how does that compile for anybody else, when -DSHA1_HEADER isn't set 
correctly? The quotes have gone missing:

	-DSHA1_HEADER='ppc/sha1.h' 

don't ask me why.

		Linus

^ permalink raw reply

* Re: [RFC] GIT user survey
From: Linus Torvalds @ 2006-06-24 20:52 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Randal L. Schwartz, Paolo Ciarrocchi, Git Mailing List
In-Reply-To: <20060624200401.GV21864@pasky.or.cz>



On Sat, 24 Jun 2006, Petr Baudis wrote:
> 
> It is also linked from the homepage, although not as prominently as it
> should be; it grew nicely over the time so it probably deserves a more
> visible link now. I will add it on the front page.

Pasky, the homepage seems a bit pointless. Each individual page is so 
small that splitting it up into six different pages is just 
counter-productive.

I'd almost suggest making it _one_ page, perhaps with some shortcuts 
within it (ie a "http://git.or.cz/index.html#tools" shortcut within the 
page instead of having a separate "http://git.or.cz/tools.html" page)

Hmm?

In contrast, the wiki frontpage actually works pretty well - it's got more 
of that kind of "multiple sub-headers all on the same page" kind of 
layout, with just extra _details_ behind the links.

I don't know about everybody else, but I get irritated at webpages that 
force me to just switch to another page to get any information. It's like 
how some web journalists split up a story over 20 pages, and each page is 
just a few paragraphs and some graphic (and the commercials, of course).

I'd much rather _scroll_ a bit, or use ^F to _search_, than have to click 
through links.

			Linus

^ permalink raw reply

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

Hi,

thank you very much for doing the extra step and using the original 
constant names. I appreciate that.

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

> @@ -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) {

Not that it matters, but this "else" could go. (Otherwise,  "--raw -p" 
would be the same as "--raw", right?)

>  		show_patch_diff(p, num_parent, dense, rev);
> -		return;
> -	default:
> -		return;
>  	}
>  }

> @@ -856,19 +846,18 @@ void diff_tree_combined(const unsigned c
> [...]
>  
> -		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');
>  	}

Just a remark: this hunk actually changes behaviour. "with_stat" meant 
that the stat was prepended before something like a patch, and therefore a 
separator was needed. If you pass only "--stat", the separator will be 
printed anyway now.

> 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))

The DIFF_FORMAT_NO_OUTPUT here makes no sense (if it was set, you unset it 
above).

> @@ -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;

<nit type=minor>
	This should be part of patch 1/7.
</nit>

> @@ -2041,55 +2028,61 @@ static void diff_summary(struct diff_fil
> [...]
>  
> -	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);

This is a very nice cleanup.

>  	}
> -	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);

Again, very nice.

>  		}
>  		show_stats(diffstat);
>  		free(diffstat);

Why not go the full nine yards, and make diffstat not a pointer, but the 
struct itself? You would avoid calloc()ing and free()ing. (Of course, 
instead of calloc()ing you have to memset() it to 0.)

> +	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);

Are we sure we do not want something like

	if (output_format / DIFF_FORMAT_DIFFSTAT > 1)
		/* output separator */

after each format (this example being after the diffstat), the condition 
being: if there is still an output format to come, add the separator?

All in all, I like this patch.

Ciao,
Dscho

^ permalink raw reply

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

On 6/24/06, Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:
>
> 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
>
> (...)
>
> What do people living in this ML think about this suvery?
> Do you have any suggestion?
> Do you think it worth the effort?

The results of the Mercurial survey have been posted there:
http://www.selenic.com/mercurial/wiki/index.cgi/UserSurvey

An interesting read.

^ permalink raw reply

* Re: [PATCH 2/7] Merge with_raw, with_stat and summary variables to output_format
From: Timo Hirvonen @ 2006-06-24 21:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: junkio, git
In-Reply-To: <Pine.LNX.4.63.0606242219320.29667@wbgn013.biozentrum.uni-wuerzburg.de>

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

> Hi,
> 
> thank you very much for doing the extra step and using the original 
> constant names. I appreciate that.
> 
> On Sat, 24 Jun 2006, Timo Hirvonen wrote:
> 
> > @@ -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) {
> 
> Not that it matters, but this "else" could go. (Otherwise,  "--raw -p" 
> would be the same as "--raw", right?)

Just tested, ./git log -p --raw displays both raw and patch.  I think it
works because I changed diff_tree_combined() to use show_raw_diff() and
show_patch_diff() directly.

It feels 'wrong' to check flags and then call a function which checks
the flags again.  This combined diff stuff is confusing.

> > @@ -856,19 +846,18 @@ void diff_tree_combined(const unsigned c
> > [...]
> >  
> > -		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');
> >  	}
> 
> Just a remark: this hunk actually changes behaviour. "with_stat" meant 
> that the stat was prepended before something like a patch, and therefore a 
> separator was needed. If you pass only "--stat", the separator will be 
> printed anyway now.

You are right, it now prints --- when it should print empty line.

> > +	int inter_name_termination = '\t';
> > +	int line_termination = options->line_termination;
> > +
> > +	if (!line_termination)
> > +		inter_name_termination = 0;
> 
> <nit type=minor>
> 	This should be part of patch 1/7.
> </nit>

That clean up was possible only after I made other changes to the code,
I think.  At least it wasn't obvious when I wrote 1/7.

> >  		show_stats(diffstat);
> >  		free(diffstat);
> 
> Why not go the full nine yards, and make diffstat not a pointer, but the 
> struct itself? You would avoid calloc()ing and free()ing. (Of course, 
> instead of calloc()ing you have to memset() it to 0.)

I was blind :)

> > +	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);
> 
> Are we sure we do not want something like
> 
> 	if (output_format / DIFF_FORMAT_DIFFSTAT > 1)
> 		/* output separator */
> 
> after each format (this example being after the diffstat), the condition 
> being: if there is still an output format to come, add the separator?

I'm not sure what you mean.

It outputs separator between (diffstat and/or summary) and patch.
There's no separator between diffstat and summary or raw and diffstat.
Should there be one?


Thanks for your comments.  Should I patch the patch or send a fixed one?
I'm currently too tired to write any code.

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

^ permalink raw reply

* Re: [RFC] GIT user survey
From: Petr Baudis @ 2006-06-24 22:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Randal L. Schwartz, Paolo Ciarrocchi, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606241344450.6483@g5.osdl.org>

Dear diary, on Sat, Jun 24, 2006 at 10:52:04PM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> said that...
> Pasky, the homepage seems a bit pointless. Each individual page is so 
> small that splitting it up into six different pages is just 
> counter-productive.

Wow, finally some more feedback on the homepage!

> I'd almost suggest making it _one_ page, perhaps with some shortcuts 
> within it (ie a "http://git.or.cz/index.html#tools" shortcut within the 
> page instead of having a separate "http://git.or.cz/tools.html" page)

Good point. When designing the multi-page layout I expected much more
stuff to end up on the homepage but it sort of didn't happen and now the
Wiki seems to work pretty well, so I have folded it all back to a single
page. I ain't no webdesign-ka but it could've turned out worse; as
usual, I'm taking patches.

The menubar should be now actually useful for quick navigation between
various Git-related resources (going to Git's gitweb using this path
should be much faster than over kernel.org, especially when gitweb.cgi
has its bad days over there).

> I don't know about everybody else, but I get irritated at webpages that 
> force me to just switch to another page to get any information. It's like 
> how some web journalists split up a story over 20 pages, and each page is 
> just a few paragraphs and some graphic (and the commercials, of course).

We could put up some commercials as well and use them for funding pizza
distributed in a round-robin fashion between the developers.

-- 
				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: [RFC] GIT user survey
From: Matthias Kestenholz @ 2006-06-24 22:15 UTC (permalink / raw)
  To: git
In-Reply-To: <94fc236b0606241455m22c4d285led04846a915267a2@mail.gmail.com>

* Adrien Beau (adrienbeau@gmail.com) wrote:
> 
> The results of the Mercurial survey have been posted there:
> http://www.selenic.com/mercurial/wiki/index.cgi/UserSurvey
> 
> An interesting read.

I find the answers to the question, what people most like to see
improved interesting: The improvement which got mentioned most often
was "merge across rename", something which git does already.

It seems, that partial checkouts and truncated history are the
only things left to implement for git from this list.

I am looking forward to a git user survey!

	Matthias

^ permalink raw reply

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

Hi,

On Sun, 25 Jun 2006, Timo Hirvonen wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > On Sat, 24 Jun 2006, Timo Hirvonen wrote:
> > 
> > > @@ -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) {
> > 
> > Not that it matters, but this "else" could go. (Otherwise,  "--raw -p" 
> > would be the same as "--raw", right?)
> 
> Just tested, ./git log -p --raw displays both raw and patch.  I think it
> works because I changed diff_tree_combined() to use show_raw_diff() and
> show_patch_diff() directly.
> 
> It feels 'wrong' to check flags and then call a function which checks
> the flags again.  This combined diff stuff is confusing.

Sorry for not checking the result, but just the patch. I also find this 
behaviour confusing. Junio?

> > > +	int inter_name_termination = '\t';
> > > +	int line_termination = options->line_termination;
> > > +
> > > +	if (!line_termination)
> > > +		inter_name_termination = 0;
> > 
> > <nit type=minor>
> > 	This should be part of patch 1/7.
> > </nit>
> 
> That clean up was possible only after I made other changes to the code,
> I think.  At least it wasn't obvious when I wrote 1/7.

It is just a minor nit, I do not think it is necessary to change the 
patch.

> > >  		show_stats(diffstat);
> > >  		free(diffstat);
> > 
> > Why not go the full nine yards, and make diffstat not a pointer, but the 
> > struct itself? You would avoid calloc()ing and free()ing. (Of course, 
> > instead of calloc()ing you have to memset() it to 0.)
> 
> I was blind :)

;-) In my experience, after staring at the code too long, you turn blind. 
This is why I like a second pair of eyeballs so much.

> > > +	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);
> > 
> > Are we sure we do not want something like
> > 
> > 	if (output_format / DIFF_FORMAT_DIFFSTAT > 1)
> > 		/* output separator */
> > 
> > after each format (this example being after the diffstat), the condition 
> > being: if there is still an output format to come, add the separator?
> 
> I'm not sure what you mean.
> 
> It outputs separator between (diffstat and/or summary) and patch.
> There's no separator between diffstat and summary or raw and diffstat.
> Should there be one?

IMHO there should be one.

> Thanks for your comments.  Should I patch the patch or send a fixed one?

I cannot speak for Junio, but I think an additional patch to clean things 
up would be the way to go.

Ciao,
Dscho

^ permalink raw reply

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

Paolo,

I've seen in the irc logs that you were wondering whether we could a
web-based survey tool. Perhaps I can setup Moodle with an
easy-to-fillout survey. Interested?


martin

^ permalink raw reply

* Re: PPC SHA-1 Updates in "pu"
From: Junio C Hamano @ 2006-06-24 23:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Petr Baudis
In-Reply-To: <Pine.LNX.4.64.0606241338370.6483@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Don't everybody see them?

No.  But I see it now.

> how does that compile for anybody else, when -DSHA1_HEADER isn't set 
> correctly? The quotes have gone missing:
>
> 	-DSHA1_HEADER='ppc/sha1.h' 
>
> don't ask me why.

That is, as usual, caused by careless shell quoting.

        : gitster; PPC_SHA1=YesPlease Meta/Make perl/Makefile
        GIT_VERSION = 1.4.1.rc1.g9adbe
        (cd perl && /usr/bin/perl Makefile.PL \
                        PREFIX="/home/junio/git-test" \
                        DEFINE="-O2 -Wall -Wdeclaration-after-statement
                        -g -fPIC -DSHA1_HEADER='"ppc/sha1.h"'
                        -DGIT_VERSION=\\\"1.4.1.rc1.g9adbe\\\"" \
                        LIBS="libgit.a xdiff/lib.a -lz")
        Unrecognized argument in LIBS ignored: 'libgit.a'
        Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
        Writing Makefile for Git

All options but to use OpenSSL SHA-1 implementation share the
same problem (Meta/Make is a thin "make" wrapper to give USE_PIC
and other C compilation flags):

        : gitster; ARM_SHA1=YesPlease Meta/Make perl/Makefile
        (cd perl && /usr/bin/perl Makefile.PL \
                        PREFIX="/home/junio/git-test" \
                        DEFINE="-O2 -Wall -Wdeclaration-after-statement
                        -g -fPIC -DSHA1_HEADER='"arm/sha1.h"'
                        -DGIT_VERSION=\\\"1.4.1.rc1.g9adbe\\\"" \
                        LIBS="libgit.a xdiff/lib.a -lz")
        Unrecognized argument in LIBS ignored: 'libgit.a'
        Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
        Writing Makefile for Git

        : gitster; MOZILLA_SHA1=YesPlease Meta/Make perl/Makefile
        (cd perl && /usr/bin/perl Makefile.PL \
                        PREFIX="/home/junio/git-test" \
                        DEFINE="-O2 -Wall -Wdeclaration-after-statement
                        -g -fPIC -DSHA1_HEADER='"mozilla-sha1/sha1.h"'
                        -DGIT_VERSION=\\\"1.4.1.rc1.g9adbe\\\"" \
                        LIBS="libgit.a xdiff/lib.a -lz")
        Unrecognized argument in LIBS ignored: 'libgit.a'
        Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
        Writing Makefile for Git

^ permalink raw reply

* Re: PPC SHA-1 Updates in "pu"
From: Petr Baudis @ 2006-06-25  1:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vd5cyt8a3.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sun, Jun 25, 2006 at 01:59:16AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > how does that compile for anybody else, when -DSHA1_HEADER isn't set 
> > correctly? The quotes have gone missing:
> >
> > 	-DSHA1_HEADER='ppc/sha1.h' 
> >
> > don't ask me why.
> 
> That is, as usual, caused by careless shell quoting.
> 
>         : gitster; PPC_SHA1=YesPlease Meta/Make perl/Makefile
>         GIT_VERSION = 1.4.1.rc1.g9adbe
>         (cd perl && /usr/bin/perl Makefile.PL \
>                         PREFIX="/home/junio/git-test" \
>                         DEFINE="-O2 -Wall -Wdeclaration-after-statement
>                         -g -fPIC -DSHA1_HEADER='"ppc/sha1.h"'
>                         -DGIT_VERSION=\\\"1.4.1.rc1.g9adbe\\\"" \
>                         LIBS="libgit.a xdiff/lib.a -lz")
>         Unrecognized argument in LIBS ignored: 'libgit.a'
>         Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
>         Writing Makefile for Git

Oops...

----

Git.pm build: Fix $DEFINE quoting and missing GIT-CFLAGS dependency

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

diff --git a/Makefile b/Makefile
index 9a59466..64375f6 100644
--- a/Makefile
+++ b/Makefile
@@ -608,10 +608,12 @@ XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare
 	rm -f $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
 
-perl/Makefile:	perl/Git.pm perl/Makefile.PL
+PERL_DEFINES = $(ALL_CFLAGS) -DGIT_VERSION='"$(GIT_VERSION)"'
+PERL_DEFINES_SQ = $(subst ','\'',$(PERL_DEFINES))
+perl/Makefile:	perl/Git.pm perl/Makefile.PL GIT-CFLAGS
 	(cd perl && $(PERL_PATH) Makefile.PL \
 		PREFIX="$(prefix)" \
-		DEFINE="$(ALL_CFLAGS) -DGIT_VERSION=\\\"$(GIT_VERSION)\\\"" \
+		DEFINE='$(PERL_DEFINES_SQ)' \
 		LIBS="$(EXTLIBS)")
 
 doc:

-- 
				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: [PATCH 07/12] Git.pm: Better error handling
From: Petr Baudis @ 2006-06-25  1:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624131731.GU21864@pasky.or.cz>

Dear diary, on Sat, Jun 24, 2006 at 03:17:31PM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> 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. :-(

---
[PATCH] Git.pm: Squash some annoying warnings in Git.xs

From: Petr Baudis <pasky@suse.cz>

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

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

diff --git a/perl/Git.xs b/perl/Git.xs
index e841e4a..d7a2b75 100644
--- a/perl/Git.xs
+++ b/perl/Git.xs
@@ -29,21 +29,18 @@ report_xs(const char *prefix, const char
 	return buf;
 }
 
-void
+void NORETURN
 die_xs(const char *err, va_list params)
 {
-	char *str;
-	str = report_xs("fatal: ", err, params);
+	char *str = report_xs("fatal: ", err, params);
 	croak(str);
 }
 
-int
+void
 error_xs(const char *err, va_list params)
 {
-	char *str;
-	str = report_xs("error: ", err, params);
+	char *str = report_xs("error: ", err, params);
 	warn(str);
-	return -1;
 }
 
 

-- 
				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: [PATCH] Introduce Git.pm (v3)
From: Junio C Hamano @ 2006-06-25  1:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606242207510.29667@wbgn013.biozentrum.uni-wuerzburg.de>

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

> My original idea: on a machine where you have no accurate diff, you at 
> least want to pass the tests, and you want to ensure you can apply a diff 
> you generated on that machine.

I remember that, but I think recently we converted t4100 and
t4101 to use pregenerated test vectors so it might not be an
issue anymore?

> This patch is just compile tested, but obviously correct 

Looks sane, thanks.

I would maybe rename the option to --inaccurate-eof and default
it to off (i.e. no --accurate-eof option).  After all we are not
talking about arbitrary inaccuracy but the particular botch of
not having "\No newline at the end of file."

^ permalink raw reply

* Re: PPC SHA-1 Updates in "pu"
From: Petr Baudis @ 2006-06-25  1:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vhd2atid1.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 10:21:30PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Pasky, can we first iron out kinks in the build procedure and
> installation before converting existing programs further?

Sure.

> The things I worry about currently are:
> 
>  - the SITELIBARCH gotcha I sent you a message about (and you
>    responded to it already -- was that an Acked-by?);

Yes. :-)

>  - RPM target -- we probably acquired a new build-dependency in
>    which case the .spec file needs to be updated;

Well, perl is currently not listed even as a runtime dependency,
so does it really need to be listed as a build dependency?

>  - Make sure Git.xs builds and installed result works fine on
>    all platforms we care about, including Cygwin and other
>    non-Linux boxes.

Unfortunately I don't have access to a lot of those. :-(

> I'd even suggest we revert the changes to git-fmt-merge-msg to
> keep it working for now, until the above worries are resolved.
> Otherwise we cannot have it in "next" safely (and I REALLY _do_
> want to have Git.pm infrastructure in "next" soonish).

Yes, that sounds reasonable.

> We can start using Git.xs and friends in some _new_ ancillary
> programs, once we solve building and installing problems for
> everybody.  That way it would help us gain portability and
> confidence without disrupting existing users.

Well, I don't think it's very likely that Git.pm per se would be buggy
on a certain specific platform - it should either work as well as
everywhere else or not build at all, in which case you have disrupted
the existing users anyway. :-) (But without disrupting anyone we won't
get any bugreports and never get it fixed.)

Perhaps other converted perl scripts can linger at least on the pu
branch?

-- 
				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

* [PATCH 1/3] rebase: allow --merge option to handle patches merged upstream
From: Eric Wong @ 2006-06-25  1:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <20060622110941.GA32261@hand.yhbt.net>

Enhance t3401-rebase-partial to test with --merge as well as
the standard am -3 strategy.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-rebase.sh             |   15 +++++++++++----
 t/t3401-rebase-partial.sh |   13 ++++++++++++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 9159477..53fb14e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -82,7 +82,10 @@ call_merge () {
 	rv=$?
 	case "$rv" in
 	0)
-		git-commit -C "$cmt" || die "commit failed: $MRESOLVEMSG"
+		if test -n "`git-diff-index HEAD`"
+		then
+			git-commit -C "$cmt" || die "commit failed: $MRESOLVEMSG"
+		fi
 		;;
 	1)
 		test -d "$GIT_DIR/rr-cache" && git-rerere
@@ -110,9 +113,13 @@ finish_rb_merge () {
 	do
 		git-read-tree `cat "$dotest/cmt.$msgnum.result"`
 		git-checkout-index -q -f -u -a
-		git-commit -C "`cat $dotest/cmt.$msgnum`"
-
-		printf "Committed %0${prec}d" $msgnum
+		if test -n "`git-diff-index HEAD`"
+		then
+			git-commit -C "`cat $dotest/cmt.$msgnum`"
+			printf "Committed %0${prec}d" $msgnum
+		else
+			printf "Already applied: %0${prec}d" $msgnum
+		fi
 		echo ' '`git-rev-list --pretty=oneline -1 HEAD | \
 					sed 's/^[a-f0-9]\+ //'`
 		msgnum=$(($msgnum + 1))
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 32dc9c5..360a670 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -37,7 +37,9 @@ test_expect_success \
 test_expect_success \
     'pick top patch from topic branch into master' \
     'git-cherry-pick my-topic-branch^0 &&
-     git-checkout -f my-topic-branch
+     git-checkout -f my-topic-branch &&
+     git-branch master-merge master &&
+     git-branch my-topic-branch-merge my-topic-branch
 '
 
 test_debug \
@@ -50,4 +52,13 @@ test_expect_success \
     'rebase topic branch against new master and check git-am did not get halted' \
     'git-rebase master && test ! -d .dotest'
 
+if test -z "$no_python"
+then
+    test_expect_success \
+	'rebase --merge topic branch that was partially merged upstream' \
+	'git-checkout -f my-topic-branch-merge &&
+	 git-rebase --merge master-merge &&
+	 test ! -d .git/.dotest-merge'
+fi
+
 test_done
-- 
1.4.0.g937a

^ permalink raw reply related

* [PATCH 0/3] rebase --merge improvements and fixes
From: Eric Wong @ 2006-06-25  1:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060622110941.GA32261@hand.yhbt.net>

These patches should address the current issues with rebase --merge
usage.  It can now do everything the original format-patch | am -3
strategy including --skip and detection of merged commits by upstream.

-- 
Eric Wong

^ permalink raw reply

* [PATCH 2/3] rebase: cleanup rebasing with --merge
From: Eric Wong @ 2006-06-25  1:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <20060622110941.GA32261@hand.yhbt.net>

We no longer have to recommit each patch to remove the parent
information we're rebasing since we're using the low-level merge
strategies directly instead of git-merge.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-rebase.sh |   33 +++++----------------------------
 1 files changed, 5 insertions(+), 28 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 53fb14e..a95ada6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -59,15 +59,16 @@ continue_merge () {
 
 	if test -n "`git-diff-index HEAD`"
 	then
+		printf "Committed: %0${prec}d" $msgnum
 		git-commit -C "`cat $dotest/current`"
 	else
-		echo "Previous merge succeeded automatically"
+		printf "Already applied: %0${prec}d" $msgnum
 	fi
+	echo ' '`git-rev-list --pretty=oneline -1 HEAD | \
+				sed 's/^[a-f0-9]\+ //'`
 
 	prev_head=`git-rev-parse HEAD^0`
-
 	# save the resulting commit so we can read-tree on it later
-	echo "$prev_head" > "$dotest/cmt.$msgnum.result"
 	echo "$prev_head" > "$dotest/prev_head"
 
 	# onto the next patch:
@@ -82,10 +83,7 @@ call_merge () {
 	rv=$?
 	case "$rv" in
 	0)
-		if test -n "`git-diff-index HEAD`"
-		then
-			git-commit -C "$cmt" || die "commit failed: $MRESOLVEMSG"
-		fi
+		return
 		;;
 	1)
 		test -d "$GIT_DIR/rr-cache" && git-rerere
@@ -103,27 +101,6 @@ call_merge () {
 }
 
 finish_rb_merge () {
-	set -e
-
-	msgnum=1
-	echo "Finalizing rebased commits..."
-	git-reset --hard "`cat $dotest/onto`"
-	end="`cat $dotest/end`"
-	while test "$msgnum" -le "$end"
-	do
-		git-read-tree `cat "$dotest/cmt.$msgnum.result"`
-		git-checkout-index -q -f -u -a
-		if test -n "`git-diff-index HEAD`"
-		then
-			git-commit -C "`cat $dotest/cmt.$msgnum`"
-			printf "Committed %0${prec}d" $msgnum
-		else
-			printf "Already applied: %0${prec}d" $msgnum
-		fi
-		echo ' '`git-rev-list --pretty=oneline -1 HEAD | \
-					sed 's/^[a-f0-9]\+ //'`
-		msgnum=$(($msgnum + 1))
-	done
 	rm -r "$dotest"
 	echo "All done."
 }
-- 
1.4.0.g937a

^ permalink raw reply related

* [PATCH 3/3] rebase: allow --skip to work with --merge
From: Eric Wong @ 2006-06-25  1:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <20060622110941.GA32261@hand.yhbt.net>

Now that we control the merge base selection, we won't be forced
into rolling things in that we wanted to skip beforehand.

Also, add a test to ensure this all works as intended.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-rebase.txt |    1 -
 git-rebase.sh                |   13 ++++++++-
 t/t3403-rebase-skip.sh       |   61 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index c339c45..9d7bcaa 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -108,7 +108,6 @@ OPTIONS
 
 --skip::
 	Restart the rebasing process by skipping the current patch.
-	This does not work with the --merge option.
 
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/git-rebase.sh b/git-rebase.sh
index a95ada6..9ad1c44 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -137,7 +137,18 @@ do
 	--skip)
 		if test -d "$dotest"
 		then
-			die "--skip is not supported when using --merge"
+			prev_head="`cat $dotest/prev_head`"
+			end="`cat $dotest/end`"
+			msgnum="`cat $dotest/msgnum`"
+			msgnum=$(($msgnum + 1))
+			onto="`cat $dotest/onto`"
+			while test "$msgnum" -le "$end"
+			do
+				call_merge "$msgnum"
+				continue_merge
+			done
+			finish_rb_merge
+			exit
 		fi
 		git am -3 --skip --resolvemsg="$RESOLVEMSG"
 		exit
diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
new file mode 100755
index 0000000..8ab63c5
--- /dev/null
+++ b/t/t3403-rebase-skip.sh
@@ -0,0 +1,61 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Eric Wong
+#
+
+test_description='git rebase --merge --skip tests'
+
+. ./test-lib.sh
+
+# we assume the default git-am -3 --skip strategy is tested independently
+# and always works :)
+
+if test "$no_python"; then
+	echo "Skipping: no python => no recursive merge"
+	test_done
+	exit 0
+fi
+
+test_expect_success setup '
+	echo hello > hello &&
+	git add hello &&
+	git commit -m "hello" &&
+	git branch skip-reference &&
+
+	echo world >> hello &&
+	git commit -a -m "hello world" &&
+	echo goodbye >> hello &&
+	git commit -a -m "goodbye" &&
+
+	git checkout -f skip-reference &&
+	echo moo > hello &&
+	git commit -a -m "we should skip this" &&
+	echo moo > cow &&
+	git add cow &&
+	git commit -m "this should not be skipped" &&
+	git branch pre-rebase skip-reference &&
+	git branch skip-merge skip-reference
+	'
+
+test_expect_failure 'rebase with git am -3 (default)' 'git rebase master'
+
+test_expect_success 'rebase --skip with am -3' '
+	git reset --hard HEAD &&
+	git rebase --skip
+	'
+test_expect_success 'checkout skip-merge' 'git checkout -f skip-merge'
+
+test_expect_failure 'rebase with --merge' 'git rebase --merge master'
+
+test_expect_success 'rebase --skip with --merge' '
+	git reset --hard HEAD &&
+	git rebase --skip
+	'
+
+test_expect_success 'merge and reference trees equal' \
+	'test -z "`git-diff-tree skip-merge skip-reference`"'
+
+test_debug 'gitk --all & sleep 1'
+
+test_done
+
-- 
1.4.0.g937a

^ permalink raw reply related

* Re: [PATCH 07/12] Git.pm: Better error handling
From: Junio C Hamano @ 2006-06-25  1:30 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Linus Torvalds
In-Reply-To: <20060624131731.GU21864@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

>> --------------------------------
>> 
>> (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>

Applied the "EXTLIBS" change in the message.  I did the same
"clean twice" change last night.  Also I will apply the
"PERL_DEFINES_SQ" change as well.  Hopefully this would make
things buildable for more people.

Thanks.

^ permalink raw reply

* [PATCH] Git.pm build: Fix quoting and missing GIT-CFLAGS dependency
From: Petr Baudis @ 2006-06-25  1:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <20060625010202.GX21864@pasky.or.cz>

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

  This one should do a better job; if we quote, let's do it proper. :-)

 Makefile |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 9a59466..fb9ffad 100644
--- a/Makefile
+++ b/Makefile
@@ -608,11 +608,15 @@ XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare
 	rm -f $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
 
-perl/Makefile:	perl/Git.pm perl/Makefile.PL
+PERL_DEFINE = $(ALL_CFLAGS) -DGIT_VERSION='"$(GIT_VERSION)"'
+PERL_DEFINE_SQ = $(subst ','\'',$(PERL_DEFINE))
+PERL_LIBS = $(EXTLIBS)
+PERL_LIBS_SQ = $(subst ','\'',$(PERL_LIBS))
+perl/Makefile:	perl/Git.pm perl/Makefile.PL GIT-CFLAGS
 	(cd perl && $(PERL_PATH) Makefile.PL \
-		PREFIX="$(prefix)" \
-		DEFINE="$(ALL_CFLAGS) -DGIT_VERSION=\\\"$(GIT_VERSION)\\\"" \
-		LIBS="$(EXTLIBS)")
+		PREFIX='$(prefix_SQ)' \
+		DEFINE='$(PERL_DEFINE_SQ)' \
+		LIBS='$(PERL_LIBS)')
 
 doc:
 	$(MAKE) -C Documentation all

-- 
				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

* [PATCH] Git.pm: Support for perl/ being built by a different compiler
From: Petr Baudis @ 2006-06-25  1:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

dst_ on #git reported that on Solaris 9, Perl was built by Sun CC
and perl/ is therefore being built with it as well, while the rest
of Git is built with gcc. The problem (the first one visible, anyway)
is that we passed perl/ even various gcc-specific options. This
separates those to a special variable.

This is not really meant for an application yet since it's not clear
if it will alone help anything.

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

 Makefile |   66 ++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/Makefile b/Makefile
index 4f0a501..6755f26 100644
--- a/Makefile
+++ b/Makefile
@@ -115,6 +115,11 @@ SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powe
 
 ### --- END CONFIGURATION SECTION ---
 
+# Those must not be GNU-specific; they are shared with perl/ which may
+# be built by a different compiler.
+BASIC_CFLAGS =
+BASIC_LDFLAGS =
+
 SCRIPT_SH = \
 	git-bisect.sh git-branch.sh git-checkout.sh \
 	git-cherry.sh git-clean.sh git-clone.sh git-commit.sh \
@@ -249,13 +254,13 @@ ifeq ($(uname_S),Darwin)
 	NEEDS_LIBICONV = YesPlease
 	## fink
 	ifeq ($(shell test -d /sw/lib && echo y),y)
-		ALL_CFLAGS += -I/sw/include
-		ALL_LDFLAGS += -L/sw/lib
+		BASIC_CFLAGS += -I/sw/include
+		BASIC_LDFLAGS += -L/sw/lib
 	endif
 	## darwinports
 	ifeq ($(shell test -d /opt/local/lib && echo y),y)
-		ALL_CFLAGS += -I/opt/local/include
-		ALL_LDFLAGS += -L/opt/local/lib
+		BASIC_CFLAGS += -I/opt/local/include
+		BASIC_LDFLAGS += -L/opt/local/lib
 	endif
 endif
 ifeq ($(uname_S),SunOS)
@@ -274,7 +279,7 @@ ifeq ($(uname_S),SunOS)
 	endif
 	INSTALL = ginstall
 	TAR = gtar
-	ALL_CFLAGS += -D__EXTENSIONS__
+	BASIC_CFLAGS += -D__EXTENSIONS__
 endif
 ifeq ($(uname_O),Cygwin)
 	NO_D_TYPE_IN_DIRENT = YesPlease
@@ -291,21 +296,22 @@ ifeq ($(uname_O),Cygwin)
 endif
 ifeq ($(uname_S),FreeBSD)
 	NEEDS_LIBICONV = YesPlease
-	ALL_CFLAGS += -I/usr/local/include
-	ALL_LDFLAGS += -L/usr/local/lib
+	BASIC_CFLAGS += -I/usr/local/include
+	BASIC_LDFLAGS += -L/usr/local/lib
 endif
 ifeq ($(uname_S),OpenBSD)
 	NO_STRCASESTR = YesPlease
 	NEEDS_LIBICONV = YesPlease
-	ALL_CFLAGS += -I/usr/local/include
-	ALL_LDFLAGS += -L/usr/local/lib
+	BASIC_CFLAGS += -I/usr/local/include
+	BASIC_LDFLAGS += -L/usr/local/lib
 endif
 ifeq ($(uname_S),NetBSD)
 	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
 		NEEDS_LIBICONV = YesPlease
 	endif
-	ALL_CFLAGS += -I/usr/pkg/include
-	ALL_LDFLAGS += -L/usr/pkg/lib -Wl,-rpath,/usr/pkg/lib
+	BASIC_CFLAGS += -I/usr/pkg/include
+	BASIC_LDFLAGS += -L/usr/pkg/lib
+	ALL_LDFLAGS += -Wl,-rpath,/usr/pkg/lib
 endif
 ifeq ($(uname_S),AIX)
 	NO_STRCASESTR=YesPlease
@@ -317,9 +323,9 @@ ifeq ($(uname_S),IRIX64)
 	NO_STRCASESTR=YesPlease
 	NO_SOCKADDR_STORAGE=YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
-	ALL_CFLAGS += -DPATH_MAX=1024
+	BASIC_CFLAGS += -DPATH_MAX=1024
 	# for now, build 32-bit version
-	ALL_LDFLAGS += -L/usr/lib32
+	BASIC_LDFLAGS += -L/usr/lib32
 endif
 ifneq (,$(findstring arm,$(uname_M)))
 	ARM_SHA1 = YesPlease
@@ -340,7 +346,7 @@ endif
 ifndef NO_CURL
 	ifdef CURLDIR
 		# This is still problematic -- gcc does not always want -R.
-		ALL_CFLAGS += -I$(CURLDIR)/include
+		BASIC_CFLAGS += -I$(CURLDIR)/include
 		CURL_LIBCURL = -L$(CURLDIR)/lib -R$(CURLDIR)/lib -lcurl
 	else
 		CURL_LIBCURL = -lcurl
@@ -361,13 +367,13 @@ ifndef NO_OPENSSL
 	OPENSSL_LIBSSL = -lssl
 	ifdef OPENSSLDIR
 		# Again this may be problematic -- gcc does not always want -R.
-		ALL_CFLAGS += -I$(OPENSSLDIR)/include
+		BASIC_CFLAGS += -I$(OPENSSLDIR)/include
 		OPENSSL_LINK = -L$(OPENSSLDIR)/lib -R$(OPENSSLDIR)/lib
 	else
 		OPENSSL_LINK =
 	endif
 else
-	ALL_CFLAGS += -DNO_OPENSSL
+	BASIC_CFLAGS += -DNO_OPENSSL
 	MOZILLA_SHA1 = 1
 	OPENSSL_LIBSSL =
 endif
@@ -379,7 +385,7 @@ endif
 ifdef NEEDS_LIBICONV
 	ifdef ICONVDIR
 		# Again this may be problematic -- gcc does not always want -R.
-		ALL_CFLAGS += -I$(ICONVDIR)/include
+		BASIC_CFLAGS += -I$(ICONVDIR)/include
 		ICONV_LINK = -L$(ICONVDIR)/lib -R$(ICONVDIR)/lib
 	else
 		ICONV_LINK =
@@ -395,13 +401,13 @@ ifdef NEEDS_NSL
 	SIMPLE_LIB += -lnsl
 endif
 ifdef NO_D_TYPE_IN_DIRENT
-	ALL_CFLAGS += -DNO_D_TYPE_IN_DIRENT
+	BASIC_CFLAGS += -DNO_D_TYPE_IN_DIRENT
 endif
 ifdef NO_D_INO_IN_DIRENT
-	ALL_CFLAGS += -DNO_D_INO_IN_DIRENT
+	BASIC_CFLAGS += -DNO_D_INO_IN_DIRENT
 endif
 ifdef NO_SYMLINK_HEAD
-	ALL_CFLAGS += -DNO_SYMLINK_HEAD
+	BASIC_CFLAGS += -DNO_SYMLINK_HEAD
 endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
@@ -420,13 +426,13 @@ ifdef NO_MMAP
 	COMPAT_OBJS += compat/mmap.o
 endif
 ifdef NO_IPV6
-	ALL_CFLAGS += -DNO_IPV6
+	BASIC_CFLAGS += -DNO_IPV6
 endif
 ifdef NO_SOCKADDR_STORAGE
 ifdef NO_IPV6
-	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in
+	BASIC_CFLAGS += -Dsockaddr_storage=sockaddr_in
 else
-	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in6
+	BASIC_CFLAGS += -Dsockaddr_storage=sockaddr_in6
 endif
 endif
 ifdef NO_INET_NTOP
@@ -434,7 +440,7 @@ ifdef NO_INET_NTOP
 endif
 
 ifdef NO_ICONV
-	ALL_CFLAGS += -DNO_ICONV
+	BASIC_CFLAGS += -DNO_ICONV
 endif
 
 ifdef PPC_SHA1
@@ -458,7 +464,7 @@ ifdef USE_PIC
 	ALL_CFLAGS += -fPIC
 endif
 ifdef NO_ACCURATE_DIFF
-	ALL_CFLAGS += -DNO_ACCURATE_DIFF
+	BASIC_CFLAGS += -DNO_ACCURATE_DIFF
 endif
 
 # Shell quote (do not use $(call) to accomodate ancient setups);
@@ -478,8 +484,12 @@ GIT_PYTHON_DIR_SQ = $(subst ','\'',$(GIT
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
-ALL_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' $(COMPAT_CFLAGS)
+BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' $(COMPAT_CFLAGS)
 LIB_OBJS += $(COMPAT_OBJS)
+
+ALL_CFLAGS += $(BASIC_CFLAGS)
+ALL_LDFLAGS += $(BASIC_LDFLAGS)
+
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 
 
@@ -608,9 +618,9 @@ XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare
 	rm -f $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
 
-PERL_DEFINE = $(ALL_CFLAGS) -DGIT_VERSION='"$(GIT_VERSION)"'
+PERL_DEFINE = $(BASIC_CFLAGS) -DGIT_VERSION='"$(GIT_VERSION)"'
 PERL_DEFINE_SQ = $(subst ','\'',$(PERL_DEFINE))
-PERL_LIBS = $(EXTLIBS)
+PERL_LIBS = $(BASIC_LDFLAGS) $(EXTLIBS)
 PERL_LIBS_SQ = $(subst ','\'',$(PERL_LIBS))
 perl/Makefile:	perl/Git.pm perl/Makefile.PL GIT-CFLAGS
 	(cd perl && $(PERL_PATH) Makefile.PL \

^ permalink raw reply related

* [PATCH 0/2] resurrect format-patch's patch id checking
From: Johannes Schindelin @ 2006-06-25  1:50 UTC (permalink / raw)
  To: git, junkio, martin


With these patches, you can say

	git format-patch --ignore-if-in-upstream upstream

to get a series of only the patches which are in HEAD, but not upstream.

The first patch adds diff_flush_patch_id() to calculate the patch id, after
a diff queue was set up. (Earlier, I sent out a version which also adds
a command line option to the diff family, but I'll postpone that until
Timo's patches went in).

The second patch adds the actual patch id checking. There are two tricky
things involved:

	- I add a pseudo-object for each patch of the upstream, which has
	  the patch id as sha1. This is no real object, but since we rely
	  on the hashes being unique for all practical purposes, and since
	  it has parsed == 0, it should be no problem.

	- To add the patch ids of the upstream, the revision walker must
	  be called twice.

	  So, if format-patch was called with a range "a..b" (a single
	  revision "a" is handled as "a..HEAD" by format-patch), a revision
	  walker is set up for "b..a", and the patch ids are calculated and
	  stored. This is done by toggling the UNINTERESTING bits of both
	  pending objects.

	  After that, the flags of all objects are reset to 0, so that the
	  revisions can be walked again. The flags of the two pending objects
	  are then reset to their original state.

Note that "--numbered" still works.

WARNING: since it is quite late in this part of the world, _and_ I am known
to produce not-always-optimal patches which could eat your children, please
give this a good beating.

Ciao,
Dscho

^ permalink raw reply

* [PATCH 1/2] add diff_flush_patch_id() to calculate the patch id
From: Johannes Schindelin @ 2006-06-25  1:51 UTC (permalink / raw)
  To: git, junkio, martin


Call it like this:

unsigned char id[20];
if (diff_flush_patch_id(diff_options, id))
	printf("And the patch id is: %s\n", sha1_to_hex(id));

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 diff.c |  139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 diff.h |    2 +
 2 files changed, 141 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 5b34f73..3beecb9 100644
--- a/diff.c
+++ b/diff.c
@@ -2027,6 +2027,145 @@ static void diff_summary(struct diff_fil
 	}
 }
 
+struct patch_id_t {
+	struct xdiff_emit_state xm;
+	SHA_CTX *ctx;
+	int patchlen;
+};
+
+static int remove_space(char *line, int len)
+{
+	int i;
+        char *dst = line;
+        unsigned char c;
+
+        for (i = 0; i < len; i++)
+                if (!isspace((c = line[i])))
+                        *dst++ = c;
+
+        return dst - line;
+}
+
+static void patch_id_consume(void *priv, char *line, unsigned long len)
+{
+	struct patch_id_t *data = priv;
+	int new_len;
+
+	/* Ignore line numbers when computing the SHA1 of the patch */
+	if (!strncmp(line, "@@ -", 4))
+		return;
+
+	new_len = remove_space(line, len);
+
+	SHA1_Update(data->ctx, line, new_len);
+	data->patchlen += new_len;
+}
+
+/* returns 0 upon success, and writes result into sha1 */
+static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	int i;
+	SHA_CTX ctx;
+	struct patch_id_t data;
+	char buffer[PATH_MAX * 4 + 20];
+
+	SHA1_Init(&ctx);
+	memset(&data, 0, sizeof(struct patch_id_t));
+	data.ctx = &ctx;
+	data.xm.consume = patch_id_consume;
+	
+	for (i = 0; i < q->nr; i++) {
+		xpparam_t xpp;
+		xdemitconf_t xecfg;
+		xdemitcb_t ecb;
+		mmfile_t mf1, mf2;
+		struct diff_filepair *p = q->queue[i];
+		int len1, len2;
+
+		if (p->status == 0)
+			return error("internal diff status error");
+		if (p->status == DIFF_STATUS_UNKNOWN)
+			continue;
+		if (diff_unmodified_pair(p))
+			continue;
+		if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
+		    (DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
+			continue;
+		if (DIFF_PAIR_UNMERGED(p))
+			continue;
+
+		diff_fill_sha1_info(p->one);
+		diff_fill_sha1_info(p->two);
+		if (fill_mmfile(&mf1, p->one) < 0 ||
+				fill_mmfile(&mf2, p->two) < 0)
+			return error("unable to read files to diff");
+
+		/* Maybe hash p->two? into the patch id? */
+		if (mmfile_is_binary(&mf2))
+			continue;
+
+		len1 = remove_space(p->one->path, strlen(p->one->path));
+		len2 = remove_space(p->two->path, strlen(p->two->path));
+		if (p->one->mode == 0)
+			len1 = snprintf(buffer, sizeof(buffer),
+					"diff--gita/%.*sb/%.*s"
+					"newfilemode%06o"
+					"---/dev/null"
+					"+++b/%.*s",
+					len1, p->one->path,
+					len2, p->two->path,
+					p->two->mode,
+					len2, p->two->path);
+		else if (p->two->mode == 0)
+			len1 = snprintf(buffer, sizeof(buffer),
+					"diff--gita/%.*sb/%.*s"
+					"deletedfilemode%06o"
+					"---a/%.*s"
+					"+++/dev/null",
+					len1, p->one->path,
+					len2, p->two->path,
+					p->one->mode,
+					len1, p->one->path);
+		else
+			len1 = snprintf(buffer, sizeof(buffer),
+					"diff--gita/%.*sb/%.*s"
+					"---a/%.*s"
+					"+++b/%.*s",
+					len1, p->one->path,
+					len2, p->two->path,
+					len1, p->one->path,
+					len2, p->two->path);
+		SHA1_Update(&ctx, buffer, len1);
+
+		xpp.flags = XDF_NEED_MINIMAL;
+		xecfg.ctxlen = 3;
+		xecfg.flags = 3;
+		ecb.outf = xdiff_outf;
+		ecb.priv = &data;
+		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+	}
+
+	SHA1_Final(sha1, &ctx);
+	return 0;
+}
+
+int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	int i;
+	int result = diff_get_patch_id(options, sha1);
+
+	for (i = 0; i < q->nr; i++)
+		diff_free_filepair(q->queue[i]);
+
+	free(q->queue);
+	q->queue = NULL;
+	q->nr = q->alloc = 0;
+
+	return result;
+}
+
 void diff_flush(struct diff_options *options)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
diff --git a/diff.h b/diff.h
index 7d7b6cd..850c15f 100644
--- a/diff.h
+++ b/diff.h
@@ -185,4 +185,6 @@ extern int run_diff_files(struct rev_inf
 
 extern int run_diff_index(struct rev_info *revs, int cached);
 
+extern int diff_flush_patch_id(struct diff_options *, unsigned char *);
+
 #endif /* DIFF_H */
-- 
1.4.0.g7a200-dirty

^ permalink raw reply related

* [PATCH 2/2] format-patch: introduce "--ignore-if-in-upstream"
From: Johannes Schindelin @ 2006-06-25  1:52 UTC (permalink / raw)
  To: git, junkio, martin


With this flag, format-patch will try very hard not to output patches which
are already in the upstream branch.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 builtin-log.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 5a8a50b..e78a9a4 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -160,6 +160,71 @@ static void reopen_stdout(struct commit 
 	freopen(filename, "w", stdout);
 }
 
+static void reset_all_objects_flags()
+{
+	int i;
+
+	for (i = 0; i < obj_allocs; i++)
+		if (objs[i])
+			objs[i]->flags = 0;
+}
+
+static int get_patch_id(struct commit *commit, struct diff_options *options,
+		unsigned char *sha1)
+{
+	diff_tree_sha1(commit->parents->item->object.sha1, commit->object.sha1,
+			"", options);
+	diffcore_std(options);
+	return diff_flush_patch_id(options, sha1);
+}
+
+static void get_patch_ids(struct rev_info *rev, struct diff_options *options)
+{
+	struct rev_info check_rev;
+	struct commit *commit;
+	struct object *o1, *o2;
+	unsigned flags1, flags2;
+	unsigned char sha1[20];
+
+	if (rev->pending.nr != 2)
+		die("Need exactly one range.");
+
+	o1 = rev->pending.objects[0].item;
+	flags1 = o1->flags;
+	o2 = rev->pending.objects[1].item;
+	flags2 = o2->flags;
+
+	if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))
+		die("Not a range.");
+
+	diff_setup(options);
+	options->recursive = 1;
+	if (diff_setup_done(options) < 0)
+		die("diff_setup_done failed");
+
+	/* given a range a..b get all patch ids for b..a */
+	init_revisions(&check_rev);
+	o1->flags ^= UNINTERESTING;
+	o2->flags ^= UNINTERESTING;
+	add_pending_object(&check_rev, o1, "o1");
+	add_pending_object(&check_rev, o2, "o2");
+	prepare_revision_walk(&check_rev);
+
+	while ((commit = get_revision(&check_rev)) != NULL) {
+		/* ignore merges */
+		if (commit->parents && commit->parents->next)
+			continue;
+
+		if (!get_patch_id(commit, options, sha1))
+			created_object(sha1, xcalloc(1, sizeof(struct object)));
+	}
+
+	/* reset for next revision walk */
+	reset_all_objects_flags();
+	o1->flags = flags1;
+	o2->flags = flags2;
+}
+
 int cmd_format_patch(int argc, const char **argv, char **envp)
 {
 	struct commit *commit;
@@ -170,6 +235,8 @@ int cmd_format_patch(int argc, const cha
 	int numbered = 0;
 	int start_number = -1;
 	int keep_subject = 0;
+	int ignore_if_in_upstream = 0;
+	struct diff_options patch_id_opts;
 	char *add_signoff = NULL;
 
 	init_revisions(&rev);
@@ -235,6 +302,8 @@ int cmd_format_patch(int argc, const cha
 			rev.mime_boundary = git_version_string;
 		else if (!strncmp(argv[i], "--attach=", 9))
 			rev.mime_boundary = argv[i] + 9;
+		else if (!strcmp(argv[i], "--ignore-if-in-upstream"))
+			ignore_if_in_upstream = 1;
 		else
 			argv[j++] = argv[i];
 	}
@@ -262,14 +331,25 @@ int cmd_format_patch(int argc, const cha
 		add_head(&rev);
 	}
 
+	if (ignore_if_in_upstream)
+		get_patch_ids(&rev, &patch_id_opts);
+
 	if (!use_stdout)
 		realstdout = fdopen(dup(1), "w");
 
 	prepare_revision_walk(&rev);
 	while ((commit = get_revision(&rev)) != NULL) {
+		unsigned char sha1[20];
+
 		/* ignore merges */
 		if (commit->parents && commit->parents->next)
 			continue;
+
+		if (ignore_if_in_upstream &&
+				!get_patch_id(commit, &patch_id_opts, sha1) &&
+				lookup_object(sha1))
+			continue;
+
 		nr++;
 		list = realloc(list, nr * sizeof(list[0]));
 		list[nr - 1] = commit;
-- 
1.4.0.g7a200-dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox