Re: [PATCH v2] compat: convert modes to use portable file type values
On Wed, Dec 3, 2014 at 9:24 PM, David Michael fedora@gmail.com wrote: --- /dev/null +++ b/compat/stat.c @@ -0,0 +1,49 @@ +#define _POSIX_C_SOURCE 200112L +#include stddef.h/* NULL */ +#include sys/stat.h /* *stat, S_IS* */ +#include sys/types.h /* mode_t */ Oops, the stddef.h line can be removed now that this is no longer testing for NULL. Let me know if this warrants a v3 if there is no other feedback. Thanks. David -- 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
[PATCH v2] compat: convert modes to use portable file type values
This adds simple wrapper functions around calls to stat(), fstat(), and lstat() that translate the operating system's native file type bits to those used by most operating systems. It also rewrites the S_IF* macros to the common values, so all file type processing is performed using the translated modes. This makes projects portable across operating systems that use different file type definitions. Only the file type bits may be affected by these compatibility functions; the file permission bits are assumed to be 0 and are passed through unchanged. Signed-off-by: David Michael fedora@gmail.com --- Changes in v2: Remove elses and use a perm_bits variable as suggested by Torsten Bögershausen. Only translate the mode on stat() success as suggested by Duy Nguyen. Add S_IFSOCK translation to support a git.c test. (I left the device types defined just to support all the POSIX file types.) Replace _POSIX_SOURCE=1 with _POSIX_C_SOURCE=200112L to properly get IEEE 1003.1a definitions for lstat(), as suggested for glibc. Makefile | 8 cache.h | 7 --- compat/stat.c | 49 + configure.ac | 23 +++ git-compat-util.h | 34 ++ 5 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 compat/stat.c diff --git a/Makefile b/Makefile index 827006b..cba3be1 100644 --- a/Makefile +++ b/Makefile @@ -191,6 +191,10 @@ all:: # Define NO_TRUSTABLE_FILEMODE if your filesystem may claim to support # the executable mode bit, but doesn't really do so. # +# Define NEEDS_MODE_TRANSLATION if your OS strays from the typical file type +# bits in mode values (e.g. z/OS defines I_SFMT to 0xFF00 as opposed to the +# usual 0xF000). +# # Define NO_IPV6 if you lack IPv6 support and getaddrinfo(). # # Define NO_UNIX_SOCKETS if your system does not offer unix sockets. @@ -1230,6 +1234,10 @@ endif ifdef NO_TRUSTABLE_FILEMODE BASIC_CFLAGS += -DNO_TRUSTABLE_FILEMODE endif +ifdef NEEDS_MODE_TRANSLATION + COMPAT_CFLAGS += -DNEEDS_MODE_TRANSLATION + COMPAT_OBJS += compat/stat.o +endif ifdef NO_IPV6 BASIC_CFLAGS += -DNO_IPV6 endif diff --git a/cache.h b/cache.h index 99ed096..f8174fe 100644 --- a/cache.h +++ b/cache.h @@ -65,13 +65,6 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long); * * The value 016 is not normally a valid mode, and * also just happens to be S_IFDIR + S_IFLNK - * - * NOTE! We *really* shouldn't depend on the S_IFxxx macros - * always having the same values everywhere. We should use - * our internal git values for these things, and then we can - * translate that to the OS-specific value. It just so - * happens that everybody shares the same bit representation - * in the UNIX world (and apparently wider too..) */ #define S_IFGITLINK016 #define S_ISGITLINK(m) (((m) S_IFMT) == S_IFGITLINK) diff --git a/compat/stat.c b/compat/stat.c new file mode 100644 index 000..c2d4711 --- /dev/null +++ b/compat/stat.c @@ -0,0 +1,49 @@ +#define _POSIX_C_SOURCE 200112L +#include stddef.h/* NULL */ +#include sys/stat.h /* *stat, S_IS* */ +#include sys/types.h /* mode_t */ + +static inline mode_t mode_native_to_git(mode_t native_mode) +{ + mode_t perm_bits = native_mode 0; + if (S_ISREG(native_mode)) + return 010 | perm_bits; + if (S_ISDIR(native_mode)) + return 004 | perm_bits; + if (S_ISLNK(native_mode)) + return 012 | perm_bits; + if (S_ISBLK(native_mode)) + return 006 | perm_bits; + if (S_ISCHR(native_mode)) + return 002 | perm_bits; + if (S_ISFIFO(native_mode)) + return 001 | perm_bits; + if (S_ISSOCK(native_mode)) + return 014 | perm_bits; + /* Non-standard type bits were given. */ + return perm_bits; +} + +int git_stat(const char *path, struct stat *buf) +{ + int rc = stat(path, buf); + if (rc == 0) + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} + +int git_fstat(int fd, struct stat *buf) +{ + int rc = fstat(fd, buf); + if (rc == 0) + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} + +int git_lstat(const char *path, struct stat *buf) +{ + int rc = lstat(path, buf); + if (rc == 0) + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} diff --git a/configure.ac b/configure.ac index 6af9647..5c1312f 100644 --- a/configure.ac +++ b/configure.ac @@ -873,6 +873,29 @@ else SNPRINTF_RETURNS_BOGUS= fi GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS]) +# +# Define NEEDS_MODE_TRANSLATION if your OS strays from the typical file type +# bits in mode values. +AC_CACHE_CHECK([whether the platform uses typical file type bits], + [ac_cv_sane_mode_bits
Re: [PATCH] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen tbo...@web.de wrote: On 12/01/2014 04:40 AM, David Michael wrote: On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de wrote: [snip] Could the code be more human-readable ? static inline mode_t mode_native_to_git(mode_t native_mode) { int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; if (S_ISBLK(native_mode)) return 006 | perm_bits; if (S_ISCHR(native_mode)) return 002 | perm_bits; if (S_ISFIFO(native_mode)) return 001 | perm_bits; /* Non-standard type bits were given. */ /* Shouldn't we die() here ?? */ return perm_bits; } Sure, I can send an updated patch with the new variable and without the elses. Regarding the question in the last comment: I was assuming if this case was ever reached, Git would handle the returned mode the same way as if it encountered an unknown/non-standard file type on a normal operating system, which could die() if needed in the function that called stat(). Should I send an updated patch that die()s there? David Not yet, please wait with a V2 patch until I finished my thinking ;-) I take back the suggestion with the die(). I was thinking how to handle unforeseen types, which may show up on the z/OS some day, So die() is not a good idea, it is better to ignore them, what the code does. Knowing that Git does not track block devices, nor character devices nor sockets, the above code could be simplyfied even more, by mapping everything which is not a directory, a file or a softlink to device type 0) This is just a suggestion, I want to here from others as well: int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK */ return perm_bits; I had considered omitting those three as well at first, but in this case it would mean that they will be unusable all together. The z/OS S_IFMT definition (i.e. the file type bit mask) is 0xFF00, and the common/translated S_IFMT definition is 0xF000. Since the S_ISxxx macros use the typical ((mode S_IFMT) == S_IFxxx) definition, they would never match a native z/OS mode after redefining S_IFMT. So translating those types isn't just for tracking files, it's to support any use of S_ISxxx anywhere in the code. It should be okay to remove any of those types if we know that Git will never need to use them. David -- 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: [PATCH] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 7:48 AM, David Michael fedora@gmail.com wrote: On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen tbo...@web.de wrote: On 12/01/2014 04:40 AM, David Michael wrote: On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de wrote: [snip] Could the code be more human-readable ? static inline mode_t mode_native_to_git(mode_t native_mode) { int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; if (S_ISBLK(native_mode)) return 006 | perm_bits; if (S_ISCHR(native_mode)) return 002 | perm_bits; if (S_ISFIFO(native_mode)) return 001 | perm_bits; /* Non-standard type bits were given. */ /* Shouldn't we die() here ?? */ return perm_bits; } Sure, I can send an updated patch with the new variable and without the elses. Regarding the question in the last comment: I was assuming if this case was ever reached, Git would handle the returned mode the same way as if it encountered an unknown/non-standard file type on a normal operating system, which could die() if needed in the function that called stat(). Should I send an updated patch that die()s there? David Not yet, please wait with a V2 patch until I finished my thinking ;-) I take back the suggestion with the die(). I was thinking how to handle unforeseen types, which may show up on the z/OS some day, So die() is not a good idea, it is better to ignore them, what the code does. Knowing that Git does not track block devices, nor character devices nor sockets, the above code could be simplyfied even more, by mapping everything which is not a directory, a file or a softlink to device type 0) This is just a suggestion, I want to here from others as well: int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK */ return perm_bits; I had considered omitting those three as well at first, but in this case it would mean that they will be unusable all together. The z/OS S_IFMT definition (i.e. the file type bit mask) is 0xFF00, and the common/translated S_IFMT definition is 0xF000. Since the S_ISxxx macros use the typical ((mode S_IFMT) == S_IFxxx) definition, they would never match a native z/OS mode after redefining S_IFMT. So translating those types isn't just for tracking files, it's to support any use of S_ISxxx anywhere in the code. It should be okay to remove any of those types if we know that Git will never need to use them. Apologies, in my pre-coffee reply, I confused myself into thinking the omitted types were going to be returned unchanged as opposed to being changed to zero. That second paragraph is irrelevant. But regarding the last paragraph: a quick grep for instances of using those file types in the Git source found S_ISFIFO and S_ISSOCK in git.c. I just noticed that I copied the list of standard file type macros from SUSv2, and S_IFSOCK was added after that. z/OS does implement S_IFSOCK, so I think I should add it to the v2 patch to support the test in git.c. Another grep found no instances of testing for block or character devices, so it should be okay to remove those from the patch if Git is unlikely to use them in the future (unless we just want to provide all 7 types from http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html ). David -- 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: [PATCH] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote: +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) It's a minor thing, but maybe test !rc instead of buf != NULL? Okay, it makes sense to only do the conversion for a successful return code. Should it test for both a zero return code and a non-null pointer? I don't know if there are any cases where passing a null pointer is legal. The standard doesn't seem to explicitly forbid it. z/OS returns -1 and sets errno to EFAULT when stat() is given NULL, but this patch should be able to be used on any platform. David -- 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: [PATCH] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 12:57 PM, Junio C Hamano gits...@pobox.com wrote: David Michael fedora@gmail.com writes: On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote: +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) It's a minor thing, but maybe test !rc instead of buf != NULL? Okay, it makes sense to only do the conversion for a successful return code. Should it test for both a zero return code and a non-null pointer? I don't know if there are any cases where passing a null pointer is legal. The standard doesn't seem to explicitly forbid it. z/OS returns -1 and sets errno to EFAULT when stat() is given NULL, but this patch should be able to be used on any platform. Huh? I am confused. Since when is it legal to give NULL as statbuf to (l)stat(2)? Wouldn't something like this be sufficient and necessary? int rc = stat(path, buf); if (rc) return rc; That is, let the underlying stat(2) diagnose any and all problems (and leave clues in errno) and parrot its return value to the caller to signal the failure? Alright, it wasn't immediately clear to me from the OpenGroup page on stat() if that would always be safe. I will just test the return code in v2. Thanks. David -- 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: [PATCH] compat: convert modes to use portable file type values
On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de wrote: [snip] Could the code be more human-readable ? static inline mode_t mode_native_to_git(mode_t native_mode) { int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; if (S_ISBLK(native_mode)) return 006 | perm_bits; if (S_ISCHR(native_mode)) return 002 | perm_bits; if (S_ISFIFO(native_mode)) return 001 | perm_bits; /* Non-standard type bits were given. */ /* Shouldn't we die() here ?? */ return perm_bits; } Sure, I can send an updated patch with the new variable and without the elses. Regarding the question in the last comment: I was assuming if this case was ever reached, Git would handle the returned mode the same way as if it encountered an unknown/non-standard file type on a normal operating system, which could die() if needed in the function that called stat(). Should I send an updated patch that die()s there? David -- 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
[PATCH] compat: convert modes to use portable file type values
This adds simple wrapper functions around calls to stat(), fstat(), and lstat() that translate the operating system's native file type bits to those used by most operating systems. It also rewrites the S_IF* macros to the common values, so all file type processing is performed using the translated modes. This makes projects portable across operating systems that use different file type definitions. Only the file type bits may be affected by these compatibility functions; the file permission bits are assumed to be 0 and are passed through unchanged. Signed-off-by: David Michael fedora@gmail.com --- Hi, This is my most recent attempt at solving the problem of z/OS using different file type values than every other OS. I believe it should be safe as long as the file type bits don't ever need to be converted back to their native values (and I didn't see any instances of that). I've been testing it by making commits to the same repositories on different operating systems and pushing those changes around, and so far there have been no issues. Can anyone foresee any problems with this method? Thanks. David Makefile | 8 cache.h | 7 --- compat/stat.c | 49 + configure.ac | 22 ++ git-compat-util.h | 32 5 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 compat/stat.c diff --git a/Makefile b/Makefile index 827006b..cba3be1 100644 --- a/Makefile +++ b/Makefile @@ -191,6 +191,10 @@ all:: # Define NO_TRUSTABLE_FILEMODE if your filesystem may claim to support # the executable mode bit, but doesn't really do so. # +# Define NEEDS_MODE_TRANSLATION if your OS strays from the typical file type +# bits in mode values (e.g. z/OS defines I_SFMT to 0xFF00 as opposed to the +# usual 0xF000). +# # Define NO_IPV6 if you lack IPv6 support and getaddrinfo(). # # Define NO_UNIX_SOCKETS if your system does not offer unix sockets. @@ -1230,6 +1234,10 @@ endif ifdef NO_TRUSTABLE_FILEMODE BASIC_CFLAGS += -DNO_TRUSTABLE_FILEMODE endif +ifdef NEEDS_MODE_TRANSLATION + COMPAT_CFLAGS += -DNEEDS_MODE_TRANSLATION + COMPAT_OBJS += compat/stat.o +endif ifdef NO_IPV6 BASIC_CFLAGS += -DNO_IPV6 endif diff --git a/cache.h b/cache.h index 99ed096..f8174fe 100644 --- a/cache.h +++ b/cache.h @@ -65,13 +65,6 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long); * * The value 016 is not normally a valid mode, and * also just happens to be S_IFDIR + S_IFLNK - * - * NOTE! We *really* shouldn't depend on the S_IFxxx macros - * always having the same values everywhere. We should use - * our internal git values for these things, and then we can - * translate that to the OS-specific value. It just so - * happens that everybody shares the same bit representation - * in the UNIX world (and apparently wider too..) */ #define S_IFGITLINK016 #define S_ISGITLINK(m) (((m) S_IFMT) == S_IFGITLINK) diff --git a/compat/stat.c b/compat/stat.c new file mode 100644 index 000..0ff1f2f --- /dev/null +++ b/compat/stat.c @@ -0,0 +1,49 @@ +#define _POSIX_SOURCE +#include stddef.h/* NULL */ +#include sys/stat.h /* *stat, S_IS* */ +#include sys/types.h /* mode_t */ + +static inline mode_t mode_native_to_git(mode_t native_mode) +{ + if (S_ISREG(native_mode)) + return 010 | (native_mode 0); + else if (S_ISDIR(native_mode)) + return 004 | (native_mode 0); + else if (S_ISLNK(native_mode)) + return 012 | (native_mode 0); + else if (S_ISBLK(native_mode)) + return 006 | (native_mode 0); + else if (S_ISCHR(native_mode)) + return 002 | (native_mode 0); + else if (S_ISFIFO(native_mode)) + return 001 | (native_mode 0); + else /* Non-standard type bits were given. */ + return native_mode 0; +} + +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} + +int git_fstat(int fd, struct stat *buf) +{ + int rc; + rc = fstat(fd, buf); + if (buf != NULL) + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} + +int git_lstat(const char *path, struct stat *buf) +{ + int rc; + rc = lstat(path, buf); + if (buf != NULL) + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} diff --git a/configure.ac b/configure.ac index 6af9647..b8eced4 100644 --- a/configure.ac +++ b/configure.ac @@ -873,6 +873,28 @@ else SNPRINTF_RETURNS_BOGUS= fi GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS]) +# +# Define NEEDS_MODE_TRANSLATION if your OS strays from the typical file type +# bits in mode
[PATCH 1/3] git-compat-util.h: Support variadic macros with the XL C compiler
When the XL C compiler is run with an appropriate language level or suboption, it defines a feature test macro to indicate support for variadic macros. This was tested on z/OS, but it should also work on AIX according to IBM documentation. Signed-off-by: David Michael fedora@gmail.com --- git-compat-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index 2107127..b31cfdd 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -778,7 +778,7 @@ void git_qsort(void *base, size_t nmemb, size_t size, #endif #endif -#if defined(__GNUC__) || (_MSC_VER = 1400) +#if defined(__GNUC__) || (_MSC_VER = 1400) || defined(__C99_MACRO_WITH_VA_ARGS) #define HAVE_VARIADIC_MACROS 1 #endif -- 1.9.3 -- 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
[PATCH 2/3] Makefile: Reorder linker flags in the git executable rule
The XL C compiler can fail due to mixing library path and object file arguments, for example when linking git while building with gmake LDFLAGS=-L$prefix/lib. This moves the ALL_LDFLAGS variable expansion in the git executable rule to be consistent with all the other linking rules. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index fcd51ac..827006b 100644 --- a/Makefile +++ b/Makefile @@ -1610,8 +1610,8 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_INFO_PATH=$(infodir_relative_SQ)' git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \ - $(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \ + $(BUILTIN_OBJS) $(LIBS) help.sp help.s help.o: common-cmds.h -- 1.9.3 -- 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
[PATCH 3/3] compat/bswap.h: Detect endianness from XL C compiler macros
There is no /usr/include/endian.h equivalent on z/OS, but the compiler will define macros to indicate endianness on host and target hardware. This adds a test for these macros as a last resort for determining byte order. Signed-off-by: David Michael fedora@gmail.com --- compat/bswap.h | 4 1 file changed, 4 insertions(+) diff --git a/compat/bswap.h b/compat/bswap.h index f6fd9a6..7fed637 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x) # define GIT_BYTE_ORDER GIT_BIG_ENDIAN # elif defined(_LITTLE_ENDIAN) !defined(_BIG_ENDIAN) # define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN +# elif defined(__THW_BIG_ENDIAN__) !defined(__THW_LITTLE_ENDIAN__) +# define GIT_BYTE_ORDER GIT_BIG_ENDIAN +# elif defined(__THW_LITTLE_ENDIAN__) !defined(__THW_BIG_ENDIAN__) +# define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN # else # error Cannot determine endianness # endif -- 1.9.3 -- 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: [PATCH 2/3] Makefile: Reorder linker flags in the git executable rule
On Sun, Oct 26, 2014 at 2:35 PM, Jeff King p...@peff.net wrote: On Sun, Oct 26, 2014 at 01:45:10PM -0400, Eric Sunshine wrote: On Sun, Oct 26, 2014 at 1:33 PM, David Michael fedora@gmail.com wrote: The XL C compiler can fail due to mixing library path and object Can you explain in the commit message the actual nature of the failure so that readers can understand more precisely how this change helps? Based on past experience, it is probably the compiler complains and refuses to run (or optionally the compiler silently ignores your LDFLAGS depending on how irritating it wants to be). But it would not hurt to be specific. Yes, the compiler refuses to run by default when a -L option occurs after a source/object file. It tries to interpret it as another file name and fails. I believe I can work around the error with an export _C89_CCMODE=1, but I thought I'd send the patch since this is the only occurrence of the problem, and the argument order is inconsistent with other linker commands in the file. IBM documentation has this to say on the noted environment variable: The default behavior of the c89/cc/c++ command is to expect all options to precede all operands. Setting this variable allows compatibility with historical implementations (other cc commands). When set to 1, the c89/cc/c++ command operates as follows: Options and operands can be interspersed. [...] Do you want me to resend the patch and reference the IBM documentation in the message? Thanks. David -- 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: [PATCH 3/3] compat/bswap.h: Detect endianness from XL C compiler macros
On Sun, Oct 26, 2014 at 2:38 PM, Jeff King p...@peff.net wrote: On Sun, Oct 26, 2014 at 01:34:26PM -0400, David Michael wrote: diff --git a/compat/bswap.h b/compat/bswap.h index f6fd9a6..7fed637 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x) # define GIT_BYTE_ORDER GIT_BIG_ENDIAN # elif defined(_LITTLE_ENDIAN) !defined(_BIG_ENDIAN) # define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN +# elif defined(__THW_BIG_ENDIAN__) !defined(__THW_LITTLE_ENDIAN__) +# define GIT_BYTE_ORDER GIT_BIG_ENDIAN +# elif defined(__THW_LITTLE_ENDIAN__) !defined(__THW_BIG_ENDIAN__) +# define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN Out of curiosity, is there ever a case where _both_ are defined? That is, would: diff --git a/compat/bswap.h b/compat/bswap.h index f6fd9a6..b78a0bd 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x) # define GIT_BYTE_ORDER GIT_BIG_ENDIAN # elif defined(_LITTLE_ENDIAN) !defined(_BIG_ENDIAN) # define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN +# elif defined(__THW_BIG_ENDIAN__) +# define GIT_BYTE_ORDER GIT_BIG_ENDIAN +# elif defined(__THW_LITTLE_ENDIAN__) +# define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN # else # error Cannot determine endianness # endif be enough, or is that used to mark some other special case? Even if not, there is not much downside to the more thorough conditions you put above, but as a reviewer I wondered if there is something else going on. I'm not 100% sure if __THW_LITTLE_ENDIAN__ will ever be defined, so I'd be okay with dropping those references completely. There is a recent version of the compiler for little endian Linux distributions, but I haven't found the documentation for it. (The product documentation still seems to only refer to the big endian Linux version.) The compiler's macro may be redundant in this case anyway, since Linux systems should have bits/endian.h supplying that information. I only used both macros for completeness; the __THW_BIG_ENDIAN__ macro (defined to 1 on z/OS and AIX) is what I actually needed here. z/OS doesn't seem to have any other compile-time byte order indicator, short of testing for the OS itself. Would you prefer the two-line patch to only test for the big endian macro, or maybe just test for __MVS__ to look at the OS? Thanks. David -- 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: Subtree in Git
On Sat, Mar 2, 2013 at 9:05 AM, Paul Campbell pcampb...@kemitix.net wrote: On Fri, Mar 1, 2013 at 2:28 AM, Kindjal kind...@gmail.com wrote: David Michael Barr b at rr-dav.id.au writes: From a quick survey, it appears there are no more than 55 patches squashed into the submitted patch. As I have an interest in git-subtree for maintaining the out-of-tree version of vcs-svn/ and a desire to improve my rebase-fu, I am tempted to make some sense of the organic growth that happened on GitHub. It doesn't appear that anyone else is willing to do this, so I doubt there will be any duplication of effort. What is the status of the work on git-subtree described in this thread? It looks like it's stalled. I hadn't been aware of that patch. Reading the thread David Michael Barr was going to try picking the patch apart into sensible chunks. Sorry for not updating the thread. I did end up moving onto other things. I quickly realised the reason for globbing all the patches together was that the individual patches were not well contained. That is single patches with multiple unrelated changes and multiple patches changing the same things in different directions. To me this means that the first step is to curate the history. If this work is still needing done I'd like to volunteer. You're most welcome. Sorry again for abandoning the thread. -- David Michael Barr -- 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: [PATCH] Improve QNX support in GIT
Hi, On Tue, Feb 26, 2013 at 12:25 PM, Matt Kraai kr...@ftbfs.org wrote: I didn't realize that QNX 6.3.2 provided getpagesize. Its header files don't provide a prototype, so when I saw the warning, I assumed it wasn't available. Since NO_GETPAGESIZE is only used by QNX, if it's OK to reintroduce the warning, NO_GETPAGESIZE might as well be removed entirely. I have been using this feature locally for building on z/OS USS. IBM decided to drop the getpagesize definition by default, as it was removed from SUSv3. There is a way to re-enable the withdrawn legacy functions, but I'd rather this option for using sysconf(_SC_PAGESIZE) since that is apparently the preferred method now anyway. So, if it's not being too much trouble, I'd vote for keeping this feature. Thanks. David -- 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
[PATCH] git-compat-util.h: Provide missing netdb.h definitions
Some platforms may lack the NI_MAXHOST and NI_MAXSERV values in their system headers, so ensure they are available. Signed-off-by: David Michael fedora@gmail.com --- NI_MAXHOST is missing from my platform, and it has no compatibility definition anywhere. $ grep -FIR NI_MAXHOST imap-send.c:char addr[NI_MAXHOST]; connect.c:static char addr[NI_MAXHOST]; I've been defining it in CFLAGS, but I noticed there is precedence for this type of definition in git-compat-util.h. This patch adds a definition for compatibility. In addition, all the documentation I've seen mentions NI_MAXHOST and NI_MAXSERV together, so this also moves the NI_MAXSERV definition to git-compat-util.h for consistency. daemon.c | 4 git-compat-util.h | 11 +++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/daemon.c b/daemon.c index 4602b46..df8c0ab 100644 --- a/daemon.c +++ b/daemon.c @@ -9,10 +9,6 @@ #define HOST_NAME_MAX 256 #endif -#ifndef NI_MAXSERV -#define NI_MAXSERV 32 -#endif - #ifdef NO_INITGROUPS #define initgroups(x, y) (0) /* nothing */ #endif diff --git a/git-compat-util.h b/git-compat-util.h index b7eaaa9..2580c6b 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -213,6 +213,17 @@ extern char *gitbasename(char *); #include openssl/err.h #endif +/* On most systems netdb.h would have given us this, but + * not on some systems (e.g. z/OS). + */ +#ifndef NI_MAXHOST +#define NI_MAXHOST 1025 +#endif + +#ifndef NI_MAXSERV +#define NI_MAXSERV 32 +#endif + /* On most systems limits.h would have given us this, but * not on some systems (e.g. GNU/Hurd). */ -- 1.8.1.2 -- 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: [PATCH] Add getenv.so for catching invalid getenv() use via LD_PRELOAD
Hi, On Sat, Jan 5, 2013 at 3:55 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Perhaps this will help the getenv bug hunting (I assume we do the hunting on Linux platform only). So far it catches this and is stuck at getenv in git_pager(). For the record: I have been testing a macro pointing getenv at an alternate implementation for the purpose of my port. This alternate function will return a unique pointer for each variable, as opposed to using the same static buffer. IBM considers this function a proprietary z/OS extension (whereas the other is labelled as conforming to half a dozen standards), but it seems to be more in-line with the behavior git expects. So I agree this patch is useful for reaching strict conformance to the published specs, but I think we can go back to assuming no known platforms are broken and unusable because of this. Thanks. David -- 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
[BUG/PATCH] setup: Copy an environment variable to avoid overwrites
It is possible for this pointer of the GIT_DIR environment variable to survive unduplicated until further getenv calls are made. The standards allow for subsequent calls of getenv to overwrite the string located at its returned pointer, and this can result in broken git operations on certain platforms. Signed-off-by: David Michael fedora@gmail.com --- I have encountered an issue with consecutive calls to getenv overwriting earlier values. Most notably, it prevents a plain git clone from working. Long story short: This value of GIT_DIR gets passed around setup.c until it reaches check_repository_format_gently. This function calls git_config_early, which eventually runs getenv(HOME). When it returns back to check_repository_format_gently, the gitdir variable contains my home directory path. The end result is that I wind up with ~/objects/ etc. and a failed repository clone. (Simply adding a bare getenv(GIT_DIR) afterwards to reset the pointer also corrects the problem.) Since other platforms are apparently working, yet this getenv behavior is supported by the standards, I am left wondering if this could be a symptom of something else being broken on my platform (z/OS). Can anyone more familiar with this part of git identify any condition that obviously should not be occurring? Thanks. setup.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/setup.c b/setup.c index f108c4b..64fb160 100644 --- a/setup.c +++ b/setup.c @@ -675,8 +675,12 @@ static const char *setup_git_directory_gently_1(int *nongit_ok) * validation. */ gitdirenv = getenv(GIT_DIR_ENVIRONMENT); -if (gitdirenv) -return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok); +if (gitdirenv) { +gitdirenv = xstrdup(gitdirenv); +ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok); +free(gitdirenv); +return ret; +} if (env_ceiling_dirs) { string_list_split(ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1); -- 1.7.11.7 -- 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: [RFC] test: Old shells and physical paths
Hi, On Thu, Dec 20, 2012 at 12:01 AM, David Aguilar dav...@gmail.com wrote: Do you know if the differences are relegated to cd, or do other common commands such as awk, grep, sed, mktemp, expr, etc. have similar issues? There are almost certainly going to be incompatibilities with other commands. The system implemented UNIX95 plus some extensions, then they began supporting UNIX03/SUSv3/POSIX.1-2001/whatever for certain commands by using an environment variable to toggle between the incompatible behaviors. Their documentation on the UNIX03 commands indicates it is still only partially supported. For example: cp supports -L and -P, but cd doesn't. I wonder if it'd be helpful to have a low-numbered test that checks the basics needed by the scripted Porcelains and test suite. It would give us an easy way to answer these questions, and could be a good way to document (in code) the capabilities we expect. I'd be in favor of something like this as well. Thanks. David P.S. In the meantime, I am handling the cd situation by replacing -P with $PHYS and prepending the following to t/test-lib.sh. set +o logical /dev/null 21 || PHYS=-P -- 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: [RFC] test: Old shells and physical paths
Hi, On Thu, Dec 20, 2012 at 12:17 AM, Junio C Hamano gits...@pobox.com wrote: Is here is a nickel, get a better shell an option? It is, somewhat. There is a pre-built port of GNU bash 2.03 for the platform, but I was trying to see how far things could go with the OS's supported shell before having to bring in unsupported dependencies. Unfortunately, I do not believe the OS fully conforms to POSIX.1-2001 yet, so that means no -P or -L without going rogue. I'll carry test fixes for this platform locally. Thanks. David -- 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: Build fixes for another obscure Unix
Hi, On Fri, Dec 14, 2012 at 2:54 AM, Joachim Schmitz j...@schmitz-digital.de wrote: For what's it worth: I ACK your HP-NonStop patch (as you can see by my comment in git-compat-util.h I was thinking along the same line) https://github.com/dm0-/git/commit/933d72a5cfdc63fa9c3c68afa2f4899d9c3f791e together with its prerequisit https://github.com/dm0-/git/commit/301032c6488aeabb94ccc81bfb6d65ff2c23b924 ACKed by: Joachim Schmitz j...@schmitz-digital.de Okay, thanks for verifying. Especially since another port needing that header was just sent to the list, I'd prefer to see some generalized feature test rather than building and maintaining an explicit OS list. No one has suggested any adjustments, so I'll send out those patches now. Thanks. David -- 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
[PATCH 1/4] Support builds when sys/param.h is missing
An option is added to the Makefile to skip the inclusion of sys/param.h. The only known platform with this condition thus far is the z/OS UNIX System Services environment. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 5 + configure.ac | 6 ++ git-compat-util.h | 2 ++ 3 files changed, 13 insertions(+) diff --git a/Makefile b/Makefile index 736ecd4..8c3a0dd 100644 --- a/Makefile +++ b/Makefile @@ -165,6 +165,8 @@ all:: # Define NO_POLL if you do not have or don't want to use poll(). # This also implies NO_SYS_POLL_H. # +# Define NO_SYS_PARAM_H if you don't have sys/param.h. +# # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # # Define NO_PREAD if you have a problem with pread() system call (e.g. @@ -1748,6 +1750,9 @@ endif ifdef NO_SYS_POLL_H BASIC_CFLAGS += -DNO_SYS_POLL_H endif +ifdef NO_SYS_PARAM_H +BASIC_CFLAGS += -DNO_SYS_PARAM_H +endif ifdef NO_INTTYPES_H BASIC_CFLAGS += -DNO_INTTYPES_H endif diff --git a/configure.ac b/configure.ac index ad215cc..317bfc6 100644 --- a/configure.ac +++ b/configure.ac @@ -699,6 +699,12 @@ AC_CHECK_HEADER([sys/poll.h], [NO_SYS_POLL_H=UnfortunatelyYes]) GIT_CONF_SUBST([NO_SYS_POLL_H]) # +# Define NO_SYS_PARAM_H if you don't have sys/param.h +AC_CHECK_HEADER([sys/param.h], +[NO_SYS_PARAM_H=], +[NO_SYS_PARAM_H=UnfortunatelyYes]) +GIT_CONF_SUBST([NO_SYS_PARAM_H]) +# # Define NO_INTTYPES_H if you don't have inttypes.h AC_CHECK_HEADER([inttypes.h], [NO_INTTYPES_H=], diff --git a/git-compat-util.h b/git-compat-util.h index 2e79b8a..ace1549 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -104,7 +104,9 @@ #endif #include errno.h #include limits.h +#ifndef NO_SYS_PARAM_H #include sys/param.h +#endif #include sys/types.h #include dirent.h #include sys/time.h -- 1.7.11.7 -- 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
[PATCH 2/4] Detect when the passwd struct is missing pw_gecos
NO_GECOS_IN_PWENT was documented with other Makefile variables but was only enforced by manually defining it to the C preprocessor. This adds support for detecting the condition with configure and defining the make variable. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 3 +++ configure.ac | 8 2 files changed, 11 insertions(+) diff --git a/Makefile b/Makefile index 8c3a0dd..735a834 100644 --- a/Makefile +++ b/Makefile @@ -1657,6 +1657,9 @@ endif ifdef NO_D_INO_IN_DIRENT BASIC_CFLAGS += -DNO_D_INO_IN_DIRENT endif +ifdef NO_GECOS_IN_PWENT +BASIC_CFLAGS += -DNO_GECOS_IN_PWENT +endif ifdef NO_ST_BLOCKS_IN_STRUCT_STAT BASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT endif diff --git a/configure.ac b/configure.ac index 317bfc6..3347c17 100644 --- a/configure.ac +++ b/configure.ac @@ -759,6 +759,14 @@ AC_CHECK_MEMBER(struct dirent.d_type, [#include dirent.h]) GIT_CONF_SUBST([NO_D_TYPE_IN_DIRENT]) # +# Define NO_GECOS_IN_PWENT if you don't have pw_gecos in struct passwd +# in the C library. +AC_CHECK_MEMBER(struct passwd.pw_gecos, +[NO_GECOS_IN_PWENT=], +[NO_GECOS_IN_PWENT=YesPlease], +[#include pwd.h]) +GIT_CONF_SUBST([NO_GECOS_IN_PWENT]) +# # Define NO_SOCKADDR_STORAGE if your platform does not have struct # sockaddr_storage. AC_CHECK_TYPE(struct sockaddr_storage, -- 1.7.11.7 -- 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
[PATCH 4/4] Declare that HP NonStop systems require strings.h
This platform previously included strings.h automatically. However, the build system now requires an explicit option to do so. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index fb78f7f..e84b0cb 100644 --- a/Makefile +++ b/Makefile @@ -1357,6 +1357,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # Added manually, see above. NEEDS_SSL_WITH_CURL = YesPlease HAVE_LIBCHARSET_H = YesPlease +HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes -- 1.7.11.7 -- 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
[PATCH 3/4] Generalize the inclusion of strings.h
The header strings.h was formerly only included for a particular platform to define strcasecmp, but another platform requiring this inclusion has been found. The build system will now include the file based on its presence determined by configure. Signed-off-by: David Michael fedora@gmail.com --- Makefile | 6 ++ configure.ac | 6 ++ git-compat-util.h | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 735a834..fb78f7f 100644 --- a/Makefile +++ b/Makefile @@ -74,6 +74,8 @@ all:: # Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks # d_type in struct dirent (Cygwin 1.5, fixed in Cygwin 1.7). # +# Define HAVE_STRINGS_H if you have strings.h and need it for strcasecmp. +# # Define NO_STRCASESTR if you don't have strcasestr. # # Define NO_MEMMEM if you don't have memmem. @@ -1892,6 +1894,10 @@ ifdef HAVE_LIBCHARSET_H EXTLIBS += $(CHARSET_LIB) endif +ifdef HAVE_STRINGS_H +BASIC_CFLAGS += -DHAVE_STRINGS_H +endif + ifdef HAVE_DEV_TTY BASIC_CFLAGS += -DHAVE_DEV_TTY endif diff --git a/configure.ac b/configure.ac index 3347c17..4176db8 100644 --- a/configure.ac +++ b/configure.ac @@ -886,6 +886,12 @@ AC_CHECK_HEADER([libcharset.h], [HAVE_LIBCHARSET_H=YesPlease], [HAVE_LIBCHARSET_H=]) GIT_CONF_SUBST([HAVE_LIBCHARSET_H]) +# +# Define HAVE_STRINGS_H if you have strings.h +AC_CHECK_HEADER([strings.h], +[HAVE_STRINGS_H=YesPlease], +[HAVE_STRINGS_H=]) +GIT_CONF_SUBST([HAVE_STRINGS_H]) # Define CHARSET_LIB if libiconv does not export the locale_charset symbol # and libcharset does CHARSET_LIB= diff --git a/git-compat-util.h b/git-compat-util.h index ace1549..d7359ef 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -99,7 +99,7 @@ #include stdlib.h #include stdarg.h #include string.h -#ifdef __TANDEM /* or HAVE_STRINGS_H or !NO_STRINGS_H? */ +#ifdef HAVE_STRINGS_H #include strings.h /* for strcasecmp() */ #endif #include errno.h -- 1.7.11.7 -- 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: [PATCH 1/4] Support builds when sys/param.h is missing
Hi, On Fri, Dec 14, 2012 at 6:41 PM, Junio C Hamano gits...@pobox.com wrote: I have this suspicion that nobody would notice if we simply stopped including the header. While I'm not aware of any subtleties it could be causing on other platforms, it does seem fine to drop sys/param.h on my test GNU/Linux systems. I can resend the series and just remove the include, if preferred. Thanks. David -- 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: Build fixes for another obscure Unix
Hi, On Thu, Dec 13, 2012 at 12:18 PM, Pyeron, Jason J CTR (US) jason.j.pyeron@mail.mil wrote: Would there be any interest in applying such individual compatibility fixes for this system, even if a full port doesn't reach completion? What are the down sides? Can your changes be shown to not impact builds on other systems? I've pushed a handful of small compatibility patches to GitHub[1] which are enough to successfully compile the project. The default values of the new variables should make them unnoticeable to other systems. Are there any concerns with this type of change? If they would be acceptable, I can try sending the first four of those patches to the list properly. (I expect the last two may be tweaked as I continue working with the port.) I do have a concern with strings.h, though. That file will be included for most people who run ./configure, when it wasn't before. Do you think it's worth making a more detailed test to see if strcasecmp is still undefined after string.h is included, rather than just testing for the header's existence? Thanks. David [1] https://github.com/dm0-/git/commits -- 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: remote-testsvn: Hangs at revision
On Wednesday, 5 December 2012 at 5:20 PM, Ramkumar Ramachandra wrote: Hi, I tried out the testsvn remote helper on a simple Subversion repository, but it seems to hang at Revision 8 indefinitely without any indication of progress. I'm currently digging in to see what went wrong. The repository I'm cloning is: $ git clone testsvn::http://python-lastfm.googlecode.com/svn/trunk/ I attempted to clone the same repository and was able to fetch 152 revisions. So the issue Ram saw might have been temporal. I did however receive a warning at the end of the clone: warning: remote HEAD refers to nonexistent ref, unable to checkout. -- David Michael Barr -- 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: [RFC] pack-objects: compression level for non-blobs
Add config pack.graphcompression similar to pack.compression. Applies to non-blob objects and if unspecified falls back to pack.compression. We may identify objects compressed with level 0 by their leading bytes. Use this to force recompression when the source and target levels mismatch. Limit its application to when the config pack.graphcompression is set. Signed-off-by: David Michael Barr b...@rr-dav.id.au (mailto:b...@rr-dav.id.au) --- builtin/pack-objects.c | 49 + 1 file changed, 45 insertions(+), 4 deletions(-) I started working on this just before taking a vacation, so it's been a little while coming. The intent is to allow selective recompression of pack data. For small objects/deltas the overhead of deflate is significant. This may improve read performance for the object graph. I ran some unscientific experiments with the chromium repository. With pack.graphcompression = 0, there was a 2.7% increase in pack size. I saw a 35% improvement with cold caches and 43% otherwise on git log --raw. I neglected to mention that this is a WIP. I get failures with certain repositories: fatal: delta size changed -- David Michael Barr -- 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
[RFC] pack-objects: compression level for non-blobs
Add config pack.graphcompression similar to pack.compression. Applies to non-blob objects and if unspecified falls back to pack.compression. We may identify objects compressed with level 0 by their leading bytes. Use this to force recompression when the source and target levels mismatch. Limit its application to when the config pack.graphcompression is set. Signed-off-by: David Michael Barr b...@rr-dav.id.au --- builtin/pack-objects.c | 49 + 1 file changed, 45 insertions(+), 4 deletions(-) I started working on this just before taking a vacation, so it's been a little while coming. The intent is to allow selective recompression of pack data. For small objects/deltas the overhead of deflate is significant. This may improve read performance for the object graph. I ran some unscientific experiments with the chromium repository. With pack.graphcompression = 0, there was a 2.7% increase in pack size. I saw a 35% improvement with cold caches and 43% otherwise on git log --raw. diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index f069462..9518daf 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -40,6 +40,7 @@ struct object_entry { unsigned long z_delta_size; /* delta data size (compressed) */ unsigned int hash; /* name hint hash */ enum object_type type; + enum object_type actual_type; enum object_type in_pack_type; /* could be delta */ unsigned char in_pack_header_size; unsigned char preferred_base; /* we do not pack this, but is available @@ -81,6 +82,8 @@ static int num_preferred_base; static struct progress *progress_state; static int pack_compression_level = Z_DEFAULT_COMPRESSION; static int pack_compression_seen; +static int pack_graph_compression_level = Z_DEFAULT_COMPRESSION; +static int pack_graph_compression_seen; static unsigned long delta_cache_size = 0; static unsigned long max_delta_cache_size = 256 * 1024 * 1024; @@ -125,14 +128,14 @@ static void *get_delta(struct object_entry *entry) return delta_buf; } -static unsigned long do_compress(void **pptr, unsigned long size) +static unsigned long do_compress(void **pptr, unsigned long size, int level) { git_zstream stream; void *in, *out; unsigned long maxsize; memset(stream, 0, sizeof(stream)); - git_deflate_init(stream, pack_compression_level); + git_deflate_init(stream, level); maxsize = git_deflate_bound(stream, size); in = *pptr; @@ -191,6 +194,18 @@ static unsigned long write_large_blob_data(struct git_istream *st, struct sha1fi return olen; } +static int check_pack_compressed(struct packed_git *p, + struct pack_window **w_curs, + off_t offset) +{ + unsigned long avail; + int compressed = 0; + unsigned char *in = use_pack(p, w_curs, offset, avail); + if (avail = 3) + compressed = !!(in[2] 0x6); + return compressed; +} + /* * we are going to reuse the existing object data as is. make * sure it is not corrupt. @@ -240,6 +255,8 @@ static void copy_pack_data(struct sha1file *f, } } +#define compression_level(type) ((type) (type) != OBJ_BLOB ? pack_graph_compression_level : pack_compression_level) + /* Return 0 if we will bust the pack-size limit */ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_entry *entry, unsigned long limit, int usable_delta) @@ -286,7 +303,7 @@ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_ent else if (entry-z_delta_size) datalen = entry-z_delta_size; else - datalen = do_compress(buf, size); + datalen = do_compress(buf, size, compression_level(entry-actual_type)); /* * The object header is a byte of 'type' followed by zero or @@ -379,6 +396,13 @@ static unsigned long write_reuse_object(struct sha1file *f, struct object_entry offset += entry-in_pack_header_size; datalen -= entry-in_pack_header_size; + if (!pack_to_stdout + pack_graph_compression_seen + check_pack_compressed(p, w_curs, offset) != !!compression_level(entry-actual_type)) { + unuse_pack(w_curs); + return write_no_reuse_object(f, entry, limit, usable_delta); + } + if (!pack_to_stdout p-index_version == 1 check_pack_inflate(p, w_curs, offset, datalen, entry-size)) { error(corrupt packed object for %s, sha1_to_hex(entry-idx.sha1)); @@ -955,6 +979,8 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type, memset(entry, 0, sizeof(*entry)); hashcpy(entry-idx.sha1, sha1); entry-hash = hash; + if (pack_graph_compression_seen) + entry-actual_type
Re: Subtree in Git
On Saturday, 27 October 2012 at 12:10 AM, Herman van Rink wrote: On 10/22/2012 04:41 PM, d...@cray.com (mailto:d...@cray.com) wrote: Herman van Rink r...@initfour.nl (mailto:r...@initfour.nl) writes: On 10/21/2012 08:32 AM, Junio C Hamano wrote: Herman van Rink r...@initfour.nl (mailto:r...@initfour.nl) writes: Junio, Could you please consider merging the single commit from my subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates In general, in areas like contrib/ where there is a volunteer area maintainer, unless the change something ultra-urgent (e.g. serious security fix) and the area maintainer is unavailable, I'm really reluctant to bypass and take a single patch that adds many things that are independent from each other. Who do you see as volunteer area maintainer for contrib/subtree? My best guess would be Dave. And he already indicated earlier in the thread to be ok with the combined patch as long as you are ok with it. Let's be clear. Junio owns the project so what he says goes, no question. I provided some review feedback which I thought would help the patches get in more easily. We really shouldn't be adding multiple features in one patch. This is easily separated into multiple patches. Then there is the issue of testcases. We should NOT have git-subtree go back to the pre-merge _ad_hoc_ test environment. We should use what the usptream project uses. That will make mainlining this much easier in the future. If Junio is ok with overriding my decisions here, that's fine. But I really don't understand why you are so hesitant to rework the patches when it should be realtively easy. Certainly easier than convincing me they are in good shape currently. :) If it's so easy to rework these patches then please do so yourself. It's been ages since I've worked on this so I would also have to re-discover everything. From a quick survey, it appears there are no more than 55 patches squashed into the submitted patch. As I have an interest in git-subtree for maintaining the out-of-tree version of vcs-svn/ and a desire to improve my rebase-fu, I am tempted to make some sense of the organic growth that happened on GitHub. It doesn't appear that anyone else is willing to do this, so I doubt there will be any duplication of effort. -- David Michael Barr -- 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 (Oct 2012, #01; Tue, 2)
On Wednesday, 3 October 2012 at 9:20 AM, Junio C Hamano wrote: * fa/remote-svn (2012-09-19) 16 commits - Add a test script for remote-svn - remote-svn: add marks-file regeneration - Add a svnrdump-simulator replaying a dump file for testing - remote-svn: add incremental import - remote-svn: Activate import/export-marks for fast-import - Create a note for every imported commit containing svn metadata - vcs-svn: add fast_export_note to create notes - Allow reading svn dumps from files via file:// urls - remote-svn, vcs-svn: Enable fetching to private refs - When debug==1, start fast-import with --stats instead of --quiet - Add documentation for the 'bidi-import' capability of remote-helpers - Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability - Add argv_array_detach and argv_array_free_detached - Add svndump_init_fd to allow reading dumps from arbitrary FDs - Add git-remote-testsvn to Makefile - Implement a remote helper for svn in C (this branch is used by fa/vcs-svn.) A GSoC project. Waiting for comments from mentors and stakeholders. I have reviewed this topic and am happy with the design and implementation. I support this topic for inclusion. Acked-by: David Michael Barr b...@rr-dav.id.au * fa/vcs-svn (2012-09-19) 4 commits - vcs-svn: remove repo_tree - vcs-svn/svndump: rewrite handle_node(), begin|end_revision() - vcs-svn/svndump: restructure node_ctx, rev_ctx handling - svndump: move struct definitions to .h (this branch uses fa/remote-svn.) A GSoC project. Waiting for comments from mentors and stakeholders. This follow-on topic I'm not so sure on, some of the design decisions make me uncomfortable and I need some convincing before I can get behind this topic. -- David Michael Barr -- 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: Using bitmaps to accelerate fetch and clone
Hi all, On Fri, Sep 28, 2012 at 3:20 AM, Jeff King p...@peff.net wrote: On Thu, Sep 27, 2012 at 07:17:42PM +0700, Nguyen Thai Ngoc Duy wrote: Operation Index V2 Index VE003 Clone 37530ms (524.06 MiB) 82ms (524.06 MiB) Fetch (1 commit back) 75ms 107ms Fetch (10 commits back) 456ms (269.51 KiB)341ms (265.19 KiB) Fetch (100 commits back) 449ms (269.91 KiB)337ms (267.28 KiB) Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB) Fetch (1 commits back) 2177ms ( 16.30 MiB)254ms ( 15.88 MiB) Fetch (10 commits back) 14340ms (185.83 MiB) 1655ms (189.39 MiB) Beautiful. And curious, why do 100-1000 and 1-1 have such big leaps in time (V2)? Agreed. I'm very excited about these numbers. +1 Definitely :-). I have shown my interest in this topic before. So I should probably say that I'm going to work on this on C Git, but slllwwwly. As this benefits the server side greatly, perhaps a GitHubber ;-) might want to work on this on C Git, for GitHub itself of course, and, as a side effect, make the rest of us happy? Yeah, GitHub is definitely interested in this. I may take a shot at it, but I know David Barr (cc'd) is also interested in such things. Yeah, I'm definitely interested, I love this stuff. -- David Michael Barr -- 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: Failing svn imports from apache.org
Hi Enrico, Repositories as old and large as ASF are the reason I created svn-fe. git-svn is known to choke on these repositories. If you have plenty of bandwidth, it might well be faster to: * Grab an ASF archive (16GB) * Use svn-fe to import the entire tree into git. * Use a simple script to extract the standard layout into a new repo. * Use git-svn to keep the new repo up-to-date. -- David Michael Barr On Saturday, 15 September 2012 at 8:07 PM, Enrico Weigelt wrote: Does anyone have an idea, what might be wrong here / how to fix it ? Here: git svn --version git-svn version 1.7.12.592.g41e7905 (svn 1.6.18) What's yours? 1.7.9.5 (ubuntu precise) I'm getting Initialized empty Git repository in /tmp/discovery/.git/ Using higher level of URL: http://svn.apache.org/repos/asf/commons/proper/discovery = http://svn.apache.org/repos/asf W: Ignoring error from SVN, path probably does not exist: (160013): Dateisystem hat keinen Eintrag: File not found: revision 100, path '/commons/proper/discovery' W: Do not be alarmed at the above message git-svn is just searching aggressively for old history. This may take a while on large repositories and then it checks the revisions. I didn't want to wait for r1301705... Does your git svn abort earlier or after checking all revs? It also scanned through thousands of revisions and then failed: W: Do not be alarmed at the above message git-svn is just searching aggressively for old history. This may take a while on large repositories mkdir .git: No such file or directory at /usr/lib/git-core/git-svn line 3669 cu -- Mit freundlichen Grüßen / Kind regards Enrico Weigelt VNC - Virtual Network Consult GmbH Head Of Development Pariser Platz 4a, D-10117 Berlin Tel.: +49 (30) 3464615-20 Fax: +49 (30) 3464615-59 enrico.weig...@vnc.biz; www.vnc.de (http://www.vnc.de) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org (mailto:majord...@vger.kernel.org) More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: [RFC 1/5] GSOC: prepare svndump for branch detection
On Sat, Aug 18, 2012 at 6:40 AM, Florian Achleitner florian.achleitner.2.6...@gmail.com wrote: Hi! This patch series should prepare vcs-svn/svndump.* for branch detection. When starting with this feature I found that the existing functions are not yet appropriate for that. These rewrites the node handling part of svndump.c, it is very invasive. The logic in handle_node is not simple, I hope that I understood every case the existing code tries to adress. At least it doesn't break an existing testcase. The series applies on top of: [PATCH/RFC v4 16/16] Add a test script for remote-svn. I could also rebase it onto master if you think it makes sense. Florian [RFC 1/5] vcs-svn: Add sha1 calculaton to fast_export and This change makes me uncomfortable. We are doubling up on hashing with fast-import. This introduces git-specific logic into vcs-svn. [RFC 2/5] svndump: move struct definitions to .h. [RFC 3/5] vcs-svn/svndump: restructure node_ctx, rev_ctx handling [RFC 4/5] vcs-svn/svndump: rewrite handle_node(), [RFC 5/5] vcs-svn: remove repo_tree I haven't read the rest of the series yet but I expect it is less controversial than the first patch. -- David Michael Barr -- 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: [PATCH/RFC v3 00/16] GSOC remote-svn
On Wed, Aug 15, 2012 at 5:13 AM, Florian Achleitner florian.achleitner.2.6...@gmail.com wrote: Hi. Version 3 of this series adds the 'bidi-import' capability, as suggested Jonathan. Diff details are attached to the patches. 04 and 05 are completely new. [PATCH/RFC v3 01/16] Implement a remote helper for svn in C. [PATCH/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile. [PATCH/RFC v3 03/16] Add svndump_init_fd to allow reading dumps from [PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via [PATCH/RFC v3 05/16] Add documentation for the 'bidi-import' [PATCH/RFC v3 06/16] remote-svn, vcs-svn: Enable fetching to private [PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir. [PATCH/RFC v3 08/16] Allow reading svn dumps from files via file:// [PATCH/RFC v3 09/16] vcs-svn: add fast_export_note to create notes [PATCH/RFC v3 10/16] Create a note for every imported commit [PATCH/RFC v3 11/16] When debug==1, start fast-import with --stats [PATCH/RFC v3 12/16] remote-svn: add incremental import. [PATCH/RFC v3 13/16] Add a svnrdump-simulator replaying a dump file [PATCH/RFC v3 14/16] transport-helper: add import|export-marks to [PATCH/RFC v3 15/16] remote-svn: add marks-file regeneration. [PATCH/RFC v3 16/16] Add a test script for remote-svn. Thank you Florian, this series was a great read. My apologies for the limited interaction over the course of summer. You have done well and engaged with the community to produce this result. Thank you Jonathan for the persistent reviews. No doubt they have contributed to the quality of the series. Thank you Junio for your dedication to reviewing the traffic on this mailing list. I will no longer be reachable on this address after Friday. I hope to make future contributions with the identity: David Michael Barr b...@rr-dav.id.au This will be my persistent address. -- David Barr -- 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: [GIT PULL] vcs-svn housekeeping
On Sat, Jul 7, 2012 at 3:10 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi Junio, The following changes since commit 58ebd9865d2bb9d42842fbac5a1c4eae49e92859: vcs-svn/svndiff.c: squelch false unused warning from gcc (2012-01-27 11:58:56 -0800) are available at: git://repo.or.cz/git/jrn.git svn-fe The first three commits duplicate changes that are already in master but were committed independently on the svn-fe branch last February. The rest are David's db/vcs-svn series which aims to address various nits noticed when merging the code back into svn-dump-fast-export: unnecessary use of git-specific functions (prefixcmp, memmem) and warnings reported by clang. Some of the patches had to change a little since v2 of db/vcs-svn, so I'll be replying with a copy of the patches for reference. David has looked the branch over and acked and tested it. Thoughts welcome, as usual. I think these are ready for pulling into master. Sorry to be so slow at this. David Barr (7): vcs-svn: drop no-op reset methods vcs-svn: avoid self-assignment in dummy initialization of pre_off vcs-svn: simplify cleanup in apply_one_window vcs-svn: use constcmp instead of prefixcmp vcs-svn: use strstr instead of memmem vcs-svn: suppress signed/unsigned comparison warnings vcs-svn: suppress a signed/unsigned comparison warning Jonathan Nieder (4): vcs-svn: allow import of 4GiB files vcs-svn: suppress -Wtype-limits warning vcs-svn: suppress a signed/unsigned comparison warning vcs-svn: allow 64-bit Prop-Content-Length Ramsay Allan Jones (1): vcs-svn: rename check_overflow and its arguments for clarity Thank you Jonathan for doing this. Definitely the result of collaborating on a series is gorgeous. I do wish I could absorb your flair for polish. -- David Barr -- 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