Git development
 help / color / mirror / Atom feed
* Re: git-rev-list diff options broken
From: Han-Wen Nienhuys @ 2007-11-06 23:38 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711062330220.4362@racer.site>

2007/11/6, Johannes Schindelin <Johannes.Schindelin@gmx.de>:

>
> Probably you want to use "git log".
>
> "git blame -L 585,589 builtin-rev-list.c" indicates that 8c1f0b44(Fix up
> rev-list option parsing) was responsible, which in turn indicates that it
> was intentional.

OK. So the man page needs fixing, right?

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

^ permalink raw reply

* [PATCH 1/2] interpolate.[ch]: Add a function to find which interpolations are active.
From: Johannes Schindelin @ 2007-11-06 23:38 UTC (permalink / raw)
  To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711062335150.4362@racer.site>


Some substitutions require pretty expensive operations.  So it makes
sense to find out which are needed to begin with.  Call
interp_find_active() to set the new "active" field in the interpolation
table.

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

	This is the patch that Rene proposed, but squashed into my earlier
	patch.

 interpolate.c |   20 ++++++++++++++++++++
 interpolate.h |    3 +++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/interpolate.c b/interpolate.c
index 6ef53f2..bbd89bb 100644
--- a/interpolate.c
+++ b/interpolate.c
@@ -102,3 +102,23 @@ unsigned long interpolate(char *result, unsigned long reslen,
 		*dest = '\0';
 	return newlen;
 }
+
+void interp_find_active(const char *orig,
+		struct interp *interps, int ninterps)
+{
+	char c;
+	int i;
+
+	for (i = 0; i < ninterps; i++)
+		interps[i].active = 0;
+
+	while ((c = *(orig++)))
+		if (c == '%')
+			/* Try to match an interpolation string. */
+			for (i = 0; i < ninterps; i++)
+				if (!prefixcmp(orig, interps[i].name + 1)) {
+					interps[i].active = 1;
+					orig += strlen(interps[i].name + 1);
+					break;
+				}
+}
diff --git a/interpolate.h b/interpolate.h
index 77407e6..d23531a 100644
--- a/interpolate.h
+++ b/interpolate.h
@@ -14,6 +14,7 @@
 struct interp {
 	const char *name;
 	char *value;
+	int active:1;
 };
 
 extern void interp_set_entry(struct interp *table, int slot, const char *value);
@@ -22,5 +23,7 @@ extern void interp_clear_table(struct interp *table, int ninterps);
 extern unsigned long interpolate(char *result, unsigned long reslen,
 				 const char *orig,
 				 const struct interp *interps, int ninterps);
+extern void interp_find_active(const char *orig,
+				struct interp *interps, int ninterps);
 
 #endif /* INTERPOLATE_H */
-- 
1.5.3.5.1597.g7191

^ permalink raw reply related

* [PATCH 2/2] pretty=format: Avoid some expensive calculations when not needed
From: Johannes Schindelin @ 2007-11-06 23:38 UTC (permalink / raw)
  To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711062335150.4362@racer.site>


Use the new function interp_find_active() to avoid calculating the
unique hash names, and other things, when they are not even asked for.

Unfortunately, we cannot reuse the result of that function, which
would be cleaner: there are more users than just git log.  Most
notably, git-archive with "$Format:...$" substitution.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 pretty.c |   55 ++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/pretty.c b/pretty.c
index 490cede..590de4c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -394,6 +394,8 @@ void format_commit_message(const struct commit *commit,
 	enum { HEADER, SUBJECT, BODY } state;
 	const char *msg = commit->buffer;
 
+	interp_find_active(format, table, ARRAY_SIZE(table));
+
 	if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
 		die("invalid interp table!");
 
@@ -407,12 +409,18 @@ void format_commit_message(const struct commit *commit,
 	/* these depend on the commit */
 	if (!commit->object.parsed)
 		parse_object(commit->object.sha1);
-	interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
-	interp_set_entry(table, IHASH_ABBREV,
+	if (table[IHASH].active)
+		interp_set_entry(table, IHASH,
+				sha1_to_hex(commit->object.sha1));
+	if (table[IHASH_ABBREV].active)
+		interp_set_entry(table, IHASH_ABBREV,
 			find_unique_abbrev(commit->object.sha1,
 				DEFAULT_ABBREV));
-	interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
-	interp_set_entry(table, ITREE_ABBREV,
+	if (table[ITREE].active)
+		interp_set_entry(table, ITREE,
+				sha1_to_hex(commit->tree->object.sha1));
+	if (table[ITREE_ABBREV].active)
+		interp_set_entry(table, ITREE_ABBREV,
 			find_unique_abbrev(commit->tree->object.sha1,
 				DEFAULT_ABBREV));
 	interp_set_entry(table, ILEFT_RIGHT,
@@ -422,22 +430,27 @@ void format_commit_message(const struct commit *commit,
 			 ? "<"
 			 : ">");
 
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			sha1_to_hex(p->item->object.sha1));
-	interp_set_entry(table, IPARENTS, parents + 1);
-
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			find_unique_abbrev(p->item->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+	if (table[IPARENTS].active) {
+		parents[1] = 0;
+		for (i = 0, p = commit->parents;
+				p && i < sizeof(parents) - 1;
+				p = p->next)
+			i += snprintf(parents + i, sizeof(parents) - i - 1,
+				" %s", sha1_to_hex(p->item->object.sha1));
+		interp_set_entry(table, IPARENTS, parents + 1);
+	}
+
+	if (table[IPARENTS_ABBREV].active) {
+		parents[1] = 0;
+		for (i = 0, p = commit->parents;
+				p && i < sizeof(parents) - 1;
+				p = p->next)
+			i += snprintf(parents + i, sizeof(parents) - i - 1,
+				" %s",
+				find_unique_abbrev(p->item->object.sha1,
+					DEFAULT_ABBREV));
+		interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+	}
 
 	for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
 		int eol;
@@ -464,7 +477,7 @@ void format_commit_message(const struct commit *commit,
 				xmemdupz(msg + i + 9, eol - i - 9);
 		i = eol;
 	}
-	if (msg[i])
+	if (table[IBODY].active && msg[i])
 		table[IBODY].value = xstrdup(msg + i);
 
 	len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
-- 
1.5.3.5.1597.g7191

^ permalink raw reply related

* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
From: Johannes Schindelin @ 2007-11-06 23:45 UTC (permalink / raw)
  To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <4730F5FA.3030705@lsrfire.ath.cx>

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

Hi,

On Wed, 7 Nov 2007, René Scharfe wrote:

> By the way, the more intrusive surgery required when using strbuf_expand()
> leads to even faster operation.  Here my measurements of most of Paul's
> test cases (best of three runs):
>
> [...]

impressive timings.  Although I wonder where the time comes from, as the 
other substitutions should not be _that_ expensive.

In any case, your approach seems much more sensible, now that we have 
strbuf.

> diff --git a/strbuf.h b/strbuf.h
> index cd7f295..95071d5 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -102,6 +102,9 @@ static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
>  	strbuf_add(sb, sb2->buf, sb2->len);
>  }
>  
> +typedef void (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
> +extern void strbuf_expand(struct strbuf *sb, const char *fmt, const char **placeholders, expand_fn_t fn, void *context);

I wonder if it would even faster (but maybe not half as readable) if 
expand_fd_t got the placeholder_index instead of the placeholder.

Ciao,
Dscho

^ permalink raw reply

* Re: git-rev-list diff options broken
From: Johannes Schindelin @ 2007-11-06 23:55 UTC (permalink / raw)
  To: hanwen; +Cc: git
In-Reply-To: <f329bf540711061538l4cdef27co2cb42a9938b2e325@mail.gmail.com>

Hi,

On Tue, 6 Nov 2007, Han-Wen Nienhuys wrote:

> 2007/11/6, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> 
> > Probably you want to use "git log".
> >
> > "git blame -L 585,589 builtin-rev-list.c" indicates that 8c1f0b44(Fix 
> > up rev-list option parsing) was responsible, which in turn indicates 
> > that it was intentional.
> 
> OK. So the man page needs fixing, right?

I guess.  Although it seems a bit involved, since the diff-formats.txt are 
not included by git-rev-list.txt itself, but by pretty-formats.txt.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add Documentation/CodingStyle
From: Andreas Ericsson @ 2007-11-07  0:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Ralf Wildenhues, git
In-Reply-To: <Pine.LNX.4.64.0711062317330.4362@racer.site>

Johannes Schindelin wrote:
> Even if our code is quite a good documentation for our coding style,
> some people seem to prefer a document describing it.
> 

Sweet. You just saved me 40 minutes of writing one just like that for
company use. I owe you a drink, and then you can tell me what movie
you alluded to ;-)

> +
> + - if you are planning a new command, consider writing it in shell or
> +   perl first, so that changes in semantics can be easily changed and
> +   discussed.  Many git commands started out like that, and a few are
> +   still scripts.

I'd skip this part though and just add a pointer to contrib/examples/,
saying something along the lines of

- if you're planning a new command, sneak a peak in contrib/examples/
  for ample study-material of retired git commands implemented in perl
  and shell.

Possibly with s/retired // on that paragraph.

There's nothing particular wrong with writing in C from the start after
all.

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

^ permalink raw reply

* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations  when not needed
From: Pierre Habouzit @ 2007-11-07  0:11 UTC (permalink / raw)
  To: René Scharfe; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <4730F5FA.3030705@lsrfire.ath.cx>

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

On Tue, Nov 06, 2007 at 11:17:14PM +0000, René Scharfe wrote:
> I haven't seen any comments on strbuf_expand.  Is it too far out?
> Here it is again, adjusted for current master and with the changes
> to strbuf.[ch] coming first:

  I have one.

>  strbuf.c |   22 +++++
>  strbuf.h |    3 
>  pretty.c |  276 ++++++++++++++++++++++++++++++++++-----------------------------
>  3 files changed, 178 insertions(+), 123 deletions(-)
> 
> diff --git a/strbuf.c b/strbuf.c
> index f4201e1..b71da99 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -129,6 +129,28 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
>  	strbuf_setlen(sb, sb->len + len);
>  }
>  
> +void strbuf_expand(struct strbuf *sb, const char *fmt,
> +                   const char **placeholders, expand_fn_t fn, void *context)
> +{
> +	char c;
> +	const char **p;
> +
> +	while ((c = *fmt++)) {
> +		if (c != '%') {
> +			strbuf_addch(sb, c);
> +			continue;
> +		}

strbuf_addch is pretty inneficient as it puts NULs each time. rather
do that (sketchy) :

{
    for (;;) {
        const char *percent = strchr(fmt, '%');
        if (!percent)
            break;
        strbuf_add(sb, fmt, percent - fmt);
        fmt = percent + 1;

        /* do your stuff */
    }
    strbuf_addstr(sb, fmt);
}

Of course it's a detail as formats will probably be short. But it's a good
example to show to people wanting to write new strbuf functions.

This nitpicking apart, the timings are impressive.

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

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

^ permalink raw reply

* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations  when not needed
From: Pierre Habouzit @ 2007-11-07  0:14 UTC (permalink / raw)
  To: René Scharfe, Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071107001112.GD4382@artemis.corp>

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

On Wed, Nov 07, 2007 at 12:11:12AM +0000, Pierre Habouzit wrote:
> On Tue, Nov 06, 2007 at 11:17:14PM +0000, René Scharfe wrote:
> > I haven't seen any comments on strbuf_expand.  Is it too far out?
> > Here it is again, adjusted for current master and with the changes
> > to strbuf.[ch] coming first:
> 
>   I have one.
> 
> >  strbuf.c |   22 +++++
> >  strbuf.h |    3 
> >  pretty.c |  276 ++++++++++++++++++++++++++++++++++-----------------------------
> >  3 files changed, 178 insertions(+), 123 deletions(-)
> > 
> > diff --git a/strbuf.c b/strbuf.c
> > index f4201e1..b71da99 100644
> > --- a/strbuf.c
> > +++ b/strbuf.c
> > @@ -129,6 +129,28 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
> >  	strbuf_setlen(sb, sb->len + len);
> >  }
> >  
> > +void strbuf_expand(struct strbuf *sb, const char *fmt,
> > +                   const char **placeholders, expand_fn_t fn, void *context)
> > +{
> > +	char c;
> > +	const char **p;
> > +
> > +	while ((c = *fmt++)) {
> > +		if (c != '%') {
> > +			strbuf_addch(sb, c);
> > +			continue;
> > +		}
> 
> strbuf_addch is pretty inneficient as it puts NULs each time. rather
> do that (sketchy) :
> 
> {
>     for (;;) {
>         const char *percent = strchr(fmt, '%');
>         if (!percent)
>             break;
>         strbuf_add(sb, fmt, percent - fmt);
>         fmt = percent + 1;
> 
>         /* do your stuff */
>     }
>     strbuf_addstr(sb, fmt);
> }


Or if we are at this level of micro-optimization:

{
    const char *percent = strchrnul(fmt, '%');
    while (*percent) {
        strbuf_add(sb, fmt, percent - fmt);
        fmt = percent + 1;

        /* do your stuff */

        percent = strchrnul(fmt, '%');
    }
    strbuf_add(sb, fmt, percent - fmt);
}


Which would require strchrnul, but it's trivial compat/ material for sure.


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

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

^ permalink raw reply

* Re: [PATCH] Add Documentation/CodingStyle
From: Junio C Hamano @ 2007-11-07  0:40 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Ralf Wildenhues, git
In-Reply-To: <Pine.LNX.4.64.0711062317330.4362@racer.site>

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

> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> new file mode 100644
> index 0000000..622b80b
> --- /dev/null
> +++ b/Documentation/CodingStyle
> @@ -0,0 +1,87 @@
> +As a popular project, we also have some guidelines to keep to the
> +code.  For git in general, two rough rules are:
> +
> + - Most importantly, we never say "It's in POSIX; we'll happily
> +   screw your system that does not conform."  We live in the
> +   real world.
> +
> + - However, we often say "Let's stay away from that construct,
> +   it's not even in POSIX".
> +

I am not sure if we want to have CodingStyle document, but the
above are not CodingStyle issues.

If we are to write this down, I'd like to have the more
important third rule, which is:

 - In spite of the above two rules, we sometimes say "Although
   this is not in POSIX, it (is so convenient | makes the code
   much more readable | has other good characteristics) and
   practically all the platforms we care about support it, so
   let's use it".  Again, we live in the real world, and it is
   sometimes a judgement call, decided based more on real world
   constraints people face than what the paper standard says.

> +For C programs:
> +
> + - Use tabs to increment, and interpret tabs as taking up to 8 spaces

What's the character for decrement?  DEL? ;-)

> +   Double negation is often harder to understand than no negation at
> +   all.
> +
> +   Some clever tricks, like using the !! operator with arithmetic
> +   constructs, can be extremely confusing to others.  Avoid them,
> +   unless there is a compelling reason to use them.

I actually think (!!var) idiom is already established in our
codebase.

> + - Use the API.  No, really.  We have a strbuf (variable length string),
> +   several arrays with the ALLOC_GROW() macro, a path_list for sorted
> +   string lists, a hash map (mapping struct objects) named
> +   "struct decorate", amongst other things.

 - When you come up with an API, document it.

> + - if you are planning a new command, consider writing it in shell or
> +   perl first, so that changes in semantics can be easily changed and
> +   discussed.  Many git commands started out like that, and a few are
> +   still scripts.

No Python allowed?

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Francesco Pretto @ 2007-11-07  0:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4unez2l.fsf@gitster.siamese.dyndns.org>

Junio C Hamano ha scritto:
> 
> Honestly speaking, I am not too thrilled about making the
> cvs-migration document much longer than what it currently is.
> 

Honestly speaking, you've spent too much time in looking for every possible
objections against these simple additions. At least it should be less than the
time I've spent in measuring every single word of this patch, hoping you could
consider them for inclusion. You gave me lot of attentions (I am grateful of this,
really) so I should probably be surprised of the cleanliness of git code, of the
rigor of the code style, of the clarity of the documentation. But unfortunately,
I am not. I simply tried to make this document more useful and helpful for a
wider audience of people that could ever consider of using git in their life.
And yes, I decided to so because I had trouble myself during initial configurations.
What's the problem if a document called "git for CVS users" is more explicated?
What's the problem if it contains as many as possible informations to set up
git in a viable way and, hopefully, to learn something on how it does work?

I'm sad. Not only because you refused a documentation patch, but because i could
have sent a "Bug: Documentation Sucks!" to the ml and i would have obtained the
same thing: nothing.

Francesco

P.S.:

>> +------------------------------------------------
>> +$ usermod -a -G $group $username
>> +------------------------------------------------
> 
> I tend to edit /etc/group with vi ;-) and I suspect these two
> commands are specific to the distro you happen to use.

You were right with usermod (groupadd is ok): that "-a" switch is redhat syntax.
Six years of Linux Standard Base and this is still an unsolved problem...

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Johannes Schindelin @ 2007-11-07  0:55 UTC (permalink / raw)
  To: Francesco Pretto; +Cc: Junio C Hamano, git
In-Reply-To: <47310ACF.4030103@gmail.com>

Hi,

On Wed, 7 Nov 2007, Francesco Pretto wrote:

> I simply tried to make this document more useful and helpful for a wider 
> audience of people that could ever consider of using git in their life. 
> And yes, I decided to so because I had trouble myself during initial 
> configurations. What's the problem if a document called "git for CVS 
> users" is more explicated? What's the problem if it contains as many as 
> possible informations to set up git in a viable way and, hopefully, to 
> learn something on how it does work?

I refrained from commenting on that patch until now, but alas, you force 
me to.

I was pretty unimpressed by the additions, as they seemed large in volume, 
but small in content.

Remember, those who read "git for CVS users" are _unwilling_ to spend the 
time reading git documentation (at least for the most part).  If they 
encounter something which is not useful to them, they will not just ignore 
it, they will stop reading.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Francesco Pretto @ 2007-11-07  1:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711070053320.4362@racer.site>

Johannes Schindelin ha scritto:
> Remember, those who read "git for CVS users" are _unwilling_ to spend the 
> time reading git documentation (at least for the most part).  If they 
> encounter something which is not useful to them, they will not just ignore 
> it, they will stop reading.
>

That's document isn't for CVS users only. It's referred on the "Git User's Manual"
speaking about shared repositories in general. I hope you agree that the time to
make your eyes jump a little below is less than the time spent googling around
if you don't find what you are looking for.

^ permalink raw reply

* [PATCH] Reteach builtin-ls-remote to understand remotes
From: Shawn O. Pearce @ 2007-11-07  1:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Prior to being made a builtin git-ls-remote understood that when
it was given a remote name we wanted it to resolve that to the
pre-configured URL and connect to that location.  That changed when
it was converted to a builtin and many of my automation tools broke.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-ls-remote.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 003580c..56f3f88 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -14,6 +14,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	unsigned flags = 0;
 	const char *uploadpack = NULL;
 
+	struct remote *remote;
 	struct transport *transport;
 	const struct ref *ref;
 
@@ -52,7 +53,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (!dest || i != argc - 1)
 		usage(ls_remote_usage);
 
-	transport = transport_get(NULL, dest);
+	remote = nongit ? NULL : remote_get(dest);
+	if (remote && !remote->url_nr)
+		die("remote %s has no configured URL", dest);
+	transport = transport_get(remote, remote ? remote->url[0] : dest);
 	if (uploadpack != NULL)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
 
-- 
1.5.3.5.1590.gfadfad

^ permalink raw reply related

* [PATCH] Mark 'git stash [message...]' as deprecated
From: Brian Downing @ 2007-11-07  0:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: tsuna, aghilesk, git, Brian Downing
In-Reply-To: <20071106085134.GD4435@artemis.corp>

Complain to STDERR unless 'git stash save' is explicitly used.
This is in preparation for completely disabling the "default save"
behavior of the command in the future.

Signed-off-by: Brian Downing <bdowning@lavos.net>
---
 Documentation/git-stash.txt |    9 ++++-----
 git-stash.sh                |    8 +++++++-
 t/t3903-stash.sh            |   14 +++++++++++++-
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index c0147b9..61cf95d 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git-stash' (list | show [<stash>] | apply [<stash>] | clear)
-'git-stash' [save] [message...]
+'git-stash' save [message...]
 
 DESCRIPTION
 -----------
@@ -39,8 +39,7 @@ OPTIONS
 save::
 
 	Save your local modifications to a new 'stash', and run `git-reset
-	--hard` to revert them.  This is the default action when no
-	subcommand is given.
+	--hard` to revert them.
 
 list::
 
@@ -119,7 +118,7 @@ perform a pull, and then unstash, like this:
 $ git pull
 ...
 file foobar not up to date, cannot merge.
-$ git stash
+$ git stash save
 $ git pull
 $ git stash apply
 ----------------------------------------------------------------
@@ -147,7 +146,7 @@ You can use `git-stash` to simplify the above, like this:
 +
 ----------------------------------------------------------------
 ... hack hack hack ...
-$ git stash
+$ git stash save
 $ edit emergency fix
 $ git commit -a -m "Fix in a hurry"
 $ git stash apply
diff --git a/git-stash.sh b/git-stash.sh
index f39bd55..a8b854a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Copyright (c) 2007, Nanako Shiraishi
 
-USAGE='[ | list | show | apply | clear]'
+USAGE='[save | list | show | apply | clear]'
 
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -223,6 +223,12 @@ help | usage)
 	if test $# -gt 0 && test "$1" = save
 	then
 		shift
+	else
+		cat >&2 <<EOF
+'git stash [message...]' is deprecated, please use
+'git stash save [message...]' instead.
+
+EOF
 	fi
 	save_stash "$*" && git-reset --hard
 	;;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 9a9a250..adfac4b 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -16,7 +16,7 @@ test_expect_success 'stash some dirty working directory' '
 	git add file &&
 	echo 3 > file &&
 	test_tick &&
-	git stash &&
+	git stash save &&
 	git diff-files --quiet &&
 	git diff-index --cached --quiet HEAD
 '
@@ -73,4 +73,16 @@ test_expect_success 'unstashing in a subdirectory' '
 	git stash apply
 '
 
+test_expect_success 'stash with no args' '
+	echo 7 > file &&
+	test_tick &&
+	git stash
+'
+
+test_expect_success 'stash with bare message' '
+	echo 8 > file &&
+	test_tick &&
+	git stash "a message"
+'
+
 test_done
-- 
1.5.3.5.1547.gf6d81-dirty

^ permalink raw reply related

* [PATCH] Disable implicit 'save' argument for 'git stash'
From: Brian Downing @ 2007-11-07  0:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: tsuna, aghilesk, git, Brian Downing
In-Reply-To: <1194395205-27905-1-git-send-email-bdowning@lavos.net>

Having 'git stash random stuff' actually stash changes is poor
user interface, due to the likelyhood of misspelling another legitimate
argument.  Require an explicit 'save' command instead.

Signed-off-by: Brian Downing <bdowning@lavos.net>
---
    This commit can be applied on top of the previous whenever it
    is decided "enough time" has passed for the hard behavior change
    of "git stash" to take place.

 git-stash.sh     |   16 +++++-----------
 t/t3903-stash.sh |    4 ++--
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index a8b854a..e900d40 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -219,17 +219,11 @@ create)
 help | usage)
 	usage
 	;;
-*)
-	if test $# -gt 0 && test "$1" = save
-	then
-		shift
-	else
-		cat >&2 <<EOF
-'git stash [message...]' is deprecated, please use
-'git stash save [message...]' instead.
-
-EOF
-	fi
+save)
+	shift
 	save_stash "$*" && git-reset --hard
 	;;
+*)
+	usage
+	;;
 esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index adfac4b..4896da0 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -73,13 +73,13 @@ test_expect_success 'unstashing in a subdirectory' '
 	git stash apply
 '
 
-test_expect_success 'stash with no args' '
+test_expect_failure 'stash with no args' '
 	echo 7 > file &&
 	test_tick &&
 	git stash
 '
 
-test_expect_success 'stash with bare message' '
+test_expect_failure 'stash with bare message' '
 	echo 8 > file &&
 	test_tick &&
 	git stash "a message"
-- 
1.5.3.5.1547.gf6d81-dirty

^ permalink raw reply related

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Aghiles @ 2007-11-07  1:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Francesco Pretto, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711070053320.4362@racer.site>

Hello,
>
> Remember, those who read "git for CVS users" are _unwilling_ to spend the
> time reading git documentation (at least for the most part).  If they
> encounter something which is not useful to them, they will not just ignore
> it, they will stop reading.
>

I must disagree with this analysis (although I didn't read the content of the
patch you are commenting). People that are not really interested in git will
find many reasons to stop reading the manual anyway. Those who really
want to migrate (such as we did) are looking for every tiny bit of information.
(We googled git commands and error messages because we didn't
find what we needed in the docs)
The docs are not perfect and somewhat unequal in content but I prefer
more information than less, at this particular stage of git development.

- Aghiles.

^ permalink raw reply

* [PATCH] git-fetch: avoid local fetching from alternate (again)
From: Shawn O. Pearce @ 2007-11-07  2:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Back in e3c6f240fd9c5bdeb33f2d47adc859f37935e2df Junio taught
git-fetch to avoid copying objects when we are fetching from
a repository that is already registered as an alternate object
database.  In such a case there is no reason to copy any objects
as we can already obtain them through the alternate.

However we need to ensure the objects are all reachable, so we
run `git rev-list --objects $theirs --not --all` to verify this.
If any object is missing or unreadable then we need to instead copy
the objects from the remote.  When a missing object is detected
the git-rev-list process will exit with a non-zero exit status,
making this condition quite easy to detect.

Although git-fetch is currently a builtin (and so is rev-list) we
really cannot invoke the traverse_objects() API at this point in the
transport code.  The object walker within traverse_objects() calls
die() as soon as it finds an object it cannot read.  If that happens
we want to resume the fetch process by calling do_fetch_pack(),
instead of terminating.  To get aroaund this we spawn git-rev-list
into a background process to prevent a die() from killing the
foreground fetch process.

We aren't interested in the output of rev-list (a list of SHA-1
object names that are reachable) or its errors (a "spurious" error
about an object not being found as we need to copy it) so we redirect
both stdout and stderr to /dev/null.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 run-command.c |    6 ++++--
 run-command.h |    1 +
 transport.c   |   48 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/run-command.c b/run-command.c
index d99a6c4..476d00c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -41,7 +41,7 @@ int start_command(struct child_process *cmd)
 		cmd->close_out = 1;
 	}
 
-	need_err = cmd->err < 0;
+	need_err = !cmd->no_stderr && cmd->err < 0;
 	if (need_err) {
 		if (pipe(fderr) < 0) {
 			if (need_in)
@@ -87,7 +87,9 @@ int start_command(struct child_process *cmd)
 			close(cmd->out);
 		}
 
-		if (need_err) {
+		if (cmd->no_stderr)
+			dup_devnull(2);
+		else if (need_err) {
 			dup2(fderr[1], 2);
 			close_pair(fderr);
 		}
diff --git a/run-command.h b/run-command.h
index 94e1e9d..1fc781d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -23,6 +23,7 @@ struct child_process {
 	unsigned close_out:1;
 	unsigned no_stdin:1;
 	unsigned no_stdout:1;
+	unsigned no_stderr:1;
 	unsigned git_cmd:1; /* if this is to be git sub-command */
 	unsigned stdout_to_stderr:1;
 };
diff --git a/transport.c b/transport.c
index f4577b7..8505a84 100644
--- a/transport.c
+++ b/transport.c
@@ -615,17 +615,56 @@ static struct ref *get_refs_via_connect(struct transport *transport)
 	return refs;
 }
 
+static int fetch_local_nocopy(struct transport *transport,
+			       int nr_heads, struct ref **to_fetch)
+{
+	struct stat sb;
+	struct child_process revlist;
+	char **argv;
+	int i, j, err;
+
+	if (stat(transport->url, &sb) || !S_ISDIR(sb.st_mode))
+		return -1;
+
+	i = 0;
+	argv = xmalloc(sizeof(*argv) * (nr_heads + 5));
+	argv[i++] = xstrdup("rev-list");
+	argv[i++] = xstrdup("--objects");
+	for (j = 0; j < nr_heads; j++)
+		argv[i++] = xstrdup(sha1_to_hex(to_fetch[j]->old_sha1));
+	argv[i++] = xstrdup("--not");
+	argv[i++] = xstrdup("--all");
+	argv[i++] = NULL;
+
+	memset(&revlist, 0, sizeof(revlist));
+	revlist.argv = (const char**)argv;
+	revlist.git_cmd = 1;
+	revlist.no_stdin = 1;
+	revlist.no_stdout = 1;
+	revlist.no_stderr = 1;
+	err = start_command(&revlist);
+	if (!err)
+		err |= finish_command(&revlist);
+
+	for (i = 0; argv[i]; i++)
+		free(argv[i]);
+	free(argv);
+	return err;
+}
+
 static int fetch_refs_via_pack(struct transport *transport,
 			       int nr_heads, struct ref **to_fetch)
 {
 	struct git_transport_data *data = transport->data;
-	char **heads = xmalloc(nr_heads * sizeof(*heads));
-	char **origh = xmalloc(nr_heads * sizeof(*origh));
+	char **heads, **origh;
 	struct ref *refs;
-	char *dest = xstrdup(transport->url);
+	char *dest;
 	struct fetch_pack_args args;
 	int i;
 
+	if (!fetch_local_nocopy(transport, nr_heads, to_fetch))
+		return 0;
+
 	memset(&args, 0, sizeof(args));
 	args.uploadpack = data->uploadpack;
 	args.keep_pack = data->keep;
@@ -634,6 +673,9 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.verbose = transport->verbose > 0;
 	args.depth = data->depth;
 
+	heads = xmalloc(nr_heads * sizeof(*heads));
+	origh = xmalloc(nr_heads * sizeof(*origh));
+	dest = xstrdup(transport->url);
 	for (i = 0; i < nr_heads; i++)
 		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
 	refs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile);
-- 
1.5.3.5.1590.gfadfad

^ permalink raw reply related

* [PATCH] Improve accuracy of check for presence of deflateBound.
From: David Symonds @ 2007-11-07  3:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Symonds; David Alan, David Symonds

From: Symonds; David Alan <dasymond@nlp2.cs.usyd.edu.au>

ZLIB_VERNUM isn't defined in some zlib versions, so this patch does a proper
linking test in autoconf to see whether deflateBound exists in zlib.

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 Makefile      |    6 ++++++
 cache.h       |    2 +-
 config.mak.in |    1 +
 configure.ac  |   20 ++++++++++++++++++++
 4 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 1a81ef1..c8bcd1d 100644
--- a/Makefile
+++ b/Makefile
@@ -98,6 +98,8 @@ all::
 # Define OLD_ICONV if your library has an old iconv(), where the second
 # (input buffer pointer) parameter is declared with type (const char **).
 #
+# Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.
+#
 # Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib"
 # that tells runtime paths to dynamic libraries;
 # "-Wl,-rpath=/path/lib" is used instead.
@@ -663,6 +665,10 @@ ifdef OLD_ICONV
 	BASIC_CFLAGS += -DOLD_ICONV
 endif
 
+ifdef NO_DEFLATE_BOUND
+	BASIC_CFLAGS += -DNO_DEFLATE_BOUND
+endif
+
 ifdef PPC_SHA1
 	SHA1_HEADER = "ppc/sha1.h"
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
diff --git a/cache.h b/cache.h
index 830d2e0..a3b1a26 100644
--- a/cache.h
+++ b/cache.h
@@ -7,7 +7,7 @@
 #include SHA1_HEADER
 #include <zlib.h>
 
-#if ZLIB_VERNUM < 0x1200
+#if defined(NO_DEFLATE_BOUND)
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
diff --git a/config.mak.in b/config.mak.in
index a3032e3..776b805 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -38,3 +38,4 @@ NO_STRCASESTR=@NO_STRCASESTR@
 NO_STRLCPY=@NO_STRLCPY@
 NO_SETENV=@NO_SETENV@
 NO_ICONV=@NO_ICONV@
+NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
diff --git a/configure.ac b/configure.ac
index ed7cc89..ab516db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,6 +182,26 @@ AC_SUBST(NEEDS_LIBICONV)
 AC_SUBST(NO_ICONV)
 test -n "$NEEDS_LIBICONV" && LIBS="$LIBS -liconv"
 #
+# Define NO_DEFLATE_BOUND if deflateBound is missing from zlib.
+AC_DEFUN([ZLIBTEST_SRC], [
+#include <zlib.h>
+
+int main(void)
+{
+	deflateBound(0, 0);
+	return 0;
+}
+])
+AC_MSG_CHECKING([for deflateBound in -lz])
+old_LIBS="$LIBS"
+LIBS="$LIBS -lz"
+AC_LINK_IFELSE(ZLIBTEST_SRC,
+	[AC_MSG_RESULT([yes])],
+	[AC_MSG_RESULT([no])
+	NO_DEFLATE_BOUND=yes])
+LIBS="$old_LIBS"
+AC_SUBST(NO_DEFLATE_BOUND)
+#
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 AC_CHECK_LIB([c], [socket],
-- 
1.5.3.1

^ permalink raw reply related

* [PATCH] Improve accuracy of check for presence of deflateBound.
From: David Symonds @ 2007-11-07  3:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, David Symonds

ZLIB_VERNUM isn't defined in some zlib versions, so this patch does a proper
linking test in autoconf to see whether deflateBound exists in zlib.

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 Makefile      |    6 ++++++
 cache.h       |    2 +-
 config.mak.in |    1 +
 configure.ac  |   20 ++++++++++++++++++++
 4 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 1a81ef1..c8bcd1d 100644
--- a/Makefile
+++ b/Makefile
@@ -98,6 +98,8 @@ all::
 # Define OLD_ICONV if your library has an old iconv(), where the second
 # (input buffer pointer) parameter is declared with type (const char **).
 #
+# Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.
+#
 # Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib"
 # that tells runtime paths to dynamic libraries;
 # "-Wl,-rpath=/path/lib" is used instead.
@@ -663,6 +665,10 @@ ifdef OLD_ICONV
 	BASIC_CFLAGS += -DOLD_ICONV
 endif
 
+ifdef NO_DEFLATE_BOUND
+	BASIC_CFLAGS += -DNO_DEFLATE_BOUND
+endif
+
 ifdef PPC_SHA1
 	SHA1_HEADER = "ppc/sha1.h"
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
diff --git a/cache.h b/cache.h
index 830d2e0..a3b1a26 100644
--- a/cache.h
+++ b/cache.h
@@ -7,7 +7,7 @@
 #include SHA1_HEADER
 #include <zlib.h>
 
-#if ZLIB_VERNUM < 0x1200
+#if defined(NO_DEFLATE_BOUND)
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
diff --git a/config.mak.in b/config.mak.in
index a3032e3..776b805 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -38,3 +38,4 @@ NO_STRCASESTR=@NO_STRCASESTR@
 NO_STRLCPY=@NO_STRLCPY@
 NO_SETENV=@NO_SETENV@
 NO_ICONV=@NO_ICONV@
+NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
diff --git a/configure.ac b/configure.ac
index ed7cc89..ab516db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,6 +182,26 @@ AC_SUBST(NEEDS_LIBICONV)
 AC_SUBST(NO_ICONV)
 test -n "$NEEDS_LIBICONV" && LIBS="$LIBS -liconv"
 #
+# Define NO_DEFLATE_BOUND if deflateBound is missing from zlib.
+AC_DEFUN([ZLIBTEST_SRC], [
+#include <zlib.h>
+
+int main(void)
+{
+	deflateBound(0, 0);
+	return 0;
+}
+])
+AC_MSG_CHECKING([for deflateBound in -lz])
+old_LIBS="$LIBS"
+LIBS="$LIBS -lz"
+AC_LINK_IFELSE(ZLIBTEST_SRC,
+	[AC_MSG_RESULT([yes])],
+	[AC_MSG_RESULT([no])
+	NO_DEFLATE_BOUND=yes])
+LIBS="$old_LIBS"
+AC_SUBST(NO_DEFLATE_BOUND)
+#
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 AC_CHECK_LIB([c], [socket],
-- 
1.5.3.1

^ permalink raw reply related

* [REPLACEMENT PATCH] Improve accuracy of check for presence of deflateBound.
From: David Symonds @ 2007-11-07  3:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, David Symonds

ZLIB_VERNUM isn't defined in some zlib versions, so this patch does a proper
linking test in autoconf to see whether deflateBound exists in zlib. Also,
setting NO_DEFLATE_BOUND will also work for folk not using autoconf.

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
	This resend keeps the ZLIB_VERNUM test in place for people who don't
	use autoconf (thanks spearce).

 Makefile      |    6 ++++++
 cache.h       |    2 +-
 config.mak.in |    1 +
 configure.ac  |   20 ++++++++++++++++++++
 4 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 1a81ef1..c8bcd1d 100644
--- a/Makefile
+++ b/Makefile
@@ -98,6 +98,8 @@ all::
 # Define OLD_ICONV if your library has an old iconv(), where the second
 # (input buffer pointer) parameter is declared with type (const char **).
 #
+# Define NO_DEFLATE_BOUND if your zlib does not have deflateBound.
+#
 # Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib"
 # that tells runtime paths to dynamic libraries;
 # "-Wl,-rpath=/path/lib" is used instead.
@@ -663,6 +665,10 @@ ifdef OLD_ICONV
 	BASIC_CFLAGS += -DOLD_ICONV
 endif
 
+ifdef NO_DEFLATE_BOUND
+	BASIC_CFLAGS += -DNO_DEFLATE_BOUND
+endif
+
 ifdef PPC_SHA1
 	SHA1_HEADER = "ppc/sha1.h"
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
diff --git a/cache.h b/cache.h
index 830d2e0..ae66de1 100644
--- a/cache.h
+++ b/cache.h
@@ -7,7 +7,7 @@
 #include SHA1_HEADER
 #include <zlib.h>
 
-#if ZLIB_VERNUM < 0x1200
+#if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
diff --git a/config.mak.in b/config.mak.in
index a3032e3..776b805 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -38,3 +38,4 @@ NO_STRCASESTR=@NO_STRCASESTR@
 NO_STRLCPY=@NO_STRLCPY@
 NO_SETENV=@NO_SETENV@
 NO_ICONV=@NO_ICONV@
+NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
diff --git a/configure.ac b/configure.ac
index ed7cc89..ab516db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,6 +182,26 @@ AC_SUBST(NEEDS_LIBICONV)
 AC_SUBST(NO_ICONV)
 test -n "$NEEDS_LIBICONV" && LIBS="$LIBS -liconv"
 #
+# Define NO_DEFLATE_BOUND if deflateBound is missing from zlib.
+AC_DEFUN([ZLIBTEST_SRC], [
+#include <zlib.h>
+
+int main(void)
+{
+	deflateBound(0, 0);
+	return 0;
+}
+])
+AC_MSG_CHECKING([for deflateBound in -lz])
+old_LIBS="$LIBS"
+LIBS="$LIBS -lz"
+AC_LINK_IFELSE(ZLIBTEST_SRC,
+	[AC_MSG_RESULT([yes])],
+	[AC_MSG_RESULT([no])
+	NO_DEFLATE_BOUND=yes])
+LIBS="$old_LIBS"
+AC_SUBST(NO_DEFLATE_BOUND)
+#
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 AC_CHECK_LIB([c], [socket],
-- 
1.5.3.1

^ permalink raw reply related

* [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-11-07  5:18 UTC (permalink / raw)
  To: gitster; +Cc: git, johannes.schindelin, Shawn Bohrer

This replaces git-clean.sh with builtin-clean.c, and moves
git-clean.sh to the examples.

This also introduces a change in behavior where the -d parameter is
required to remove an entire directory of untracked files even when
the directory is passed as a path.  For example:

   git clean dir/

Now requires

   git clean -d dir/

if 'dir' only contains untracked files.  This is consistent with the
old behavior when two or more paths were specified.

Thanks to Johannes Schindelin for the conversion to using the
parse-options API.

Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---
 Makefile                                      |    3 +-
 builtin-clean.c                               |  134 +++++++++++++++++++++++++
 builtin.h                                     |    1 +
 git-clean.sh => contrib/examples/git-clean.sh |    0 
 git.c                                         |    1 +
 5 files changed, 138 insertions(+), 1 deletions(-)
 create mode 100644 builtin-clean.c
 rename git-clean.sh => contrib/examples/git-clean.sh (100%)

diff --git a/Makefile b/Makefile
index c427fee..932ff08 100644
--- a/Makefile
+++ b/Makefile
@@ -209,7 +209,7 @@ BASIC_LDFLAGS =
 
 SCRIPT_SH = \
 	git-bisect.sh git-checkout.sh \
-	git-clean.sh git-clone.sh git-commit.sh \
+	git-clone.sh git-commit.sh \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
 	git-repack.sh git-request-pull.sh \
@@ -325,6 +325,7 @@ BUILTIN_OBJS = \
 	builtin-check-attr.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
+	builtin-clean.o \
 	builtin-commit-tree.o \
 	builtin-count-objects.o \
 	builtin-describe.o \
diff --git a/builtin-clean.c b/builtin-clean.c
new file mode 100644
index 0000000..fb2feb5
--- /dev/null
+++ b/builtin-clean.c
@@ -0,0 +1,134 @@
+/*
+ * "git clean" builtin command
+ *
+ * Copyright (C) 2007 Shawn Bohrer
+ *
+ * Based on git-clean.sh by Pavel Roskin
+ */
+
+#include "builtin.h"
+#include "cache.h"
+#include "dir.h"
+#include "parse-options.h"
+
+static int force;
+
+static const char *const builtin_clean_usage[] = {
+	"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...",
+	NULL
+};
+
+static int git_clean_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "clean.requireforce")) {
+		force = !git_config_bool(var, value);
+	}
+	return 0;
+}
+
+int cmd_clean(int argc, const char **argv, const char *prefix)
+{
+	int j;
+	int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;
+	int ignored_only = 0, baselen = 0;
+	struct strbuf directory;
+	struct dir_struct dir;
+	const char *path = ".";
+	const char *base = "";
+	static const char **pathspec;
+	struct option options[] = {
+		OPT__QUIET(&quiet),
+		OPT__DRY_RUN(&show_only),
+		OPT_BOOLEAN('f', NULL, &force, "force"),
+		OPT_BOOLEAN('d', NULL, &remove_directories,
+				"remove whole directories"),
+		OPT_BOOLEAN('x', NULL, &ignored, "remove ignored files, too"),
+		OPT_BOOLEAN('X', NULL, &ignored_only,
+				"remove only ignored files"),
+		OPT_END()
+	};
+
+	git_config(git_clean_config);
+	argc = parse_options(argc, argv, options, builtin_clean_usage, 0);
+
+	memset(&dir, 0, sizeof(dir));
+	if (ignored_only) {
+		dir.show_ignored =1;
+		dir.exclude_per_dir = ".gitignore";
+	}
+
+	if (ignored && ignored_only)
+		die("-x and -X cannot be used together");
+
+	if (!show_only && !force)
+		die("clean.requireForce set and -n or -f not given; refusing to clean");
+
+	dir.show_other_directories = 1;
+
+	if (!ignored) {
+		dir.exclude_per_dir = ".gitignore";
+		if (!access(git_path("info/exclude"), F_OK)) {
+			char *exclude_path = git_path("info/exclude");
+			add_excludes_from_file(&dir, exclude_path);
+		}
+	}
+
+	pathspec = get_pathspec(prefix, argv);
+	read_cache();
+	read_directory(&dir, path, base, baselen, pathspec);
+	strbuf_init(&directory, 0);
+
+	for (j = 0; j < dir.nr; ++j) {
+		struct dir_entry *ent = dir.entries[j];
+		int len, pos;
+		struct cache_entry *ce;
+		struct stat st;
+
+		/*
+		 * Remove the '/' at the end that directory
+		 * walking adds for directory entries.
+		 */
+		len = ent->len;
+		if (len && ent->name[len-1] == '/')
+			len--;
+		pos = cache_name_pos(ent->name, len);
+		if (0 <= pos)
+			continue;	/* exact match */
+		pos = -pos - 1;
+		if (pos < active_nr) {
+			ce = active_cache[pos];
+			if (ce_namelen(ce) == len &&
+			    !memcmp(ce->name, ent->name, len))
+				continue; /* Yup, this one exists unmerged */
+		}
+
+		/* remove the files */
+		if (!lstat(ent->name, &st) && (S_ISDIR(st.st_mode))) {
+			strbuf_addstr(&directory, ent->name);
+			if (show_only && remove_directories) {
+				printf("Would remove %s\n", directory.buf);
+			} else if (quiet && remove_directories) {
+				remove_dir_recursively(&directory, 0);
+			} else if (remove_directories) {
+				printf("Removing %s\n", ent->name);
+				remove_dir_recursively(&directory, 0);
+			} else if (show_only) {
+				printf("Would not remove %s\n", directory.buf);
+			} else {
+				printf("Not removing %s\n", directory.buf);
+			}
+			strbuf_reset(&directory);
+		} else {
+			if (show_only) {
+				printf("Would remove %s\n", ent->name);
+				continue;
+			} else if (!quiet) {
+				printf("Removing %s\n", ent->name);
+			}
+			unlink(ent->name);
+		}
+	}
+
+	strbuf_release(&directory);
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 525107f..5476a92 100644
--- a/builtin.h
+++ b/builtin.h
@@ -24,6 +24,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
+extern int cmd_clean(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
diff --git a/git-clean.sh b/contrib/examples/git-clean.sh
similarity index 100%
rename from git-clean.sh
rename to contrib/examples/git-clean.sh
diff --git a/git.c b/git.c
index 204a6f7..3fa8e4d 100644
--- a/git.c
+++ b/git.c
@@ -293,6 +293,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: [PATCH] Reteach builtin-ls-remote to understand remotes
From: Junio C Hamano @ 2007-11-07  6:24 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071107012920.GA9961@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Prior to being made a builtin git-ls-remote understood that when
> it was given a remote name we wanted it to resolve that to the
> pre-configured URL and connect to that location.  That changed when
> it was converted to a builtin and many of my automation tools broke.

Thanks.  I will squash this in.

---
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
new file mode 100755
index 0000000..6ec5f7c
--- /dev/null
+++ b/t/t5512-ls-remote.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git ls-remote'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	>file &&
+	git add file &&
+	test_tick &&
+	git commit -m initial &&
+	git tag mark &&
+	git show-ref --tags -d | sed -e "s/ /	/" >expected.tag &&
+	(
+		echo "$(git rev-parse HEAD)	HEAD"
+		git show-ref -d	| sed -e "s/ /	/"
+	) >expected.all &&
+
+	git remote add self $(pwd)/.git
+
+'
+
+test_expect_success 'ls-remote --tags .git' '
+
+	git ls-remote --tags .git >actual &&
+	diff -u expected.tag actual
+
+'
+
+test_expect_success 'ls-remote .git' '
+
+	git ls-remote .git >actual &&
+	diff -u expected.all actual
+
+'
+
+test_expect_success 'ls-remote --tags self' '
+
+	git ls-remote --tags self >actual &&
+	diff -u expected.tag actual
+
+'
+
+test_expect_success 'ls-remote self' '
+
+	git ls-remote self >actual &&
+	diff -u expected.all actual
+
+'
+
+test_done

^ permalink raw reply related

* Re: [PATCH] git-fetch: avoid local fetching from alternate (again)
From: Junio C Hamano @ 2007-11-07  6:24 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071107024118.GA11043@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Back in e3c6f240fd9c5bdeb33f2d47adc859f37935e2df Junio taught
> git-fetch to avoid copying objects when we are fetching from
> a repository that is already registered as an alternate object
> database.  In such a case there is no reason to copy any objects
> as we can already obtain them through the alternate.

Well spotted.  It would be a good idea to commit the big comment
from contrib/examples/git-fetch.sh to fetch_local_nocopy()
function, which would have made us realize that the patch does
not refrain from applying this optimization even when shallow
is in effect.  But I think that is actually a good change.

The run-command change and the main part of the fix are
logically independent.

The regression the patch fixes should be testable with a
script.  Please have a new test for it.

Thanks.

^ permalink raw reply

* [PATCH] send-email: apply --suppress-from to S-o-b and cc-cmd
From: Uwe Kleine-König @ 2007-11-07  7:34 UTC (permalink / raw)
  To: git; +Cc: Uwe Kleine-König, Ryan Anderson

From: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>

Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
Cc: Ryan Anderson <ryan@michonline.com>
---
Hello,

I don't see the sense in adding the sender to Cc: from Signed-off-by
lines but not from From:.  If someone is convinced it makes sense, I'm
willing to send a new patch that uses a different option.

Cc'd Ryan as the author of --suppress-from.

I already tried to send this from my work's email account, but it seems
to have problems to send mails to vger (and lists.arm.linux.org.uk).
Sorry Ryan if you get this mail once more.

Best regards
Uwe

 Documentation/git-send-email.txt |    3 +--
 git-send-email.perl              |    5 +++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index e38b702..659215a 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -113,8 +113,7 @@ The --cc option must be repeated for each user you want on the cc list.
 	is not set, this will be prompted for.
 
 --suppress-from, --no-suppress-from::
-        If this is set, do not add the From: address to the cc: list, if it
-        shows up in a From: line.
+        If this is set, do not add the From: address to the cc: list.
         Default is the value of 'sendemail.suppressfrom' configuration value;
         if that is unspecified, default to --no-suppress-from.
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 96051bc..f4b8f96 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -88,8 +88,7 @@ Options:
 
    --smtp-ssl     If set, connects to the SMTP server using SSL.
 
-   --suppress-from Suppress sending emails to yourself if your address
-                  appears in a From: line. Defaults to off.
+   --suppress-from Suppress sending emails to yourself. Defaults to off.
 
    --thread       Specify that the "In-Reply-To:" header should be set on all
                   emails. Defaults to on.
@@ -730,6 +729,7 @@ foreach my $t (@files) {
 			if (/^(Signed-off-by|Cc): (.*)$/i && $signed_off_cc) {
 				my $c = $2;
 				chomp $c;
+				next if ($c eq $sender and $suppress_from);
 				push @cc, $c;
 				printf("(sob) Adding cc: %s from line '%s'\n",
 					$c, $_) unless $quiet;
@@ -745,6 +745,7 @@ foreach my $t (@files) {
 			my $c = $_;
 			$c =~ s/^\s*//g;
 			$c =~ s/\n$//g;
+			next if ($c eq $sender and $suppress_from);
 			push @cc, $c;
 			printf("(cc-cmd) Adding cc: %s from: '%s'\n",
 				$c, $cc_cmd) unless $quiet;
-- 
1.5.3.4

^ permalink raw reply related

* Re: git pull opinion
From: Uwe Kleine-König @ 2007-11-07  7:06 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Aghiles, git
In-Reply-To: <4730AD48.2050907@obry.net>

Hello,

> I'm using:
> 
> $ git config --global alias.update '!git stash && git pull && git stash apply'
I wonder how this works, if the merge produces conflicts...

Best regards
Uwe

-- 
Uwe Kleine-König

http://www.google.com/search?q=1+year+in+days

^ 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