[PATCH v2] compat: Fix read() of 2GB and more on Mac OS X
Previously, filtering 2GB or more through an external filter (see test) failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with: error: read from external filter cat failed error: cannot feed the input to external filter cat error: cat died of signal 13 error: external filter cat failed 141 error: external filter cat failed The reason was that read() immediately returns with EINVAL if nbyte = 2GB. According to POSIX [1], if the value of nbyte passed to read() is greater than SSIZE_MAX, the result is implementation-defined. The write function has the same restriction [2]. Since OS X still supports running 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also imposed on 64-bit executables under certain conditions. For write, the problem has been addressed in a earlier commit [6c642a]. The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Unfortunately, '#undef read' is needed at a few places to avoid expanding the compat macro in constructs like 'vtbl-read(...)'. Note that 'git add' exits with 0 even if it prints filtering errors to stderr. The test, therefore, checks stderr. 'git add' should probably be changed (sometime in another commit) to exit with nonzero if filtering fails. The test could then be changed to use test_must_fail. Thanks to the following people for their suggestions: Johannes Sixt j...@kdbg.org John Keeping j...@keeping.me.uk Jonathan Nieder jrnie...@gmail.com Kyle J. McKay mack...@gmail.com Torsten Bögershausen tbo...@web.de [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html [6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3 compate/clipped-write.c: large write(2) fails on Mac OS X/XNU Signed-off-by: Steffen Prohaska proha...@zib.de --- Makefile | 8 builtin/var.c | 1 + config.mak.uname | 1 + git-compat-util.h | 5 + streaming.c | 1 + t/t0021-conversion.sh | 14 ++ 6 files changed, 30 insertions(+) diff --git a/Makefile b/Makefile index 3588ca1..0f69e24 100644 --- a/Makefile +++ b/Makefile @@ -69,6 +69,9 @@ all:: # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt # doesn't support GNU extensions like --check and --statistics # +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than +# INT_MAX bytes at once (e.g. MacOS X). +# # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than # INT_MAX bytes at once (e.g. MacOS X). # @@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS MSGFMT += --check --statistics endif +ifdef NEEDS_CLIPPED_READ + BASIC_CFLAGS += -DNEEDS_CLIPPED_READ + COMPAT_OBJS += compat/clipped-read.o +endif + ifdef NEEDS_CLIPPED_WRITE BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE COMPAT_OBJS += compat/clipped-write.o diff --git a/builtin/var.c b/builtin/var.c index aedbb53..e59f5ba 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -38,6 +38,7 @@ static struct git_var git_vars[] = { { , NULL }, }; +#undef read static void list_vars(void) { struct git_var *ptr; diff --git a/config.mak.uname b/config.mak.uname index b27f51d..5c10726 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin) NO_MEMMEM = YesPlease USE_ST_TIMESPEC = YesPlease HAVE_DEV_TTY = YesPlease + NEEDS_CLIPPED_READ = YesPlease NEEDS_CLIPPED_WRITE = YesPlease COMPAT_OBJS += compat/precompose_utf8.o BASIC_CFLAGS += -DPRECOMPOSE_UNICODE diff --git a/git-compat-util.h b/git-compat-util.h index 115cb1d..a227127 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -185,6 +185,11 @@ typedef unsigned long uintptr_t; #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NEEDS_CLIPPED_READ +ssize_t clipped_read(int fd, void *buf, size_t nbyte); +#define read(x,y,z) clipped_read((x),(y),(z)) +#endif + #ifdef NEEDS_CLIPPED_WRITE ssize_t clipped_write(int fildes, const void *buf, size_t nbyte); #define write(x,y,z) clipped_write((x),(y),(z)) diff --git a/streaming.c b/streaming.c index debe904..c1fe34a 100644 --- a/streaming.c +++ b/streaming.c @@ -99,6 +99,7 @@ int close_istream(struct git_istream *st) return r; } +#undef read ssize_t read_istream(struct git_istream *st, void *buf, size_t sz) { return st-vtbl-read(st, buf, sz); diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index e50f0f7..b92e6cb 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' ' test_must_fail git add test.fc ' +test -n $GIT_TEST_LONG test_set_prereq EXPENSIVE + +test_expect_success EXPENSIVE 'filter large file' ' + git config filter.largefile.smudge cat + git config
Re: [PATCH] submodule: prevent warning in summary output
Hi Brian, On 19/08/13 05:31, brian m. carlson wrote: When git submodule summary is run and there is a deleted submodule, there is an warning from git rev-parse: fatal: Not a git repository: '.vim/pathogen/.git' Silence this warning, since it is fully expected that a deleted submodule will not be a git repository. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2979197..66ee621 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1071,7 +1071,7 @@ cmd_summary() { missing_dst= test $mod_src = 16 - ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null + ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null 21 missing_src=t test $mod_dst = 16 I wonder if there are other useful errors this will silence unintentionally. Perhaps this would be better (untested) test $mod_src = 16 test -e $name/.git ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null missing_src=t Having said that there are precedents for both in git-submodule.sh. If there aren't any errors worth catching here then your way is probably cleaner than mine. - C -- 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
Notes and submodules
Hello, Is it possible to keep submodules notes in the super project ? Thanks -- Francis -- 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 v2] compat: Fix read() of 2GB and more on Mac OS X
Am 19.08.2013 08:38, schrieb Steffen Prohaska: +test_expect_success EXPENSIVE 'filter large file' ' + git config filter.largefile.smudge cat + git config filter.largefile.clean cat + for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB Shouldn't you count to 2049 to get a file that is over 2GB? + echo 2GB filter=largefile .gitattributes + git add 2GB 2err + ! test -s err + rm -f 2GB + git checkout -- 2GB 2err + ! test -s err +' -- Hannes -- 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 v2] compat: Fix read() of 2GB and more on Mac OS X
On Aug 19, 2013, at 9:54 AM, John Keeping j...@keeping.me.uk wrote: You've created compat/clipped-read.c, but... Makefile | 8 builtin/var.c | 1 + config.mak.uname | 1 + git-compat-util.h | 5 + streaming.c | 1 + t/t0021-conversion.sh | 14 ++ 6 files changed, 30 insertions(+) ... it's not included here. Did you forget to git add? Indeed. How embarrassing. Thanks for spotting this. I'll send v3 in a minute. Stefffen-- 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 v3] compat: Fix read() of 2GB and more on Mac OS X
Previously, filtering 2GB or more through an external filter (see test) failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with: error: read from external filter cat failed error: cannot feed the input to external filter cat error: cat died of signal 13 error: external filter cat failed 141 error: external filter cat failed The reason was that read() immediately returns with EINVAL if nbyte = 2GB. According to POSIX [1], if the value of nbyte passed to read() is greater than SSIZE_MAX, the result is implementation-defined. The write function has the same restriction [2]. Since OS X still supports running 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also imposed on 64-bit executables under certain conditions. For write, the problem has been addressed in a earlier commit [6c642a]. The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Unfortunately, '#undef read' is needed at a few places to avoid expanding the compat macro in constructs like 'vtbl-read(...)'. Note that 'git add' exits with 0 even if it prints filtering errors to stderr. The test, therefore, checks stderr. 'git add' should probably be changed (sometime in another commit) to exit with nonzero if filtering fails. The test could then be changed to use test_must_fail. Thanks to the following people for their suggestions: Johannes Sixt j...@kdbg.org John Keeping j...@keeping.me.uk Jonathan Nieder jrnie...@gmail.com Kyle J. McKay mack...@gmail.com Torsten Bögershausen tbo...@web.de [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html [6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3 compate/clipped-write.c: large write(2) fails on Mac OS X/XNU Signed-off-by: Steffen Prohaska proha...@zib.de --- Makefile | 8 builtin/var.c | 1 + compat/clipped-read.c | 13 + config.mak.uname | 1 + git-compat-util.h | 5 + streaming.c | 1 + t/t0021-conversion.sh | 14 ++ 7 files changed, 43 insertions(+) create mode 100644 compat/clipped-read.c diff --git a/Makefile b/Makefile index 3588ca1..0f69e24 100644 --- a/Makefile +++ b/Makefile @@ -69,6 +69,9 @@ all:: # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt # doesn't support GNU extensions like --check and --statistics # +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than +# INT_MAX bytes at once (e.g. MacOS X). +# # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than # INT_MAX bytes at once (e.g. MacOS X). # @@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS MSGFMT += --check --statistics endif +ifdef NEEDS_CLIPPED_READ + BASIC_CFLAGS += -DNEEDS_CLIPPED_READ + COMPAT_OBJS += compat/clipped-read.o +endif + ifdef NEEDS_CLIPPED_WRITE BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE COMPAT_OBJS += compat/clipped-write.o diff --git a/builtin/var.c b/builtin/var.c index aedbb53..e59f5ba 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -38,6 +38,7 @@ static struct git_var git_vars[] = { { , NULL }, }; +#undef read static void list_vars(void) { struct git_var *ptr; diff --git a/compat/clipped-read.c b/compat/clipped-read.c new file mode 100644 index 000..6962f67 --- /dev/null +++ b/compat/clipped-read.c @@ -0,0 +1,13 @@ +#include ../git-compat-util.h +#undef read + +/* + * Version of read that will write at most INT_MAX bytes. + * Workaround a xnu bug on Mac OS X + */ +ssize_t clipped_read(int fd, void *buf, size_t nbyte) +{ + if (nbyte INT_MAX) + nbyte = INT_MAX; + return read(fd, buf, nbyte); +} diff --git a/config.mak.uname b/config.mak.uname index b27f51d..5c10726 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin) NO_MEMMEM = YesPlease USE_ST_TIMESPEC = YesPlease HAVE_DEV_TTY = YesPlease + NEEDS_CLIPPED_READ = YesPlease NEEDS_CLIPPED_WRITE = YesPlease COMPAT_OBJS += compat/precompose_utf8.o BASIC_CFLAGS += -DPRECOMPOSE_UNICODE diff --git a/git-compat-util.h b/git-compat-util.h index 115cb1d..a227127 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -185,6 +185,11 @@ typedef unsigned long uintptr_t; #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NEEDS_CLIPPED_READ +ssize_t clipped_read(int fd, void *buf, size_t nbyte); +#define read(x,y,z) clipped_read((x),(y),(z)) +#endif + #ifdef NEEDS_CLIPPED_WRITE ssize_t clipped_write(int fildes, const void *buf, size_t nbyte); #define write(x,y,z) clipped_write((x),(y),(z)) diff --git a/streaming.c b/streaming.c index debe904..c1fe34a 100644 --- a/streaming.c +++ b/streaming.c @@ -99,6 +99,7 @@ int close_istream(struct git_istream *st) return r; }
Re: [PATCH gitk 0/4] gitk support for git log -L
Paul Mackerras pau...@samba.org writes: Hi Thomas, On Wed, Jul 31, 2013 at 03:17:41PM +0200, Thomas Rast wrote: Jens Lehmann jens.lehm...@web.de writes: Am 29.07.2013 21:37, schrieb Thomas Rast: Thomas Rast tr...@inf.ethz.ch writes: Thomas Rast tr...@inf.ethz.ch writes: Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk. [...] One thing I worry about is having gitk storing in memory not just the history graph but also all the diffs (assuming I have understood correctly what you're doing). Gitk's memory consumption is already pretty large. However, I can't see an alternative at this point. I don't think there is one. log -L is pretty much an all or nothing thing at this point. I suppose if we really found that the diffs are regularly too big to be manageable for gitk, we could invent a porcelain mode where 'log -L' just prints the detected commits and corresponding line ranges, and then have a new option to diff-tree to let it again filter that range. But note that ordinary 'git log -L' also buffers the entire set of diffs within less. The memory consumption of gitk to hold the same diffs in memory should be only a small factor of what less uses in the same scenario. Furthermore, users will typically ask for a small region of code (one function, or some such), so the diffs themselves are usually quite small, nowhere near the size of the full commit diffs. Unfortunately it's turning out to be harder than I hoped. gitk runs the arguments through git-rev-parse, which only knows that -n gets an unstuck argument. Consequently, gitk accepts an unstuck -n but only stuck forms of -S and -G. Excuse my ignorance, but what do you mean by stuck vs. unstuck? Whether the option value is a separate argument in argv, or directly stuck to the option. stuck: gitk -L:foo:main.c unstuck: gitk -L :foo:main.c Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'. -- Thomas Rast trast@{inf,student}.ethz.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: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X
On 08/19/2013 10:20 AM, Johannes Sixt wrote: Am 19.08.2013 08:38, schrieb Steffen Prohaska: +test_expect_success EXPENSIVE 'filter large file' ' +git config filter.largefile.smudge cat +git config filter.largefile.clean cat +for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB Shouldn't you count to 2049 to get a file that is over 2GB? Would it be possible to offload the looping from shell to a real program? So for example truncate -s 2049M filename should do the job. That would create a file reading all bytes as zeros being larger as 2G. If truncate is not available, what about dd? Stefan signature.asc Description: OpenPGP digital signature
Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X
Am 19.08.2013 08:38, schrieb Steffen Prohaska: Note that 'git add' exits with 0 even if it prints filtering errors to stderr. The test, therefore, checks stderr. 'git add' should probably be changed (sometime in another commit) to exit with nonzero if filtering fails. The test could then be changed to use test_must_fail. Thanks for this hint. I was not aware of this behavior. Of course, we do *not* want to use test_must_fail because git add generally must not fail for files with more than 2GB. (Architectures with a 32bit size_t are a different matter, of course.) -- Hannes -- 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 v2] compat: Fix read() of 2GB and more on Mac OS X
On Aug 19, 2013, at 10:20 AM, Johannes Sixt j...@kdbg.org wrote: Am 19.08.2013 08:38, schrieb Steffen Prohaska: +test_expect_success EXPENSIVE 'filter large file' ' +git config filter.largefile.smudge cat +git config filter.largefile.clean cat +for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB Shouldn't you count to 2049 to get a file that is over 2GB? No. INT_MAX = 2GB - 1 works. INT_MAX + 1 = 2GB fails. It tests exactly at the boundary. Steffen -- 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: Does Git now have any C struct version history tracking mechanism?
Nazri Ramliy ayieh...@gmail.com writes: On Sun, Aug 18, 2013 at 6:33 PM, Zhan Jianyu nasa4...@gmail.com wrote: Such a requirement came into my mind when I am tracking a gloomy C struct , with lengthy list of elements which are either elaborated or opaque. So I use git blame to track it down and found that its original version is quite simple and intuitive. So I think I could just slice out every snapshot of this struct, reading every changelog , to get a better knowledge of what it is and why it should be like this. It seems quite helpful but the process is quite cumbersome. So I wonder if there is already some mechanism fulfilling my requirement in Git. If it doesn't, would it be worthy adding one ? It's already merged to git.git's master quite recently in ed73fe56428eecd2b635473f6a517a183c4713a3 (back in June). You'd invoke git log like this: $ git log -L :struct_or_function_name:filename.c and it will show you the commits and the specific hunks that affect the struct or function name. It's still a bit rough on the edges, for example, doing the following in git.git: $ git log -L :rev_cmdline_info:revision.h Shows three commits (a765499, ca92e59 and 281eee4) where the second one does not touch the struct at all (if you do git show ca92e59 you might gain an insight as to how -L works). Hmm, IIUC that's actually not a bug or even a roughness; it's an artifact of how the :pattern:file syntax is defined. It takes the first _funcname line_ matching 'pattern', up to (but excluding) the next funcname line. The default funcname rule (in the absence of any patterns) does not match '#define...' on account of the '#'. The default funcname pattern for 'cpp' (if you manually configured your git.git repo to set this attribute; it doesn't by default) never matches a leading '#´ either. So it's no surprise that the tracked range extends a few more lines after the struct. Or is there an issue that I'm missing? -- Thomas Rast trast@{inf,student}.ethz.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: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X
Am 19.08.2013 10:25, schrieb Stefan Beller: On 08/19/2013 10:20 AM, Johannes Sixt wrote: Am 19.08.2013 08:38, schrieb Steffen Prohaska: +test_expect_success EXPENSIVE 'filter large file' ' +git config filter.largefile.smudge cat +git config filter.largefile.clean cat +for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB Shouldn't you count to 2049 to get a file that is over 2GB? Would it be possible to offload the looping from shell to a real program? So for example truncate -s 2049M filename should do the job. That would create a file reading all bytes as zeros being larger as 2G. If truncate is not available, what about dd? The point is exactly to avoid external dependencies. Our dd on Windows doesn't do the right thing with seek=2GB (it makes the file twice as large as expected). -- Hannes -- 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
Reply.
I have a project for you in the tune of One Hundred Five Million EUR,Please reply for specifics. -- 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 v3] compat: Fix read() of 2GB and more on Mac OS X
On Mon, Aug 19, 2013 at 4:21 AM, Steffen Prohaska proha...@zib.de wrote: Previously, filtering 2GB or more through an external filter (see test) failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with: error: read from external filter cat failed error: cannot feed the input to external filter cat error: cat died of signal 13 error: external filter cat failed 141 error: external filter cat failed Signed-off-by: Steffen Prohaska proha...@zib.de --- Makefile | 8 builtin/var.c | 1 + compat/clipped-read.c | 13 + config.mak.uname | 1 + git-compat-util.h | 5 + streaming.c | 1 + t/t0021-conversion.sh | 14 ++ 7 files changed, 43 insertions(+) create mode 100644 compat/clipped-read.c diff --git a/Makefile b/Makefile index 3588ca1..0f69e24 100644 --- a/Makefile +++ b/Makefile @@ -69,6 +69,9 @@ all:: # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt # doesn't support GNU extensions like --check and --statistics # +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than +# INT_MAX bytes at once (e.g. MacOS X). +# # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than # INT_MAX bytes at once (e.g. MacOS X). Is it likely that we would see a platform requiring only one or the other CLIPPED? Would it make sense to combine these into a single NEEDS_CLIPPED_IO? # @@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS MSGFMT += --check --statistics endif +ifdef NEEDS_CLIPPED_READ + BASIC_CFLAGS += -DNEEDS_CLIPPED_READ + COMPAT_OBJS += compat/clipped-read.o +endif + ifdef NEEDS_CLIPPED_WRITE BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE COMPAT_OBJS += compat/clipped-write.o diff --git a/builtin/var.c b/builtin/var.c index aedbb53..e59f5ba 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -38,6 +38,7 @@ static struct git_var git_vars[] = { { , NULL }, }; -- 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: Notes and submodules
On Mon, Aug 19, 2013 at 10:13 AM, Francis Moreau francis.m...@gmail.com wrote: Hello, Is it possible to keep submodules notes in the super project ? Not easily. I guess it depends on what you want to use the notes for. In order for notes to be generally useful (i.e. show up in logs, surviving a notes prune, etc.) they really must reside in the same repo as the annotated objects [1]. Now, if all your interaction with notes happens through scripts that you control, then I guess it would be possible to hack this in some sort of semi-workable way, but you would still have to make sure never to run git notes prune in the super project. I guess the real question here is: Why would you want to do this? and is there maybe some other way your use case can be accomodated? ...Johan [1]: If you were to annotate objects in a submodule, but then store the notes objects in the super project, it would be impossible for git log in the submodule to find the notes objects, and your log would show no notes. Similarly, a git log in the super project would see a lot of notes objects pointing to non-existing objects (because those objects live in the submodule), hence the notes objects would be removed when running git notes prune in the super project. -- Johan Herland, jo...@herland.net www.herland.net -- 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 v2] compat: Fix read() of 2GB and more on Mac OS X
On 2013-08-19 08.38, Steffen Prohaska wrote: [snip] diff --git a/builtin/var.c b/builtin/var.c index aedbb53..e59f5ba 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -38,6 +38,7 @@ static struct git_var git_vars[] = { { , NULL }, }; +#undef read This is techically right for this very version of the code, but not really future proof, if someone uses read() further down in the code (in a later version) I think the problem comes from further up: -- struct git_var { const char *name; const char *(*read)(int); }; - could the read be replaced by readfn ? === diff --git a/streaming.c b/streaming.c index debe904..c1fe34a 100644 --- a/streaming.c +++ b/streaming.c @@ -99,6 +99,7 @@ int close_istream(struct git_istream *st) return r; } +#undef read Same possible future problem as above. When later someone uses read, the original (buggy) read() will be used, and not the re-defined clipped_read() from git-compat-util.h -- 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 v4] compat: Fix read() of 2GB and more on Mac OS X
Previously, filtering 2GB or more through an external filter (see test) failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with: error: read from external filter cat failed error: cannot feed the input to external filter cat error: cat died of signal 13 error: external filter cat failed 141 error: external filter cat failed The reason was that read() immediately returns with EINVAL if nbyte = 2GB. According to POSIX [1], if the value of nbyte passed to read() is greater than SSIZE_MAX, the result is implementation-defined. The write function has the same restriction [2]. Since OS X still supports running 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also imposed on 64-bit executables under certain conditions. For write, the problem has been addressed in a earlier commit [6c642a]. The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. It is very likely that the read() and write() wrappers are always used together. To avoid introducing another option, NEEDS_CLIPPED_WRITE is changed to NEEDS_CLIPPED_IO and used to activate both wrappers. To avoid expanding the read compat macro in constructs like 'vtbl-read(...)', 'read' is renamed to 'readfn' in two cases. The solution seems more robust than using '#undef read'. Note that 'git add' exits with 0 even if it prints filtering errors to stderr. The test, therefore, checks stderr. 'git add' should probably be changed (sometime in another commit) to exit with nonzero if filtering fails. The test could then be changed to use test_must_fail. Thanks to the following people for their suggestions: Johannes Sixt j...@kdbg.org John Keeping j...@keeping.me.uk Jonathan Nieder jrnie...@gmail.com Kyle J. McKay mack...@gmail.com Torsten Bögershausen tbo...@web.de Eric Sunshine sunsh...@sunshineco.com [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html [6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3 compate/clipped-write.c: large write(2) fails on Mac OS X/XNU Signed-off-by: Steffen Prohaska proha...@zib.de --- Makefile | 10 +- builtin/var.c| 10 +- compat/{clipped-write.c = clipped-io.c} | 11 ++- config.mak.uname | 2 +- git-compat-util.h| 5 - streaming.c | 4 ++-- t/t0021-conversion.sh| 14 ++ 7 files changed, 41 insertions(+), 15 deletions(-) rename compat/{clipped-write.c = clipped-io.c} (53%) diff --git a/Makefile b/Makefile index 3588ca1..f54134d 100644 --- a/Makefile +++ b/Makefile @@ -69,8 +69,8 @@ all:: # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt # doesn't support GNU extensions like --check and --statistics # -# Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than -# INT_MAX bytes at once (e.g. MacOS X). +# Define NEEDS_CLIPPED_IO if your read(2) and/or write(2) cannot handle more +# than INT_MAX bytes at once (e.g. Mac OS X). # # Define HAVE_PATHS_H if you have paths.h and want to use the default PATH # it specifies. @@ -1493,9 +1493,9 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS MSGFMT += --check --statistics endif -ifdef NEEDS_CLIPPED_WRITE - BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE - COMPAT_OBJS += compat/clipped-write.o +ifdef NEEDS_CLIPPED_IO + BASIC_CFLAGS += -DNEEDS_CLIPPED_IO + COMPAT_OBJS += compat/clipped-io.o endif ifneq (,$(XDL_FAST_HASH)) diff --git a/builtin/var.c b/builtin/var.c index aedbb53..06f8459 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -28,7 +28,7 @@ static const char *pager(int flag) struct git_var { const char *name; - const char *(*read)(int); + const char *(*readfn)(int); }; static struct git_var git_vars[] = { { GIT_COMMITTER_IDENT, git_committer_info }, @@ -43,8 +43,8 @@ static void list_vars(void) struct git_var *ptr; const char *val; - for (ptr = git_vars; ptr-read; ptr++) - if ((val = ptr-read(0))) + for (ptr = git_vars; ptr-readfn; ptr++) + if ((val = ptr-readfn(0))) printf(%s=%s\n, ptr-name, val); } @@ -53,9 +53,9 @@ static const char *read_var(const char *var) struct git_var *ptr; const char *val; val = NULL; - for (ptr = git_vars; ptr-read; ptr++) { + for (ptr = git_vars; ptr-readfn; ptr++) { if (strcmp(var, ptr-name) == 0) { - val = ptr-read(IDENT_STRICT); + val = ptr-readfn(IDENT_STRICT); break; } } diff --git a/compat/clipped-write.c b/compat/clipped-io.c similarity index 53% rename from compat/clipped-write.c rename
Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X
On Mon, Aug 19, 2013 at 8:41 AM, Steffen Prohaska proha...@zib.de wrote: The reason was that read() immediately returns with EINVAL if nbyte = 2GB. According to POSIX [1], if the value of nbyte passed to read() is greater than SSIZE_MAX, the result is implementation-defined. Yeah, the OS X filesystem layer is an incredible piece of shit. Not only doesn't it follow POSIX, it fails *badly*. Because OS X kernel engineers apparently have the mental capacity of a retarded rodent on crack. Linux also refuses to actually read more than a maximum value in one go (because quite frankly, doing more than 2GB at a time is just not reasonable, especially in unkillable disk wait), but at least Linux gives you the partial read, so that the usual read until you're happy works (which you have to do anyway with sockets, pipes, NFS intr mounts, etc etc). Returning EINVAL is a sign of a diseased mind. I hate your patch for other reasons, though: The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Why do you do that? We already _have_ wrapper functions for read(), namely xread(). Exactly because you basically have to, in order to handle signals on interruptible filesystems (which aren't POSIX either, but at least sanely so) or from other random sources. And to handle the you can't do reads that big issue. So why isn't the patch much more straightforward? Like the attached totally untested one that just limits the read/write size to 8MB (which is totally arbitrary, but small enough to not have any latency issues even on slow disks, and big enough that any reasonable IO subsystem will still get good throughput). And by totally untested I mean that it actually passes the git test suite, but since I didn't apply your patch nor do I have OS X anywhere, I can't actually test that it fixes *your* problem. But it should. Linus patch.diff Description: Binary data
Re: [PATCH] Git segmentation faults if submodule path is empty.
Updated the patch and the patch submission. -- 8 -- Git segmentation faults if submodule path is empty. Git fails due to a segmentation fault if a submodule path is empty. Here is an example .gitmodules that will cause a segmentation fault: [submodule foo-module] path url = http://host/repo.git $ git status Segmentation fault (core dumped) This occurs because in the function parse_submodule_config_option, the variable 'value' is assumed to be null, and when passed as an argument to xstrdup a segmentation fault occurs if it is indeed null. This is the case when using the .gitmodules example above. This patch addresses the issue by checking to make sure 'value' is not null before using it as an argument to xstrdup. For some configuration options, such as fetchRecurseSubmodules, an empty value is valid. If the option being read is 'path', an empty value is not valid, and so an error message is printed. Signed-off-by: Jharrod LaFon jlafon at eyesopen.com --- submodule.c|6 ++ t/t1307-null-submodule-path.sh | 14 ++ 2 files changed, 20 insertions(+) create mode 100755 t/t1307-null-submodule-path.sh diff --git a/submodule.c b/submodule.c index 1821a5b..1a2cf30 100644 --- a/submodule.c +++ b/submodule.c @@ -134,6 +134,9 @@ int parse_submodule_config_option(const char *var, const char *value) return 0; if (!strcmp(key, path)) { + if (!value) + return config_error_nonbool(var); + config = unsorted_string_list_lookup(config_name_for_path, value); if (config) free(config-util); @@ -151,6 +154,9 @@ int parse_submodule_config_option(const char *var, const char *value) } else if (!strcmp(key, ignore)) { char *name_cstr; + if (!value) + return config_error_nonbool(var); + if (strcmp(value, untracked) strcmp(value, dirty) strcmp(value, all) strcmp(value, none)) { warning(Invalid parameter \%s\ for config option \submodule.%s.ignore\, value, var); diff --git a/t/t1307-null-submodule-path.sh b/t/t1307-null-submodule-path.sh new file mode 100755 index 000..a4470a8 --- /dev/null +++ b/t/t1307-null-submodule-path.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +test_description='test empty submodule path' +. ./test-lib.sh + +setup() { +echo '[submodule test] path' .gitmodules +} + +test_expect_success 'git status with empty submodule path should not seg fault' ' +setup +test_must_fail git status +' +test_done -- 1.7.9.5 On Aug 16, 2013, at 2:52 PM, Jeff King p...@peff.net wrote: On Fri, Aug 16, 2013 at 10:59:35AM -0700, Jharrod LaFon wrote: Here is an updated patch with a test. Bits like this that should not be part of the commit message should either go after the --- lines near the diffstat, or should come before a scissors line, like: Here is my new patch. -- 8 -- Git segmentation faults etc... That way git-am can do the right thing, and the maintainer does not have to fix up your patch by hand. diff --git a/submodule.c b/submodule.c index 1821a5b..1e4acfd 100644 --- a/submodule.c +++ b/submodule.c @@ -134,6 +134,9 @@ int parse_submodule_config_option(const char *var, const char *value) return 0; if (!strcmp(key, path)) { +if (!value) +return config_error_nonbool(var); Your indentation looks like two spaces here, which does not match the rest of the block. It should be one tab. @@ -151,6 +154,9 @@ int parse_submodule_config_option(const char *var, const char *value) } else if (!strcmp(key, ignore)) { char *name_cstr; +if (!value) +return config_error_nonbool(var); + Ditto here. diff --git a/t/t1307-null-submodule-path.sh b/t/t1307-null-submodule-path.sh new file mode 100644 index 000..eeda2cb --- /dev/null +++ b/t/t1307-null-submodule-path.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +test_description='test empty submodule path' +. ./test-lib.sh + +setup() { +(printf [submodule \test\]\n +printf path\n +printf url) .gitmodules +} You can use single-quotes to avoid having to backslash the embedded double-quotes. And I do not see any reason to use printf rather than the more readable echo, which can drop the \n. And is there any point in having the url field? The presence of a valueless bool path should be enough, no? It may be easier to see what it is we are testing without the extraneous parameter. With those changes, you could even put it all on one line: echo '[submodule test] path' .gitmodules -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
Re: [PATCH v3] compat: Fix read() of 2GB and more on Mac OS X
Eric Sunshine sunsh...@sunshineco.com writes: +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than +# INT_MAX bytes at once (e.g. MacOS X). +# # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than # INT_MAX bytes at once (e.g. MacOS X). Is it likely that we would see a platform requiring only one or the other CLIPPED? Would it make sense to combine these into a single NEEDS_CLIPPED_IO? I am slightly negative to that suggestion for two reasons. - Does MacOS X clip other IO operations? Do we need to invent yet another NEEDS_CLIPPED, e.g. NEEDS_CLIPPED_LSEEK? A single NEEDS_CLIPPED_IO may sound attractive for its simplicity (e.g. on a system that only needs NEEDS_CLIPPED_WRITE, we will unnecessarily chop a big read into multiple reads, but that does not affect the correctness of the operation, only performance but the actual IO cost will dominate it anyway). If we know there are 47 different IO operations that might need clipping, that simplicity is certainly a good thing to have. I somehow do not think the set of operations will grow that large, though. - NEEDS_CLIPPED_IO essentially says only those who clip their writes would clip their reads (and vice versa), which is not all that different from saying only Apple would clip their IO, which in turn defeats the notion of let's use a generic NEEDS_CLIPPED without limiting the workaround to specific platforms somewhat. -- 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 v4] compat: Fix read() of 2GB and more on Mac OS X
On Aug 19, 2013, at 6:04 PM, Linus Torvalds torva...@linux-foundation.org wrote: I hate your patch for other reasons, though: The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Why do you do that? We already _have_ wrapper functions for read(), namely xread(). Exactly because you basically have to, in order to handle signals on interruptible filesystems (which aren't POSIX either, but at least sanely so) or from other random sources. And to handle the you can't do reads that big issue. So why isn't the patch much more straightforward? The first version was more straightforward [1]. But reviewers suggested that the compat wrappers would be the right way to do it and showed me that it has been done like this before [2]. I haven't submitted anything in a while, so I tried to be a kind person and followed the suggestions. I started to hate the patch a bit (maybe less than you), but I wasn't brave enough to reject the suggestions. This is why the patch became what it is. I'm happy to rework it again towards your suggestion. I would also remove the compat wrapper for write(). But I got a bit tired. I'd appreciate if I received more indication whether a version without compat wrappers would be accepted. Steffen [1] http://article.gmane.org/gmane.comp.version-control.git/232455 [2] 6c642a8 compate/clipped-write.c: large write(2) fails on Mac OS X/XNU-- 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 (Aug 2013, #04; Thu, 15)
Johannes Sixt j...@kdbg.org writes: Am 16.08.2013 00:36, schrieb Junio C Hamano: Due to unfortunate regressions, two topics had to be reverted: * An attempted fix to git stash save, to detect that going back to the state of the HEAD needs to lose killed files, and/or untracked files in a killed directory, to prevent the command from proceeding without --force. This used ls-files -k that was unusably slow. * An attempted enhancement to allow @ to be used to name HEAD. This rewrote @ in a ref where it shouldn't have, e.g. refs/@/foo. You still need to remove corresponding paragraphs from the release notes. Indeed. Thanks for reminding. -- 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
CPPCheck found 24 high risk bugs in Git v.1.8.3.4
I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? v/r Roderick (Rick) Koch Information Assurance rick.k...@tbe.com CPPCheck_on_Git.v1.8.3.4.xlsx Description: CPPCheck_on_Git.v1.8.3.4.xlsx
Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X
Linus Torvalds torva...@linux-foundation.org writes: I hate your patch for other reasons, though: The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Why do you do that? We already _have_ wrapper functions for read(), namely xread(). Exactly because you basically have to, in order to handle signals on interruptible filesystems (which aren't POSIX either, but at least sanely so) or from other random sources. And to handle the you can't do reads that big issue. The same argument applies to xwrite(), but currently we explicitly catch EINTR and EAGAIN knowing that on sane systems these are the signs that we got interrupted. Do we catch EINVAL unconditionally in the same codepath? Could EINVAL on saner systems mean completely different thing (like our caller is passing bogus parameters to underlying read/write, which is a program bug we would want to catch)? So why isn't the patch much more straightforward? Like the attached totally untested one that just limits the read/write size to 8MB (which is totally arbitrary, but small enough to not have any latency issues even on slow disks, and big enough that any reasonable IO subsystem will still get good throughput). Ahh. OK, not noticing EINVAL unconditionally, but always feed IOs in chunks that are big enough for sane systems but small enough for broken ones. That makes sense. Could somebody on MacOS X test this? 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: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X
Steffen Prohaska proha...@zib.de writes: I'm happy to rework it again towards your suggestion. I would also remove the compat wrapper for write(). But I got a bit tired. I'd appreciate if I received more indication whether a version without compat wrappers would be accepted. I think it is a reasonable way forward to remove the writer side wrapper and doing large IO in reasonably big (but small enough not to trigger MacOS X limitations) chunks in both read/write direction. Linus, thanks for a dose of sanity. -- 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 gitk 0/4] gitk support for git log -L
Thomas Rast tr...@inf.ethz.ch writes: Whether the option value is a separate argument in argv, or directly stuck to the option. stuck: gitk -L:foo:main.c unstuck: gitk -L :foo:main.c Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'. I somehow thought that we encourage the stuck/sticked form, to reduce things the users need to remember to cope better with options with optional value. -- 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 v4] compat: Fix read() of 2GB and more on Mac OS X
On Mon, Aug 19, 2013 at 10:16 AM, Junio C Hamano gits...@pobox.com wrote: Linus Torvalds torva...@linux-foundation.org writes: The same argument applies to xwrite(), but currently we explicitly catch EINTR and EAGAIN knowing that on sane systems these are the signs that we got interrupted. Do we catch EINVAL unconditionally in the same codepath? No, and we shouldn't. If EINVAL happens, it will keep happening. But with the size limiter, it doesn't matter, since we won't hit the OS X braindamage. Could EINVAL on saner systems mean completely different thing (like our caller is passing bogus parameters to underlying read/write, which is a program bug we would want to catch)? Yes. Even on OS X, it means that - it's just that OS X notion of what is bogus is pure crap. But the thing is, looping on EINVAL would be wrong even on OS X, since unless you change the size, it will keep happening forever. But with the limit IO to 8MB (or whatever) patch, the issue is moot. If you get an EINVAL, it will be due to something else being horribly horribly wrong. Linus -- 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] Git segmentation faults if submodule path is empty.
Jharrod LaFon jla...@eyesopen.com writes: Updated the patch and the patch submission. Getting a lot warmer ;-). -- 8 -- Git segmentation faults if submodule path is empty. If this is meant to replace the MUA's Subject: line, then please add Subject: prefix, like the example at the end of this message. Our commit titles by convention omit the final full-stop. Git fails due to a segmentation fault if a submodule path is empty. Please do not indent the body of the commit log message. Flush them to the left. Here is an example .gitmodules that will cause a segmentation fault: Please have a blank line before an example added for illustration in the log message. [submodule foo-module] path url = http://host/repo.git $ git status Segmentation fault (core dumped) Indenting such an illustration, and having a blank line after it, are good things. Please continue doing so. This occurs because in the function parse_submodule_config_option, the Again, please do not indent the body text of the log message. variable 'value' is assumed to be null, and when passed as an argument to xstrdup a segmentation fault occurs if it is indeed null. This is the case when using the .gitmodules example above. It is not assumed to be null, is it? This patch addresses the issue by checking to make sure 'value' is not null before using it as an argument to xstrdup. For some configuration options, such as fetchRecurseSubmodules, an empty value is valid. If the option being read is 'path', an empty value is not valid, and so an error message is printed. We like to write the log message in the imperative mood, as if we are ordering the codebase to make it so. Signed-off-by: Jharrod LaFon jlafon at eyesopen.com Please do not do cute at here. With a Signed-off-by, you are already agreeing to Developer's Certificate of Origin 1.1, cluase (d): (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. --- submodule.c|6 ++ t/t1307-null-submodule-path.sh | 14 ++ Can we have the test not in a brand new test script file, but at the end of an existing one? diff --git a/t/t1307-null-submodule-path.sh b/t/t1307-null-submodule-path.sh new file mode 100755 index 000..a4470a8 --- /dev/null +++ b/t/t1307-null-submodule-path.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +test_description='test empty submodule path' +. ./test-lib.sh + +setup() { +echo '[submodule test] path' .gitmodules +} No SP after redirection operator, i.e. echo '[submodule test] path' .gitmodules Also it does not make much sense to have a helper script that is used only once (for that matter, it does not make much sense to add a new script file to add a single two-liner test). Here is to illustrate all the points mentioned above. -- 8 -- From: Jharrod LaFon jla...@eyesopen.com Subject: avoid segfault on submodule.*.path set to an empty true Date: Mon, 19 Aug 2013 09:26:56 -0700 Git fails due to a segmentation fault if a submodule path is empty. Here is an example .gitmodules that will cause a segmentation fault: [submodule foo-module] path url = http://host/repo.git $ git status Segmentation fault (core dumped) This is because the parsing of submodule.*.path is not prepared to see a value-less true and assumes that the value is always non-NULL (parsing of ignore has the same problem). Fix it by checking the NULL-ness of value and complain with config_error_nonbool(). Signed-off-by: Jharrod LaFon jla...@eyesopen.com Signed-off-by: Junio C Hamano gits...@pobox.com --- submodule.c| 6 ++ t/t7400-submodule-basic.sh | 10 ++ 2 files changed, 16 insertions(+) diff --git a/submodule.c b/submodule.c index 3f0a3f9..c0f93c2 100644 --- a/submodule.c +++ b/submodule.c @@ -134,6 +134,9 @@ int parse_submodule_config_option(const char *var, const char *value) return 0; if (!strcmp(key, path)) { + if (!value) + return config_error_nonbool(var); + config = unsorted_string_list_lookup(config_name_for_path, value); if (config) free(config-util); @@ -151,6 +154,9 @@ int parse_submodule_config_option(const char *var, const char *value) } else if (!strcmp(key, ignore)) { char *name_cstr; + if (!value) + return config_error_nonbool(var); + if (strcmp(value, untracked) strcmp(value, dirty) strcmp(value, all) strcmp(value, none)) {
[ANNOUNCE] Git v1.8.4-rc4
A release candidate Git v1.8.4-rc4 is now available for testing at the usual places. The only changes since -rc3 are reversion of two topics that introduced regressions. Hopefully the final at the end of this week and then we will start the next cycle, most likely to be for 1.8.5. This is a bit delayed than I planned early last week, but sometimes real life happens. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: f390da8ea032c92c673d4cc6c4d8818379db344d git-1.8.4.rc4.tar.gz 90a9df6b2ada9a0ab9c8711e03d77244e7310c1e git-htmldocs-1.8.4.rc4.tar.gz 4e6ed2c0307ba538257bdc9f233dd574b419f411 git-manpages-1.8.4.rc4.tar.gz The following public repositories all have a copy of the v1.8.4-rc4 tag and the master branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.8.4 Release Notes (draft) Backward compatibility notes (for Git 2.0) -- When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics that pushes: - only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch, if you are pushing to the same remote as you fetch from; or - only the current branch to the branch with the same name, if you are pushing to a remote that is not where you usually fetch from. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. Updates since v1.8.3 Foreign interfaces, subsystems and ports. * Cygwin port has been updated for more recent Cygwin 1.7. * git rebase -i now honors --strategy and -X options. * Git-gui has been updated to its 0.18.0 version. * MediaWiki remote helper (in contrib/) has been updated to use the credential helper interface from Git.pm. * Update build for Cygwin 1.[57]. Torsten Bögershausen reports that this is fine with Cygwin 1.7 ($gmane/225824) so let's try moving it ahead. * The credential helper to talk to keychain on OS X (in contrib/) has been updated to kick in not just when talking http/https but also imap(s) and smtp. * Remote transport helper has been updated to report errors and maintain ref hierarchy used to keep track of its own state better. * With export remote-helper protocol, (1) a push that tries to update a remote ref whose name is different from the pushing side does not work yet, and (2) the helper may not know how to do --dry-run; these problematic cases are disabled for now. * git-remote-hg/bzr (in contrib/) updates. * git-remote-mw (in contrib/) hints users to check the certificate, when https:// connection failed. * git-remote-mw (in contrib/) adds a command to allow previewing the contents locally before pushing it out, when working with a MediaWiki remote. UI, Workflows Features * Sample post-receive-email hook script got an enhanced replacement multimail (in contrib/). * Also in contrib/ is a new contacts script that runs git blame to find out the people who may be interested in a set of changes. * git clean command learned an interactive mode. * The --head option to git show-ref was only to add
Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
From: Koch, Rick (Subcontractor) rick.k...@tbe.com Sent: Monday, August 19, 2013 6:09 PM I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? v/r Roderick (Rick) Koch Information Assurance rick.k...@tbe.com What OS version / CPPCheck version was this checked on? In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. Philip description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2803 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2802 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2805 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c syslog.c 46 uninitvar(CppCheck) \git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c 419 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156 uninitvar(CppCheck) \git-master\merge-recursive.c merge-recursive.c 1887 uninitvar(CppCheck) \git-master\notes.c notes.c 805 uninitvar(CppCheck) \git-master\notes.c notes.c 805 deallocret(CppCheck) \git-master\pretty.c pretty.c 677 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125 doubleFree(CppCheck) \git-master\shell.c shell.c 130 -- 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: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
On Mon, Aug 19, 2013 at 09:03:21PM +0100, Philip Oakley wrote: In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. [...] description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 Hm. That code in v1.8.3.4 reads: if (pathspec) while (pathspec[pc]) pc++; What's the problem? If pathspec is not properly terminated, we can run off the end, but I do see anything to indicate that is the case. What does the nullPointer check mean here? wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 Line 588 does not have formatted I/O at all. Are these line numbers somehow not matching what I have in v1.8.3.4? nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 This one looks like: if (tag *tag show_valid_bit (ce-ce_flags CE_VALID)) { static char alttag[4]; memcpy(alttag, tag, 3); if (isalpha(tag[0])) where the final line is 144. But we have explicitly checked that tag is not NULL... doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 This one looks like: if (...) { free(buf); die(...); } ... free(buf); which might look like a double free if you do not know that die() will never return (it is properly annotated for gcc, but I don't know whether CppCheck understands such things). So out of the 4 entries I investigated, none of them looks like an actual problem. But I'm not even sure I am looking at the right place; these don't even seem like things that would cause a false positive in a static analyzer. -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: [ANNOUNCE] Git v1.8.4-rc4
On Aug 19, 2013, at 12:59, Junio C Hamano wrote: Performance, Internal Implementation, etc. * On Cygwin, we used to use our own lstat(2) emulation that is allegedly faster than the platform one in codepaths where some of the information it returns did not matter, but it started to bite us in a few codepaths where the trick it uses to cheat does show breakages. This emulation has been removed and we use the native lstat(2) emulation supplied by Cygwin now. * The function attributes extensions are used to catch mistakes in use of our own variadic functions that use NULL sentinel at the end (i.e. like execl(3)) and format strings (i.e. like printf(3)). * The code to allow configuration data to be read from in-tree blob objects is in. This may help working in a bare repository and submodule updates. * Fetching between repositories with many refs employed O(n^2) algorithm to match up the common objects, which has been corrected. * The original way to specify remote repository using .git/branches/ used to have a nifty feature. The code to support the feature was still in a function but the caller was changed not to call it 5 years ago, breaking that feature and leaving the supporting code unreachable. The dead code has been removed. * git pack-refs that races with new ref creation or deletion have been susceptible to lossage of refs under right conditions, which has been tightened up. * We read loose and packed rerferences in two steps, but after ^^ deciding to read a loose ref but before actually opening it to read s/rerferences/references/ -- 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: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
Jeff King p...@peff.net writes: On Mon, Aug 19, 2013 at 09:03:21PM +0100, Philip Oakley wrote: In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. [...] description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 Hm. That code in v1.8.3.4 reads: if (pathspec) while (pathspec[pc]) pc++; What's the problem? If pathspec is not properly terminated, we can run off the end, but I do see anything to indicate that is the case. What does the nullPointer check mean here? wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 Line 588 does not have formatted I/O at all. Are these line numbers somehow not matching what I have in v1.8.3.4? nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 This one looks like: if (tag *tag show_valid_bit (ce-ce_flags CE_VALID)) { static char alttag[4]; memcpy(alttag, tag, 3); if (isalpha(tag[0])) where the final line is 144. But we have explicitly checked that tag is not NULL... doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 This one looks like: if (...) { free(buf); die(...); } ... free(buf); which might look like a double free if you do not know that die() will never return (it is properly annotated for gcc, but I don't know whether CppCheck understands such things). So out of the 4 entries I investigated, none of them looks like an actual problem. But I'm not even sure I am looking at the right place; these don't even seem like things that would cause a false positive in a static analyzer. And the ones I picked at random looks totally bogus, too. uninitvar(CppCheck) \git-master\notes.c notes.c 805 uninitvar(CppCheck) \git-master\notes.c notes.c 805 That is int combine_notes_concatenate(unsigned char *cur_sha1, const unsigned char *new_sha1) { char *cur_msg = NULL, *new_msg = NULL, *buf; unsigned long cur_len, new_len, buf_len; enum object_type cur_type, new_type; int ret; /* read in both note blob objects */ if (!is_null_sha1(new_sha1)) new_msg = read_sha1_file(new_sha1, new_type, new_len); 805:if (!new_msg || !new_len || new_type != OBJ_BLOB) { free(new_msg); return 0; } new_msg starts out to be NULL, if we did not run read_sha1_file(), it will still be NULL and if() will not look at new_len/new_type so their being uninitialized does not matter. If we did run read_sha1_file(), and if it returns a non-NULL new_msg, both of these are filled. If read_sha1_file() returns a NULL new_msg, again the other two does not matter. In short, the analyzer seems to be giving useless noise for this one. -- 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] Git segmentation faults if submodule path is empty.
On Mon, Aug 19, 2013 at 11:56:17AM -0700, Junio C Hamano wrote: Jharrod LaFon jla...@eyesopen.com writes: Updated the patch and the patch submission. Getting a lot warmer ;-). Thanks, I agree with all of the stuff you said. The end result that you included looks good to me. -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: [PATCH] Git segmentation faults if submodule path is empty.
Jharrod LaFon jla...@eyesopen.com writes: I will keep trying this until it's perfect, and I thank you for the help. When I resubmit this, would you like me to include your sign-off line as well? If the one I attached at the end of the message you are responding to looks fine to you, I'd just apply it without having you to reroll. Also, the end of the test script was not included in your message. Sorry, but I am not sure what you meant by this. I reworked your example to make it the second test in an existing test script. There are many existing tests after that so it is natural that we wouldn't see the end of the file in the patch context. diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 5ee97b0..a39d074 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -18,6 +18,16 @@ test_expect_success 'setup - initial commit' ' git branch initial ' +test_expect_success 'configuration parsing' ' +test_when_finished rm -f .gitmodules +cat .gitmodules -\EOF +[submodule s] +path +ignore +EOF +test_must_fail git status +' + test_expect_success 'setup - repository in init subdirectory' ' mkdir init ( -- 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 v2 1/2] submodule: fix confusing variable name
Am 17.08.2013 19:25, schrieb brian m. carlson: cmd_summary reads the output of git diff, but reads in the submodule path into a variable called name. Since this variable does not contain the name of the submodule, but the path, rename it to be clearer what data it actually holds. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Thanks, this one is looking good to me. Acked-by: Jens Lehmann jens.lehm...@web.de --- git-submodule.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2979197..38520db 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1032,13 +1032,13 @@ cmd_summary() { # Get modified modules cared by user modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- $@ | sane_egrep '^:([0-7]* )?16' | - while read mod_src mod_dst sha1_src sha1_dst status name + while read mod_src mod_dst sha1_src sha1_dst status sm_path do # Always show modules deleted or type-changed (blob-module) - test $status = D -o $status = T echo $name continue + test $status = D -o $status = T echo $sm_path continue # Also show added or modified modules which are checked out - GIT_DIR=$name/.git git-rev-parse --git-dir /dev/null 21 - echo $name + GIT_DIR=$sm_path/.git git-rev-parse --git-dir /dev/null 21 + echo $sm_path done ) -- 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 v2 2/2] submodule: don't print status output with ignore=all
Am 17.08.2013 19:25, schrieb brian m. carlson: git status prints information for submodules, but it should ignore the status of those which have submodule.name.ignore set to all. Fix it so that it does properly ignore those which have that setting either in .git/config or in .gitmodules. Not ignored are submodules that are added, deleted, or moved (which is essentially a combination of the first two) because it is not easily possible to determine the old path once a move has occurred, nor is it easily possible to detect which adds and deletions are moves and which are not. This also preserves the previous behavior of always listing modules which are to be deleted. Sounds sane. Even though we should be able to follow renames by inspecting the old .gitmodules content with the new --blob option of git config, I doubt it'll be worth the hassle. Thanks for fixing two known test breakages. Acked-by: Jens Lehmann jens.lehm...@web.de Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- git-submodule.sh | 7 +++ t/t7508-status.sh | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 38520db..c1ba0f8 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1036,6 +1036,13 @@ cmd_summary() { do # Always show modules deleted or type-changed (blob-module) test $status = D -o $status = T echo $sm_path continue + # Respect the ignore setting for --for-status. + if test -n $for_status + then + name=$(module_name $sm_path) + ignore_config=$(get_submodule_config $name ignore none) + test $status != A -a $ignore_config = all continue + fi # Also show added or modified modules which are checked out GIT_DIR=$sm_path/.git git-rev-parse --git-dir /dev/null 21 echo $sm_path diff --git a/t/t7508-status.sh b/t/t7508-status.sh index ac3d0fe..fb89fb9 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1316,7 +1316,7 @@ test_expect_success --ignore-submodules=all suppresses submodule summary ' test_i18ncmp expect output ' -test_expect_failure '.gitmodules ignore=all suppresses submodule summary' ' +test_expect_success '.gitmodules ignore=all suppresses submodule summary' ' git config --add -f .gitmodules submodule.subname.ignore all git config --add -f .gitmodules submodule.subname.path sm git status output @@ -1324,7 +1324,7 @@ test_expect_failure '.gitmodules ignore=all suppresses submodule summary' ' git config -f .gitmodules --remove-section submodule.subname ' -test_expect_failure '.git/config ignore=all suppresses submodule summary' ' +test_expect_success '.git/config ignore=all suppresses submodule summary' ' git config --add -f .gitmodules submodule.subname.ignore none git config --add -f .gitmodules submodule.subname.path sm git config --add submodule.subname.ignore all -- 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] submodule: prevent warning in summary output
Am 19.08.2013 10:13, schrieb Chris Packham: Hi Brian, On 19/08/13 05:31, brian m. carlson wrote: When git submodule summary is run and there is a deleted submodule, there is an warning from git rev-parse: fatal: Not a git repository: '.vim/pathogen/.git' Silence this warning, since it is fully expected that a deleted submodule will not be a git repository. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2979197..66ee621 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1071,7 +1071,7 @@ cmd_summary() { missing_dst= test $mod_src = 16 -! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null +! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null 21 missing_src=t test $mod_dst = 16 I wonder if there are other useful errors this will silence unintentionally. Perhaps this would be better (untested) test $mod_src = 16 test -e $name/.git ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null missing_src=t Having said that there are precedents for both in git-submodule.sh. If there aren't any errors worth catching here then your way is probably cleaner than mine. I'd prefer a way to not drop all errors too. We should be able to use the status variable to see if the submodule is deleted and then skip the rev-parse all together, or am I missing something here? -- 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] git-send-email: kill $prompting variable
On Fri, Aug 16, 2013 at 05:34:04PM +, Rasmus Villemoes wrote: The variable $prompting is weird. It is only read in one place (when deciding whether to prompt for a Message-ID to use in In-Reply-To), and it will be false unless we've taken the completely unrelated branch filling in @initial_to. Prompting should be done if the info is needed, not if some unrelated item had to be prompted for. So kill $prompting. The prompting flag dates back to 1f038a0 from late 2005. I _think_ the intent was that you could use certain command lines to specify the required information (like initial compose subject line, sender, etc), and then send-email would skip prompting for the optional information (like in-reply-to). That makes it easier to use in a batch mode in which the user does not want to be prompted (they do not have to give a blank --in-reply-to to prevent the prompt). Over the years, the set of items which triggered prompting (and which depended on previous prompts) has grown and shrunk, and most prompts do not respect the $prompting system at all. So I kind of doubt that anybody will care if it goes away; it does not make much sense at this point. However, your patch will make the default be to ask about the initial message-id. Which is likely going to annoy people, as it is not necessary (and people who care can specify it on the command line). Would we want to get rid of it entirely? -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: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
On 08/19/2013 07:09 PM, Koch, Rick (Subcontractor) wrote: I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? Hi, if you're using cppcheck as found at https://github.com/danmar/cppcheck or http://sourceforge.net/apps/trac/cppcheck/ you really need to review the results, as there are many false positives. I used that tool for my contributions so far (bug fixes as reported by cppcheck). However you *really* need to manually review any message cppcheck generates. This is because git is using a C, asm-like coding style for many routines, whereas that cppcheck is rather optimized to find typical C++ errors. And the styles vary wildy! (cppcheck tries to become no false positives, but it's hard I guess) I am running that cppcheck tool on git regulary (cppcheck master branch on git master branch), and review for real findings, you're welcome to do so as well. :) There are other static code analyzers, which have slightly different goals, such as http://css.csail.mit.edu/stack/ which has an incredibly low false positive rate (I found none as of now). However I think having different tools is a great thing, but you'd need to know your tools. ;) Stefan signature.asc Description: OpenPGP digital signature
Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
From: Koch, Rick (Subcontractor) rick.k...@tbe.com Ran CPPCheck 1.5.6 on Windows-XP. Hi Rick, Thank you for the clarification. Normal practice on the list is to use Reply All, so everyone can participate in the discussion. It looks like most of the reports are false positives. My bikeshedding thought would be that it is common in Git to inspect all the call sites such that they don't create the various problems, rather than protect against the problems within the various functions, which may be a cause of the reports (i.e. different philosophical approach to checking). regards Philip --- v/r Roderick (Rick) Koch OSF - Information Assurance Team Teledyne / Sentar Inc. Work: 256-726-1253 rick.k...@tbe.com -Original Message- From: Philip Oakley [mailto:philipoak...@iee.org] Sent: Monday, August 19, 2013 3:03 PM To: Koch, Rick (Subcontractor); Git List Subject: Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4 From: Koch, Rick (Subcontractor) rick.k...@tbe.com Sent: Monday, August 19, 2013 6:09 PM I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? v/r Roderick (Rick) Koch Information Assurance rick.k...@tbe.com What OS version / CPPCheck version was this checked on? In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. Philip description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2803 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2802 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2805 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c syslog.c 46 uninitvar(CppCheck) \git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c 419 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156 uninitvar(CppCheck) \git-master\merge-recursive.c merge-recursive.c 1887 uninitvar(CppCheck) \git-master\notes.c notes.c 805 uninitvar(CppCheck) \git-master\notes.c notes.c 805 deallocret(CppCheck) \git-master\pretty.c pretty.c 677 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125 doubleFree(CppCheck) \git-master\shell.c shell.c 130 -- 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 v4] compat: Fix read() of 2GB and more on Mac OS X
On Aug 19, 2013, at 10:16, Junio C Hamano wrote: Linus Torvalds torva...@linux-foundation.org writes: So why isn't the patch much more straightforward? Like the attached totally untested one that just limits the read/write size to 8MB (which is totally arbitrary, but small enough to not have any latency issues even on slow disks, and big enough that any reasonable IO subsystem will still get good throughput). Ahh. OK, not noticing EINVAL unconditionally, but always feed IOs in chunks that are big enough for sane systems but small enough for broken ones. That makes sense. Could somebody on MacOS X test this? I tested this on both i386 (OS X 32-bit intel) and x86_64 (OS X 64-bit intel). What I tested: 1. I started with branch pu: (965adb10 Merge branch 'sg/bash-prompt-lf-in-cwd-test' into pu) 2. I added Steffen's additional test (modified to always run) to t0021: diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index e50f0f7..b92e6cb 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -190,4 +190,16 @@ test_expect_success 'required filter clean failure' ' test_must_fail git add test.fc ' +test_expect_success 'filter large file' ' + git config filter.largefile.smudge cat + git config filter.largefile.clean cat + for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB + echo 2GB filter=largefile .gitattributes + git add 2GB 2err + ! test -s err + rm -f 2GB + git checkout -- 2GB 2err + ! test -s err +' + test_done 3. I verified that the test fails with an unpatched build on both 32- bit and 64-bit. 4. I applied Linus's unmodified patch to wrapper.c. 5. I tested again. The t0021 test now passes on 64-bit. It still fails on 32-bit for another reason unrelated to Linus's patch. It fails when attempting the git add 2GB line from the new 'filter large file' part of the test. The failure with backtrace: git(16806,0xa095c720) malloc: *** mmap(size=2147487744) failed (error code=12) *** error: can't allocate region *** set a breakpoint in malloc_error_break to debug # NOTE: error code 12 is ENOMEM on OS X Breakpoint 1, 0x97f634b1 in malloc_error_break () (gdb) bt #0 0x97f634b1 in malloc_error_break () #1 0x97f5e49f in szone_error () #2 0x97e8b876 in allocate_pages () #3 0x97e8c062 in large_and_huge_malloc () #4 0x97e831c8 in szone_malloc () #5 0x97e82fb8 in malloc_zone_malloc () #6 0x97e8c7b2 in realloc () #7 0x00128abe in xrealloc (ptr=0x0, size=2147483649) at wrapper.c:100 #8 0x00111a8c in strbuf_grow (sb=0xbfffe634, extra=2147483648) at strbuf.c:74 #9 0x00112bb9 in strbuf_read (sb=0xbfffe634, fd=6, hint=2548572518) at strbuf.c:349 #10 0x0009b899 in apply_filter (path=value temporarily unavailable, due to optimizations, src=0x100 ' ' repeats 200 times..., len=2147483648, dst=0xbfffe774, cmd=0x402980 cat) at convert.c:407 #11 0x0009c6f6 in convert_to_git (path=0x4028b4 2GB, src=0x100 ' ' repeats 200 times..., len=2147483648, dst=0xbfffe774, checksafe=SAFE_CRLF_WARN) at convert.c:764 #12 0x0010bb38 in index_mem (sha1=0x402330 , buf=0x100, size=2147483648, type=OBJ_BLOB, path=0x4028b4 2GB, flags=1) at sha1_file.c:3044 #13 0x0010bf57 in index_core [inlined] () at /private/var/tmp/src/git/ sha1_file.c:3101 #14 0x0010bf57 in index_fd (sha1=0x402330 , fd=5, st=0xbfffe900, type=OBJ_BLOB, path=0x4028b4 2GB, flags=1) at sha1_file.c:3139 #15 0x0010c05e in index_path (sha1=0x402330 , path=0x4028b4 2GB, st=0xbfffe900, flags=1) at sha1_file.c:3157 #16 0x000e82f4 in add_to_index (istate=0x1a8820, path=0x4028b4 2GB, st=0xbfffe900, flags=0) at read-cache.c:665 #17 0x000e87c8 in add_file_to_index (istate=0x1a8820, path=0x4028b4 2GB, flags=0) at read-cache.c:694 #18 0x440a in cmd_add (argc=value temporarily unavailable, due to optimizations, argv=0xb584, prefix=0x0) at builtin/add.c:299 #19 0x2e1f in run_builtin [inlined] () at /private/var/tmp/src/git/ git.c:303 #20 0x2e1f in handle_internal_command (argc=2, argv=0xb584) at git.c:466 #21 0x32d4 in run_argv [inlined] () at /private/var/tmp/src/git/ git.c:512 #22 0x32d4 in main (argc=2, av=0xbfffe28c) at git.c:595 The size 2147487744 is 2GB + 4096 bytes. Apparently git does not support a filter for a file unless the file can fit entirely into git's memory space. Normally a single 2GB + 4096 byte allocation works in an OS X 32-bit process, but something else is apparently eating up a large portion of the memory space in this case (perhaps an mmap'd copy?). In any case, if the file being filtered was closer to 4GB in size it would always fail on 32-bit regardless. The fact that the entire file is read into memory when applying the filter does not seem like a good thing (see #7-#10 above). --Kyle -- 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
What's cooking in git.git (Aug 2013, #05; Mon, 19)
What's cooking in git.git (Aug 2013, #05; Mon, 19) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. An extra release candidate -rc4 has been tagged and pushed out. Hopefully this will be the last one before the final release of 1.8.4. As I expect we will have two more cycles of 1.8.x by the end of the year and then 2.0 early next year, we may want to merge these for 2.0 topics to 'next' for real, starting the next cycle. 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] * es/rebase-i-respect-core-commentchar (2013-08-18) 1 commit - rebase -i: fix cases ignoring core.commentchar Will merge to and cook in 'next'. * jx/branch-vv-always-compare-with-upstream (2013-08-18) 3 commits - status: always show tracking branch even no change - branch: mark missing tracking branch as gone - branch: not report invalid tracking branch * nd/fetch-into-shallow (2013-08-18) 6 commits - list-objects: mark more commits as edges in mark_edges_uninteresting - list-objects: reduce one argument in mark_edges_uninteresting - upload-pack: delegate rev walking in shallow fetch to pack-objects - shallow: add setup_temporary_shallow() - shallow: only add shallow graft points to new shallow file - move setup_alternate_shallow and write_shallow_commits to shallow.c * sb/diff-delta-remove-needless-comparison (2013-08-18) 1 commit - create_delta_index: simplify condition always evaluating to true Will merge to and cook in 'next'. * sg/bash-prompt-lf-in-cwd-test (2013-08-18) 1 commit - bash prompt: test the prompt with newline in repository path Will merge to and cook in 'next'. * jl/some-submodule-config-are-not-boolean (2013-08-19) 1 commit - avoid segfault on submodule.*.path set to an empty true Will merge to and cook in 'next'. -- [Stalled] * tf/gitweb-ss-tweak (2013-07-15) 4 commits - gitweb: make search help link less ugly - gitweb: omit the repository owner when it is unset - gitweb: vertically centre contents of page footer - gitweb: ensure OPML text fits inside its box Comments? * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits - ### DONTMERGE: needs better explanation on what config they need - pack-refs.c: Add missing call to git_config() - show-ref.c: Add missing call to git_config() The changes themselves are probably good, but it is unclear what basic setting needs to be read for which exact operation. Waiting for clarification. $gmane/228294 * jh/shorten-refname (2013-05-07) 4 commits - t1514: refname shortening is done after dereferencing symbolic refs - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin - t1514: Demonstrate failure to correctly shorten refs/remotes/origin/HEAD - t1514: Add tests of shortening refnames in strict/loose mode When remotes/origin/HEAD is not a symbolic ref, rev-parse --abbrev-ref remotes/origin/HEAD ought to show origin, not origin/HEAD, which is fixed with this series (if it is a symbolic ref that points at remotes/origin/something, then it should show origin/something and it already does). Expecting a reroll, as an early part of a larger series. $gmane/225137 * jk/list-objects-sans-blobs (2013-06-06) 4 commits . archive: ignore blob objects when checking reachability . list-objects: optimize revs-blob_objects = 0 case . upload-archive: restrict remote objects with reachability check . clear parsed flag when we free tree buffers Attempt to allow archive --remote=$there $arbitrary_sha1 while keeping the reachability safety. Seems to break some tests in a trivial and obvious way. * mg/more-textconv (2013-05-10) 7 commits - grep: honor --textconv for the case rev:path - grep: allow to use textconv filters - t7008: demonstrate behavior of grep with textconv - cat-file: do not die on --textconv without textconv filters - show: honor --textconv for blobs - diff_opt: track whether flags have been set explicitly - t4030: demonstrate behavior of show with textconv Make git grep and git show pay attention to --textconv when dealing with blob objects. I thought this was pretty well designed and executed, but it seems there are some doubts on the list; kicked back to 'pu'. * jc/format-patch (2013-04-22) 2 commits - format-patch: --inline-single - format-patch: rename no_inline field A new option to send a single patch to the standard output to be appended at the bottom of a message. I personally have no need for this, but it was easy enough to cobble together. Tests, docs and stripping out more MIMEy stuff are left as exercises to interested parties. Not ready for inclusion.
Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X
On Mon, Aug 19, 2013 at 2:56 PM, Kyle J. McKay mack...@gmail.com wrote: The fact that the entire file is read into memory when applying the filter does not seem like a good thing (see #7-#10 above). Yeah, that's horrible. Its likely bad for performance too, because even if you have enough memory, it blows everything out of the L2/L3 caches, and if you don't have enough memory it obviously causes other problems. So it would probably be a great idea to make the filtering code able to do things in smaller chunks, but I suspect that the patch to chunk up xread/xwrite is the right thing to do anyway. Linus -- 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: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
- Original Message - From: Philip Oakley philipoak...@iee.org From: Koch, Rick (Subcontractor) rick.k...@tbe.com Sent: Monday, August 19, 2013 6:09 PM I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? v/r Roderick (Rick) Koch Information Assurance rick.k...@tbe.com What OS version / CPPCheck version was this checked on? In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. Philip description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2803 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2802 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2805 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c syslog.c 46 This looks like a possible, based on http://bytes.com/topic/c/answers/215084-can-realloc-potentially-cause-memory-leak (Mac's reply, with tweaks) Misuse of realloc CAN cause a memory leak, but only when allocation fails if realloc fails, the memory previously pointed to by 'str = realloc(str, ++str_len + 1)' will still be claimed, but you will have lost your only pointer to it, because realloc returns NULL on failure. This is a memory leak. We (those using the compat function) then only provide a warning, so it could repeat endlessly. Eric (cc'd) may be able to clarify if this is a possibility. uninitvar(CppCheck) \git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c 419 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156 uninitvar(CppCheck) \git-master\merge-recursive.c merge-recursive.c 1887 uninitvar(CppCheck) \git-master\notes.c notes.c 805 uninitvar(CppCheck) \git-master\notes.c notes.c 805 deallocret(CppCheck) \git-master\pretty.c pretty.c 677 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125 doubleFree(CppCheck) \git-master\shell.c shell.c 130 -- Philip -- 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: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
This one seems real, although it's quite theoretical. It should only happen in cases where the log-message contains %1, the initial malloc passed and reallocing two more bytes failed. However, what's much more of a disaster: pos is used after the call to realloc might have moved the memory! I guess something like this should fix both issues. Sorry about the lack of indentation, it seems Gmail has regressed, and the old compose mode is somehow gone... (also sorry for triple-posting to some of you, Gmail seems particularly broken today) diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c index d015e43..0641f4e 100644 --- a/compat/win32/syslog.c +++ b/compat/win32/syslog.c @@ -43,11 +43,14 @@ void syslog(int priority, const char *fmt, ...) va_end(ap); while ((pos = strstr(str, %1)) != NULL) { - str = realloc(str, ++str_len + 1); - if (!str) { + char *tmp = realloc(str, ++str_len + 1); + if (!tmp) { warning(realloc failed: '%s', strerror(errno)); + free(str); return; } + pos = tmp + (pos - str); + str = tmp; memmove(pos + 2, pos + 1, strlen(pos)); pos[1] = ' '; } On Tue, Aug 20, 2013 at 12:55 AM, Philip Oakley philipoak...@iee.org wrote: - Original Message - From: Philip Oakley philipoak...@iee.org From: Koch, Rick (Subcontractor) rick.k...@tbe.com Sent: Monday, August 19, 2013 6:09 PM I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? v/r Roderick (Rick) Koch Information Assurance rick.k...@tbe.com What OS version / CPPCheck version was this checked on? In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. Philip description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2803 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2802 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2805 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c syslog.c 46 This looks like a possible, based on http://bytes.com/topic/c/answers/215084-can-realloc-potentially-cause-memory-leak (Mac's reply, with tweaks) Misuse of realloc CAN cause a memory leak, but only when allocation fails if realloc fails, the memory previously pointed to by 'str = realloc(str, ++str_len + 1)' will still be claimed, but you will have lost your only pointer to it, because realloc returns NULL on failure. This is a memory leak. We (those using the compat function) then only provide a warning, so it could repeat endlessly. Eric (cc'd) may be able to clarify if this is a possibility. uninitvar(CppCheck) \git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c 419 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156 uninitvar(CppCheck) \git-master\merge-recursive.c merge-recursive.c 1887 uninitvar(CppCheck) \git-master\notes.c notes.c 805 uninitvar(CppCheck) \git-master\notes.c notes.c 805 deallocret(CppCheck) \git-master\pretty.c pretty.c 677 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125 doubleFree(CppCheck) \git-master\shell.c shell.c 130 -- Philip -- 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 PATCHv4] repack: rewrite the shell script in C.
Hi, so today I compared the argument lists of the repack shell script with the C rewrite passed on to the pack-objects command and fixed some corner cases (-A -d --unpack-unreachable=%s should only pass --unpack-unreachable=%s once to pack-objects) Also I fixed some missing smaller options (--quiet, --no-reuse-delta, --no-reuse-object). I have run the test suite several times successfully now, trying to find a pattern for the non-deterministically bug, which appears to only occur in 1 out of 4 test suite runs. The test suite has around 40 calls to repack and 35 calls to gc, which calls repack internally. That alone covers quite a lot of the repack options, but the debugging for the test cases is no fun, as the the calls to repack are usually not the main concern of the respective tests. There are however t7700-repack.sh t7701-repack-unpack-unreachable.sh It was suggested earlier, and I think it's a good idea to enhance those tests. Anyway, here is an updated version of the repack rewrite. Stefan --8-- From f6da16ac3ca71aa746fe6d9224b06e6cc4e7a104 Mon Sep 17 00:00:00 2001 From: Stefan Beller stefanbel...@googlemail.com Date: Fri, 16 Aug 2013 02:08:47 +0200 Subject: [RFC PATCHv4] repack: rewrite the shell script in C. This is the beginning of the rewrite of the repacking. All tests have been positive at least once now. However there is still a non-deterministic error occuring in about 1 out of 4 test suite runs (usually in 7701 or 9301, but could also occur in 5501 or 3306 iirc) Signed-off-by: Stefan Beller stefanbel...@googlemail.com --- Makefile| 2 +- builtin.h | 1 + builtin/repack.c| 363 git-repack.sh = contrib/examples/git-repack.sh | 0 git.c | 1 + 5 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 builtin/repack.c rename git-repack.sh = contrib/examples/git-repack.sh (100%) diff --git a/Makefile b/Makefile index ef442eb..4ec5bbe 100644 --- a/Makefile +++ b/Makefile @@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh SCRIPT_SH += git-quiltimport.sh SCRIPT_SH += git-rebase.sh SCRIPT_SH += git-remote-testgit.sh -SCRIPT_SH += git-repack.sh SCRIPT_SH += git-request-pull.sh SCRIPT_SH += git-stash.sh SCRIPT_SH += git-submodule.sh @@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o BUILTIN_OBJS += builtin/remote.o BUILTIN_OBJS += builtin/remote-ext.o BUILTIN_OBJS += builtin/remote-fd.o +BUILTIN_OBJS += builtin/repack.o BUILTIN_OBJS += builtin/replace.o BUILTIN_OBJS += builtin/rerere.o BUILTIN_OBJS += builtin/reset.o diff --git a/builtin.h b/builtin.h index 8afa2de..b56cf07 100644 --- a/builtin.h +++ b/builtin.h @@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const char *prefix); extern int cmd_remote(int argc, const char **argv, const char *prefix); extern int cmd_remote_ext(int argc, const char **argv, const char *prefix); extern int cmd_remote_fd(int argc, const char **argv, const char *prefix); +extern int cmd_repack(int argc, const char **argv, const char *prefix); extern int cmd_repo_config(int argc, const char **argv, const char *prefix); extern int cmd_rerere(int argc, const char **argv, const char *prefix); extern int cmd_reset(int argc, const char **argv, const char *prefix); diff --git a/builtin/repack.c b/builtin/repack.c new file mode 100644 index 000..a87900e --- /dev/null +++ b/builtin/repack.c @@ -0,0 +1,363 @@ +/* + * The shell version was written by Linus Torvalds (2005) and many others. + * This is a translation into C by Stefan Beller (2013) + */ + +#include builtin.h +#include cache.h +#include dir.h +#include parse-options.h +#include run-command.h +#include sigchain.h +#include strbuf.h +#include string-list.h +#include argv-array.h + +static int delta_base_offset = 0; + +static const char *const git_repack_usage[] = { + N_(git repack [options]), + NULL +}; + +static int repack_config(const char *var, const char *value, void *cb) +{ + if (!strcmp(var, repack.usedeltabaseoffset)) { + delta_base_offset = git_config_bool(var, value); + return 0; + } + return git_default_config(var, value, cb); +} + +static void remove_temporary_files() { + DIR *dir; + struct dirent *e; + char *prefix, *path; + + prefix = mkpathdup(.tmp-%d-pack, getpid()); + path = mkpathdup(%s/pack, get_object_directory()); + + dir = opendir(path); + while ((e = readdir(dir)) != NULL) { + if (!prefixcmp(e-d_name, prefix)) { + struct strbuf fname = STRBUF_INIT; + strbuf_addf(fname, %s/%s, path, e-d_name); + unlink(strbuf_detach(fname, NULL)); + } + } + free(prefix); + free(path); + closedir(dir); +} + +static void remove_pack_on_signal(int
Re: [PATCH] Git segmentation faults if submodule path is empty.
It looks great to me. Thanks, -- Jharrod LaFon OpenEye Scientific Software On Aug 19, 2013, at 2:54 PM, Junio C Hamano gits...@pobox.com wrote: Jharrod LaFon jla...@eyesopen.com writes: I will keep trying this until it's perfect, and I thank you for the help. When I resubmit this, would you like me to include your sign-off line as well? If the one I attached at the end of the message you are responding to looks fine to you, I'd just apply it without having you to reroll. Also, the end of the test script was not included in your message. Sorry, but I am not sure what you meant by this. I reworked your example to make it the second test in an existing test script. There are many existing tests after that so it is natural that we wouldn't see the end of the file in the patch context. diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 5ee97b0..a39d074 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -18,6 +18,16 @@ test_expect_success 'setup - initial commit' ' git branch initial ' +test_expect_success 'configuration parsing' ' + test_when_finished rm -f .gitmodules + cat .gitmodules -\EOF + [submodule s] + path + ignore + EOF + test_must_fail git status +' + test_expect_success 'setup - repository in init subdirectory' ' mkdir init ( -- 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] submodule: prevent warning in summary output
When git submodule summary is run and there is a deleted submodule, there is an warning from git rev-parse: fatal: Not a git repository: '.vim/pathogen/.git' Silence this warning, since it is fully expected that a deleted submodule will not be a git repository. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- I hesitated to add the test for $status because it will end up having no effect since we exclude that case later. However, for correctness, I included it. git-submodule.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-submodule.sh b/git-submodule.sh index 2979197..eec3135 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1070,7 +1070,10 @@ cmd_summary() { missing_src= missing_dst= + test $status = D missing_src=t + test $mod_src = 16 + test -e $name/.git ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null missing_src=t -- 1.8.4.rc3 -- 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: Does Git now have any C struct version history tracking mechanism?
On Mon, Aug 19, 2013 at 4:37 PM, Thomas Rast tr...@inf.ethz.ch wrote: Hmm, IIUC that's actually not a bug or even a roughness; it's an artifact of how the :pattern:file syntax is defined. It takes the first _funcname line_ matching 'pattern', up to (but excluding) the next funcname line. ... Or is there an issue that I'm missing? No you're correct. I should have used the following instead for the demo: $ git log -L /struct\ rev_cmdline_info/,/^}/:revision.h Moral: You'd hit your target better if you work on your aim. nazri -- 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] Document smart http
This may provide some clues for those who want to modify smart http code as smart http is pretty much undocumented. Smart http document so far is a few commit messages and the source code. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-fetch-pack.txt | 11 +++-- Documentation/git-receive-pack.txt| 12 +- Documentation/git-send-pack.txt | 9 +++- Documentation/git-upload-pack.txt | 9 +++- Documentation/technical/pack-protocol.txt | 69 +++ 5 files changed, 103 insertions(+), 7 deletions(-) diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt index 1e71754..85a9437 100644 --- a/Documentation/git-fetch-pack.txt +++ b/Documentation/git-fetch-pack.txt @@ -9,10 +9,7 @@ git-fetch-pack - Receive missing objects from another repository SYNOPSIS [verse] -'git fetch-pack' [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag] - [--upload-pack=git-upload-pack] - [--depth=n] [--no-progress] - [-v] [host:]directory [refs...] +'git fetch-pack' [options] [host:]directory [refs...] DESCRIPTION --- @@ -90,6 +87,12 @@ be in a separate packet, and the list must end with a flush packet. --no-progress:: Do not show the progress. +--stateless-rpc:: + Smart HTTP mode. + +--lock-pack:: + Issue lock command to the remote helper via stdout. + -v:: Run verbosely. diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt index b1f7dc6..0b2029c 100644 --- a/Documentation/git-receive-pack.txt +++ b/Documentation/git-receive-pack.txt @@ -9,7 +9,7 @@ git-receive-pack - Receive what is pushed into the repository SYNOPSIS [verse] -'git-receive-pack' directory +'git-receive-pack' [options] directory DESCRIPTION --- @@ -35,6 +35,16 @@ are not fast-forwards. OPTIONS --- +--stateless-rpc:: + Smart HTTP mode. + +--advertise-refs:: + Only the initial ref advertisement is output then exits + immediately. + +--quiet:: + Make unpack-objects at the receive-pack end quiet. + directory:: The repository to sync into. diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt index dc3a568..6cee3d4 100644 --- a/Documentation/git-send-pack.txt +++ b/Documentation/git-send-pack.txt @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository SYNOPSIS [verse] -'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory [ref...] +'git send-pack' [options] [host:]directory [ref...] DESCRIPTION --- @@ -52,6 +52,13 @@ OPTIONS Send a thin pack, which records objects in deltified form based on objects not included in the pack to reduce network traffic. +--stateless-rpc:: + Smart HTTP mode. + +--helper-status: + Issue status commands (e.g. ok or error) to the remote + helper via stdout. + host:: A remote host to house the repository. When this part is specified, 'git-receive-pack' is invoked via diff --git a/Documentation/git-upload-pack.txt b/Documentation/git-upload-pack.txt index 0abc806..ce9a455 100644 --- a/Documentation/git-upload-pack.txt +++ b/Documentation/git-upload-pack.txt @@ -9,7 +9,7 @@ git-upload-pack - Send objects packed back to git-fetch-pack SYNOPSIS [verse] -'git-upload-pack' [--strict] [--timeout=n] directory +'git-upload-pack' [options] directory DESCRIPTION --- @@ -31,6 +31,13 @@ OPTIONS --timeout=n:: Interrupt transfer after n seconds of inactivity. +--stateless-rpc:: + Smart HTTP mode. + +--advertise-refs:: + Only the initial ref advertisement is output then exits + immediately. + directory:: The repository to sync from. diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index b898e97..7590394 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -546,3 +546,72 @@ An example client/server communication might look like this: S: 0018ok refs/heads/debug\n S: 002ang refs/heads/master non-fast-forward\n + +Smart HTTP Transport + + +Smart HTTP protocol is basically git protocol on top of http. The +base protocol is modified slightly to fit HTTP processing model: no +bidirectional full-duplex connections, the program may read the +request, write a response and must exit. Any negotiation is broken +down into many separate GET or POST requests. The server is +stateless. The client must send enough information so that the server +can recreate the previous state. + +Reference Discovery +--- + +The server end always sends the list of references in both push and +fetch cases. This ref list is retrieved by the client's sending HTTP +GET request to a smart http