Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.
From: Shawn O. Pearce @ 2008-09-30 23:16 UTC (permalink / raw)
  To: Raphael Zimmerer; +Cc: git
In-Reply-To: <1222816390-9141-1-git-send-email-killekulla@rdrz.de>

Raphael Zimmerer <killekulla@rdrz.de> wrote:
> Here's a trivial patch that adds "-Z" and "--null" options to "git
> grep" equal to GNU's grep.
> So things like 'git grep -l -Z "$FOO" | xargs -0 sed -i "s/$FOO/$BOO/"'
> are more comfortable.

Elsewhere in Git we call this "-z", like "git ls-tree -z", "git
log -z".  Should we match grep or git convention here?
 
>  Documentation/git-grep.txt |    6 ++++++
>  builtin-grep.c             |    7 +++++++
>  grep.c                     |   14 +++++++++++---
>  grep.h                     |    1 +

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] parse-opt: migrate fmt-merge-msg.
From: Shawn O. Pearce @ 2008-09-30 22:46 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git, gitster
In-Reply-To: <20080930211643.GA16879@artemis.corp>

Pierre Habouzit <madcoder@debian.org> wrote:
> On Tue, Sep 30, 2008 at 07:10:14PM +0000, Shawn O. Pearce wrote:
> > Pierre Habouzit <madcoder@debian.org> wrote:
> > > On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> > > > Pierre Habouzit <madcoder@debian.org> wrote:
> > > > > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > 
> > I'd appreciate an updated series if you can send it.  am -3 isn't
> > "easily" applying it.
> 
> Okay, I will then, but FWIW it means that when you'll try to merge this
> in next it'll conflict at that time, so I'm not sure there's a huge win
> for you at that point.

It may actually be a good idea to rebase this against master.

Reading Junio's notes for sg/merge-options (the branch this conflict
is coming out of) it sounds like we'd want to revert that anyway.
Its been around since April and Junio was talking about it needing
to be in a 1.7.0 release.  Its not going to graduate anytime soon.

IOW I'm quite tempted to revert sg/merge-options and cancel the
branch out of next.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] doc: enhance git describe --tags help
From: Shawn O. Pearce @ 2008-09-30 22:26 UTC (permalink / raw)
  To: Uwe Kleine-KKKnig
  Cc: Pierre Habouzit, Erez Zilber, git@vger.kernel.org, open-iscsi,
	Junio C Hamano, Andreas Ericsson
In-Reply-To: <20080930221453.GA13659@strlen.de>

Uwe Kleine-KKKnig <ukleinek@strlen.de> wrote:
> On Tue, Sep 30, 2008 at 12:04:49PM -0700, Shawn O. Pearce wrote:
> > Uwe Kleine-KKKnig <ukleinek@strlen.de> wrote:
> > >
> > > IMHO --tags should behave as Erez expected (because it's what I
> > > expected, too).
> > 
> > I don't disagree.  I've been tempted to write a patch to change the
> > behavior of git-describe so that --tags and --all control what names
> > are inserted into the candidate list, but don't control the ordering
> > of their selection.
> > 
> > I think this is all that is needed to make the behavior do what you
> > and Erez expected.  But its a pretty big change in the results if
> > you are passing in --all or --tags today.
>
> But it matches the documentation, and the expectations of Erez, me and
> (at least initially) Pierre.
> 
> My POV is still:  If you pass --all or --tags you have to be able to
> handle if a lw tag is used in the answer.

I was agreeing with you.  I've long felt that the --tags and --all
behavior of git-describe was wrong.  But something in the back of
my mind tells me Junio felt otherwise.

Its a change in behavior.  Today users are getting annotated tags
back from `git describe --tags` even if lightweight tags are closer.
Once this code change is in they'll start to get lightweight tags.

Previously `git describe --tags` never gave a lightweight tag if
there was at least one annotated tag in the history.  Now it will
start to give the lightweight tags.  Some users may see that as a
breakage.  Especially after the 1.6 "dashless" change...

> > -static int all;	/* Default to annotated tags only */
> > -static int tags;	/* But allow any tags if --tags is specified */
> > +static int all;	/* Any valid ref can be used */
> > +static int tags;	/* Either lightweight or annotated tags */
>
> Mmmh, IMHO the comment for tags is misleading, its either annotated only
> or both.

Oh, yes, right.  Thanks.  I'll clean it up.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] doc: enhance git describe --tags help
From: Uwe Kleine-König @ 2008-09-30 22:14 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Pierre Habouzit, Erez Zilber, git@vger.kernel.org, open-iscsi,
	Junio C Hamano, Andreas Ericsson
In-Reply-To: <20080930190449.GG21310@spearce.org>

Hi Shawn,

On Tue, Sep 30, 2008 at 12:04:49PM -0700, Shawn O. Pearce wrote:
> Uwe Kleine-KKKnig <ukleinek@strlen.de> wrote:
> > On Mon, Sep 29, 2008 at 08:01:27AM -0700, Shawn O. Pearce wrote:
> > > --tags::
> > > 	If a lightweight tag exactly matches, output it.  If no
> > > 	annotated tag is found in the ancestry but a lightweight
> > > 	tag is found, output the lightweight tag.
> >
> > IMHO --tags should behave as Erez expected (because it's what I
> > expected, too).  As --tags currently behaves it's only usable in very
> > rare cases (most of the time it only makes a difference on repos without
> > any annotated tag).
> > 
> > When do you pass --tags?  Only if a lightweight tag is OK for an answer.
> > And then I would prefer a "near" lightweight tag to a "farer" annotated
> > one.
> 
> I don't disagree.  I've been tempted to write a patch to change the
> behavior of git-describe so that --tags and --all control what names
> are inserted into the candidate list, but don't control the ordering
> of their selection.
> 
> I think this is all that is needed to make the behavior do what you
> and Erez expected.  But its a pretty big change in the results if
> you are passing in --all or --tags today.
But it matches the documentation, and the expectations of Erez, me and
(at least initially) Pierre.

My POV is still:  If you pass --all or --tags you have to be able to
handle if a lw tag is used in the answer.

> -static int all;	/* Default to annotated tags only */
> -static int tags;	/* But allow any tags if --tags is specified */
> +static int all;	/* Any valid ref can be used */
> +static int tags;	/* Either lightweight or annotated tags */
Mmmh, IMHO the comment for tags is misleading, its either annotated only
or both.

Best regards and thanks,
Uwe

^ permalink raw reply

* Re: [PATCH] parse-opt: migrate fmt-merge-msg.
From: Pierre Habouzit @ 2008-09-30 21:16 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20080930191014.GH21310@spearce.org>

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

On Tue, Sep 30, 2008 at 07:10:14PM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> > > Pierre Habouzit <madcoder@debian.org> wrote:
> > > > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > > 
> > > Near as I can tell, this is based upon a merge commit in next.
> > 
> > Hmm I've always sent my patches this way, and I believe you can git am
> > -3 them on top of master easily. I can send you the updated series if
> > you want.
> 
> I'd appreciate an updated series if you can send it.  am -3 isn't
> "easily" applying it.  Here I define "easy" as "the patch applies
> without me needing to resolve conflicts":
> 
>  $ git co -b ph/parseopt master
>  $ git am -3 -s X
>  Applying: parse-opt: migrate fmt-merge-msg.
>  error: patch failed: builtin-fmt-merge-msg.c:5
>  error: builtin-fmt-merge-msg.c: patch does not apply
>  Using index info to reconstruct a base tree...
>  Falling back to patching base and 3-way merge...
>  CONFLICT (content): Merge conflict in builtin-fmt-merge-msg.c
>  Recorded preimage for 'builtin-fmt-merge-msg.c'
>  Failed to merge in the changes.
>  Patch failed at 0001.
>  When you have resolved this problem run "git am -3 --resolved".
>  If you would prefer to skip this patch, instead run "git am -3 --skip".
>  To restore the original branch and stop patching run "git am -3 --abort".

Okay, I will then, but FWIW it means that when you'll try to merge this
in next it'll conflict at that time, so I'm not sure there's a huge win
for you at that point.

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

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

^ permalink raw reply

* Re: [PATCH 1/6] gitweb: action in path with use_pathinfo
From: Jakub Narebski @ 2008-09-30 21:00 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Lea Wiemann
In-Reply-To: <cb7bb73a0809300553o7496f4c1me14ddf55b31fe4a6@mail.gmail.com>

On Tue, 30 September 2008, Giuseppe Bilotta wrote:
> On Tue, Sep 30, 2008 at 1:22 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Tue, 30 Sep 2008, Giuseppe "Oblomov" Bilotta wrote:
>>> On Tue, Sep 30, 2008 at 10:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:

>>>> Or we could just scrap and revert adding href(..., -replay=>1).
>>>> There is much trouble with getting it right and performing well,
>>>> and it is less useful than I thought (at least now).
>>>
>>> Dunno, the idea in itself is not bad. We just have to get it right ;)
>>
>> It is not easy to get it right, especially that there are multivalued
>> parameters like @extra_options, see e.g. commit 7863c612
> 
> So we just let values be arrays. Or isn't this enough?

I don't think it would be that simple.


Let me elaborate a bit about complications and difficulties
within href() code.

First there are two names of parameters: the short name like 'a',
or 'h', or 'f' which is used in links to make them short (and
which is a bit historical legacy), and the long names like 'action',
'hash' or 'file_name' which are used for readability; then there are
also variables which hold values of parameters, like $action,
$hash and $file_name (which were source of long names for parameters).
href() has to provide translation between those two (well, three)
names; long names are used as names of "named parameters" to href(),
while short names are used when generating URL; %mapping provides
mapping between those two.

Second, href() must distinguish between gitweb options/parameters
like 'file_name'=>$file_name and extra options like '-full'=>1
or '-replay'=>1; additionally we want to have options output in
some definite order, with more important options first.  This
is provided by %mapping (to filter out) and @mapping (to sort).

Third, there are various sources of values of parameters, and
parameters used.  There are parameters specified explicitly in
href() call, and there are also implicit parameters: 'project'
parameter is implicitly added as it is needed in almost all
gitweb links (you can override it and generate projectless link by
using 'project'=>undef), and for '-replay'=>1 all parameters from
current URL are added.  Parameters specified explicitly have preference
over implicit or '-replay' ones.  

Now for '-replay' parameters might come from CGI query string, from
path_info part of URL, and perhaps in the future also from command
line.  To avoid duplicated code and other problems we should either
get parameters from variables (like in your code, but which doesn't
cover @extra_options well, but it could; or using "long name" to
variable ref mapping), or have some place where we save parameters
as we extract it from CGI query string, from path_info part of URL,
and in the future probably also from command line options/parameters.

Fourth, there is a little matter of _some_ parameters be multivalued;
currently it is only @extra_options / 'opt', but this may change in
the future, while _most_ are ordinary scalar values.  Printing arrayref
isn't something we want to do...


That is third and fourth which caused problems in the past with
href(..., -replay=>1)...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH, resent] fix openssl headers conflicting with custom SHA1 implementations
From: Shawn O. Pearce @ 2008-09-30 20:51 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0809301645340.3635@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:
> On Tue, 30 Sep 2008, Shawn O. Pearce wrote:
> > Nicolas Pitre <nico@cam.org> wrote:
> > > On ARM I have the following compilation errors:
> > ...
> > > This is a bit ugly but given the rat nest of system includes we have I 
> > > don't know how to solve this any better.
> > 
> > Hmm.  On Linux x86 with your change:
> > 
> > In file included from imap-send.c:32:
> > cache.h: In function 'create_ce_mode':
> > cache.h:186: error: 'S_IFLNK' undeclared (first use in this function)
> > 
> > I think that last hunk which moved the openssl includes to be before
> > cache.h is the problem.
> 
> With that, my build on ARM bombs out with:
> 
>     CC imap-send.o
> In file included from /usr/include/openssl/kssl.h:71,
>                  from /usr/include/openssl/ssl.h:191,
>                  from imap-send.c:30:
> /usr/include/ctype.h:102: error: expected expression before ']' token

*sigh*

Yea, its a bit ugly due to the rats nest of system includes.
Right now I don't see how we can include your patch, it breaks a
major platform for us.  But obviously my "fix" is also bogus and
won't get ARM working again.

Any other ideas we can try?  'cause I don't have any right now.  :-|

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH, resent] fix openssl headers conflicting with custom SHA1 implementations
From: Nicolas Pitre @ 2008-09-30 20:46 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20080930203928.GN21310@spearce.org>

On Tue, 30 Sep 2008, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@cam.org> wrote:
> > On ARM I have the following compilation errors:
> ...
> > This is a bit ugly but given the rat nest of system includes we have I 
> > don't know how to solve this any better.
> 
> Hmm.  On Linux x86 with your change:
> 
> In file included from imap-send.c:32:
> cache.h: In function 'create_ce_mode':
> cache.h:186: error: 'S_IFLNK' undeclared (first use in this function)
> 
> I think that last hunk which moved the openssl includes to be before
> cache.h is the problem.  This squashed in seems to make it my build
> work again:
> 
> diff --git a/imap-send.c b/imap-send.c
> index 01f1c9a..6b8c66e 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -22,6 +22,7 @@
>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> +#include "cache.h"
>  #ifdef NO_OPENSSL
>  typedef void *SSL;
>  #else
> @@ -29,8 +30,6 @@ typedef void *SSL;
>  #include <openssl/err.h>
>  #endif
>  
> -#include "cache.h"
> -
>  struct store_conf {
>  	char *name;
>  	const char *path; /* should this be here? its interpretation is driver-specific */

With that, my build on ARM bombs out with:

    CC imap-send.o
In file included from /usr/include/openssl/kssl.h:71,
                 from /usr/include/openssl/ssl.h:191,
                 from imap-send.c:30:
/usr/include/ctype.h:102: error: expected expression before ']' token
/usr/include/ctype.h:102: error: expected ')' before '!=' token
/usr/include/ctype.h:103: error: expected expression before ']' token
/usr/include/ctype.h:103: error: expected ')' before '!=' token
/usr/include/ctype.h:105: error: expected expression before ']' token
/usr/include/ctype.h:105: error: expected ')' before '!=' token
/usr/include/ctype.h:110: error: expected expression before ']' token
/usr/include/ctype.h:110: error: expected ')' before '!=' token
/usr/include/ctype.h:116: error: expected declaration specifiers or '...' before '(' token
/usr/include/ctype.h:116: error: expected declaration specifiers or '...' before numeric constant
/usr/include/ctype.h:119: error: expected declaration specifiers or '...' before '(' token
/usr/include/ctype.h:119: error: expected declaration specifiers or '...' before numeric constant
/usr/include/ctype.h:191: error: expected declaration specifiers or '...' before '(' token
/usr/include/ctype.h:191: error: expected declaration specifiers or '...' before numeric constant
/usr/include/ctype.h:192: error: redefinition of 'sane_case'
git-compat-util.h:338: error: previous definition of 'sane_case' was here
/usr/include/ctype.h: In function 'sane_case':
/usr/include/ctype.h:192: error: number of arguments doesn't match prototype
git-compat-util.h:338: error: prototype declaration
/usr/include/ctype.h:193: error: '__c' undeclared (first use in this function)
/usr/include/ctype.h:193: error: (Each undeclared identifier is reported only once
/usr/include/ctype.h:193: error: for each function it appears in.)
/usr/include/ctype.h: At top level:
/usr/include/ctype.h:197: error: expected declaration specifiers or '...' before '(' token
/usr/include/ctype.h:197: error: expected declaration specifiers or '...' before numeric constant
/usr/include/ctype.h:198: error: redefinition of 'sane_case'
/usr/include/ctype.h:192: error: previous definition of 'sane_case' was here
/usr/include/ctype.h: In function 'sane_case':
/usr/include/ctype.h:198: error: number of arguments doesn't match prototype
/usr/include/ctype.h:192: error: prototype declaration
/usr/include/ctype.h:199: error: '__c' undeclared (first use in this function)
make: *** [imap-send.o] Error 1


Nicolas

^ permalink raw reply

* Re: [PATCH, resent] fix openssl headers conflicting with custom SHA1 implementations
From: Shawn O. Pearce @ 2008-09-30 20:39 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0809301621091.3635@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:
> On ARM I have the following compilation errors:
...
> This is a bit ugly but given the rat nest of system includes we have I 
> don't know how to solve this any better.

Hmm.  On Linux x86 with your change:

In file included from imap-send.c:32:
cache.h: In function 'create_ce_mode':
cache.h:186: error: 'S_IFLNK' undeclared (first use in this function)

I think that last hunk which moved the openssl includes to be before
cache.h is the problem.  This squashed in seems to make it my build
work again:

diff --git a/imap-send.c b/imap-send.c
index 01f1c9a..6b8c66e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,6 +22,7 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include "cache.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #else
@@ -29,8 +30,6 @@ typedef void *SSL;
 #include <openssl/err.h>
 #endif
 
-#include "cache.h"
-
 struct store_conf {
 	char *name;
 	const char *path; /* should this be here? its interpretation is driver-specific */

-- 
Shawn.

^ permalink raw reply related

* [PATCH] Use "git_config_string" to simplify "builtin-gc.c" code where "prune_expire" is set
From: David Bryson @ 2008-09-30 20:28 UTC (permalink / raw)
  To: git
In-Reply-To: <20080930190549.GA9869@eratosthenes.cryptobackpack.org>


Signed-off-by: David Bryson <david@statichacks.org>
---
Manage to fix the const char ** compile error, third time is a
harm(hopefully!)

 builtin-gc.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index fac200e..7af65bb 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,7 +26,7 @@ static int pack_refs = 1;
 static int aggressive_window = -1;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static char *prune_expire = "2.weeks.ago";
+static const char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
@@ -57,15 +57,12 @@ static int gc_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "gc.pruneexpire")) {
-		if (!value)
-			return config_error_nonbool(var);
-		if (strcmp(value, "now")) {
+		if (value && strcmp(value, "now")) {
 			unsigned long now = approxidate("now");
 			if (approxidate(value) >= now)
 				return error("Invalid %s: '%s'", var, value);
 		}
-		prune_expire = xstrdup(value);
-		return 0;
+		return git_config_string(&prune_expire, var, value);
 	}
 	return git_default_config(var, value, cb);
 }
-- 
1.6.0.2

^ permalink raw reply related

* Re: [PATCH 4/4 v2] cygwin: Use native Win32 API for stat
From: Shawn O. Pearce @ 2008-09-30 20:26 UTC (permalink / raw)
  To: Marcus Griep
  Cc: Dmitry Potapov, Johannes Sixt, git, Junio C Hamano, Alex Riesen
In-Reply-To: <48E23E5B.7020404@griep.us>

Marcus Griep <marcus@griep.us> wrote:
> Dmitry Potapov wrote:
> > lstat/stat functions in Cygwin are very slow, because they try to emulate
> > some *nix things that Git does not actually need. This patch adds Win32
> > specific implementation of these functions for Cygwin.
> 
> Can't wait to see this patch in next or master!  If you recall my benchmarks
> from earlier, the speed-up is pretty good for cygwin users working with
> large repositories.
> 
> > Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
> 
> Thanks for the work, Dmitry!

Thanks folks.  I'm scheduling this for 'next'.  Lets see how
it goes...

-- 
Shawn.

^ permalink raw reply

* [PATCH, resent] fix openssl headers conflicting with custom SHA1 implementations
From: Nicolas Pitre @ 2008-09-30 20:22 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

On ARM I have the following compilation errors:

    CC fast-import.o
In file included from cache.h:8,
                 from builtin.h:6,
                 from fast-import.c:142:
arm/sha1.h:14: error: conflicting types for 'SHA_CTX'
/usr/include/openssl/sha.h:105: error: previous declaration of 'SHA_CTX' was here
arm/sha1.h:16: error: conflicting types for 'SHA1_Init'
/usr/include/openssl/sha.h:115: error: previous declaration of 'SHA1_Init' was here
arm/sha1.h:17: error: conflicting types for 'SHA1_Update'
/usr/include/openssl/sha.h:116: error: previous declaration of 'SHA1_Update' was here
arm/sha1.h:18: error: conflicting types for 'SHA1_Final'
/usr/include/openssl/sha.h:117: error: previous declaration of 'SHA1_Final' was here
make: *** [fast-import.o] Error 1

This is because openssl header files are always included in 
git-compat-util.h since commit 684ec6c63c whenever NO_OPENSSL is not 
set, which somehow brings in <openssl/sha1.h> clashing with the custom 
ARM version.  Compilation of git is probably broken on PPC too for the 
same reason.

Turns out that the only file requiring openssl/ssl.h and openssl/err.h 
is imap-send.c.  But only moving those problematic includes there 
doesn't solve the issue as it also includes cache.h which brings in the 
conflicting header.  So also conditionally including SHA1_HEADER allows 
for not including it when compiling imap-send.c.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

This is a bit ugly but given the rat nest of system includes we have I 
don't know how to solve this any better.

diff --git a/Makefile b/Makefile
index 3c0664a..a0f86dd 100644
--- a/Makefile
+++ b/Makefile
@@ -1242,6 +1242,9 @@ endif
 git-%$X: %.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
+imap-send.o: imap-send.c GIT-CFLAGS
+	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -USHA1_HEADER $<
+
 git-imap-send$X: imap-send.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
diff --git a/cache.h b/cache.h
index de8c2b6..d93e086 100644
--- a/cache.h
+++ b/cache.h
@@ -5,7 +5,10 @@
 #include "strbuf.h"
 #include "hash.h"
 
+#ifdef SHA1_HEADER
 #include SHA1_HEADER
+#endif
+
 #include <zlib.h>
 
 #if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200
diff --git a/git-compat-util.h b/git-compat-util.h
index db2836f..044f62b 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -99,11 +99,6 @@
 #include <iconv.h>
 #endif
 
-#ifndef NO_OPENSSL
-#include <openssl/ssl.h>
-#include <openssl/err.h>
-#endif
-
 /* On most systems <limits.h> would have given us this, but
  * not on some systems (e.g. GNU/Hurd).
  */
diff --git a/imap-send.c b/imap-send.c
index af7e08c..01f1c9a 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,11 +22,15 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include "cache.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
+#else
+#include <openssl/ssl.h>
+#include <openssl/err.h>
 #endif
 
+#include "cache.h"
+
 struct store_conf {
 	char *name;
 	const char *path; /* should this be here? its interpretation is driver-specific */




Nicolas

^ permalink raw reply related

* Re: [GUILT] Use git_editor
From: Josef 'Jeff' Sipek @ 2008-09-30 20:22 UTC (permalink / raw)
  To: Alan Jenkins; +Cc: git
In-Reply-To: <48E27A87.9060205@tuffmail.co.uk>

On Tue, Sep 30, 2008 at 08:14:15PM +0100, Alan Jenkins wrote:
> Josef 'Jeff' Sipek wrote:
> > On Tue, Sep 30, 2008 at 07:43:50PM +0100, Alan Jenkins wrote:
> >   
> >> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> >>
> >> diff --git a/guilt b/guilt
> >> index 1cef7b9..8d023e9 100755
> >> --- a/guilt
> >> +++ b/guilt
> >> @@ -693,10 +693,6 @@ fi
> >>  series="$GUILT_DIR/$branch/series"
> >>  applied="$GUILT_DIR/$branch/status"
> >>  
> >> -# determine an editor to use for anything interactive (fall back to vi)
> >> -editor="vi"
> >> -[ ! -z "$EDITOR" ] && editor="$EDITOR"
> >> -
> >>     
> >
> > Looks good from a quick glance. Any chance there's a git_pager?
> >
> > Josef 'Jeff' Sipek.
> 
> Not in git-sh-setup.  I did check :-).

Interesting! There are definitely places in git that invoke _a_
pager...maybe there's a patch to git that need creating :)

Josef 'Jeff' Sipek.

-- 
Failure is not an option,
It comes bundled with your Microsoft product.

^ permalink raw reply

* Re: [PATCH] Use "git_config_string" to simplify "builtin-gc.c" code where "prune_expire" is set
From: Shawn O. Pearce @ 2008-09-30 20:04 UTC (permalink / raw)
  To: David Bryson; +Cc: git
In-Reply-To: <20080930195355.GA14499@eratosthenes.cryptobackpack.org>

David Bryson <david@statichacks.org> wrote:
> diff --git a/builtin-gc.c b/builtin-gc.c
> index fac200e..6260652 100644
> --- a/builtin-gc.c
> +++ b/builtin-gc.c
> @@ -57,15 +57,12 @@ static int gc_config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  	if (!strcmp(var, "gc.pruneexpire")) {
> -		if (!value)
> -			return config_error_nonbool(var);
> -		if (strcmp(value, "now")) {
> +		if (value && strcmp(value, "now")) {
>  			unsigned long now = approxidate("now");
>  			if (approxidate(value) >= now)
>  				return error("Invalid %s: '%s'", var, value);
>  		}
> -		prune_expire = xstrdup(value);
> -		return 0;
> +		return git_config_string(&prune_expire, var, value);

The message formatting was a lot better.  But:

builtin-gc.c:65: warning: passing argument 1 of 'git_config_string' from incompatible pointer type

>From http://git.or.cz/gitwiki/Janitor:

  This is because the first argument to "git_config_string" should
  be a "const char **", not a "char **". There will be a compile
  time warning if a "char **" is passed instead of a "const char **"
  and the patch will be rejected. (And no, casting the "char **"
  into a "const char **" is not a good solution either.)"

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] git-gui: Implement a 'clone' subcommand
From: Shawn O. Pearce @ 2008-09-30 19:58 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Petr Baudis
In-Reply-To: <20080930195330.GA4413@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Petr Baudis <pasky@suse.cz> wrote:
> > This enables git-gui to be started with the clone dialog opened right
> > away, possibly with the URL prefilled when it is passed as another
> > argument. git-gui can then be e.g. registered as the git:// protocol
> > handler.
> > 
> > This is just a simple implementation - we construct the front actions
> > page, then throw it away immediately; I wanted to avoid unnecessary
> > refactoring and complication of the code, though.
> > 
> > Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
> > 
> > ---
> >  Documentation/git-gui.txt         |    5 +++++
> >  git-gui/git-gui.sh                |   21 ++++++++++++++++++---
> >  git-gui/lib/choose_repository.tcl |   11 ++++++++++-
> >  3 files changed, 33 insertions(+), 4 deletions(-)
> 
> This looks fine [...]

I take that back.  "git gui clone git://repo.or.cz/alt-git.git"
didn't show the URL in the clone dialog for me.  This is on top
of your previous series, including the "locator" feature, and
with two locators configured in my .git/config.

-- 
Shawn.

^ permalink raw reply

* [PATCH] Improve git-log documentation wrt file filters
From: martin f. krafft @ 2008-09-30 19:57 UTC (permalink / raw)
  To: git; +Cc: martin f. krafft
In-Reply-To: <20080930192553.GI21310@spearce.org>

The need for "--" in the git-log synopsis was previously unclear and
confusing. This patch makes it a little clearer.

Thanks to hyy <yiyihu@gmail.com> for his help.

Signed-off-by: martin f. krafft <madduck@madduck.net>
---
 Documentation/git-log.txt |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 0446bad..5a3a625 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -8,7 +8,7 @@ git-log - Show commit logs
 
 SYNOPSIS
 --------
-'git log' <option>...
+'git log' [<options>] [<since>..<until>] [[--] <path>...]
 
 DESCRIPTION
 -----------
@@ -57,8 +57,10 @@ include::diff-options.txt[]
 	Note that only message is considered, if also a diff is shown
 	its size is not included.
 
-<path>...::
-	Show only commits that affect any of the specified paths.
+[--] <path>...::
+	Show only commits that affect any of the specified paths. To
+	prevent confusion with options and branch names, paths may need to be
+	prefixed with "-- " to separate them from options or refnames.
 
 
 include::rev-list-options.txt[]
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] Use "git_config_string" to simplify "builtin-gc.c" code where "prune_expire" is set
From: David Bryson @ 2008-09-30 19:53 UTC (permalink / raw)
  To: git
In-Reply-To: <20080930190549.GA9869@eratosthenes.cryptobackpack.org>


Signed-off-by: David Bryson <david@statichacks.org>
---
 builtin-gc.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index fac200e..6260652 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -57,15 +57,12 @@ static int gc_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "gc.pruneexpire")) {
-		if (!value)
-			return config_error_nonbool(var);
-		if (strcmp(value, "now")) {
+		if (value && strcmp(value, "now")) {
 			unsigned long now = approxidate("now");
 			if (approxidate(value) >= now)
 				return error("Invalid %s: '%s'", var, value);
 		}
-		prune_expire = xstrdup(value);
-		return 0;
+		return git_config_string(&prune_expire, var, value);
 	}
 	return git_default_config(var, value, cb);
 }
-- 
1.6.0.2

^ permalink raw reply related

* Re: [PATCH] git-gui: Implement a 'clone' subcommand
From: Shawn O. Pearce @ 2008-09-30 19:53 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Petr Baudis
In-Reply-To: <1222793501-17997-1-git-send-email-pasky@suse.cz>

Petr Baudis <pasky@suse.cz> wrote:
> This enables git-gui to be started with the clone dialog opened right
> away, possibly with the URL prefilled when it is passed as another
> argument. git-gui can then be e.g. registered as the git:// protocol
> handler.
> 
> This is just a simple implementation - we construct the front actions
> page, then throw it away immediately; I wanted to avoid unnecessary
> refactoring and complication of the code, though.
> 
> Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
> 
> ---
>  Documentation/git-gui.txt         |    5 +++++
>  git-gui/git-gui.sh                |   21 ++++++++++++++++++---
>  git-gui/lib/choose_repository.tcl |   11 ++++++++++-
>  3 files changed, 33 insertions(+), 4 deletions(-)

This looks fine, except for the diffstat.  You can't patch
the docs and the code in the same patch as they are in two
different repositories...  :-|

Also, I don't know if you've noticed but I think tg is
sending duplicate "To" headers in the messages:

	From: Petr Baudis <pasky@suse.cz>
*	To: git@vger.kernel.org
	Cc: spearce@spearce.org, Petr Baudis <petr.baudis@novartis.com>
	Subject: [PATCH] git-gui: Implement a 'clone' subcommand
	Date: Tue, 30 Sep 2008 18:51:41 +0200
	Message-Id: <1222793501-17997-1-git-send-email-pasky@suse.cz>
	X-Mailer: git-send-email 1.5.6.3.392.g292f1
*	To: git@vger.kernel.org
	Sender: git-owner@vger.kernel.org
	Precedence: bulk
	List-ID: <git.vger.kernel.org>
	X-Mailing-List: git@vger.kernel.org
 
-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Use "git_config_string" to simplify "builtin-gc.c" code where "prune_expire" is set
From: Shawn O. Pearce @ 2008-09-30 19:37 UTC (permalink / raw)
  To: David Bryson; +Cc: git
In-Reply-To: <20080930190549.GA9869@eratosthenes.cryptobackpack.org>

David Bryson <david@statichacks.org> wrote:
> Hi all,
> This is a quick janitor patch and my first patch sent to the git mailing
> list.  Please let me know if I have done any process of the patch
> submission wrong, and if I got the Janitorial task done in the proper
> fashion.

This part of the message probably should go after the "---" line.
Everything above "---" goes into the commit message, everything
after it is automatically cut by git-am.

This sort of "Hi, I'm new" isn't very useful in the long term in
the project logs, but its still useful to introduce yourself.  ;-)
 
> Signed-off-by: David Bryson <david@statichacks.org>
> ---
>  builtin-gc.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin-gc.c b/builtin-gc.c
> index fac200e..6260652 100644
> --- a/builtin-gc.c
> +++ b/builtin-gc.c
> @@ -57,15 +57,12 @@ static int gc_config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  	if (!strcmp(var, "gc.pruneexpire")) {
> -		if (!value)
> -			return config_error_nonbool(var);
> -		if (strcmp(value, "now")) {
> +		if (value && strcmp(value, "now")) {
>  			unsigned long now = approxidate("now");
>  			if (approxidate(value) >= now)
>  				return error("Invalid %s: '%s'", var, value);
>  		}
> -		prune_expire = xstrdup(value);
> -		return 0;
> +		return git_config_string(&prune_expire, var, value);
>  	}
>  	return git_default_config(var, value, cb);
>  }

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Improve git-log documentation wrt file filters
From: Shawn O. Pearce @ 2008-09-30 19:25 UTC (permalink / raw)
  To: martin f. krafft; +Cc: git
In-Reply-To: <1222785372-32336-1-git-send-email-madduck@madduck.net>

"martin f. krafft" <madduck@madduck.net> wrote:
> The need for "--" in the git-log synopsis was previously unclear and
> confusing. This patch makes it a little clearer.
 
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 0446bad..b7409f8 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -8,7 +8,7 @@ git-log - Show commit logs
>  
>  SYNOPSIS
>  --------
> -'git log' <option>...
> +'git log' [<options>] [<since>..<until>] [[--] <path>...]

This part is really useful.

>  DESCRIPTION
>  -----------
> @@ -57,8 +57,10 @@ include::diff-options.txt[]
>  	Note that only message is considered, if also a diff is shown
>  	its size is not included.
>  
> -<path>...::
> -	Show only commits that affect any of the specified paths.
> +-- <path>...::
> +	Show only commits that affect any of the specified paths. To
> +        prevent confusion with options and branch names, paths should be
> +        prefixed with "-- " to separate them from options or refnames.

Indentation here looks wrong, especially on the last two lines.

I'm also not sure we want to say "-- <path>" in the header for
this section.  The -- is optional but I'm reading this as though it
is required.  Maybe "... paths can be prefixed with -- ..." instead?

-- 
Shawn.

^ permalink raw reply

* [PATCH] Use "git_config_string" to simplify "builtin-gc.c" code where "prune_expire" is set
From: David Bryson @ 2008-09-30 19:05 UTC (permalink / raw)
  To: git

Hi all,
This is a quick janitor patch and my first patch sent to the git mailing
list.  Please let me know if I have done any process of the patch
submission wrong, and if I got the Janitorial task done in the proper
fashion.

Signed-off-by: David Bryson <david@statichacks.org>
---
 builtin-gc.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index fac200e..6260652 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -57,15 +57,12 @@ static int gc_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "gc.pruneexpire")) {
-		if (!value)
-			return config_error_nonbool(var);
-		if (strcmp(value, "now")) {
+		if (value && strcmp(value, "now")) {
 			unsigned long now = approxidate("now");
 			if (approxidate(value) >= now)
 				return error("Invalid %s: '%s'", var, value);
 		}
-		prune_expire = xstrdup(value);
-		return 0;
+		return git_config_string(&prune_expire, var, value);
 	}
 	return git_default_config(var, value, cb);
 }
-- 
1.6.0.2

^ permalink raw reply related

* Re: [GUILT] Use git_editor
From: Alan Jenkins @ 2008-09-30 19:14 UTC (permalink / raw)
  To: Josef 'Jeff' Sipek; +Cc: git
In-Reply-To: <20080930190744.GO32444@josefsipek.net>

Josef 'Jeff' Sipek wrote:
> On Tue, Sep 30, 2008 at 07:43:50PM +0100, Alan Jenkins wrote:
>   
>> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>>
>> diff --git a/guilt b/guilt
>> index 1cef7b9..8d023e9 100755
>> --- a/guilt
>> +++ b/guilt
>> @@ -693,10 +693,6 @@ fi
>>  series="$GUILT_DIR/$branch/series"
>>  applied="$GUILT_DIR/$branch/status"
>>  
>> -# determine an editor to use for anything interactive (fall back to vi)
>> -editor="vi"
>> -[ ! -z "$EDITOR" ] && editor="$EDITOR"
>> -
>>     
>
> Looks good from a quick glance. Any chance there's a git_pager?
>
> Josef 'Jeff' Sipek.
>   

Not in git-sh-setup.  I did check :-).

Alan

^ permalink raw reply

* Re: [PATCH] parse-opt: migrate fmt-merge-msg.
From: Shawn O. Pearce @ 2008-09-30 19:10 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git, gitster
In-Reply-To: <20080930084019.GB11453@artemis.corp>

Pierre Habouzit <madcoder@debian.org> wrote:
> On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> > Pierre Habouzit <madcoder@debian.org> wrote:
> > > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > 
> > Near as I can tell, this is based upon a merge commit in next.
> 
> Hmm I've always sent my patches this way, and I believe you can git am
> -3 them on top of master easily. I can send you the updated series if
> you want.

I'd appreciate an updated series if you can send it.  am -3 isn't
"easily" applying it.  Here I define "easy" as "the patch applies
without me needing to resolve conflicts":

 $ git co -b ph/parseopt master
 $ git am -3 -s X
 Applying: parse-opt: migrate fmt-merge-msg.
 error: patch failed: builtin-fmt-merge-msg.c:5
 error: builtin-fmt-merge-msg.c: patch does not apply
 Using index info to reconstruct a base tree...
 Falling back to patching base and 3-way merge...
 CONFLICT (content): Merge conflict in builtin-fmt-merge-msg.c
 Recorded preimage for 'builtin-fmt-merge-msg.c'
 Failed to merge in the changes.
 Patch failed at 0001.
 When you have resolved this problem run "git am -3 --resolved".
 If you would prefer to skip this patch, instead run "git am -3 --skip".
 To restore the original branch and stop patching run "git am -3 --abort".

:-\

-- 
Shawn.

^ permalink raw reply

* Re: [GUILT] Use git_editor
From: Josef 'Jeff' Sipek @ 2008-09-30 19:07 UTC (permalink / raw)
  To: Alan Jenkins; +Cc: git
In-Reply-To: <48E27366.3080503@tuffmail.co.uk>

On Tue, Sep 30, 2008 at 07:43:50PM +0100, Alan Jenkins wrote:
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> 
> diff --git a/guilt b/guilt
> index 1cef7b9..8d023e9 100755
> --- a/guilt
> +++ b/guilt
> @@ -693,10 +693,6 @@ fi
>  series="$GUILT_DIR/$branch/series"
>  applied="$GUILT_DIR/$branch/status"
>  
> -# determine an editor to use for anything interactive (fall back to vi)
> -editor="vi"
> -[ ! -z "$EDITOR" ] && editor="$EDITOR"
> -

Looks good from a quick glance. Any chance there's a git_pager?

Josef 'Jeff' Sipek.

-- 
Bad pun of the week: The formula 1 control computer suffered from a race
condition

^ permalink raw reply

* Re: [PATCH] doc: enhance git describe --tags help
From: Shawn O. Pearce @ 2008-09-30 19:04 UTC (permalink / raw)
  To: Uwe Kleine-KKKnig
  Cc: Pierre Habouzit, Erez Zilber, git@vger.kernel.org, open-iscsi,
	Junio C Hamano
In-Reply-To: <20080930095641.GA9001@strlen.de>

Uwe Kleine-KKKnig <ukleinek@strlen.de> wrote:
> On Mon, Sep 29, 2008 at 08:01:27AM -0700, Shawn O. Pearce wrote:
> > --tags::
> > 	If a lightweight tag exactly matches, output it.  If no
> > 	annotated tag is found in the ancestry but a lightweight
> > 	tag is found, output the lightweight tag.
>
> IMHO --tags should behave as Erez expected (because it's what I
> expected, too).  As --tags currently behaves it's only usable in very
> rare cases (most of the time it only makes a difference on repos without
> any annotated tag).
> 
> When do you pass --tags?  Only if a lightweight tag is OK for an answer.
> And then I would prefer a "near" lightweight tag to a "farer" annotated
> one.

I don't disagree.  I've been tempted to write a patch to change the
behavior of git-describe so that --tags and --all control what names
are inserted into the candidate list, but don't control the ordering
of their selection.

I think this is all that is needed to make the behavior do what you
and Erez expected.  But its a pretty big change in the results if
you are passing in --all or --tags today.

--8<--
[WIP] Change meaning of --tags and --all

---
 builtin-describe.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin-describe.c b/builtin-describe.c
index ec404c8..fd54fec 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -15,8 +15,8 @@ static const char * const describe_usage[] = {
 };
 
 static int debug;	/* Display lots of verbose info */
-static int all;	/* Default to annotated tags only */
-static int tags;	/* But allow any tags if --tags is specified */
+static int all;	/* Any valid ref can be used */
+static int tags;	/* Either lightweight or annotated tags */
 static int longformat;
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
@@ -112,8 +112,6 @@ static int compare_pt(const void *a_, const void *b_)
 {
 	struct possible_tag *a = (struct possible_tag *)a_;
 	struct possible_tag *b = (struct possible_tag *)b_;
-	if (a->name->prio != b->name->prio)
-		return b->name->prio - a->name->prio;
 	if (a->depth != b->depth)
 		return a->depth - b->depth;
 	if (a->found_order != b->found_order)
-- 
1.6.0.2.513.g6dbd


-- 
Shawn.

^ 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