Git development
 help / color / mirror / Atom feed
* [PATCH] block-sha1: guard gcc extensions with __GNUC__
From: Nicolas Pitre @ 2009-08-18 19:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Sebastian Schuberth, Artur Skawina, Johannes Sixt, msysGit,
	Linus Torvalds, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0908181516510.6044@xanadu.home>

With this, the code should now be portable to any C compiler.

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

On Tue, 18 Aug 2009, Nicolas Pitre wrote:

> On Tue, 18 Aug 2009, Junio C Hamano wrote:
> 
> > Another issue, especially with your "openssl sha1 removal" patch, is if we
> > can assume gcc everywhere.  As far as I can tell, block-sha1/sha1.c will
> > be the first unconditional use of inline asm or statement expression on
> > i386/amd64.  Are folks on Solaris and other platforms Ok with this?
> 
> I guess we can guard the first with ifdef(__GNUC__) which should help 
> people with MSVC.  That should take care of x86 at least.

Here it is.

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index d31f2e3..92d9121 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -9,7 +9,7 @@
 
 #include "sha1.h"
 
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
 
 /*
  * Force usage of rol or ror by selecting the one with the smaller constant.
@@ -54,7 +54,7 @@
 
 #if defined(__i386__) || defined(__x86_64__)
   #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
-#elif defined(__arm__)
+#elif defined(__GNUC__) && defined(__arm__)
   #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
 #else
   #define setW(x, val) (W(x) = (val))

^ permalink raw reply related

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Junio C Hamano @ 2009-08-18 19:40 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Sebastian Schuberth, Artur Skawina, Johannes Sixt, msysGit,
	Linus Torvalds, Git Mailing List
In-Reply-To: <7vk511dk11.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Another issue, especially with your "openssl sha1 removal" patch, is if we
> can assume gcc everywhere.  As far as I can tell, block-sha1/sha1.c will
> be the first unconditional use of inline asm or statement expression on
> i386/amd64.  Are folks on Solaris and other platforms Ok with this?

Ahhh, your bswap.h is a no-op unless you use gcc, so my worry was
unfounded.

^ permalink raw reply

* learning curve (aka Git User's Survey 2009 partial summary, part 2 - from first 10)
From: Nicolas Sebrecht @ 2009-08-18 19:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Nicolas Sebrecht, git
In-Reply-To: <200908181900.03356.jnareb@gmail.com>

The 18/08/09, Jakub Narebski wrote:

> > I believe it would be interesting to know who (from the question 6.)
> > think what later. We may expect that people of the grade 4 and 5 ("can
> > offer advice" and "know it very well") underestimate the difficulty to
> > learn Git.
> 
> Well, you can do some analysis yourself, using just created filters at
> http://www.survs.com/shareResults?survey=2PIMZGU0&rndm=678J66QRA2

Thank you very much.

> For example:
> 
> ================================================|
>                   |    proficiency with Git     |
> To learn          |-----------------------------|
>                   | any*[%] | 1-3 [%] | 4-5 [%] |
> ------------------------------------------------|
> Very easy         |     4%  |     3%  |     5%  |
> Easy              |    20%  |    19%  |    22%  |
> Reasonably easy   |    55%  |    55%  |    56%  |
> Hard              |    19%  |    20%  |    16%  |
> Very hard         |     2%  |     3%  |     1%  |
> ------------------------------------------------|
> Total respondents |   3005  |   1786  |   1212  |
> ================================================|

This looks consistent. Here are some more grained summaries based on
your filters:


                      =========================================================
                      |                       TO LEARN (%)                    |
===============================================================================
Proficiency           | very easy | easy | reasonably easy | hard | very hard |
===============================================================================
no filter (any)       |      4    |  20  |       55        |  19  |     2     |
――――――――――――――――――――――|―――――――――――|――――――|―――――――――――――――――|――――――|―――――――――――|
1. novice             |      3    |  12  |       49        |  25  |    11     |
2. casual             |      2    |  17  |       54        |  25  |     3     |
3. everyday use       |      4    |  21  |       56        |  18  |     2     |
4. can offer advice   |      4    |  23  |       56        |  17  |     1     |
5. know it very well  |     11    |  20  |       54        |  14  |     1     |
===============================================================================

                      =========================================================
                      |                       TO USE   (%)                    |
===============================================================================
Proficiency           | very easy | easy | reasonably easy | hard | very hard |
===============================================================================
no filter (any)       |      9    |  37  |        45       |   8  |     1     |
――――――――――――――――――――――|―――――――――――|――――――|―――――――――――――――――|――――――|―――――――――――|
1. novice             |      5    |  17  |        47       |  25  |     7     |
2. casual             |      5    |  27  |        51       |  15  |     2     |
3. everyday use       |      8    |  36  |        47       |   8  |     1     |
4. can offer advice   |     11    |  44  |        40       |   4  |     0     |
5. know it very well  |     22    |  37  |        36       |   5  |     1     |
===============================================================================

What appears here first is that the averages of the third category
("everyday day use") are _very_ near from the averages of the whole users.
This gives a good confidence in the results.


Simplified summary:

                      ===============================================
                      |                  TO LEARN (%)               |
=====================================================================
Proficiency           | (very) easy | reasonably easy | (very) hard |
=====================================================================
no filter (any)       |     24      |        55       |      21     |
――――――――――――――――――――――|―――――――――――――|―――――――――――――――――|―――――――――――――|
1. novice             |     15      |        49       |      36     |
2. casual             |     19      |        54       |      28     |
----------------------|-------------|-----------------|-------------|
3. everyday use       |     25      |        56       |      20     |
----------------------|-------------|-----------------|-------------|
4. can offer advice   |     27      |        56       |      18     |
5. know it very well  |     31      |        54       |      15     |
=====================================================================

                      ===============================================
                      |                  TO USE   (%)               |
=====================================================================
Proficiency           | (very) easy | reasonably easy | (very) hard |
=====================================================================
no filter (any)       |     36      |        45       |       9     |
――――――――――――――――――――――|―――――――――――――|―――――――――――――――――|―――――――――――――|
1. novice             |     22      |        47       |      32     |
2. casual             |     32      |        51       |      17     |
----------------------|-------------|-----------------|-------------|
3. everyday use       |     44      |        47       |       9     |
----------------------|-------------|-----------------|-------------|
4. can offer advice   |     55      |        40       |       4     |
5. know it very well  |     59      |        36       |       6     |
=====================================================================


-- 
Nicolas Sebrecht

^ permalink raw reply

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Brandon Casey @ 2009-08-18 19:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, Sebastian Schuberth, Artur Skawina, Johannes Sixt,
	msysGit, Linus Torvalds, Git Mailing List
In-Reply-To: <7vk511dk11.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> Another issue, especially with your "openssl sha1 removal" patch, is if we
> can assume gcc everywhere.  As far as I can tell, block-sha1/sha1.c will
> be the first unconditional use of inline asm or statement expression on
> i386/amd64.  Are folks on Solaris and other platforms Ok with this?

The SUNWspro compiler doesn't set __i386__.  Instead it sets __i386, and
I think __x86_64 and __amd64 where appropriate.  So, compilation with
the SUNWspro compiler on x86 is currently unaffected by these changes and
falls back to the generic routines.

It seems that v5.10 of the compiler can grok both the __asm__ statements
and the ({...}) naked block notation and passes all of the tests when the
block_sha1 code is modified to add defined(__i386) to each of the macro
statements.

The 5.8 version cannot grok the naked block, and requires spelling __asm__
as __asm for inline assembly.  Even then it appears that there is a bug in
the assembly that is produced (a google search told me so), so the assembly
code does not successfully compile.

I haven't had much time to think about how or whether to address this.

Adding something like the following would get ugly real quick:

   (defined(__i386) && defined(__SUNPRO_C) && (__SUNPRO_C >= 0x5100))

For now, the code compiles fine using the SUNWspro compiler on x86 even if
it is suboptimal compared to gcc.  It is still an improvement over the
mozilla code.

-brandon

^ permalink raw reply

* Re: Continue git clone after interruption
From: Nicolas Pitre @ 2009-08-18 20:01 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Tomasz Kontusz, git
In-Reply-To: <m3fxbpneqe.fsf@localhost.localdomain>

On Tue, 18 Aug 2009, Jakub Narebski wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > On Tue, 18 Aug 2009, Tomasz Kontusz wrote:
> > 
> > > Ok, so it looks like it's not implementable without some kind of cache
> > > server-side, so the server would know what the pack it was sending
> > > looked like.
> > > But here's my idea: make server send objects in different order (the
> > > newest commit + whatever it points to first, then next one,then
> > > another...). Then it would be possible to look at what we got, tell
> > > server we have nothing, and want [the newest commit that was not
> > > complete]. I know the reason why it is sorted the way it is, but I think
> > > that the way data is stored after clone is clients problem, so the
> > > client should reorganize packs the way it wants.
> > 
> > That won't buy you much.  You should realize that a pack is made of:
> > 
> > 1) Commit objects.  Yes they're all put together at the front of the pack,
> >    but they roughly are the equivalent of:
> > 
> > 	git log --pretty=raw | gzip | wc -c
> > 
> >    For the Linux repo as of now that is around 32 MB.
> 
> For my clone of Git repository this gives 3.8 MB
>  
> > 2) Tree and blob objects.  Those are the bulk of the content for the top 
> >    commit.  The top commit is usually not delta compressed because we 
> >    want fast access to the top commit, and that is used as the base for 
> >    further delta compression for older commits.  So the very first 
> >    commit is whole at the front of the pack right after the commit 
> >    objects.  you can estimate the size of this data with:
> > 
> > 	git archive --format=tar HEAD | gzip | wc -c
> > 
> >    On the same Linux repo this is currently 75 MB.
> 
> On the same Git repository this gives 2.5 MB

Interesting to see that the commit history is larger than the latest 
source tree.  Probably that would be the same with the Linux kernel as 
well if all versions since the beginning with adequate commit logs were 
included in the repo.

> > 3) Delta objects.  Those are making the rest of the pack, plus a couple 
> >    tree/blob objects that were not found in the top commit and are 
> >    different enough from any object in that top commit not to be 
> >    represented as deltas.  Still, the majority of objects for all the 
> >    remaining commits are delta objects.
> 
> You forgot that delta chains are bound by pack.depth limit, which
> defaults to 50.  You would have then additional full objects.

Sure, but that's probably not significant.  the delta chain depth is 
limited, but not the width.  A given base object can have unlimited 
delta "children", and so on at each depth level.

> The single packfile for this (just gc'ed) Git repository is 37 MB.
> Much more than 3.8 MB + 2.5 MB = 6.3 MB.

What I'm saying is that most of that 37 MB - 6.3 MB = 31 MB is likely to 
be occupied by deltas.

> [cut]
> 
> There is another way which we can go to implement resumable clone.
> Let's git first try to clone whole repository (single pack; BTW what
> happens if this pack is larger than file size limit for given
> filesystem?).

We currently fail.  Seems that no one ever had a problem with that so 
far. We'd have to split the pack stream into multiple packs on the 
receiving end.  But frankly, if you have a repository large enough to 
bust your filesystem's file size limit then maybe you should seriously 
reconsider your choice of development environment.

> If it fails, client ask first for first half of of
> repository (half as in bisect, but it is server that has to calculate
> it).  If it downloads, it will ask server for the rest of repository.
> If it fails, it would reduce size in half again, and ask about 1/4 of
> repository in packfile first.

Problem people with slow links have won't be helped at all with this.  
What if the network connection gets broken only after 49% of the 
transfer and that took 3 hours to download?  You'll attempt a 25% size 
transfer which would take 1.5 hour despite the fact that you already 
spent that much time downloading that first 1/4 of the repository 
already.  And yet what if you're unlucky and now the network craps on 
you after 23% of that second attempt?

I think it is better to "prime" the repository with the content of the 
top commit in the most straight forward manner using git-archive which 
has the potential to be fully restartable at any point with little 
complexity on the server side.


Nicolas

^ permalink raw reply

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Nicolas Pitre @ 2009-08-18 20:10 UTC (permalink / raw)
  To: Brandon Casey
  Cc: Junio C Hamano, Sebastian Schuberth, Artur Skawina, Johannes Sixt,
	msysGit, Linus Torvalds, Git Mailing List
In-Reply-To: <XJM0H8pTiCJpryS-arPltHCHwsm0djqVixaH1NwBqT2pci2MA9karw@cipher.nrlssc.navy.mil>

On Tue, 18 Aug 2009, Brandon Casey wrote:

> The SUNWspro compiler doesn't set __i386__.  Instead it sets __i386, and
> I think __x86_64 and __amd64 where appropriate.  So, compilation with
> the SUNWspro compiler on x86 is currently unaffected by these changes and
> falls back to the generic routines.
> 
> It seems that v5.10 of the compiler can grok both the __asm__ statements
> and the ({...}) naked block notation and passes all of the tests when the
> block_sha1 code is modified to add defined(__i386) to each of the macro
> statements.

Does it really implement the gcc inline assembly syntax?


Nicolas

^ permalink raw reply

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Junio C Hamano @ 2009-08-18 20:17 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Junio C Hamano, Sebastian Schuberth, Artur Skawina, Johannes Sixt,
	msysGit, Linus Torvalds, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0908181411320.6044@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> On Tue, 18 Aug 2009, Junio C Hamano wrote:
>
>> To reduce confusion, you may want to rename compat/bswap.h to something
>> like compat/ntohl-htonl-fix.h ;-)
>
> Bah.  If you wish, you can edit the patch directly for this, unless you 
> really prefer me to repost.  Maybe we might want to add a 8-byte 
> versions of those as well eventually, which is why I chose a more 
> generic name.

Ok, here is what I came up with after many squashing...

-- >8 --
From: Nicolas Pitre <nico@cam.org>
Date: Tue, 18 Aug 2009 12:43:08 -0400
Subject: [PATCH] bswap: avoid potentially inefficient ntohl/htonl on i386/x86-64

Johannes Sixt reports that on Windows ntohl()/htonl() are not found in
<arpa/inet.h>, and as a minimal fix we need to include <winsock2.h>
instead.  Sebastian Schuberth points out that they are implemented as
out-of-line functions on Windows, which defeats these byteorder "macros"
used for performance.

Use bswap instruction through gcc inline asm instead on i386/x86-64
as a generic solution to this.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Nicolas Pitre <nico@cam.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 block-sha1/sha1.c |    4 +---
 compat/bswap.h    |   19 +++++++++++++++++++
 git-compat-util.h |    2 ++
 3 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 compat/bswap.h

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index 464cb25..51a27c1 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -4,9 +4,7 @@
  * and to avoid unnecessary copies into the context array.
  */
 
-#include <string.h>
-#include <arpa/inet.h>
-
+#include "../git-compat-util.h"
 #include "sha1.h"
 
 #if defined(__i386__) || defined(__x86_64__)
diff --git a/compat/bswap.h b/compat/bswap.h
new file mode 100644
index 0000000..78fd2df
--- /dev/null
+++ b/compat/bswap.h
@@ -0,0 +1,19 @@
+/*
+ * Let's make sure we always have a sane definition for ntohl()/htonl().
+ * Some libraries define those as a function call, just to perform byte
+ * swapping, bringing significant overhead to what should be a simple
+ * operation.
+ */
+
+#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
+
+#define bswap32(x) ({ \
+	unsigned int __res; \
+	__asm__("bswap %0" : "=r" (__res) : "0" (x)); \
+	__res; })
+#undef ntohl
+#undef htonl
+#define ntohl(x) bswap32(x)
+#define htonl(x) bswap32(x)
+
+#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index 9f941e4..000859e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -176,6 +176,8 @@ extern char *gitbasename(char *);
 #endif
 #endif
 
+#include "compat/bswap.h"
+
 /* General helper functions */
 extern void usage(const char *err) NORETURN;
 extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
-- 
1.6.4.245.g50659

^ permalink raw reply related

* Re: [PATCH] git-log: allow --decorate[=short|full]
From: Junio C Hamano @ 2009-08-18 20:18 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Jeff King, Junio C Hamano, Git Mailing List
In-Reply-To: <8c5c35580908150723h6e4c8fcay43b708969a86ae27@mail.gmail.com>

Lars Hjemli <hjemli@gmail.com> writes:

> diff --git a/builtin-log.c b/builtin-log.c
> index 3817bf1..1ed4c76 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -61,8 +61,15 @@ static void cmd_log_init(int argc, const char
> **argv, const char *prefix,
>  	for (i = 1; i < argc; i++) {
>  		const char *arg = argv[i];
>  		if (!strcmp(arg, "--decorate")) {
> -			load_ref_decorations();
> -			rev->show_decorations = 1;
> +			rev->show_decorations = DECORATE_SHORT_REFS;
> +		} else if (!prefixcmp(arg, "--decorate=")) {
> +			const char *v = skip_prefix(arg, "--decorate=");
> +			if (!strcmp(v, "full"))
> +				rev->show_decorations = DECORATE_FULL_REFS;
> +			else if (!strcmp(v, "short"))
> +				rev->show_decorations = DECORATE_SHORT_REFS;
> +			else
> +				die("invalid --decorate option: %s", arg);
>  		} else if (!strcmp(arg, "--source")) {
>  			rev->show_source = 1;
>  		} else if (!strcmp(arg, "-h")) {
> @@ -70,6 +77,8 @@ static void cmd_log_init(int argc, const char
> **argv, const char *prefix,
>  		} else
>  			die("unrecognized argument: %s", arg);
>  	}
> +	if (rev->show_decorations)
> +		load_ref_decorations(rev->show_decorations);
>  }

If you are deciding whether full refs are given or short ones when you
call load_ref_decorations(), I do not think there is any reason for you to
change the type of rev->short_decorations from bool to enum.  Shouldn't
you maintain a local variable in this function and pass it down to this
call instead?

That is, something like this on top (I had to fix the breakage your MUA
has done to your patch, so there might be some fuzz around whitespace).


 builtin-log.c |   13 ++++++++-----
 log-tree.c    |    2 +-
 revision.h    |    2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 2a0f5f7..25e21ed 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -35,6 +35,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		      struct rev_info *rev)
 {
 	int i;
+	int decoration_style = 0;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
@@ -61,13 +62,13 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--decorate")) {
-			rev->show_decorations = DECORATE_SHORT_REFS;
+			decoration_style = DECORATE_SHORT_REFS;
 		} else if (!prefixcmp(arg, "--decorate=")) {
 			const char *v = skip_prefix(arg, "--decorate=");
 			if (!strcmp(v, "full"))
-				rev->show_decorations = DECORATE_FULL_REFS;
+				decoration_style = DECORATE_FULL_REFS;
 			else if (!strcmp(v, "short"))
-				rev->show_decorations = DECORATE_SHORT_REFS;
+				decoration_style = DECORATE_SHORT_REFS;
 			else
 				die("invalid --decorate option: %s", arg);
 		} else if (!strcmp(arg, "--source")) {
@@ -77,8 +78,10 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		} else
 			die("unrecognized argument: %s", arg);
 	}
-	if (rev->show_decorations)
-		load_ref_decorations(rev->show_decorations);
+	if (decoration_style) {
+		rev->show_decorations = 1;
+		load_ref_decorations(decoration_style);
+	}
 }
 
 /*
diff --git a/log-tree.c b/log-tree.c
index 1c767c9..1c9eefe 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -25,7 +25,7 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 	struct object *obj = parse_object(sha1);
 	if (!obj)
 		return 0;
-	if (!cb_data || *(int *)cb_data & DECORATE_SHORT_REFS)
+	if (!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS)
 		refname = prettify_refname(refname);
 	add_name_decoration("", refname, obj);
 	while (obj->type == OBJ_TAG) {
diff --git a/revision.h b/revision.h
index 9a644ee..b10984b 100644
--- a/revision.h
+++ b/revision.h
@@ -59,6 +59,7 @@ struct rev_info {
 			rewrite_parents:1,
 			print_parents:1,
 			show_source:1,
+			show_decorations:1,
 			reverse:1,
 			reverse_output_stage:1,
 			cherry_pick:1,
@@ -98,7 +99,6 @@ struct rev_info {
 	const char	*subject_prefix;
 	int		no_inline;
 	int		show_log_size;
-	int		show_decorations;
 
 	/* Filter by commit log message */
 	struct grep_opt	grep_filter;

^ permalink raw reply related

* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Junio C Hamano @ 2009-08-18 20:20 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthieu Moy, git, Johannes Schindelin
In-Reply-To: <20090818174509.GA27518@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Aug 18, 2009 at 03:01:52PM +0200, Matthieu Moy wrote:
> ...
> I think yours is nicer, especially as we have just added the
> '-p|--patch' option, as well. With what is there now, you can do "git
> stash -p", but not "git stash -p -k".
>
>> But I may have missed its drawbacks ;-)
>
> The only I can think of is that bogus input will provoke 'save'. So
> something like:
>
>   git stash --apply
>
> will invoke "git stash save --apply", which doesn't even complain. It
> just tries to make a stash with message --apply. Now of course this
> input is obviously bogus, but probably the right thing to do is
> complain.
>
> OTOH, I think it is the fault of "git stash save --apply" for not doing
> the complaining, so your patch really isn't making it worse. Probably it
> should barf on anything unrecognized starting with a '-', and allow '--'
> to separate the message from the rest of the options (in the rare case
> that you want a message starting with '-').

Sounds like a sane approach.

I am Ok with the idea of queuing a patch to update js/stash-dwim ea41cfc
(Make 'git stash -k' a short form for 'git stash save --keep-index',
2009-07-27) with an implementation along this line; the "stash -p" one
f300fab (DWIM 'git stash save -p' for 'git stash -p', 2009-08-13) also
may need to adjusted (it could become no-op, I suspect).

^ permalink raw reply

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Linus Torvalds @ 2009-08-18 20:25 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Brandon Casey, Junio C Hamano, Sebastian Schuberth, Artur Skawina,
	Johannes Sixt, msysGit, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0908181607240.6044@xanadu.home>



On Tue, 18 Aug 2009, Nicolas Pitre wrote:
> 
> Does it really implement the gcc inline assembly syntax?

A fair number of compilers do. The Intel compiler does too.

The reason? The gcc inline asm is as close to a standard there is, and is 
fairly expressive without being the mess that is MSC. So if you do inline 
asm at all (and considering the target market for a C compiler, most do), 
gcc asm is a good thing to aim for.

			Linus

^ permalink raw reply

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Brandon Casey @ 2009-08-18 20:29 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Junio C Hamano, Sebastian Schuberth, Artur Skawina, Johannes Sixt,
	msysGit, Linus Torvalds, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0908181607240.6044@xanadu.home>

Nicolas Pitre wrote:
> On Tue, 18 Aug 2009, Brandon Casey wrote:
> 
>> The SUNWspro compiler doesn't set __i386__.  Instead it sets __i386, and
>> I think __x86_64 and __amd64 where appropriate.  So, compilation with
>> the SUNWspro compiler on x86 is currently unaffected by these changes and
>> falls back to the generic routines.
>>
>> It seems that v5.10 of the compiler can grok both the __asm__ statements
>> and the ({...}) naked block notation and passes all of the tests when the
>> block_sha1 code is modified to add defined(__i386) to each of the macro
>> statements.
> 
> Does it really implement the gcc inline assembly syntax?

Yes, I think it does.  I actually extracted the assembly from sha1.c and tested
in a separate test program.  The v5.10 sunwspro compiler compiles and executes
it correctly.

-brandon

^ permalink raw reply

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Junio C Hamano @ 2009-08-18 20:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, Sebastian Schuberth, Artur Skawina, Johannes Sixt,
	msysGit, Linus Torvalds, Git Mailing List
In-Reply-To: <7vprasc1vu.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Nicolas Pitre <nico@cam.org> writes:
>
>> On Tue, 18 Aug 2009, Junio C Hamano wrote:
>>
>>> To reduce confusion, you may want to rename compat/bswap.h to something
>>> like compat/ntohl-htonl-fix.h ;-)
>>
>> Bah.  If you wish, you can edit the patch directly for this, unless you 
>> really prefer me to repost.  Maybe we might want to add a 8-byte 
>> versions of those as well eventually, which is why I chose a more 
>> generic name.
>
> Ok, here is what I came up with after many squashing...

Meh, our mails crossed.  I'll chuck this one and use your

    [PATCH] make sure byte swapping is optimal for git

patch.  Do you want default_swab32 be mmoved inside the

    #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))

block?

^ permalink raw reply

* [PATCH][resend 2] git-svn: Use GIT_SSH setting if SVN_SSH is not set
From: Karthik R @ 2009-08-18 20:31 UTC (permalink / raw)
  To: git

If SVN_SSH is defined, it will be used; else value in
GIT_SSH is copied to SVN_SSH & then, only on Windows,
the \s are escaped.

On Windows, the shell-variables must be set as follows
    GIT_SSH="C:\Program Files\PuTTY\plink.exe"
    SVN_SSH="C:\\Program Files\\PuTTY\\plink.exe"

See http://code.google.com/p/msysgit/issues/detail?id=305
---
Trying again following Dscho's comment (removed MSWin32) & using
logic from Junio (don't muck with user's SVN_SSH; else use GIT_SSH
to seed SVN_SSH & adjust the \s on Windows)

Previous life of patch:
[PATCH] GIT_SSH does not override ssh in git-svn
[PATCH][resend] git-svn: Respect GIT_SSH setting

 git-svn.perl |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index b0bfb74..ef86d10 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -21,6 +21,15 @@ $Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 
'git-svn';
 $Git::SVN::Ra::_log_window_size = 100;
 $Git::SVN::_minimize_url = 'unset';
 
+if (! exists $ENV{SVN_SSH}) {
+    if (exists $ENV{GIT_SSH}) {
+        $ENV{SVN_SSH} = $ENV{GIT_SSH};
+        if ($^O eq 'msys') {
+            $ENV{SVN_SSH} =~ s/\\/\\\\/g;
+        }
+    }
+}
+
 $Git::SVN::Log::TZ = $ENV{TZ};
 $ENV{TZ} = 'UTC';
 $| = 1; # unbuffer STDOUT
-- 
1.5.4.3

^ permalink raw reply related

* Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
From: Nicolas Pitre @ 2009-08-18 20:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Sebastian Schuberth, Artur Skawina, Johannes Sixt, msysGit,
	Linus Torvalds, Git Mailing List
In-Reply-To: <7v7hx0c1at.fsf@alter.siamese.dyndns.org>

On Tue, 18 Aug 2009, Junio C Hamano wrote:

> Meh, our mails crossed.  I'll chuck this one and use your
> 
>     [PATCH] make sure byte swapping is optimal for git
> 
> patch.  Do you want default_swab32 be mmoved inside the
> 
>     #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
> 
> block?

Not necessarily.  It is generic code that other compilers/architectures 
might use as well.


Nicolas

^ permalink raw reply

* Re: Continue git clone after interruption
From: Jakub Narebski @ 2009-08-18 21:02 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Tomasz Kontusz, git
In-Reply-To: <alpine.LFD.2.00.0908181537360.6044@xanadu.home>

On Tue, 18 Aug 2009, Nicolas Pitre wrote:
> On Tue, 18 Aug 2009, Jakub Narebski wrote:
>> Nicolas Pitre <nico@cam.org> writes:

>>> That won't buy you much.  You should realize that a pack is made of:
>>> 
>>> 1) Commit objects.  Yes they're all put together at the front of the pack,
>>>    but they roughly are the equivalent of:
>>> 
>>> 	git log --pretty=raw | gzip | wc -c
>>> 
>>>    For the Linux repo as of now that is around 32 MB.
>> 
>> For my clone of Git repository this gives 3.8 MB
>>  
>>> 2) Tree and blob objects.  Those are the bulk of the content for the top 
>>>    commit. [...]  You can estimate the size of this data with:
>>> 
>>> 	git archive --format=tar HEAD | gzip | wc -c
>>> 
>>>    On the same Linux repo this is currently 75 MB.
>> 
>> On the same Git repository this gives 2.5 MB
> 
> Interesting to see that the commit history is larger than the latest 
> source tree.  Probably that would be the same with the Linux kernel as 
> well if all versions since the beginning with adequate commit logs were 
> included in the repo.

Note that having reflog and/or patch management interface like StGit,
and frequently reworking commits (e.g. using rebase) means more commit
objects in repository.

Also Git repository has 3 independent branches: 'man', 'html' and 'todo',
from whose branches objects are not included in "git archive HEAD".

> 
>>> 3) Delta objects.  Those are making the rest of the pack, plus a couple 
>>>    tree/blob objects that were not found in the top commit and are 
>>>    different enough from any object in that top commit not to be 
>>>    represented as deltas.  Still, the majority of objects for all the 
>>>    remaining commits are delta objects.
>> 
>> You forgot that delta chains are bound by pack.depth limit, which
>> defaults to 50.  You would have then additional full objects.
> 
> Sure, but that's probably not significant.  the delta chain depth is 
> limited, but not the width.  A given base object can have unlimited 
> delta "children", and so on at each depth level.

You can probably get number and size taken by delta and non-delta (base)
objects in the packfile somehow.  Neither "git verify-pack -v <packfile>"
nor contrib/stats/packinfo.pl did help me arrive at this data.

>> The single packfile for this (just gc'ed) Git repository is 37 MB.
>> Much more than 3.8 MB + 2.5 MB = 6.3 MB.
> 
> What I'm saying is that most of that 37 MB - 6.3 MB = 31 MB is likely to 
> be occupied by deltas.

True.
 
>> [cut]
>> 
>> There is another way which we can go to implement resumable clone.
>> Let's git first try to clone whole repository (single pack; BTW what
>> happens if this pack is larger than file size limit for given
>> filesystem?).
> 
> We currently fail.  Seems that no one ever had a problem with that so 
> far. We'd have to split the pack stream into multiple packs on the 
> receiving end.  But frankly, if you have a repository large enough to 
> bust your filesystem's file size limit then maybe you should seriously 
> reconsider your choice of development environment.

Do we fail gracefully (with an error message), or does git crash then?

If I remember correctly FAT28^W FAT32 has maximum file size of 2 GB.
FAT is often used on SSD, on USB drive.  Although if you have  2 GB
packfile, you are doing something wrong, or UGFWIINI (Using Git For
What It Is Not Intended).
 
>> If it fails, client ask first for first half of of
>> repository (half as in bisect, but it is server that has to calculate
>> it).  If it downloads, it will ask server for the rest of repository.
>> If it fails, it would reduce size in half again, and ask about 1/4 of
>> repository in packfile first.
> 
> Problem people with slow links have won't be helped at all with this.  
> What if the network connection gets broken only after 49% of the 
> transfer and that took 3 hours to download?  You'll attempt a 25% size 
> transfer which would take 1.5 hour despite the fact that you already 
> spent that much time downloading that first 1/4 of the repository 
> already.  And yet what if you're unlucky and now the network craps on 
> you after 23% of that second attempt?

A modification then.

First try ordinary clone.  If it fails because network is unreliable,
check how much we did download, and ask server for packfile of slightly
smaller size; this means that we are asking server for approximate pack
size limit, not for bisect-like partitioning revision list.

> I think it is better to "prime" the repository with the content of the 
> top commit in the most straight forward manner using git-archive which 
> has the potential to be fully restartable at any point with little 
> complexity on the server side.

But didn't it make fully restartable 2.5 MB part out of 37 MB packfile?

A question about pack protocol negotiation.  If clients presents some
objects as "have", server can and does assume that client has all 
prerequisites for such objects, e.g. for tree objects that it has
all objects for files and directories inside tree; for commit it means
all ancestors and all objects in snapshot (have top tree, and its 
prerequisites).  Do I understand this correctly?

If we have partial packfile which crashed during downloading, can we
extract from it some full objects (including blobs)?  Can we pass
tree and blob objects as "have" to server, and is it taken into account?
Perhaps instead of separate step of resumable-downloading of top commit
objects (in snapshot), we can pass to server what we did download in
full?


BTW. because of compression it might be more difficult to resume 
archive creation in the middle, I think...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* RE: interaction between --graph and --simplify-by-decoration
From: Adam Simpkins @ 2009-08-18 20:55 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Git Mailing List

On Friday, July 31, 2009 4:11am, "Santi Béjar" <santi@agolina.net> said:
> Hello,
> 
>   I've found that in some cases the --graph and
> --simplify-by-decoration don't work well together. If you do this in
> the git.git repository:

Thanks for reporting the problem, I apologize for taking so long to
investigate and respond.


> * | | f29ac4f (tag: v1.6.3-rc2) GIT 1.6.3-rc2
>  / /
> | | *   66996ec Sync with 1.6.2.4

> you can see that f29ac4f looks like it does not have any parents while
> the correct parent is 66996ec which seems to have no children. But if
> you omit the --oneline you can see that there are a lot of "root"-like
> commits (f01f109, a48f5d7, f29ac4f,...).

Yes, there's a bug in graph_is_interesting().  When processing
f29ac4f, the graph code thinks that 66996ec isn't interesting and
won't get displayed in the output, so it doesn't prepare the graph
lines to show lines to 66996ec.

I'll submit a patch shortly.

--
Adam Simpkins
adam@adamsimpkins.net

^ permalink raw reply

* Re: Continue git clone after interruption
From: Nicolas Pitre @ 2009-08-18 21:32 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Tomasz Kontusz, git
In-Reply-To: <200908182302.10619.jnareb@gmail.com>

On Tue, 18 Aug 2009, Jakub Narebski wrote:

> You can probably get number and size taken by delta and non-delta (base)
> objects in the packfile somehow.  Neither "git verify-pack -v <packfile>"
> nor contrib/stats/packinfo.pl did help me arrive at this data.

Documentation for verify-pack says:

|When specifying the -v option the format used is:
|
|        SHA1 type size size-in-pack-file offset-in-packfile
|
|for objects that are not deltified in the pack, and
|
|        SHA1 type size size-in-packfile offset-in-packfile depth base-SHA1
|
|for objects that are deltified.

So a simple script should be able to give you the answer.

> >> (BTW what happens if this pack is larger than file size limit for 
> >> given filesystem?).
> > 
> > We currently fail.  Seems that no one ever had a problem with that so 
> > far. We'd have to split the pack stream into multiple packs on the 
> > receiving end.  But frankly, if you have a repository large enough to 
> > bust your filesystem's file size limit then maybe you should seriously 
> > reconsider your choice of development environment.
> 
> Do we fail gracefully (with an error message), or does git crash then?

If the filesystem is imposing the limit, it will likely return an error 
on the write() call and we'll die().  If the machine has a too small 
off_t for the received pack then we also die("pack too large for current 
definition of off_t").

> If I remember correctly FAT28^W FAT32 has maximum file size of 2 GB.
> FAT is often used on SSD, on USB drive.  Although if you have  2 GB
> packfile, you are doing something wrong, or UGFWIINI (Using Git For
> What It Is Not Intended).

Hopefully you're not performing a 'git clone' off of a FAT filesystem.  
For physical transport you may repack with the appropriate switches.

> >> If it fails, client ask first for first half of of
> >> repository (half as in bisect, but it is server that has to calculate
> >> it).  If it downloads, it will ask server for the rest of repository.
> >> If it fails, it would reduce size in half again, and ask about 1/4 of
> >> repository in packfile first.
> > 
> > Problem people with slow links have won't be helped at all with this.  
> > What if the network connection gets broken only after 49% of the 
> > transfer and that took 3 hours to download?  You'll attempt a 25% size 
> > transfer which would take 1.5 hour despite the fact that you already 
> > spent that much time downloading that first 1/4 of the repository 
> > already.  And yet what if you're unlucky and now the network craps on 
> > you after 23% of that second attempt?
> 
> A modification then.
> 
> First try ordinary clone.  If it fails because network is unreliable,
> check how much we did download, and ask server for packfile of slightly
> smaller size; this means that we are asking server for approximate pack
> size limit, not for bisect-like partitioning revision list.

If the download didn't reach past the critical point (75 MB in my linux 
repo example) then you cannot validate the received data and you've 
wasted that much bandwidth.

> > I think it is better to "prime" the repository with the content of the 
> > top commit in the most straight forward manner using git-archive which 
> > has the potential to be fully restartable at any point with little 
> > complexity on the server side.
> 
> But didn't it make fully restartable 2.5 MB part out of 37 MB packfile?

The front of the pack is the critical point.  If you get enough to 
create the top commit then further transfers can be done incrementally 
with only the deltas between each commits.

> A question about pack protocol negotiation.  If clients presents some
> objects as "have", server can and does assume that client has all 
> prerequisites for such objects, e.g. for tree objects that it has
> all objects for files and directories inside tree; for commit it means
> all ancestors and all objects in snapshot (have top tree, and its 
> prerequisites).  Do I understand this correctly?

That works only for commits.

> If we have partial packfile which crashed during downloading, can we
> extract from it some full objects (including blobs)?  Can we pass
> tree and blob objects as "have" to server, and is it taken into account?

No.

> Perhaps instead of separate step of resumable-downloading of top commit
> objects (in snapshot), we can pass to server what we did download in
> full?

See above.

> BTW. because of compression it might be more difficult to resume 
> archive creation in the middle, I think...

Why so?  the tar+gzip format is streamable.


Nicolas

^ permalink raw reply

* Re: [RFC] Enable compilation by Makefile for the MSVC toolchain
From: Johannes Schindelin @ 2009-08-18 21:35 UTC (permalink / raw)
  To: Pau Garcia i Quiles
  Cc: Marius Storm-Olsen, msysgit, git, lznuaa, bonzini, kusmabite
In-Reply-To: <3af572ac0908181007q7622091eqc78861e00c3a9e45@mail.gmail.com>

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

Hi,

On Tue, 18 Aug 2009, Pau Garcia i Quiles wrote:

> On Tue, Aug 18, 2009 at 4:11 PM, Johannes
> Schindelin<Johannes.Schindelin@gmx.de> wrote:
>
> > On Tue, 18 Aug 2009, Marius Storm-Olsen wrote:
> >
> >>  So, instead of rely on these vcproj files which *will* go stale, we 
> >> can  simply use the same Makefile system which everyone else is 
> >> using. :)  After all, we're just compiling with a different compiler. 
> >> The end  result will still rely on the *msysGit environment* to 
> >> function, so we  already require it. Thus, GNU Make is present, and 
> >> we can use it.
> >
> > We can also use sed or perl to generate/modify the .vcproj files, or 
> > run CMake (once Pau got it to build), and package the stuff using zip 
> > (once I got that to build).
> 
> Re: package the stuff using ZIP, it's trivial to do with CPack (part of 
> the CMake chain). Just tell me what you want to get and I'll implement 
> it.

Well, I tried to tell you already what I want.  A simple and unsupervised 
script that downloads, compiles, installs and commits CMake.

Ciao,
Dscho

^ permalink raw reply

* Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d
From: Fredrik Kuivinen @ 2009-08-18 21:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin <johannes.schindelin@gmx.de> Alex Riesen,
	Git, Josh ben Jore
In-Reply-To: <7viqhaipg0.fsf@alter.siamese.dyndns.org>

On Thu, Jul 30, 2009 at 9:45 AM, Junio C Hamano<gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that
>> "unmerged" path, while the other branch added something else to the same
>> path, and decided to add that at an alternative path, and the intent of
>> that is so that it can safely resolve the "renamed" side to its final
>> destination.  The added update_file() call is about finishing that
>> conflict resolution the code forgets to do.
>
> By the way, (I am CC'ing Fredrik, the author of the merge-recursive.py,
> the original implementation, even though I haven't seen him around here
> for a long time. Two "thieves" are also CC'ed), I think the way recursive
> strategy tries to handle rename/add and rename/rename conflicts by coming
> up with a temporary pathname is fundamentally wrong.

It was a long time ago I looked at the merge stuff (or any git code in
fact) so I
may have forgotten some things. However, as far as I can see your analysis is
correct and the temporary pathname stuff can be avoided when the conflicts
only involve files (and no directories). As you wrote, for D/F
conflicts one need
the temporary pathnames in order to build a tree which is used in the virtual
ancestor.

[Nice analysis of when temporary pathnames are needed and when they are not
needed snipped.]

- Fredrik

PS. I am sorry for the late reply.

^ permalink raw reply

* Re: x86 SHA1: Faster than OpenSSL
From: Andy Polyakov @ 2009-08-18 21:26 UTC (permalink / raw)
  To: George Spelvin; +Cc: git
In-Reply-To: <20090803034741.23415.qmail@science.horizon.com>

George Spelvin wrote:
> (Work in progress, state dump to mailing list archives.)
> 
> This started when discussing git startup overhead due to the dynamic
> linker.  One big contributor is the openssl library, which is used only
> for its optimized x86 SHA-1 implementation.  So I took a look at it,
> with an eye to importing the code directly into the git source tree,
> and decided that I felt like trying to do better.
> 
> The original code was excellent, but it was optimized when the P4 was new.

Even though last revision took place when "the P4 was new" and even
triggered by its appearance, *all-round* performance was and will always
be the prime goal. This means that improvements on some particular
micro-architecture is always weighed against losses on others [and
compromise is considered of so required]. Please note that I'm *not*
trying to diminish George's effort by saying that proposed code is
inappropriate, on the contrary I'm nothing but grateful! Thanks, George!
I'm only saying that it will be given thorough consideration. Well, I've
actually given the consideration and outcome is already committed:-) See
http://cvs.openssl.org/chngview?cn=18513. I don't deliver +17%, only
+12%, but at the cost of Intel Atom-specific optimizations. I used this
opportunity to optimize even for Intel Atom core, something I was
planning to do at some point anyway...

>   http://www.openssl.org/~appro/cryptogams/cryptogams-0.tar.gz
> - "tar xz cryptogams-0.tar.gz"

If there is interest I can pack new tar ball with updated modules.

> An open question is how to add appropriate CPU detection to the git
> build scripts. (Note that `uname -m`, which it currently uses to select
> the ARM code, does NOT produce the right answer if you're using a 32-bit
> compiler on a 64-bit platform.)

It's not only that. As next subscriber noted problem on MacOS X, it
[MacOS X] uses slightly different assembler convention and ELF modules
can't be compiled on MacOS X. OpenSSL perlasm takes care of several
assembler flavors and executable formats, including MacOS X. I'm talking
about

> +++ Makefile	2009-08-02 06:44:44.000000000 -0400
> +%.s : %.pl x86asm.pl x86unix.pl
> +	perl $< elf > $@
                ^^^ this argument.

Cheers. A.

^ permalink raw reply

* [PATCH 1/3] stash: accept -k as a shortcut for --keep-index
From: Matthieu Moy @ 2009-08-18 21:38 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1250631523-10524-1-git-send-email-Matthieu.Moy@imag.fr>


Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-stash.txt |    4 ++--
 git-stash.sh                |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 4b15459..1b5392a 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]]
+'git stash' [save [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]]
 'git stash' clear
 'git stash' create
 
@@ -42,7 +42,7 @@ is also possible).
 OPTIONS
 -------
 
-save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::
+save [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]::
 
 	Save your local modifications to a new 'stash', and run `git reset
 	--hard` to revert them.  This is the default action when no
diff --git a/git-stash.sh b/git-stash.sh
index 567aa5d..856a2d5 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,7 +7,7 @@ USAGE="list [<options>]
    or: $dashless drop [-q|--quiet] [<stash>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
-   or: $dashless [save [--keep-index] [-q|--quiet] [<message>]]
+   or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -132,7 +132,7 @@ save_stash () {
 	while test $# != 0
 	do
 		case "$1" in
-		--keep-index)
+		-k|--keep-index)
 			keep_index=t
 			;;
 		--no-keep-index)
-- 
1.6.4.rc2.31.g2d7d7

^ permalink raw reply related

* [PATCH 2/3] stash: accept options also when subcommand 'save' is omitted
From: Matthieu Moy @ 2009-08-18 21:38 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1250631523-10524-2-git-send-email-Matthieu.Moy@imag.fr>

This allows in particular 'git stash -k which is shorter than
'git stash save -k', and not ambiguous.

Testcase taken from Johannes Schindelin <johannes.schindelin@gmx.de>.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-stash.txt |    2 +-
 git-stash.sh                |    8 +++++++-
 t/t3903-stash.sh            |    8 ++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1b5392a..7e515ce 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [save [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]]
+'git stash' [save] [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]
 'git stash' clear
 'git stash' create
 
diff --git a/git-stash.sh b/git-stash.sh
index 856a2d5..bb36bc7 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,7 +7,7 @@ USAGE="list [<options>]
    or: $dashless drop [-q|--quiet] [<stash>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
-   or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
+   or: $dashless [save] [-k|--keep-index] [-q|--quiet] [<message>]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -354,6 +354,12 @@ apply_to_branch () {
 	drop_stash $stash
 }
 
+case "$1" in
+    -*)
+	set "save" "$@"
+	;;
+esac
+
 # Main command set
 case "$1" in
 list)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 7a3fb67..0e831e0 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -200,4 +200,12 @@ test_expect_success 'drop -q is quiet' '
 	test ! -s output.out
 '
 
+test_expect_success 'stash -k' '
+       echo bar3 > file &&
+       echo bar4 > file2 &&
+       git add file2 &&
+       git stash -k &&
+       test bar,bar4 = $(cat file),$(cat file2)
+'
+
 test_done
-- 
1.6.4.rc2.31.g2d7d7

^ permalink raw reply related

* [PATCH] graph API: use a new color when starting a brand new column
From: Adam Simpkins @ 2009-08-18 21:41 UTC (permalink / raw)
  To: git; +Cc: Allan Caffee, Junio C Hamano

Use a new color for commits that don't have any previously printed
children.  The following command demonstrates the changes:

  git log --graph --pretty=tformat:'%h %s%n' -7 481c7a6 18b0793

Now the two independent lines of development are displayed with
different colors, instead of both using the same color.

Signed-off-by: Adam Simpkins <simpkins@facebook.com>
---
 graph.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/graph.c b/graph.c
index e466770..f3226ec 100644
--- a/graph.c
+++ b/graph.c
@@ -225,7 +225,12 @@ struct git_graph *graph_init(struct rev_info *opt)
 	graph->num_columns = 0;
 	graph->num_new_columns = 0;
 	graph->mapping_size = 0;
-	graph->default_column_color = 0;
+	/*
+	 * Start the column color at the maximum value, since we'll
+	 * always increment it for the first commit we output.
+	 * This way we start at 0 for the first commit.
+	 */
+	graph->default_column_color = COLUMN_COLORS_MAX - 1;
 
 	/*
 	 * Allocate a reasonably large default number of columns
@@ -499,11 +504,14 @@ static void graph_update_columns(struct git_graph *graph)
 			     parent;
 			     parent = next_interesting_parent(graph, parent)) {
 				/*
-				 * If this is a merge increment the current
+				 * If this is a merge, or the start of a new
+				 * childless column, increment the current
 				 * color.
 				 */
-				if (graph->num_parents > 1)
+				if (graph->num_parents > 1 ||
+				    !is_commit_in_columns) {
 					graph_increment_column_color(graph);
+				}
 				graph_insert_into_new_columns(graph,
 							      parent->item,
 							      &mapping_idx);
-- 
1.6.4.314.g5667f

^ permalink raw reply related

* [PATCH 0/3] short syntaxes for 'git stash'
From: Matthieu Moy @ 2009-08-18 21:38 UTC (permalink / raw)
  To: git, gitster
In-Reply-To: <7vbpmcc1sc.fsf@alter.siamese.dyndns.org>

This small patch serie is based on the following commit in pu:

dda1f2a Implement 'git stash save --patch'

It is meant to replace two commits already there:

ea41cfc Make 'git stash -k' a short form for 'git stash save --keep-index'
f300fab DWIM 'git stash save -p' for 'git stash -p'

The first (git stash -k) has the drawback of forcing one to use one
and only one option when the 'save' command is ommited. My approach is
mostly based on the simple hack:

+case "$1" in
+    -*)
+       set "save" "$@"
+       ;;
+esac
+

and accepts an arbitrary number of options.

The second (DWIM git stash save -p) becomes unnecessary afterwards.

^ permalink raw reply

* Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted
From: Johannes Schindelin @ 2009-08-18 21:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthieu Moy, git
In-Reply-To: <20090818174509.GA27518@coredump.intra.peff.net>

Hi,

On Tue, 18 Aug 2009, Jeff King wrote:

> On Tue, Aug 18, 2009 at 03:01:52PM +0200, Matthieu Moy wrote:
> 
> > Hmm, googling a bit, I just noticed that there's already something in
> > pu:
> > ea41cfc4f (Make 'git stash -k' a short form for 'git stash save --keep-index')
> > which also does the trick, while adding a -k alias for --keep-index.
> >
> > [...]
> > 
> > Mine has at least two advantages:
> > 
> > * It won't require changing the code again when new options are added
> >   to 'git stash save'.
> > 
> > * It works with 'git stash -k -q' for example, while the other
> >   proposal checks that $# == 1, which won't work if there are more
> >   than one option.
> 
> I think yours is nicer, especially as we have just added the
> '-p|--patch' option, as well. With what is there now, you can do "git
> stash -p", but not "git stash -p -k".

But it is sloppy, in that it blindly accepts options that might be valid 
for several subcommands, not just "save".

That was the reason I did not implement it this way.

But we do not have such ambiguous options yet.

Or do we?  Look at what "list" accepts!

So please register my objection.

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