Git development
 help / color / mirror / Atom feed
* Re: Ideas for qgit
From: Pavel Roskin @ 2006-06-23 20:59 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550606231112l6ca67799m7dddfabcee055045@mail.gmail.com>

On Fri, 2006-06-23 at 20:12 +0200, Marco Costalba wrote:
> On 6/23/06, Pavel Roskin <proski@gnu.org> wrote:
> > Hi, Marco!
> >
> > As promised, here's what I would like to see in qgit:
> >
> > 1) Bookmarks or quick tags (qtags).  It may be useful to mark some
> > commits to make it easier to navigate qgit.  Yet I don't want them to
> > mix with real tags.  Perhaps qgit could save them separately, e.g.
> > in .git/refs/qtags to facilitate navigation.  qtags should appear
> > separately in the popup menu.
> >
> 
> Currently we have two types of tags, signed and simple.

I guess they should be shown a bit differently.

>  If it is
> possible I would really like to stay with git tags, because bookmarks
> seems to me like reinventing the (broken) wheel.

OK, I don't insist.  Then we need a user-friendlier implementation of
Ctrl-Rightclick in the revision list.  I stumbled upon it accidentally,
and it looked like a bug at the first glance, when the file box suddenly
went turquoise and stopped showing modified files.  I still couldn't
figure out how to turn it off without going to the Patch tab and setting
diff to parent.

> Git is very good in tagging and untagging and IMHO we should stay with
> them, perhaps in the simple version. I really don't see any advantage
> nor immediate and less in the long term to almost duplicate that
> functionality and loosing proven git native tag handling features.

OK

> > 2) The "Patch" tab should be redesigned so that the diff can be shown
> > against the parent or against head/tag/qtag.  Users are not supposed to
> > enter SHA1.  If they have to, then it only confirms that qgit needs
> > qtags.
> >
> 
> Well, you can write also a ref name in SHA1 field. Try with
> "v2.6.17-rc6" or "v1.4.0".
> Perhaps this should be better documented and the SHA1 name is misleading.

I think it also shows that a wrong widget is used.  Probably an editable
listbox with all tag and branch names would be better.

> > 3) It would be nice to have some minimal navigating capabilities on the
> > Patch tab.  At least it should be possible to go up and down the
> > revision list and go to any head/tag/qtag/stgit patch.  It would
> > eliminate the need to switch to the "Rev list" too often.
> >
> 
> This make me think of biting the bullet and append patch information
> _also_ below revision description, yes, a la gitk. I think this is
> really what you feel is missing.

Maybe.  "Rev list" and "Patch" represent essentially the same thing
shown differently.  They can be replaced with one tab if it offers
enough flexibility to use the screen space differently for different
tasks.

> > 4) Some bisect support would be nice, at least as good as in gitk.
> > Actually, I'm not using bisect too much, but it's probably because I'm
> > not debugging Wine these days.  Everything else is intelligible :-)
> >
> 
> I don't use bisect and I don't know the gitk implementation.
> I will investigate when I found some time. Not a top priority, at
> least for my kind of workflow ;-)

I agree.

> > 5) Branch view based on reflog.  Probably there should be an interface
> > allowing to limit the displayed revisions to one branch.  I think qgit
> > should still load all revisions that it loads now, but if users start
> > complaining about performance too much, maybe qgit should have an option
> > to load only the logged branch.  The problem is that some parents will
> > be unavailable in the view.
> >
> 
> I think I didn't understand this. Isn't there command line arguments
> for narrowing loaded revision set?

As far as I understand, reflog is only used to resolve references
specified as branch@date.  It doesn't seem to be well integrated with
the rest of git.

> > 7) qgit command line should be documented.  "qgit --help" should display
> > help on stdout.
> >
> 
> All the stuff in command line is sent directly to git-rev-parse. I
> think this is good because avoids any overlapping between qgit and git
> options and guarantees future compatibility with _any_  git option.

This also means that features not related to git-rev-parse (including
StGIT and possible reflog support) cannot be controlled from the command
line.  It's not critical now, but it may become more important.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: Is anybody actually using git-cherry.sh?
From: Johannes Schindelin @ 2006-06-23 20:43 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20060623180658.GA24022@coredump.intra.peff.net>

Hi Peff,

On Fri, 23 Jun 2006, Jeff King wrote:

> It looks like patch-id does a flush whenever a sha1 is found at the 
> beginning of a line; diff-tree lines simply have the 'diff-tree ' part 
> ignored.

Ah! Everything makes sense now. Thanks!

Ciao,
Dscho

^ permalink raw reply

* Re: Added macro support to qgit
From: Pavel Roskin @ 2006-06-23 20:15 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550606231044j4586ea65v191f86a869237b84@mail.gmail.com>

On Fri, 2006-06-23 at 19:44 +0200, Marco Costalba wrote:

> Or just "commands"? in any case I agree "macro" was not a good choice.

For a menubar entry, something short is preferred, so "Commands" or
"Actions" would be better than "external commands".  Emacs calls it
"Tools".  But for a menu entry and the dialog title, "external commands"
would be better.

> The bug is qgit lets you write the first foo bar, before you press NEW
> button. It shouldn't. Also some buttons enable/disable policy could be
> good.

Definitely.

> > What happens to the arguments qgit is asking for if a multiline entry is
> > executed?  I understand they are prepended to the first line.  This is
> > not quite logical.  Wouldn't it be better to have a shell like notation
> > for them?

I meant appended, sorry.

> I thought of commands sequence as a quick way to run some simple
> commands as git pull, git push or similar without writing a bash
> script, i.e. no $1 for arguments. If you need something more complex
> the external script is supposed to be the proper way.

I think the checkbox controlling whether to ask for arguments allows
selection between "simple" and "complex" commands.

> Perhaps we could remove the external script single edit line and use
> only the multiline edit to let user insert commands or script.

I agree.

> > I see the macros are saved in the qgit configuration for the
> > user .qt/qgitrc, like this:

> Well, this file is really not meant to be view nor to be modified by
> hand, it is mainly a qgit 'private' thing stuff. Being qgit a GUI tool
> with a (nice ;-)  ) settings dialog, configuration file is mainly used
> for persistency, not for browsing/setup.

OK

> I hope to fix the external commands interface bugs this week-end.

I'm looking forward to testing it.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* x86 asm SHA1 (draft)
From: linux @ 2006-06-23 17:18 UTC (permalink / raw)
  To: git

As long as I was hacking on PowerPC asm, I figured I might as well take
a crack at the openssl dependency, too.  This is a draft x86 SHA1 that
is a little over 2x faster than the C version on a Pentium M.  I haven't
yet started competing with the OpenSSL code.

This might be useful for the folks who are careful about licensing and
don't like to get mixed up in the OpenSSL/GPL license tangle.

Work in progress, but it functions.  Public domain.


--- /dev/null	2006-04-13 05:29:14.000000000 -0400
+++ sha1x86.S	2006-06-23 09:14:21.000000000 -0400
@@ -0,0 +1,233 @@
+	.text
+#define K1 0x5a827999
+#define K2 0x6ed9eba1
+#define K3 0x8f1bbcdc
+#define K4 0xca62c1d6
+
+#define A %edi
+#define B %ebx
+#define C %ecx
+#define D %edx
+#define E %ebp
+
+#define T %eax
+
+#define MIX(base) \
+	movl	60-base(%esp),T;	\
+	xorl	52-base(%esp),T;	\
+	xorl	28-base(%esp),T;	\
+	xorl	8-base(%esp),T;		\
+	roll	$1,T
+
+/*
+ *In these choice functions, C is the value most recently modified
+ * (It was the B that was rotated in the previous round), so schedule its
+ * use as late as possible.
+ */
+
+/* Choice function: bitwise b ? c : d = ((d ^ c) & b) ^ d */
+#define F1(b,c,d,e) \
+	movl	d,T;	\
+	xorl	c,T;	\
+	andl	b,T;	\
+	roll	$30,b;	\
+	xorl	d,T;	\
+	addl	T,e
+
+/* Parity function: b ^ c ^ d = (b ^ d) ^ c */
+#define F2(b,c,d,e) \
+	movl	b,T;	\
+	roll	$30,b;	\
+	xorl	d,T;	\
+	xorl	c,T;	\
+	addl	T,e
+
+/* Majority function: (b&c) | (c&d) | (d&b) = (b&d) + ((b^d)&c) */
+#define F3(b,c,d,e) \
+	movl	b,T;	\
+	andl	d,T;	\
+	addl	T,e;	\
+	movl	b,T;	\
+	roll	$30,b;	\
+	xorl	d,T;	\
+	andl	c,T;	\
+	addl	T,e
+
+/*
+ * Register assignments:
+ * %eax - temp
+ * %esi - Pointer to input data
+ * %edi, %ebx, %ecx, %edx, %ebp - A..E
+ */
+
+ /*
+  * The basic round:
+  * e += ROTL(e,5) + F(b,c,d) + W[i] + 0x5a827999
+  */
+
+/* This version fetches (and swaps) data from %esi */
+#define ROUND_LOAD(F,a,b,c,d,e,K) \
+	lodsl;		\
+	addl	$K,e;	\
+	bswap	T;	\
+	addl	T,e;	\
+	pushl	T;	\
+	F(b,c,d,e);	\
+	movl	a,T;	\
+	roll	$5,T;	\
+	addl	T,e
+
+/* The standard round: compute the new W value and push it on the stack */
+#define ROUND_MIX(F,a,b,c,d,e,K) \
+	MIX(0);		\
+	addl	$K,e;	\
+	addl	T,e;	\
+	pushl	T;	\
+	F(b,c,d,e);	\
+	movl	a,T;	\
+	roll	$5,T;	\
+	addl	T,e
+
+/* Mix the W[] value, but do NOT push it, as it's never used */
+#define ROUND_LAST(F,a,b,c,d,e,K,base) \
+	MIX(base);	\
+	addl	$K,e;	\
+	addl	T,e;	\
+	F(b,c,d,e);	\
+	movl	a,T;	\
+	roll	$5,T;	\
+	addl	T,e
+
+/* Args are context (A..E, then a W[] array), then input data */
+.globl shaHashBlock
+	.type	shaHashBlock, @function
+shaHashBlock:
+	pushl	%ebp
+	pushl	%ebx
+	pushl	%esi
+	pushl	%edi
+/* Args now start at 20(%esp) */
+
+	movl	20(%esp),T
+	movl	24(%esp),%esi
+
+	movl	  (T),A
+	movl	 4(T),B
+	movl	 8(T),C
+	movl	12(T),D
+	movl	16(T),E
+
+	ROUND_LOAD(F1, A,B,C,D,E, K1);
+	ROUND_LOAD(F1, E,A,B,C,D, K1);
+	ROUND_LOAD(F1, D,E,A,B,C, K1);
+	ROUND_LOAD(F1, C,D,E,A,B, K1);
+	ROUND_LOAD(F1, B,C,D,E,A, K1);
+
+	ROUND_LOAD(F1, A,B,C,D,E, K1);
+	ROUND_LOAD(F1, E,A,B,C,D, K1);
+	ROUND_LOAD(F1, D,E,A,B,C, K1);
+	ROUND_LOAD(F1, C,D,E,A,B, K1);
+	ROUND_LOAD(F1, B,C,D,E,A, K1);
+
+	ROUND_LOAD(F1, A,B,C,D,E, K1);
+	ROUND_LOAD(F1, E,A,B,C,D, K1);
+	ROUND_LOAD(F1, D,E,A,B,C, K1);
+	ROUND_LOAD(F1, C,D,E,A,B, K1);
+	ROUND_LOAD(F1, B,C,D,E,A, K1);
+
+	ROUND_LOAD(F1, A,B,C,D,E, K1);
+	ROUND_MIX(F1, E,A,B,C,D, K1);
+	ROUND_MIX(F1, D,E,A,B,C, K1);
+	ROUND_MIX(F1, C,D,E,A,B, K1);
+	ROUND_MIX(F1, B,C,D,E,A, K1);
+
+	ROUND_MIX(F2, A,B,C,D,E, K2);
+	ROUND_MIX(F2, E,A,B,C,D, K2);
+	ROUND_MIX(F2, D,E,A,B,C, K2);
+	ROUND_MIX(F2, C,D,E,A,B, K2);
+	ROUND_MIX(F2, B,C,D,E,A, K2);
+
+	ROUND_MIX(F2, A,B,C,D,E, K2);
+	ROUND_MIX(F2, E,A,B,C,D, K2);
+	ROUND_MIX(F2, D,E,A,B,C, K2);
+	ROUND_MIX(F2, C,D,E,A,B, K2);
+	ROUND_MIX(F2, B,C,D,E,A, K2);
+
+	ROUND_MIX(F2, A,B,C,D,E, K2);
+	ROUND_MIX(F2, E,A,B,C,D, K2);
+	ROUND_MIX(F2, D,E,A,B,C, K2);
+	ROUND_MIX(F2, C,D,E,A,B, K2);
+	ROUND_MIX(F2, B,C,D,E,A, K2);
+
+	ROUND_MIX(F2, A,B,C,D,E, K2);
+	ROUND_MIX(F2, E,A,B,C,D, K2);
+	ROUND_MIX(F2, D,E,A,B,C, K2);
+	ROUND_MIX(F2, C,D,E,A,B, K2);
+	ROUND_MIX(F2, B,C,D,E,A, K2);
+
+	ROUND_MIX(F3, A,B,C,D,E, K3);
+	ROUND_MIX(F3, E,A,B,C,D, K3);
+	ROUND_MIX(F3, D,E,A,B,C, K3);
+	ROUND_MIX(F3, C,D,E,A,B, K3);
+	ROUND_MIX(F3, B,C,D,E,A, K3);
+
+	ROUND_MIX(F3, A,B,C,D,E, K3);
+	ROUND_MIX(F3, E,A,B,C,D, K3);
+	ROUND_MIX(F3, D,E,A,B,C, K3);
+	ROUND_MIX(F3, C,D,E,A,B, K3);
+	ROUND_MIX(F3, B,C,D,E,A, K3);
+
+	ROUND_MIX(F3, A,B,C,D,E, K3);
+	ROUND_MIX(F3, E,A,B,C,D, K3);
+	ROUND_MIX(F3, D,E,A,B,C, K3);
+	ROUND_MIX(F3, C,D,E,A,B, K3);
+	ROUND_MIX(F3, B,C,D,E,A, K3);
+
+	ROUND_MIX(F3, A,B,C,D,E, K3);
+	ROUND_MIX(F3, E,A,B,C,D, K3);
+	ROUND_MIX(F3, D,E,A,B,C, K3);
+	ROUND_MIX(F3, C,D,E,A,B, K3);
+	ROUND_MIX(F3, B,C,D,E,A, K3);
+
+	ROUND_MIX(F2, A,B,C,D,E, K4);
+	ROUND_MIX(F2, E,A,B,C,D, K4);
+	ROUND_MIX(F2, D,E,A,B,C, K4);
+	ROUND_MIX(F2, C,D,E,A,B, K4);
+	ROUND_MIX(F2, B,C,D,E,A, K4);
+
+	ROUND_MIX(F2, A,B,C,D,E, K4);
+	ROUND_MIX(F2, E,A,B,C,D, K4);
+	ROUND_MIX(F2, D,E,A,B,C, K4);
+	ROUND_MIX(F2, C,D,E,A,B, K4);
+	ROUND_MIX(F2, B,C,D,E,A, K4);
+
+	ROUND_MIX(F2, A,B,C,D,E, K4);
+	ROUND_MIX(F2, E,A,B,C,D, K4);
+	ROUND_MIX(F2, D,E,A,B,C, K4);
+	ROUND_MIX(F2, C,D,E,A,B, K4);
+	ROUND_MIX(F2, B,C,D,E,A, K4);
+
+	ROUND_MIX(F2, A,B,C,D,E, K4);
+	ROUND_MIX(F2, E,A,B,C,D, K4);
+	ROUND_LAST(F2, D,E,A,B,C, K4, 0);
+	ROUND_LAST(F2, C,D,E,A,B, K4, 4);
+	ROUND_LAST(F2, B,C,D,E,A, K4, 8);
+
+	addl	$77*4,%esp
+
+	movl	20(%esp),T
+
+	addl	A,  (T)
+	addl	B, 4(T)
+	addl	C, 8(T)
+	addl	D,12(T)
+	addl	E,16(T)
+
+	popl	%edi
+	popl	%esi
+	popl	%ebx
+	popl	%ebp
+
+	ret
+
+	.size	shaHashBlock, .-shaHashBlock
--- /dev/null	2006-04-13 05:29:14.000000000 -0400
+++ sha1asm.h	2006-06-23 10:24:35.578683250 -0400
@@ -0,0 +1,12 @@
+#include <stdint.h>
+#include <stddef.h>	/* For size_t */
+
+typedef struct sha_ctx {
+  uint32_t hash[5];
+  uint32_t data[16];
+  uint32_t sizeL, sizeH;
+} SHA_CTX;
+
+void SHA1_Init(SHA_CTX *ctx);
+void SHA1_Update(SHA_CTX *ctx, const void *dataIn, size_t len);
+void SHA1_Final(unsigned char hashout[20], SHA_CTX *ctx);
--- /dev/null	2006-04-13 05:29:14.000000000 -0400
+++ sha1asm.c	2006-06-23 10:25:26.798757250 -0400
@@ -0,0 +1,163 @@
+
+#include <string.h>	/* For memcpy */
+#include <arpa/inet.h>	/* For htonl */
+
+#include "sha1asm.h"
+
+#if ASM
+extern void shaHashBlock(SHA_CTX *ctx, uint32_t const *input);
+#else
+
+/* 
+ * This chunk of this file are subject to the Mozilla Public
+ * License Version 1.1 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.mozilla.org/MPL/
+ * 
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ * 
+ * The Original Code is SHA 180-1 Reference Implementation (Compact version)
+ * 
+ * The Initial Developer of the Original Code is Paul Kocher of
+ * Cryptography Research.  Portions created by Paul Kocher are 
+ * Copyright (C) 1995-9 by Cryptography Research, Inc.  All
+ * Rights Reserved.
+ * 
+ * Contributor(s):
+ *
+ *     Paul Kocher
+ * 
+ * Alternatively, this portion of this file may be used under the
+ * terms of the GNU General Public License Version 2 or later (the
+ * "GPL"), in which case the provisions of the GPL are applicable 
+ * instead of those above.  If you wish to allow use of your 
+ * version of this file only under the terms of the GPL and not to
+ * allow others to use your version of this file under the MPL,
+ * indicate your decision by deleting the provisions above and
+ * replace them with the notice and other provisions required by
+ * the GPL.  If you do not delete the provisions above, a recipient
+ * may use your version of this file under either the MPL or the
+ * GPL.
+ */
+
+#define ROTL(X,n) (((X) << (n)) | ((X) >> (32-(n))))
+
+static void
+shaHashBlock(SHA_CTX *ctx, uint32_t const *input)
+{
+	int i;
+	uint32_t A,B,C,D,E,T;
+	uint32_t W[80];
+
+	for (i = 0; i < 16; i++)
+		W[i] = ntohl(input[i]);
+	for (i = 16; i < 80; i++) {
+		T = W[i-3] ^ W[i-8] ^ W[i-14] ^ W[i-16];
+		W[i] = ROTL(T, 1);
+	}
+
+	A = ctx->hash[0];
+	B = ctx->hash[1];
+	C = ctx->hash[2];
+	D = ctx->hash[3];
+	E = ctx->hash[4];
+
+	for (i = 0; i < 20; i++) {
+		T = ROTL(A,5) + (((C^D)&B)^D)     + E + W[i] + 0x5a827999;
+		E = D; D = C; C = ROTL(B, 30); B = A; A = T;
+	}
+	for (i = 20; i < 40; i++) {
+		T = ROTL(A,5) + (B^C^D)           + E + W[i] + 0x6ed9eba1;
+		E = D; D = C; C = ROTL(B, 30); B = A; A = T;
+	}
+	for (i = 40; i < 60; i++) {
+		T = ROTL(A,5) + (B&C) + (D&(B^C)) + E + W[i] + 0x8f1bbcdc;
+		E = D; D = C; C = ROTL(B, 30); B = A; A = T;
+	}
+	for (i = 60; i < 80; i++) {
+		T = ROTL(A,5) + (B^C^D)           + E + W[i] + 0xca62c1d6;
+		E = D; D = C; C = ROTL(B, 30); B = A; A = T;
+	}
+
+	ctx->hash[0] += A;
+	ctx->hash[1] += B;
+	ctx->hash[2] += C;
+	ctx->hash[3] += D;
+	ctx->hash[4] += E;
+}
+#endif
+
+/*
+ * The following part of the file is NOT subject to the above license, and is
+ * instead placed in the public domain.
+ */
+
+void
+SHA1_Init(SHA_CTX *ctx)
+{
+  /* Initialize H with the magic constants (see FIPS180 for constants)
+   */
+  ctx->hash[0] = 0x67452301;
+  ctx->hash[1] = 0xefcdab89;
+  ctx->hash[2] = 0x98badcfe;
+  ctx->hash[3] = 0x10325476;
+  ctx->hash[4] = 0xc3d2e1f0;
+
+  ctx->sizeH = ctx->sizeL = 0;
+}
+
+void
+SHA1_Update(SHA_CTX *ctx, const void *data, size_t len)
+{
+	unsigned pos = ctx->sizeL % 64;
+
+	ctx->sizeL += len;
+	ctx->sizeH += (ctx->sizeL < (uint32_t)len);
+	ctx->sizeH += len >> 16 >> 16;	/* In case size_t is 64 bits */
+
+	/* Leading partial block */
+	if (pos) {
+		unsigned avail = 64 - pos;
+		if (avail > len)
+			goto end;
+		memcpy((char *)ctx->data + pos, data, avail);
+		data = (char const *)data + avail;
+		len -= avail;
+		shaHashBlock(ctx, ctx->data);
+	}
+	/* Full blocks */
+	while (len >= 64) {
+		shaHashBlock(ctx, data);
+		data = (char const *)data + 64;
+		len -= 64;
+	}
+	pos = 0;
+end:
+	/* Buffer trailing partial block */
+	memcpy((char *)ctx->data + pos, data, len);
+}
+
+void
+SHA1_Final(unsigned char hashout[20], SHA_CTX *ctx)
+{
+  static unsigned char const padding[64] = { 0x80, 0 /* more zeros */ };
+  uint32_t sizeL = ctx->sizeL;
+  uint32_t sizeH = ctx->sizeH;
+  int i;
+
+  /* Append final padding, leaving 8 bytes free */
+  SHA1_Update(ctx, padding, 64 - ((sizeL + 8) % 64));
+  ctx->data[14] = htonl(sizeH << 3 | sizeL >> 29);
+  ctx->data[15] = htonl(sizeL << 3);
+
+  shaHashBlock(ctx, ctx->data);
+
+  for (i = 0; i < 5; i++)
+  	((uint32_t *)hashout)[i] = htonl(ctx->hash[i]);
+
+   memset(ctx, 0, sizeof *ctx);
+}
+

^ permalink raw reply

* Re: Tracking CVS
From: Jon Smirl @ 2006-06-23 18:14 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060623023131.GK21864@pasky.or.cz>

My cg-sync script looks like this now:

#!/usr/bin/env bash
cg-rm -a
cg-status -wnSs \? | xargs cg-add

But when are no files to add or remove I get this:

[jonsmirl@jonsmirl mozilla]$ cg sync
cg-rm: no files to remove
cg-add: usage: cg-add [-N] [-r] FILE...

This is definitely a useful script for tracking CVS. I'd like to see
it added to cogito once it is fully debugged.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Ideas for qgit
From: Marco Costalba @ 2006-06-23 18:12 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git
In-Reply-To: <1151035711.25640.6.camel@dv>

On 6/23/06, Pavel Roskin <proski@gnu.org> wrote:
> Hi, Marco!
>
> As promised, here's what I would like to see in qgit:
>
> 1) Bookmarks or quick tags (qtags).  It may be useful to mark some
> commits to make it easier to navigate qgit.  Yet I don't want them to
> mix with real tags.  Perhaps qgit could save them separately, e.g.
> in .git/refs/qtags to facilitate navigation.  qtags should appear
> separately in the popup menu.
>

Currently we have two types of tags, signed and simple. If it is
possible I would really like to stay with git tags, because bookmarks
seems to me like reinventing the (broken) wheel.

Git is very good in tagging and untagging and IMHO we should stay with
them, perhaps in the simple version. I really don't see any advantage
nor immediate and less in the long term to almost duplicate that
functionality and loosing proven git native tag handling features.

> 2) The "Patch" tab should be redesigned so that the diff can be shown
> against the parent or against head/tag/qtag.  Users are not supposed to
> enter SHA1.  If they have to, then it only confirms that qgit needs
> qtags.
>

Well, you can write also a ref name in SHA1 field. Try with
"v2.6.17-rc6" or "v1.4.0".
Perhaps this should be better documented and the SHA1 name is misleading.

> 3) It would be nice to have some minimal navigating capabilities on the
> Patch tab.  At least it should be possible to go up and down the
> revision list and go to any head/tag/qtag/stgit patch.  It would
> eliminate the need to switch to the "Rev list" too often.
>

This make me think of biting the bullet and append patch information
_also_ below revision description, yes, a la gitk. I think this is
really what you feel is missing.

> 4) Some bisect support would be nice, at least as good as in gitk.
> Actually, I'm not using bisect too much, but it's probably because I'm
> not debugging Wine these days.  Everything else is intelligible :-)
>

I don't use bisect and I don't know the gitk implementation.
I will investigate when I found some time. Not a top priority, at
least for my kind of workflow ;-)

> 5) Branch view based on reflog.  Probably there should be an interface
> allowing to limit the displayed revisions to one branch.  I think qgit
> should still load all revisions that it loads now, but if users start
> complaining about performance too much, maybe qgit should have an option
> to load only the logged branch.  The problem is that some parents will
> be unavailable in the view.
>

I think I didn't understand this. Isn't there command line arguments
for narrowing loaded revision set?

> 7) qgit command line should be documented.  "qgit --help" should display
> help on stdout.
>

All the stuff in command line is sent directly to git-rev-parse. I
think this is good because avoids any overlapping between qgit and git
options and guarantees future compatibility with _any_  git option.

Again, being qgit a GUI tool we could avoid some command line only
application's habits. In the latter case there's nothing better to do,
but we can run a nicely formatted handbook in an independent window
just pressing F1 and be sure to never mess with any current and future
possible git option.

        Marco

^ permalink raw reply

* Re: Is anybody actually using git-cherry.sh?
From: Jeff King @ 2006-06-23 18:06 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606231818140.29667@wbgn013.biozentrum.uni-wuerzburg.de>

On Fri, Jun 23, 2006 at 06:22:05PM +0200, Johannes Schindelin wrote:

> from git-cherry.sh:
> 
> -- snip --
> for c in $inup
> do
>         git-diff-tree -p $c
> done | git-patch-id |
> while read id name
> do
>         echo $name >>$patch/$id
> done
> -- snap --
>
> AFAICS this _must_ be broken.  git-diff-tree -p <ent> does not emit
> "diff-tree <sha1>", and neither "commit <sha1>" lines. So this code
> would yield just one file, treating all diffs as one huge diff. A
> quick fix would be this change (without the patch I sent out earlier):

Maybe I don't understand what you're saying, but it works fine here
(using latest head of master):
  $ for c in origin origin^; do git-diff-tree -p $c; done | git-patch-id
  1511e4e276ccc98ecf0ea31dad1bc9010869fdaf f60349aa786d519368938d7b6e5bb2006eccb0cf
  86ce7eeedd87a78cd8cac79adb6d4d968ece9e53 50f575fc9836704d45a5f732125b8f58103425a4

It looks like patch-id does a flush whenever a sha1 is found at the
beginning of a line; diff-tree lines simply have the 'diff-tree ' part
ignored.

-Peff

^ permalink raw reply

* Re: Added macro support to qgit
From: Marco Costalba @ 2006-06-23 17:44 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git
In-Reply-To: <1151024452.5205.46.camel@dv>

Hi Pavel,

> If I understand correctly, qgit doesn't do that.  It calls external
> commands that are not implemented internally, and it doesn't aggregate
> anything.  Then why not call them "external commands"?
>

Or just "commands"? in any case I agree "macro" was not a good choice.

> The interface is quite confusing.  I see 5 buttons on top, all of which
> are enabled, plus one button labeled as "...", three checkboxes, one
> single-line entry and one multiline entry.  I have no I idea where to
> start.  Should I click "new" or white something and then click "new"?
> And where's "Cancel"?.
>
> That's what I have tried:
>
> Enter "foo bar" in the multiline entry.
> Click "New"
> Enter "foobar"
> Click "New"
> Enter "abc"
> Select "foobar"
>
> Now "Run external script" is selected and "foo bar" is gone!  I believe
> user input should not be discarded without a warning.
>

The bug is qgit lets you write the first foo bar, before you press NEW
button. It shouldn't. Also some buttons enable/disable policy could be
good.

> I also tried something more meaningful.  I create a "pull" macro as an
> external script "stg pull".  It didn't work.  Am I supposed to supply
> full path?  Does it understand arguments?  It the script supposed to be
> in a certain format?  OK, stg is written in python, but how about
> cg-status, a shell script?  It doesn't seen to work either.
>

You are not supposed to supply full path, any executable file should
be OK and yes, you could supply arguments. You don't see nothing
because of a silly bug.

> What happens to the arguments qgit is asking for if a multiline entry is
> executed?  I understand they are prepended to the first line.  This is
> not quite logical.  Wouldn't it be better to have a shell like notation
> for them?
>

I thought of commands sequence as a quick way to run some simple
commands as git pull, git push or similar without writing a bash
script, i.e. no $1 for arguments. If you need something more complex
the external script is supposed to be the proper way.

Perhaps we could remove the external script single edit line and use
only the multiline edit to let user insert commands or script.

> I see the macros are saved in the qgit configuration for the
> user .qt/qgitrc, like this:
>

Well, this file is really not meant to be view nor to be modified by
hand, it is mainly a qgit 'private' thing stuff. Being qgit a GUI tool
with a (nice ;-)  ) settings dialog, configuration file is mainly used
for persistency, not for browsing/setup.

I hope to fix the external commands interface bugs this week-end.

Thanks for your reports
Marco

^ permalink raw reply

* Is anybody actually using git-cherry.sh?
From: Johannes Schindelin @ 2006-06-23 16:22 UTC (permalink / raw)
  To: git

Hi,

from git-cherry.sh:

-- snip --
for c in $inup
do
        git-diff-tree -p $c
done | git-patch-id |
while read id name
do
        echo $name >>$patch/$id
done
-- snap --

AFAICS this _must_ be broken. git-diff-tree -p <ent> does not emit 
"diff-tree <sha1>", and neither "commit <sha1>" lines. So this code would 
yield just one file, treating all diffs as one huge diff. A quick fix 
would be this change (without the patch I sent out earlier):

+	echo "diff-tree $c"
 	git-diff-tree -p $c

Am I wrong?

Ciao,
Dscho

^ permalink raw reply

* CVS import broken?
From: Johannes Schindelin @ 2006-06-23 16:14 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git

Hi,

I track quite a few CVS repos with 'git-cvsimport -k -i', but recently it 
stopped working (of course, I was not reimporting, but incrementally 
tracking them). I think it is the introduction of one-index-per-branch 
policy:

> fatal: index file mmap failed (Invalid argument)
> unable to write to git-update-index:  at /.../git-cvsimport line 606, <CVS> line 277425.

It seems that git-cvsimport makes a temporary file of size 0, which cannot 
get mmap()ed, because it has size 0.

Any help appreciated,
Dscho

^ permalink raw reply

* Re: [PATCH] patch-id: "diff-tree" => "commit"
From: Johannes Schindelin @ 2006-06-23 16:07 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.64.0606230849290.6483@g5.osdl.org>

Hi,

On Fri, 23 Jun 2006, Linus Torvalds wrote:

> On Fri, 23 Jun 2006, Johannes Schindelin wrote:
> > 
> > Some time ago we changed git-log in a massive way, and one consequence is
> > that the keyword changed. Adjust patch-id for that.
> 
> Ahh. Yes. Except I think you should allow both, for historical reasons (ie 
> not remove the old case).

Hmm. If you are alluding to mailboxes, where there could be mails from 
older git versions, then this might not be enough. Look at my patch, for 
example. There is no "diff-tree", and no "commit".

However, the only official user of patch-id is git-cherry (and indirectly, 
all users of git-cherry). And this user works on data which is generated 
on the fly, i.e. there will be no "diff-tree" at the beginning.

Of course, there is a Pandora's box: a line in a commit message is much 
more likely to start with "commit <sha1>" than "diff-tree <sha1>". So my 
patch probably breaks many cases.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] patch-id: "diff-tree" => "commit"
From: Linus Torvalds @ 2006-06-23 15:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0606231731280.29667@wbgn013.biozentrum.uni-wuerzburg.de>



On Fri, 23 Jun 2006, Johannes Schindelin wrote:
> 
> Some time ago we changed git-log in a massive way, and one consequence is
> that the keyword changed. Adjust patch-id for that.

Ahh. Yes. Except I think you should allow both, for historical reasons (ie 
not remove the old case).

		Linus

^ permalink raw reply

* [FYI/PATCH] diff: add diff_flush_patch_id()
From: Johannes Schindelin @ 2006-06-23 15:44 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: martin.langhoff, git
In-Reply-To: <Pine.LNX.4.63.0606231431420.29667@wbgn013.biozentrum.uni-wuerzburg.de>


Call it like this:

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

This patch also adds a switch "--with-patch-id" to the diff family, to
print out the patch id before each patch.

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

---


	> > > 	- add a DIFF_FORMAT_PATCH_ID
	> > 
	> > Please don't add any DIFF_FORMAT_*.  I'm cleaning the diff output code
	> > and replacing diff_options.output_format with one-bit flags.
	> 
	> Okay. For the purposes of git-format-patch, this is not needed 
	> anyway, but rather a function which takes two tree objects and 
	> returns the patch id. When you are finished it should be easy to 
	> add this as a display format.

	Timo, I am prepared to redo this patch when you finished
	the cleanup.

 diff.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 diff.h |    3 +
 2 files changed, 132 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 5b34f73..1140d54 100644
--- a/diff.c
+++ b/diff.c
@@ -1460,6 +1460,8 @@ int diff_opt_parse(struct diff_options *
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->with_stat = 1;
 	}
+	else if (!strcmp(arg, "--with-patch-id"))
+		options->with_patch_id = 1;
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!strncmp(arg, "-l", 2))
@@ -2027,12 +2029,139 @@ static void diff_summary(struct diff_fil
 	}
 }
 
+struct patch_id_t {
+	struct xdiff_emit_state xm;
+	SHA_CTX *ctx;
+	int patchlen;
+};
+
+static int remove_space(char *line, int len)
+{
+	int i;
+        char *dst = line;
+        unsigned char c;
+
+        for (i = 0; i < len; i++)
+                if (!isspace((c = line[i])))
+                        *dst++ = c;
+
+        return dst - line;
+}
+
+static void patch_id_consume(void *priv, char *line, unsigned long len)
+{
+	struct patch_id_t *data = priv;
+	int new_len;
+
+	/* Ignore line numbers when computing the SHA1 of the patch */
+	if (!strncmp(line, "@@ -", 4))
+		return;
+
+	new_len = remove_space(line, len);
+
+	SHA1_Update(data->ctx, line, new_len);
+	data->patchlen += new_len;
+}
+
+/* returns 0 upon success, and writes result into sha1 */
+int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	int i;
+	SHA_CTX ctx;
+	struct patch_id_t data;
+	char buffer[PATH_MAX * 4 + 20];
+
+	SHA1_Init(&ctx);
+	memset(&data, 0, sizeof(struct patch_id_t));
+	data.ctx = &ctx;
+	data.xm.consume = patch_id_consume;
+	
+	for (i = 0; i < q->nr; i++) {
+		xpparam_t xpp;
+		xdemitconf_t xecfg;
+		xdemitcb_t ecb;
+		mmfile_t mf1, mf2;
+		struct diff_filepair *p = q->queue[i];
+		int len1, len2;
+
+		if (p->status == 0)
+			return error("internal diff status error");
+		if (p->status == DIFF_STATUS_UNKNOWN)
+			continue;
+		if (diff_unmodified_pair(p))
+			continue;
+		if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
+		    (DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
+			continue;
+		if (DIFF_PAIR_UNMERGED(p))
+			continue;
+
+		diff_fill_sha1_info(p->one);
+		diff_fill_sha1_info(p->two);
+		if (fill_mmfile(&mf1, p->one) < 0 ||
+				fill_mmfile(&mf2, p->two) < 0)
+			return error("unable to read files to diff");
+
+		/* Maybe hash p->two? into the patch id? */
+		if (mmfile_is_binary(&mf2))
+			continue;
+
+		len1 = remove_space(p->one->path, strlen(p->one->path));
+		len2 = remove_space(p->two->path, strlen(p->two->path));
+		if (p->one->mode == 0)
+			len1 = snprintf(buffer, sizeof(buffer),
+					"diff--gita/%.*sb/%.*s"
+					"newfilemode%06o"
+					"---/dev/null"
+					"+++b/%.*s",
+					len1, p->one->path,
+					len2, p->two->path,
+					p->two->mode,
+					len2, p->two->path);
+		else if (p->two->mode == 0)
+			len1 = snprintf(buffer, sizeof(buffer),
+					"diff--gita/%.*sb/%.*s"
+					"deletedfilemode%06o"
+					"---a/%.*s"
+					"+++/dev/null",
+					len1, p->one->path,
+					len2, p->two->path,
+					p->one->mode,
+					len1, p->one->path);
+		else
+			len1 = snprintf(buffer, sizeof(buffer),
+					"diff--gita/%.*sb/%.*s"
+					"---a/%.*s"
+					"+++b/%.*s",
+					len1, p->one->path,
+					len2, p->two->path,
+					len1, p->one->path,
+					len2, p->two->path);
+		SHA1_Update(&ctx, buffer, len1);
+
+		xpp.flags = XDF_NEED_MINIMAL;
+		xecfg.ctxlen = 3;
+		xecfg.flags = 3;
+		ecb.outf = xdiff_outf;
+		ecb.priv = &data;
+		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+	}
+
+	SHA1_Final(sha1, &ctx);
+	return 0;
+}
+
 void diff_flush(struct diff_options *options)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	int i;
 	int diff_output_format = options->output_format;
 	struct diffstat_t *diffstat = NULL;
+	unsigned char sha1[20];
+
+	if (options->with_patch_id && !diff_flush_patch_id(options, sha1))
+		printf("patch-id %s\n", sha1_to_hex(sha1));
 
 	if (diff_output_format == DIFF_FORMAT_DIFFSTAT || options->with_stat) {
 		diffstat = xcalloc(sizeof (struct diffstat_t), 1);
diff --git a/diff.h b/diff.h
index 7d7b6cd..29aac52 100644
--- a/diff.h
+++ b/diff.h
@@ -27,6 +27,7 @@ struct diff_options {
 	unsigned recursive:1,
 		 with_raw:1,
 		 with_stat:1,
+		 with_patch_id:1,
 		 tree_in_recursive:1,
 		 binary:1,
 		 full_index:1,
@@ -185,4 +186,6 @@ extern int run_diff_files(struct rev_inf
 
 extern int run_diff_index(struct rev_info *revs, int cached);
 
+extern int diff_flush_patch_id(struct diff_options *, unsigned char *);
+
 #endif /* DIFF_H */
-- 
1.4.0.g319e-dirty

^ permalink raw reply related

* [PATCH] patch-id: "diff-tree" => "commit"
From: Johannes Schindelin @ 2006-06-23 15:36 UTC (permalink / raw)
  To: git, junkio


Some time ago we changed git-log in a massive way, and one consequence is
that the keyword changed. Adjust patch-id for that.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 patch-id.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/patch-id.c b/patch-id.c
index edbc4aa..01845be 100644
--- a/patch-id.c
+++ b/patch-id.c
@@ -40,8 +40,8 @@ static void generate_id_list(void)
 		char *p = line;
 		int len;
 
-		if (!memcmp(line, "diff-tree ", 10))
-			p += 10;
+		if (!memcmp(line, "commit ", 7))
+			p += 7;
 
 		if (!get_sha1_hex(p, n)) {
 			flush_current_id(patchlen, sha1, &ctx);
-- 
1.4.1.rc1.g406e

^ permalink raw reply related

* Re: What's in git.git and announcing v1.4.1-rc1
From: Linus Torvalds @ 2006-06-23 14:59 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Git Mailing List, Linux Kernel Mailing List
In-Reply-To: <Pine.LNX.4.63.0606231305000.29667@wbgn013.biozentrum.uni-wuerzburg.de>



On Fri, 23 Jun 2006, Johannes Schindelin wrote:
> > 
> >  - default to red/green for old/new lines. That's the norm, I'd think.
> 
> ... and which happens to be useless for 10% of the male population (and 
> even more if you look specifically at Asian people). But then, I just 
> pasted that part from somewhere else.

Sure. 

(Although I think it's 7% in general, and more in certain populations, 
some Western European countries included)

Which just means that we should have some way to let people set their own 
colors.

The _default_ should be the one people expect, though.

		Linus

^ permalink raw reply

* Re: What's in git.git and announcing v1.4.1-rc1
From: Johannes Schindelin @ 2006-06-23 14:25 UTC (permalink / raw)
  To: Pádraig Brady
  Cc: Linus Torvalds, Junio C Hamano, Git Mailing List,
	Linux Kernel Mailing List
In-Reply-To: <449BF508.9040207@draigBrady.com>

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

Hi,

On Fri, 23 Jun 2006, Pádraig Brady wrote:

> So 10% of the male population need to learn traffic light positions 
> rather than colours?

A friend of mine was at the military and had to check new recruits for 
color-blindness. Only after the 20th color-blind man in a row he realized 
for the first time in hist life that it was _him_, being the color-blind.

Ciao,
Dscho

^ permalink raw reply

* Re: What's in git.git and announcing v1.4.1-rc1
From: Pádraig Brady @ 2006-06-23 14:04 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Linus Torvalds, Junio C Hamano, Git Mailing List,
	Linux Kernel Mailing List
In-Reply-To: <Pine.LNX.4.63.0606231305000.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 22 Jun 2006, Linus Torvalds wrote:
> 
> 
>>On Thu, 22 Jun 2006, Junio C Hamano wrote:
>>
>>> - diff --color (Johannes).
>>
>> - default to red/green for old/new lines. That's the norm, I'd think.
> 
> 
> ... and which happens to be useless for 10% of the male population (and 
> even more if you look specifically at Asian people). But then, I just 
> pasted that part from somewhere else.

:)

So 10% of the male population need to learn
traffic light positions rather than colours?

I'm red/green colour blind which means I can't
distinguish _subtley_ different shades of red and green.

vim is another fondue fork offender as it merges
syntax highlighting and diff colours in diff mode (vimdiff).
I put the following in ~/.vimrc to disable that madness:

if &diff
    "I'm only interested in diff colours
    syntax off
endif

Pádraig.

^ permalink raw reply

* Re: [PATCH] Customizable error handlers
From: Petr Baudis @ 2006-06-23 13:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060623133227.27854.65567.stgit@machine.or.cz>

Dear diary, on Fri, Jun 23, 2006 at 03:32:27PM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 5d543d2..e954002 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -36,9 +36,13 @@ #endif
>  #endif
>  
>  /* General helper functions */
> -extern void usage(const char *err) NORETURN;
> -extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
> -extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> +void usage(const char *err) NORETURN;
> +void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
> +int error(const char *err, ...) __attribute__((format (printf, 1, 2)));

Wah, this kind of slipped through. Below is a patch without the
externs removed. Sorry about the noise.

>  int error(const char *err, ...)
>  {
>  	va_list params;
> +	int ret;
>  
>  	va_start(params, err);
> -	report("error: ", err, params);
> +	ret = error_routine(err, params);
>  	va_end(params);
> -	return -1;
> +	return ret;
>  }

BTW, I don't mind if you just force the return value to -1 here.
It might be saner.

---

[PATCH] Customizable error handlers

This patch makes the usage(), die() and error() handlers customizable.
Nothing in the git code itself uses that but many other libgit users
(like Git.pm) will.

This is implemented using the mutator functions primarily because you
cannot directly modifying global variables of libgit from a program that
dlopen()ed it, apparently. But having functions for that is a better API
anyway.

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

---

diff --git a/git-compat-util.h b/git-compat-util.h
index 5d543d2..0c5ceb3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -40,6 +40,10 @@ extern void usage(const char *err) NORET
 extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
+extern void set_usage_routine(void (*routine)(const char *err) NORETURN);
+extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
+extern void set_error_routine(int (*routine)(const char *err, va_list params));
+
 #ifdef NO_MMAP
 
 #ifndef PROT_READ
diff --git a/usage.c b/usage.c
index 1fa924c..445456d 100644
--- a/usage.c
+++ b/usage.c
@@ -12,28 +12,68 @@ static void report(const char *prefix, c
 	fputs("\n", stderr);
 }
 
-void usage(const char *err)
+void usage_builtin(const char *err)
 {
 	fprintf(stderr, "usage: %s\n", err);
 	exit(129);
 }
 
+void die_builtin(const char *err, va_list params)
+{
+	report("fatal: ", err, params);
+	exit(128);
+}
+
+int error_builtin(const char *err, va_list params)
+{
+	report("error: ", err, params);
+	return -1;
+}
+
+
+/* If we are in a dlopen()ed .so write to a global variable would segfault
+ * (ugh), so keep things static. */
+static void (*usage_routine)(const char *err) NORETURN = usage_builtin;
+static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;
+static int (*error_routine)(const char *err, va_list params) = error_builtin;
+
+void set_usage_routine(void (*routine)(const char *err) NORETURN)
+{
+	usage_routine = routine;
+}
+
+void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN)
+{
+	die_routine = routine;
+}
+
+void set_error_routine(int (*routine)(const char *err, va_list params))
+{
+	error_routine = routine;
+}
+
+
+void usage(const char *err)
+{
+	usage_routine(err);
+}
+
 void die(const char *err, ...)
 {
 	va_list params;
 
 	va_start(params, err);
-	report("fatal: ", err, params);
+	die_routine(err, params);
 	va_end(params);
-	exit(128);
 }
 
 int error(const char *err, ...)
 {
 	va_list params;
+	int ret;
 
 	va_start(params, err);
-	report("error: ", err, params);
+	ret = error_routine(err, params);
 	va_end(params);
-	return -1;
+	return ret;
 }

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

^ permalink raw reply related

* [PATCH] git-commit: allow -e option anywhere on command line
From: Jeff King @ 2006-06-23 13:43 UTC (permalink / raw)
  To: junkio; +Cc: git

Previously, the command 'git-commit -e -m foo' would ignore the '-e' option
because the '-m' option overwrites the no_edit flag during sequential
option parsing. Now we cause -e to reset the no_edit flag after all
options are parsed.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-commit.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index 6dd04fd..4bb16db 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -199,6 +199,7 @@ only=
 logfile=
 use_commit=
 amend=
+edit_flag=
 no_edit=
 log_given=
 log_message=
@@ -246,7 +247,7 @@ do
       shift
       ;;
   -e|--e|--ed|--edi|--edit)
-      no_edit=
+      edit_flag=t
       shift
       ;;
   -i|--i|--in|--inc|--incl|--inclu|--includ|--include)
@@ -384,6 +385,7 @@ do
       ;;
   esac
 done
+case "$edit_flag" in t) no_edit= ;; esac 
 
 ################################################################
 # Sanity check options
-- 
1.4.1.rc1.gf603-dirty

^ permalink raw reply related

* [PATCH] Customizable error handlers
From: Petr Baudis @ 2006-06-23 13:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This patch makes the usage(), die() and error() handlers customizable.
Nothing in the git code itself uses that but many other libgit users
(like Git.pm) will.

This is implemented using the mutator functions primarily because you
cannot directly modifying global variables of libgit from a program that
dlopen()ed it, apparently. But having functions for that is a better API
anyway.

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

 git-compat-util.h |   10 +++++++---
 usage.c           |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 5d543d2..e954002 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -36,9 +36,13 @@ #endif
 #endif
 
 /* General helper functions */
-extern void usage(const char *err) NORETURN;
-extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
-extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+void usage(const char *err) NORETURN;
+void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
+int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+
+void set_usage_routine(void (*routine)(const char *err) NORETURN);
+void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
+void set_error_routine(int (*routine)(const char *err, va_list params));
 
 #ifdef NO_MMAP
 
diff --git a/usage.c b/usage.c
index 1fa924c..445456d 100644
--- a/usage.c
+++ b/usage.c
@@ -12,28 +12,68 @@ static void report(const char *prefix, c
 	fputs("\n", stderr);
 }
 
-void usage(const char *err)
+void usage_builtin(const char *err)
 {
 	fprintf(stderr, "usage: %s\n", err);
 	exit(129);
 }
 
+void die_builtin(const char *err, va_list params)
+{
+	report("fatal: ", err, params);
+	exit(128);
+}
+
+int error_builtin(const char *err, va_list params)
+{
+	report("error: ", err, params);
+	return -1;
+}
+
+
+/* If we are in a dlopen()ed .so write to a global variable would segfault
+ * (ugh), so keep things static. */
+static void (*usage_routine)(const char *err) NORETURN = usage_builtin;
+static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;
+static int (*error_routine)(const char *err, va_list params) = error_builtin;
+
+void set_usage_routine(void (*routine)(const char *err) NORETURN)
+{
+	usage_routine = routine;
+}
+
+void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN)
+{
+	die_routine = routine;
+}
+
+void set_error_routine(int (*routine)(const char *err, va_list params))
+{
+	error_routine = routine;
+}
+
+
+void usage(const char *err)
+{
+	usage_routine(err);
+}
+
 void die(const char *err, ...)
 {
 	va_list params;
 
 	va_start(params, err);
-	report("fatal: ", err, params);
+	die_routine(err, params);
 	va_end(params);
-	exit(128);
 }
 
 int error(const char *err, ...)
 {
 	va_list params;
+	int ret;
 
 	va_start(params, err);
-	report("error: ", err, params);
+	ret = error_routine(err, params);
 	va_end(params);
-	return -1;
+	return ret;
 }

^ permalink raw reply related

* Re: [PATCH] Introduce Git.pm (v3)
From: Petr Baudis @ 2006-06-23 12:45 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <e7g079$8qt$1@sea.gmane.org>

Dear diary, on Fri, Jun 23, 2006 at 08:03:23AM CEST, I got a letter
where Jakub Narebski <jnareb@gmail.com> said that...
> Perhaps Git.pm should provide also generic, pure Perl (and slower)
> fallback implementation (when for some reason we cannot compile XS).

I fiercely want to avoid this if there is any other possible way to go
about it - this is a path to hell of massive code duplication and
additional work, as the number of routines will grow. If it is question
of spending many developer-hours uselessly duplicating code in a way
that'll be much slower than possible anyway OR building with -fPIC... ;-)

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

^ permalink raw reply

* Re: [PATCH] git-merge --squash
From: Thomas Glanzmann @ 2006-06-23 12:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0606231433370.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Hello,

> Isn't this the same as 'git-cherry-pick -n'? I often do a poor man's StGIT 
> by cherry picking my way through a messy branch, often combining patches 
> by '-n'.

yes it is. I didn't know about the cherry-pick -n option. Thanks.

        Thomas

^ permalink raw reply

* Re: [PATCH] Introduce Git.pm (v3)
From: Petr Baudis @ 2006-06-23 12:39 UTC (permalink / raw)
  To: Junio C Hamano, Eric W. Biederman; +Cc: git
In-Reply-To: <7vejxgckq9.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Fri, Jun 23, 2006 at 10:57:50AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Also, is there any real problem with just using -fPIC?
> 
> Personally, not really, but I consider it a workaround having to
> compile with -fPIC (being able to compile with -fPIC is a
> feature).

Well, for the .xs you do need an .so and for that you apparently need
-fPIC on most architectures, so there's no way around it.

There's a patch to build libgit.so, would you take it as an excuse to
always compile with -fPIC? ;-)

> Doesn't it have performance implications to use -fPIC when you
> do not have to?

No idea here.

> By the way, you also need to adjust the testsuite so that it
> finds the Perl modules from freshly built tree before
> installing.  I think (but haven't checked yet) the stuff written
> in Python does that already, so you might want to mimic it.

It should be enough to -I../perl/blib/lib -I../perl/blib/arch/auto/Git.


Dear diary, on Fri, Jun 23, 2006 at 02:04:17PM CEST, I got a letter
where "Eric W. Biederman" <ebiederm@xmission.com> said that...
> The question is why are we building with a .so?

To make use of it in Git.pm - it can call libgit routines directly from
inside of Perl, but for that it needs to dynamically link libgit to the
Perl process on the fly (using dlopen()).

We _can_ avoid the .so, but that involved producing a new perl
executable with libgit statically linked to it, which is quite
impractical, so to say.

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

^ permalink raw reply

* Re: [PATCH] git-merge --squash
From: Johannes Schindelin @ 2006-06-23 12:36 UTC (permalink / raw)
  To: Thomas Glanzmann; +Cc: Junio C Hamano, git
In-Reply-To: <20060623122501.GD15631@cip.informatik.uni-erlangen.de>

Hi,

On Fri, 23 Jun 2006, Thomas Glanzmann wrote:

> Hello Junio,
> 
> > So in that sense I would imagine --squash is not really useless
> > in such a situation as I made it sound like, but at the same
> > time I suspect people might be better off to use tools like
> > StGIT which are specially designed to support such a workflow if
> > they were to do this.
> 
> thanks for --squash. So --squash is basically a 'suck multiple deltas
> from another branch into ., but don't commit it'. I very often use that
> way of work flow. I do small and many commits, and when I am done I
> merge them to one a bit bigger one and submit it upstream. I useally use
> 'one branch per feature'.

Isn't this the same as 'git-cherry-pick -n'? I often do a poor man's StGIT 
by cherry picking my way through a messy branch, often combining patches 
by '-n'.

Ciao,
Dscho

^ permalink raw reply

* Re: git-format-patch builtin isn't using git-cherry?
From: Johannes Schindelin @ 2006-06-23 12:33 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: martin.langhoff, git
In-Reply-To: <20060623152321.2c20e9f8.tihirvon@gmail.com>

Hi,

On Fri, 23 Jun 2006, Timo Hirvonen wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > Basically, it will involve the following recipe:
> > 
> > 	- add a DIFF_FORMAT_PATCH_ID
> 
> Please don't add any DIFF_FORMAT_*.  I'm cleaning the diff output code
> and replacing diff_options.output_format with one-bit flags.

Okay. For the purposes of git-format-patch, this is not needed anyway, but 
rather a function which takes two tree objects and returns the patch id. 
When you are finished it should be easy to add this as a display format.

Ciao,
Dscho

^ 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