Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-18 Thread Karsten Blees
Am 09.12.2013 15:03, schrieb Karsten Blees:
> 3.) Inject individual fields via macro
> 
> Instead of injecting a struct hashmap_entry, which implies alignment to 
> sizeof(struct hashmap_entry), we could inject the individual fields, e.g.
> 
>  #define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int 
> __hash;
> 
>  struct name_entry {
>HASHMAP_ENTRY_HEADER
>int namelen;
>char *name;
>  };
> 

I've tried this as well. However, the change is much more intrusive, and 
produces lots of strict-aliasing warnings in GCC 4.4, probably due to this bug 
[1]. So I don't think its a good solution. For anyone interested, the patch can 
be found at [2].

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42032
[2] https://github.com/kblees/git/commits/kb/hashmap-v5-fixes-macro


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-18 Thread Karsten Blees
Am 18.12.2013 14:10, schrieb Karsten Blees:
> + printf("sizeof(pointer+int) (%u) is not a "
> +"multiple of sizeof(pointer) (%u)!\n",
> +sizeof(struct pointer_int),
> +sizeof(void *));

Should be:
-  sizeof(struct pointer_int),
-  sizeof(void *));
+  (unsigned) sizeof(struct pointer_int),
+  (unsigned) sizeof(void *));

(sizeof() is 'unsigned int' on 32-bit and 'long unsigned int' on 64-bit; 
without the cast, "%u" produces warnings on 64-bit and "%lu" on 32-bit...)

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-18 Thread Karsten Blees
Am 10.12.2013 00:45, schrieb Jonathan Nieder:
> Karsten Blees wrote:
> 
>> Googling some more, I believe the most protable way to achieve this
>> via 'compiler settings' is
>>
>>  #pragma pack(push)
>>  #pragma pack(4)
>>  struct hashmap_entry {
>>struct hashmap_entry *next;
>>unsigned int hash;
>>  };
>>  #pragma pack(pop)
>>
>> This is supported by at least GCC, MSVC and HP (see your link). The
>> downside is that we cannot use macros (in git-compat-util.h) to emit
>> #pragmas. But we wouldn't have to, as compilers aren't supposed to
>> barf on unknown #pragmas.
> 
> Technically this can be done using macros:
> 
>  #if (gcc)
>  #define BEGIN_PACKED _Pragma("pack(push,4)")
>  #define END_PACKED _Pragma("pack(pop)")
>  #elif (msvc)
>  #define BEGIN_PACKED __pragma(pack(push,4))
>  #define END_PACKED __pragma(pack(pop))
>  #else
>   /* Just tolerate a little padding. */
>  #define BEGIN_PACKED
>  #define END_PACKED
>  #end
> 
> Then you can do:
> 
>  BEGIN_PACKED
>  struct hashmap_entry {
>   ...
>  };
>  END_PACKED
> 

Sorry for the delay...

My intention with #pragma pack was to support as many compilers / platforms as 
possible (even though I can't test them all). From what I could find on the 
'net, support for the _Pragma operator is slim. And as the MSVC build is 32-bit 
only (AFAIK), this is pretty much a GCC-only solution (and thus equivalent, but 
much more verbose than the initial __attribute__((__packed__)) variant).

> Whether that's nicer or uglier than the alternatives I leave to you.
> ;-)

If it was really up to me ( :-) ), I'd like to do this:

-- 8< ---
Subject: [PATCH] hashmap.h: make sure map entries are tightly packed

Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but struct hashmap_entry needs to be packed for this to work. E.g.

 struct name_entry {
   struct hashmap_entry ent; /* consists of pointer + int */
   int namelen;
   char *name;
 };

should be 24 instead of 32 bytes.

Packing structures is compiler-specific, most compilers support [1]:

 #pragma pack(n) - to set structure packing
 #pragma pack()  - to reset structure packing to default

Compilers are supposed to ignore unknown #pragmas, so using this without
further #if  guards should be safe.

Wasting a few bytes for padding is acceptable, however, compiling the rest
of git with packed structures (and thus mis-aligned arrays of structs) is
not. Add a test to ensure that '#pragma pack()' really resets the packing.

[1] Compiler docs regarding #pragma pack:
http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Structure_002dPacking-Pragmas.html#Structure_002dPacking-Pragmas
http://msdn.microsoft.com/en-us/library/2e70t5y1%28v=vs.80%29.aspx
http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#pragma-PACK
http://clang.llvm.org/docs/UsersManual.html#microsoft-extensions
http://uw714doc.sco.com/en/SDK_cprog/_Preprocessing_Directives.html
http://osr507doc.sco.com/en/tools/ANSI_F.3.13_Preprocessing.html
http://docs.oracle.com/cd/E19422-01/819-3690/Pragmas_App.html#73499
http://publib.boulder.ibm.com/infocenter/comphelp/v7v91/index.jsp?topic=%2Fcom.ibm.vacpp7a.doc%2Fcompiler%2Fref%2Frnpgpack.htm

Supposedly wants #pragma pack(0) to reset:
http://techpubs.sgi.com/library/tpl/cgi-bin/getdoc.cgi/0650/bks/SGI_Developer/books/Pragmas/sgi_html/ch04.html

Not supported:
http://software.intel.com/sites/products/documentation/doclib/iss/2013/compiler/cpp-lin/GUID-DD32852C-A0F9-4AC6-BF67-D10D064CC87A.htm

Signed-off-by: Karsten Blees 
---
 hashmap.h  |  7 +++
 t/t0011-hashmap.sh |  6 ++
 test-hashmap.c | 17 +
 3 files changed, 30 insertions(+)

diff --git a/hashmap.h b/hashmap.h
index a816ad4..93b330b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -15,10 +15,17 @@ extern unsigned int memihash(const void *buf, size_t len);
 
 /* data structures */
 
+/*
+ * Set struct packing to 4 so that we don't waste memory on 64-bit systems.
+ * Struct hashmap_entry is used as prefix to other (un-packed) structures,
+ * so this won't cause alignment issues e.g. for odd elements in an array.
+ */
+#pragma pack(4)
 struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
 };
+#pragma pack()
 
 typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key,
const void *keydata);
diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index 391e2b6..3f3c90b 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -237,4 +237,10 @@ test_expect_success 'grow / shrink' '
 
 '
 
+test_expect_success '"#pragma pack()" resets packing to default' '
+
+   test_hashmap "pragma-pack" "ok"
+
+'
+
 test_done
diff --git a/test-hashmap.c b/test-hashmap.c
index f5183fb..2d5a63a 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -36,6 +36,12 @@ static struct test_entry *alloc_test_entry(int hash, char 
*key, int klen,

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Jonathan Nieder
Karsten Blees wrote:

> (Besides, __attribute__((aligned)) / __declspec(align) can only
> _increase_ the alignment, so aligned(1) would have no effect).

Good catch.

> Googling some more, I believe the most protable way to achieve this
> via 'compiler settings' is
>
>  #pragma pack(push)
>  #pragma pack(4)
>  struct hashmap_entry {
>struct hashmap_entry *next;
>unsigned int hash;
>  };
>  #pragma pack(pop)
>
> This is supported by at least GCC, MSVC and HP (see your link). The
> downside is that we cannot use macros (in git-compat-util.h) to emit
> #pragmas. But we wouldn't have to, as compilers aren't supposed to
> barf on unknown #pragmas.

Technically this can be done using macros:

 #if (gcc)
 #  define BEGIN_PACKED _Pragma("pack(push,4)")
 #  define END_PACKED _Pragma("pack(pop)")
 #elif (msvc)
 #  define BEGIN_PACKED __pragma(pack(push,4))
 #  define END_PACKED __pragma(pack(pop))
 #else
/* Just tolerate a little padding. */
 #  define BEGIN_PACKED
 #  define END_PACKED
 #end

Then you can do:

 BEGIN_PACKED
 struct hashmap_entry {
...
 };
 END_PACKED

Whether that's nicer or uglier than the alternatives I leave to you.
;-)

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Karsten Blees
Am 09.12.2013 21:08, schrieb Jonathan Nieder:
> Karsten Blees wrote:
> 
>> GCC supports __packed__ as of 2.3 (1992), so any other compilers
>> that copied the __attribute__ feature probably won't complain.
> 
> Alas, it looks like HP C doesn't support __packed__ (not that I
> care much about HP C):
> 
>  
> http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes
> 

Thanks for the link

> Maybe a macro expanding to __attribute__((aligned(1))) on the fields
> would work?  The same macro could expand to __declspec(align(1)) in
> the MSVC build.
> 

But alignment is not the same as packing. We still want the structure to be 
8-byte aligned (i.e. variables of the type should start at 8-byte boundaries). 
We just don't want the size of the structure to be padded to a multiple of 8, 
so that we can extend it without penalty. (Besides, __attribute__((aligned)) / 
__declspec(align) can only _increase_ the alignment, so aligned(1) would have 
no effect).

Googling some more, I believe the most protable way to achieve this via 
'compiler settings' is

 #pragma pack(push)
 #pragma pack(4)
 struct hashmap_entry {
   struct hashmap_entry *next;
   unsigned int hash;
 };
 #pragma pack(pop)

This is supported by at least GCC, MSVC and HP (see your link). The downside is 
that we cannot use macros (in git-compat-util.h) to emit #pragmas. But we 
wouldn't have to, as compilers aren't supposed to barf on unknown #pragmas.


However, considering the portability issues, the macro solution (injecting just 
the two fields instead of a struct) becomes more and more attractive in my 
mind...

Karsten


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Junio C Hamano
Jeff King  writes:

> On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote:
>
>> Junio C Hamano  writes:
>> 
>> > * jk/pack-bitmap (2013-11-18) 22 commits
>> [...]
>> Peff can decide if he wants to reroll with my nits or not; either way
>> I'm all for moving it forward and aiming for one of the next releases.
>
> I'm going to be a bit slow this week, as I'm traveling in China.
>
> I have at least one more local fix queued up (one of the re-rolls
> introduced a use-after-free). Your comments make sense to me, though
> some of them are "if this is not too hard", and I haven't looked yet to
> see how hard some of the requisite refactoring would be. So expect at
> least one more re-roll, and I'll try to incorporate your comments.
> Thanks for giving it a careful reading.
>
> As an aside, we have been running the last version sent to the list
> (modulo the fix I mentioned above) on github.com for a week or two
> (previously we were running the old, based-on-v1.8.4 version). So it is
> getting exercised.

;-).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Jeff King
On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote:

> Junio C Hamano  writes:
> 
> > * jk/pack-bitmap (2013-11-18) 22 commits
> [...]
> Peff can decide if he wants to reroll with my nits or not; either way
> I'm all for moving it forward and aiming for one of the next releases.

I'm going to be a bit slow this week, as I'm traveling in China.

I have at least one more local fix queued up (one of the re-rolls
introduced a use-after-free). Your comments make sense to me, though
some of them are "if this is not too hard", and I haven't looked yet to
see how hard some of the requisite refactoring would be. So expect at
least one more re-roll, and I'll try to incorporate your comments.
Thanks for giving it a careful reading.

As an aside, we have been running the last version sent to the list
(modulo the fix I mentioned above) on github.com for a week or two
(previously we were running the old, based-on-v1.8.4 version). So it is
getting exercised.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Junio C Hamano
Felipe Contreras  writes:

> To take hostage the whole patch series because of the name
> --refspec (which is perfectly fine) is just an excuse to not fix
> the issues already present.

I actually was wondering why you are taking the fixes to the hostage
to force a wrong option name.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Jonathan Nieder
Karsten Blees wrote:

> GCC supports __packed__ as of 2.3 (1992), so any other compilers
> that copied the __attribute__ feature probably won't complain.

Alas, it looks like HP C doesn't support __packed__ (not that I
care much about HP C):

 
http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes

Maybe a macro expanding to __attribute__((aligned(1))) on the fields
would work?  The same macro could expand to __declspec(align(1)) in
the MSVC build.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Junio C Hamano
Karsten Blees  writes:

>> * kb/fast-hashmap (2013-11-18) 14 commits
>>   (merged to 'next' on 2013-12-06 at f90be3d) 
>
> Damn, a day too late :-) I found these two glitches today...is a
> fixup patch OK or should I do a reroll (or separate patch on top)?

A separate patch on top would be the most appropriate.  People have
been looking at the change since mid November, and nobody noticed
the problem; having a separate fix on top is a good way to document
what the specific gotcha that can be easily missed is.

I think the patch you attached describes the issue well, possibly
with a retitle (perhaps "hashmap.h: make sure map entries are
tightly packed", or something.)

Thanks.

> --- 8< ---
> Subject: [PATCH] fixup! add a hashtable implementation that supports O(1) 
> removal
>
> Use 'unsigned int' for hash-codes everywhere.
>
> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
> memory on 64-bit systems. This is already documented in api-hashmap.txt,
> but needs '__attribute__((__packed__))' to work. Reduces e.g.
>
>  struct name_entry {
>  struct hashmap_entry ent;
>  int namelen;
>  char *name;
>  };
>
> from 32 to 24 bytes.
>
> Signed-off-by: Karsten Blees 
> ---
>  hashmap.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hashmap.h b/hashmap.h
> index f5b3b61..b64567b 100644
> --- a/hashmap.h
> +++ b/hashmap.h
> @@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len);
>  
>  /* data structures */
>  
> -struct hashmap_entry {
> +struct __attribute__((__packed__)) hashmap_entry {
>   struct hashmap_entry *next;
>   unsigned int hash;
>  };
> @@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int 
> free_entries);
>  
>  /* hashmap_entry functions */
>  
> -static inline void hashmap_entry_init(void *entry, int hash)
> +static inline void hashmap_entry_init(void *entry, unsigned int hash)
>  {
>   struct hashmap_entry *e = entry;
>   e->hash = hash;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Junio C Hamano
Karsten Blees  writes:

> Am 07.12.2013 00:52, schrieb Junio C Hamano:
>
>> * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit
>>  - gitignore.txt: clarify recursive nature of excluded directories
>> 
>>  Originally merged to 'next' on 2013-11-13
>> 
>>  Kicked back to 'pu' to replace with a newer reroll ($gmane/237814
>>  looked OK but there seems to have some loose ends in the
>>  discussion).
>
> I'm unaware of any loose ends, could you clarify?
>
> Btw. $gmane/237814 seems to be a different topic, the version in next (and 
> now in pu) was $gmane/237429.

Ah, my mistake. I thought I still had $gmane/237416 and needed to
wait for the discussion to conclude; in fact, a later one in that
thread $gmane/237429 is what came out of that discussion, so this is
good to go, perhaps with an amend with Acked-by: peff.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-09 Thread Karsten Blees
Am 08.12.2013 11:20, schrieb Thomas Rast:
> Karsten Blees  writes:
> 
>> Am 07.12.2013 23:23, schrieb Thomas Rast:
>>> Karsten Blees  writes:
>>>
 Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
 memory on 64-bit systems. This is already documented in api-hashmap.txt,
 but needs '__attribute__((__packed__))' to work. Reduces e.g.
>>>
>>> You'd have to guard __attribute__((__packed__)) with some compiler
>>> detection in git-compat-util.h though.
>>>
>>
>> Isn't that already handled? __attribute__ is already widely used
>> (e.g. for printf formats), and platforms that don't support it define
>> it as empty (e.g. MSVC). Or do you mean I should account for
>> compiler-specific variants (#pragma pack...)?
> 
> True, __attribute__ expands to nothing on unknown compilers, but what
> does the compiler do when it sees an unknown attribute?  If some of them
> choke, you need a separate macro.
> 
> I'm a bit confused myself though, many attributes have special macros in
> git-compat-util.h but others we just use in the code.
> 

So what do you propose? I basically see three options:

1.) Trial and error

GCC supports __packed__ as of 2.3 (1992), so any other compilers that copied 
the __attribute__ feature probably won't complain.

2.) Accept the wasted memory

Moving the hash code from the hash table (as in hash.[ch]) to the entry is 
already a big improvement, as it no longer multiplies hash code + wasted bytes 
with the load factor. 64-bit software uses more memory in general, so we could 
just live with it (and only fix the documentation in api-hashmap.txt).

3.) Inject individual fields via macro

Instead of injecting a struct hashmap_entry, which implies alignment to 
sizeof(struct hashmap_entry), we could inject the individual fields, e.g.

 #define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int __hash;

 struct name_entry {
   HASHMAP_ENTRY_HEADER
   int namelen;
   char *name;
 };


What do you think?

Karsten
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-08 Thread Thomas Rast
Karsten Blees  writes:

> Am 07.12.2013 23:23, schrieb Thomas Rast:
>> Karsten Blees  writes:
>> 
>>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
>>> memory on 64-bit systems. This is already documented in api-hashmap.txt,
>>> but needs '__attribute__((__packed__))' to work. Reduces e.g.
>> 
>> You'd have to guard __attribute__((__packed__)) with some compiler
>> detection in git-compat-util.h though.
>> 
>
> Isn't that already handled? __attribute__ is already widely used
> (e.g. for printf formats), and platforms that don't support it define
> it as empty (e.g. MSVC). Or do you mean I should account for
> compiler-specific variants (#pragma pack...)?

True, __attribute__ expands to nothing on unknown compilers, but what
does the compiler do when it sees an unknown attribute?  If some of them
choke, you need a separate macro.

I'm a bit confused myself though, many attributes have special macros in
git-compat-util.h but others we just use in the code.

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Karsten Blees
Am 07.12.2013 23:23, schrieb Thomas Rast:
> Karsten Blees  writes:
> 
>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
>> memory on 64-bit systems. This is already documented in api-hashmap.txt,
>> but needs '__attribute__((__packed__))' to work. Reduces e.g.
> 
> You'd have to guard __attribute__((__packed__)) with some compiler
> detection in git-compat-util.h though.
> 

Isn't that already handled? __attribute__ is already widely used (e.g. for 
printf formats), and platforms that don't support it define it as empty (e.g. 
MSVC). Or do you mean I should account for compiler-specific variants (#pragma 
pack...)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Thomas Rast
Karsten Blees  writes:

> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
> memory on 64-bit systems. This is already documented in api-hashmap.txt,
> but needs '__attribute__((__packed__))' to work. Reduces e.g.

You'd have to guard __attribute__((__packed__)) with some compiler
detection in git-compat-util.h though.

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Felipe Contreras
On Sat, Dec 7, 2013 at 4:03 AM, Felipe Contreras
 wrote:
> Junio C Hamano wrote:
>> * fc/transport-helper-fixes (2013-11-13) 12 commits
>>  - remote-bzr: support the new 'force' option
>>  - transport-helper: add support to delete branches
>>  - fast-export: add support to delete refs
>>  - fast-import: add support to delete refs
>>  - transport-helper: add support for old:new refspec
>>  - fast-export: add new --refspec option
>>  - fast-export: improve argument parsing
>>  - test-hg.sh: tests are now expected to pass
>>  - transport-helper: check for 'forced update' message
>>  - transport-helper: add 'force' to 'export' helpers
>>  - transport-helper: don't update refs in dry-run
>>  - transport-helper: mismerge fix
>>
>>  Updates transport-helper, fast-import and fast-export to allow the
>>  ref mapping and ref deletion in a way similar to the natively
>>  supported transports.
>>
>>  The option name "--refspec" needs to be rethought. It does not mean
>>  what refspec usually means, even though it shares the same syntax
>>  with refspec; calling it --refspec only because it shares the same
>>  syntax is like calling it --asciistring and does not make sense.
>
> Not true. remote..fetch is a refspec, and doesn't specify what to fetch,
> so does the "refspec" capability in remote helpers, and so does the proposed
> --refspec option.
>
> It is exactly what it already means.

Not to mention that you can apply the first part of the series without
any problems:

>>  - fast-export: improve argument parsing
>>  - test-hg.sh: tests are now expected to pass
>>  - transport-helper: check for 'forced update' message
>>  - transport-helper: add 'force' to 'export' helpers
>>  - transport-helper: don't update refs in dry-run
>>  - transport-helper: mismerge fix

Even:

>>  - remote-bzr: support the new 'force' option

The rest is for old:new :to-delete refspecs.

To take hostage the whole patch series because of the name --refspec
(which is perfectly fine) is just an excuse to not fix the issues
already present.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Karsten Blees
Am 07.12.2013 00:52, schrieb Junio C Hamano:

> * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit
>  - gitignore.txt: clarify recursive nature of excluded directories
> 
>  Originally merged to 'next' on 2013-11-13
> 
>  Kicked back to 'pu' to replace with a newer reroll ($gmane/237814
>  looked OK but there seems to have some loose ends in the
>  discussion).

I'm unaware of any loose ends, could you clarify?

Btw. $gmane/237814 seems to be a different topic, the version in next (and now 
in pu) was $gmane/237429.


> * kb/fast-hashmap (2013-11-18) 14 commits
>   (merged to 'next' on 2013-12-06 at f90be3d) 

Damn, a day too late :-) I found these two glitches today...is a fixup patch OK 
or should I do a reroll (or separate patch on top)?

Thanks,
Karsten

--- 8< ---
Subject: [PATCH] fixup! add a hashtable implementation that supports O(1) 
removal

Use 'unsigned int' for hash-codes everywhere.

Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but needs '__attribute__((__packed__))' to work. Reduces e.g.

 struct name_entry {
 struct hashmap_entry ent;
 int namelen;
 char *name;
 };

from 32 to 24 bytes.

Signed-off-by: Karsten Blees 
---
 hashmap.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hashmap.h b/hashmap.h
index f5b3b61..b64567b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len);
 
 /* data structures */
 
-struct hashmap_entry {
+struct __attribute__((__packed__)) hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
 };
@@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int 
free_entries);
 
 /* hashmap_entry functions */
 
-static inline void hashmap_entry_init(void *entry, int hash)
+static inline void hashmap_entry_init(void *entry, unsigned int hash)
 {
struct hashmap_entry *e = entry;
e->hash = hash;
-- 
1.8.5.1.178.g0a9afc1.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Matthieu Moy
Junio C Hamano  writes:

> * mm/mv-file-to-no-such-dir-with-slash (2013-12-04) 1 commit
>  - mv: let 'git mv file no-such-dir/' error out

Why is it "stalled"? I think I took all remarks into account.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Thomas Rast
Junio C Hamano  writes:

> * jk/pack-bitmap (2013-11-18) 22 commits
>  - compat/mingw.h: Fix the MinGW and msvc builds
>  - pack-bitmap: implement optional name_hash cache
>  - t/perf: add tests for pack bitmaps
>  - t: add basic bitmap functionality tests
>  - count-objects: recognize .bitmap in garbage-checking
>  - repack: consider bitmaps when performing repacks
>  - repack: handle optional files created by pack-objects
>  - repack: turn exts array into array-of-struct
>  - repack: stop using magic number for ARRAY_SIZE(exts)
>  - pack-objects: implement bitmap writing
>  - rev-list: add bitmap mode to speed up object lists
>  - pack-objects: use bitmaps when packing objects
>  - pack-bitmap: add support for bitmap indexes
>  - documentation: add documentation for the bitmap format
>  - ewah: compressed bitmap implementation
>  - compat: add endianness helpers
>  - sha1_file: export `git_open_noatime`
>  - revision: allow setting custom limiter function
>  - pack-objects: factor out name_hash
>  - pack-objects: refactor the packing list
>  - revindex: export new APIs
>  - sha1write: make buffer const-correct
>  (this branch is tangled with jk/name-pack-after-byte-representation.)
>
>  Borrows the bitmap index into packfiles from JGit to speed up
>  enumeration of objects involved in a commit range without having to
>  fully traverse the history.

Peff can decide if he wants to reroll with my nits or not; either way
I'm all for moving it forward and aiming for one of the next releases.

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-07 Thread Felipe Contreras
Junio C Hamano wrote:
> * fc/transport-helper-fixes (2013-11-13) 12 commits
>  - remote-bzr: support the new 'force' option
>  - transport-helper: add support to delete branches
>  - fast-export: add support to delete refs
>  - fast-import: add support to delete refs
>  - transport-helper: add support for old:new refspec
>  - fast-export: add new --refspec option
>  - fast-export: improve argument parsing
>  - test-hg.sh: tests are now expected to pass
>  - transport-helper: check for 'forced update' message
>  - transport-helper: add 'force' to 'export' helpers
>  - transport-helper: don't update refs in dry-run
>  - transport-helper: mismerge fix
> 
>  Updates transport-helper, fast-import and fast-export to allow the
>  ref mapping and ref deletion in a way similar to the natively
>  supported transports.
> 
>  The option name "--refspec" needs to be rethought. It does not mean
>  what refspec usually means, even though it shares the same syntax
>  with refspec; calling it --refspec only because it shares the same
>  syntax is like calling it --asciistring and does not make sense.

Not true. remote..fetch is a refspec, and doesn't specify what to fetch,
so does the "refspec" capability in remote helpers, and so does the proposed
--refspec option.

It is exactly what it already means.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Dec 2013, #02; Fri, 6)

2013-12-06 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

The tip of 'next' has been rewound, ejecting a few topics that
used to be there.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* jn/git-gui-chmod+x (2013-11-25) 1 commit
 - git-gui: chmod +x po2msg, windows/git-gui.sh

 Parked here until I get the same change back from the upstream
 git-gui tree.


* jn/gitk-chmod+x (2013-11-25) 1 commit
 - gitk: chmod +x po2msg

 Parked here until I get the same change back from the upstream gitk
 tree.


* jk/name-pack-after-byte-representation (2013-12-05) 2 commits
 - pack-objects: name pack files after trailer hash
 - sha1write: make buffer const-correct
 (this branch is tangled with jk/pack-bitmap.)


* nd/negative-pathspec (2013-12-06) 3 commits
 - pathspec.c: support adding prefix magic to a pathspec with mnemonic magic
 - Support pathspec magic :(exclude) and its short form :!
 - glossary-content.txt: rephrase magic signature part


* nd/transport-positive-depth-only (2013-12-06) 1 commit
 - clone,fetch: catch non positive --depth option value


* zk/difftool-counts (2013-12-06) 1 commit
 - difftool: display the number of files in the diff queue in the prompt

--
[Graduated to "master"]

* ak/submodule-foreach-quoting (2013-09-27) 1 commit
  (merged to 'next' on 2013-10-14 at d77c5f1)
 + submodule foreach: skip eval for more than one argument

 A behavior change, but maybe a worthwhile one: "git submodule
 foreach" was treating its arguments as part of a single command to
 be concatenated and passed to a shell, making writing buggy scripts
 too easy.

 This patch preserves the old "just pass it to the shell" behavior
 when a single argument is passed to 'git submodule foreach' and
 moves to a new "skip the shell and use the arguments passed
 unmolested" behavior when more than one argument is passed.

 The old behavior (always concatenating and passing to the shell)
 was similar to the 'ssh' command, while the new behavior (switching
 on the number of arguments) is what 'xterm -e' does.

 May need more thought to make sure this change is advertised well;
 scripts that used multiple arguments but added their own extra
 layer of quoting are broken, and the users need to adjust them.


* bc/http-100-continue (2013-10-31) 3 commits
  (merged to 'next' on 2013-11-01 at e12ae23)
 + remote-curl: fix large pushes with GSSAPI
 + remote-curl: pass curl slot_results back through run_slot
 + http: return curl's AUTHAVAIL via slot_results

 Issue "100 Continue" responses to help use of GSS-Negotiate
 authentication scheme over HTTP transport when needed.


* jc/bundle (2013-11-12) 1 commit
  (merged to 'next' on 2013-11-21 at 535b046)
 + bundle: use argv-array

 Code clean-up.


* jc/merge-base-reflog (2013-10-29) 2 commits
  (merged to 'next' on 2013-11-01 at 6114764)
 + merge-base: teach "--fork-point" mode
 + merge-base: use OPT_CMDMODE and clarify the command line parsing

 Code the logic in "pull --rebase" that figures out a fork point
 from reflog entries in C.


* jc/ref-excludes (2013-11-01) 5 commits
  (merged to 'next' on 2013-11-04 at fac1ed0)
 + rev-parse: introduce --exclude= to tame wildcards
 + rev-list --exclude: export add/clear-ref-exclusion and ref-excluded API
 + rev-list --exclude: tests
 + document --exclude option
 + revision: introduce --exclude= to tame wildcards

 People often wished a way to tell "git log --branches" (and "git
 log --remotes --not --branches") to exclude some local branches
 from the expansion of "--branches" (similarly for "--tags", "--all"
 and "--glob=").  Now they have one.


* jh/loose-object-dirs-creation-race (2013-10-28) 1 commit
  (merged to 'next' on 2013-11-01 at 3169b0f)
 + sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs

 Two processes creating loose objects at the same time could have
 failed unnecessarily when they happened to have created objects
 whose names share the same first byte.


* jk/remove-experimental-loose-object-support (2013-11-21) 1 commit
  (merged to 'next' on 2013-11-21 at d37bab7)
 + drop support for "experimental" loose objects

 Read-only support for experimental loose-object format, in which
 users could optionally choose to write in their loose objects for a
 short while between v1.4.3 to v1.5.3 era, has been dropped.


* jk/replace-perl-in-built-scripts (2013-10-29) 1 commit
  (merged to 'next' on 2013-11-01 at 2384e29)
 + use @@PERL@@ in built scripts


* jk/robustify-parse-commit (2013-10-24) 6 commits
  (merged to 'next' on 2013-11-01 at 2bfbaab)
 + checkout: do not die when leaving broken detached HEAD
 + use parse_commit_or_die instead of custom message
 + use parse_commit_or_die instea