Re: [PATCH 2/2] describe: Add documentation for --abbrev=+
On Sat, Aug 23, 2014 at 02:13:22PM -0300, Jonh Wendell wrote: --- Documentation/git-describe.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt index d20ca40..e291770 100644 --- a/Documentation/git-describe.txt +++ b/Documentation/git-describe.txt @@ -54,6 +54,12 @@ OPTIONS abbreviated object name, use n digits, or as many digits as needed to form a unique object name. An n of 0 will suppress long format, only showing the closest tag. + + + + Did you intend to have two lines with just plus signs here? I'm not aware of anywhere else in the Documentation where we do that. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] Undefine strlcpy if needed.
On 24/08/14 05:35, Benoit Sigoure wrote: On OS X, strlcpy is already #define'd, which causes warnings in all the files that include `git-compat-util.h'. Note that this only occurs when building without running ./configure. Signed-off-by: Benoit Sigoure tsuna...@gmail.com --- Resending with the SOB line I forgot. git-compat-util.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index f587749..8c001e2 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -495,6 +495,9 @@ extern char *gitstrcasestr(const char *haystack, const char *needle); #endif #ifdef NO_STRLCPY +#ifdef strlcpy +#undef strlcpy +#endif If strlcpy is #defined, then presumably NO_STRLCPY should not be set, no? #define strlcpy gitstrlcpy extern size_t gitstrlcpy(char *, const char *, size_t); #endif Hmm, which version of OS X are we talking about? config.mak.uname contains this: ifeq ($(shell expr $(uname_R) : '[15]\.'),2) NO_STRLCPY = YesPlease What does ./configure put in config.mak.autogen for NO_STRLCPY? (sorry, I don't have access to any version of OS X, so I can't offer much help on this). ATB, Ramsay Jones -- 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] Undefine strlcpy if needed.
On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Hmm, which version of OS X are we talking about? OS X 10.9.4: $ uname -a Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun 3 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64 config.mak.uname contains this: ifeq ($(shell expr $(uname_R) : '[15]\.'),2) NO_STRLCPY = YesPlease What does ./configure put in config.mak.autogen for NO_STRLCPY? NO_STRLCPY= I guess I saw all the warnings because I did just a “git pull —rebase make -j8” without running “make configure ./configure”. -- Benoit tsuna Sigoure -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] describe: Add documentation for --abbrev=+
2014-08-24 3:35 GMT-03:00 brian m. carlson sand...@crustytoothpaste.net: On Sat, Aug 23, 2014 at 02:13:22PM -0300, Jonh Wendell wrote: --- Documentation/git-describe.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt index d20ca40..e291770 100644 --- a/Documentation/git-describe.txt +++ b/Documentation/git-describe.txt @@ -54,6 +54,12 @@ OPTIONS abbreviated object name, use n digits, or as many digits as needed to form a unique object name. An n of 0 will suppress long format, only showing the closest tag. + + + + Did you intend to have two lines with just plus signs here? I'm not aware of anywhere else in the Documentation where we do that. -- In my tests just one line was not enough to produce a proper line break in the html output. With two lines like above the output in man and html are ok (just 1 line break). -- Jonh Wendell http://www.bani.com.br -- 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: Re: Relative submodule URLs
Hi, since the mail got quite long. To avoid 'tl;dr', I talk about two topics in this mail: * Submodule settings for default remote (complex, future) * New --with--remote parameter for 'git submodule' (simple, now) Depending on your interest you might want to skip the first part of the email. I think they are two separate topics. Please only answer to either one and remove the other. That way we split the thread here and not mix the two together anymore. On Fri, Aug 22, 2014 at 12:00:07PM -0400, Marc Branchaud wrote: I think you're on the right path. However I'd suggest something like the following: [submodule] remote = remote_for_relative_submodules (e.g. `upstream`) I think remote.default would be more generally useful, especially when working with detached checkouts. Depends what workflow you have. Especially for submodules where the default remote might change from branch to branch this is not necessarily true. The following drawbacks in relation to submodules come to my mind: * You can not transport such configuration to the server. In case you are developing on a branch which has changes in a forked submodule that would be useful. * When your development in superproject and submodule gets merged to a stable branch (i.e. master) you also may not want that other remote anymore. So a setting, that can be per branch, might be preferred. * When your development gets pushed to a different remote the settings do not change. I.e. once part of the upstream repository the settings should possibly disappear. * You might only want to fork a certain submodule (since thats the only one you need to make changes in) in your branch. Then you need this setting to be per submodule. So to sum up a default remote setting which would be generally useful for submodules needs the following properties (IMO): * pushable * per branch * per remote * per submodule All of these being optional, so in case you have a local mirror, including submodules, of some project in which you develop with your team you might just want to set the default remote once for all submodules. I have not completely thought that through but the special ref idea[3] described by Jonathan seems to make it possible to implement all these properties. [branch.name] submoduleRemote = remote_for_relative_submodule If I understand correctly, you want this so that your branch can be a fork of only the super-repo while the submodules are not forked and so they should keep tracking their original repo. To me this seems to be going in the opposite direction of having branches recursively apply to submodules, which I think most of us want. I disagree. While recursive branches might make sense in some situations in most it does not. Consider a project in which you use a library which is separately maintained. You develop on featureA in your project and discover a bug in the submodule which you fix on a branch (which is then tracked in the submodule). Here it does not make sense to call your branch in the submodule featureA, since the submodule has no knowledge at all (and should not) about this featureA. While having said that, for a simple workflow while developing a certain feature recursive branches make sense. Lets say as a temporary local branch you could have that featureA branch in your submodule and just commit any changes you need in the submodule on that branch (including extensions and stuff). Later in the process you divide up that branch in the submodule into cleanups, bugfixes, extensions, ... to push it upstream for review and integration. A branch should fork the entire repo, including its submodules. The implication is that if you want to push that branch somewhere, that somewhere needs to be able to accept the forks of the submodules *even if those submodules aren't changed in your branch* because at the very least the branch ref has to exist in the submodules' repositories. I disagree here as well. As the distributed nature of git allows to have different remotes, I think its perfectly legitimate to just fork the repositories you need to change. It should be easy to work on a repository that is forked in its entirety, but it should also be possible (and properly supported) to only fork some submodules. I know it does make the situation more complex, but I think we should properly define the goal beforehand, so we do not exclude any use-cases. Then we can go ahead and just implement the simpler stuff (like entire repo forks) first, while making sure we do not block the more complex use-cases. With absolute-path submodules, the push is a simple as creating the branch ref in the submodules' home repositories -- even if the main somewhere you're pushing to isn't one of those repositories. With relative-path submodules, the push's target repo *must* also have the submodules in their proper places, so that they can get updated.
Re: [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR
Hi, On Aug 23, 2014 12:26 PM, Jeff King p...@peff.net wrote: Since dd0b72c (bash prompt: use bash builtins to check stash state, 2011-04-01), git-prompt checks whether we have a stash by looking for $GIT_DIR/refs/stash. Generally external programs should never do this, because they would miss packed-refs. Not sure whether the prompt script is external program or not, but doesn't matter, this is the right thing to do. That commit claims that packed-refs does not pack refs/stash, but that is not quite true. It does pack the ref, but due to a bug, fails to prune the ref. When we fix that bug, we would want to be doing the right thing here. Signed-off-by: Jeff King p...@peff.net --- I know we are pretty sensitive to forks in the prompt code (after all, that was the point of dd0b72c). This patch is essentially a reversion of this hunk of dd0b72c, and is definitely safe. I'm not sure, but if I remember correctly (don't have the means to check it at the moment, sorry) in that commit I also added a 'git pack-ref' invocation to the relevant test(s?) to guard us against breakages due to changes in 'git pack-refs'. If that is so, then I think those invocations should be removed as well, as they'll become useless. Best, GáborN�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf
[PATCH] bisect: save heap memory. allocate only the required amount
Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c @@ -215,10 +215,13 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n } qsort(array, cnt, sizeof(*array), compare_commit_dist); for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100]; + sprintf(name, dist=%d, array[i].distance); + int name_len = strlen(name); + struct name_decoration *r = xmalloc(sizeof(*r) + name_len); struct object *obj = (array[i].commit-object); - sprintf(r-name, dist=%d, array[i].distance); + memcpy(r-name, name, name_len + 1); r-next = add_decoration(name_decoration, obj, r); p-item = array[i].commit; p = p-next; -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bisect: save heap memory. allocate only the required amount
On 24.08.2014 16:17, Arjun Sreedharan wrote: Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c @@ -215,10 +215,13 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n } qsort(array, cnt, sizeof(*array), compare_commit_dist); for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100]; Would it make sense to convert the 'name' into a git strbuf? Please have a look at Documentation/technical/api-strbuf.txt + sprintf(name, dist=%d, array[i].distance); + int name_len = strlen(name); + struct name_decoration *r = xmalloc(sizeof(*r) + name_len); struct object *obj = (array[i].commit-object); - sprintf(r-name, dist=%d, array[i].distance); + memcpy(r-name, name, name_len + 1); r-next = add_decoration(name_decoration, obj, r); p-item = array[i].commit; p = p-next; -- 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 ASAP, IT'S URGENT.
Attention: I have a business Proposal that will be of benefit to the both of us and I shall be compensating you with Forty percent at the final conclusion. If you are interested please reply ASAP, so I can send you more detail on how we are going to proceed I await your urgent response, Regards, Mr. Larry Markson Please reply to my alternative E-mail Address below: larrymarks...@asia.com -- 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] bisect: save heap memory. allocate only the required amount
On 24/08/14 15:17, Arjun Sreedharan wrote: Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c @@ -215,10 +215,13 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n } qsort(array, cnt, sizeof(*array), compare_commit_dist); for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100]; + sprintf(name, dist=%d, array[i].distance); + int name_len = strlen(name); + struct name_decoration *r = xmalloc(sizeof(*r) + name_len); struct object *obj = (array[i].commit-object); declaration(s) after statement. - sprintf(r-name, dist=%d, array[i].distance); + memcpy(r-name, name, name_len + 1); r-next = add_decoration(name_decoration, obj, r); p-item = array[i].commit; p = p-next; HTH ATB, Ramsay Jones -- 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 2/3] Introduce GIT_MMAP_LIMIT to allow testing expected mmap size
On Aug 22, 2014, at 6:31 PM, Junio C Hamano gits...@pobox.com wrote: Steffen Prohaska proha...@zib.de writes: + if (limit == -1) { + const char *env = getenv(GIT_MMAP_LIMIT); + limit = env ? atoi(env) * 1024 : 0; ... this should then be changed to atol(env), and ... In the real codepath (not debugging aid like this) we try to avoid atoi/atol so that we can catch errors like feeding 123Q and parsing it as 123. But it is OK to be loose in an debugging aid. If I were doing this code, I actually would call git_parse_ulong() and not define it in terms of kilobytes, though. Thanks for suggesting this. I wasn't aware of git_parse_ulong(). I think it makes very much sense to use it, even though it's only a testing aid. I'll send a PATCH v5 series. 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
[PATCH v5 2/4] Change GIT_ALLOC_LIMIT check to use git_parse_ulong()
GIT_ALLOC_LIMIT limits xmalloc()'s size, which is of type size_t. Better use git_parse_ulong() to parse the environment variable, so that the postfixes 'k', 'm', and 'g' can be used; and use size_t to store the limit for consistency. The change to size_t has no direct practical impact, because we use GIT_ALLOC_LIMIT to test small sizes. The cast of size in the call to die() is changed to uintmax_t to match the format string PRIuMAX. Signed-off-by: Steffen Prohaska proha...@zib.de --- t/t1050-large.sh | 2 +- wrapper.c| 16 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/t/t1050-large.sh b/t/t1050-large.sh index aea4936..e7657ab 100755 --- a/t/t1050-large.sh +++ b/t/t1050-large.sh @@ -13,7 +13,7 @@ test_expect_success setup ' echo X | dd of=large2 bs=1k seek=2000 echo X | dd of=large3 bs=1k seek=2000 echo Y | dd of=huge bs=1k seek=2500 - GIT_ALLOC_LIMIT=1500 + GIT_ALLOC_LIMIT=1500k export GIT_ALLOC_LIMIT ' diff --git a/wrapper.c b/wrapper.c index bc1bfb8..69d1c9b 100644 --- a/wrapper.c +++ b/wrapper.c @@ -11,14 +11,18 @@ static void (*try_to_free_routine)(size_t size) = do_nothing; static void memory_limit_check(size_t size) { - static int limit = -1; - if (limit == -1) { - const char *env = getenv(GIT_ALLOC_LIMIT); - limit = env ? atoi(env) * 1024 : 0; + static size_t limit = SIZE_MAX; + if (limit == SIZE_MAX) { + const char *var = GIT_ALLOC_LIMIT; + unsigned long val = 0; + const char *env = getenv(var); + if (env !git_parse_ulong(env, val)) + die(Failed to parse %s, var); + limit = val; } if (limit size limit) - die(attempting to allocate %PRIuMAX over limit %d, - (intmax_t)size, limit); + die(attempting to allocate %PRIuMAX over limit %PRIuMAX, + (uintmax_t)size, (uintmax_t)limit); } try_to_free_t set_try_to_free_routine(try_to_free_t routine) -- 2.1.0.8.gd3b6067 -- 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 v5 1/4] convert: Refactor would_convert_to_git() to single arg 'path'
It is only the path that matters in the decision whether to filter or not. Clarify this by making path the single argument of would_convert_to_git(). Signed-off-by: Steffen Prohaska proha...@zib.de --- convert.h | 5 ++--- sha1_file.c | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/convert.h b/convert.h index 0c2143c..c638b33 100644 --- a/convert.h +++ b/convert.h @@ -40,10 +40,9 @@ extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst); extern int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst); -static inline int would_convert_to_git(const char *path, const char *src, - size_t len, enum safe_crlf checksafe) +static inline int would_convert_to_git(const char *path) { - return convert_to_git(path, src, len, NULL, checksafe); + return convert_to_git(path, NULL, 0, NULL, 0); } /* diff --git a/sha1_file.c b/sha1_file.c index 3f70b1d..00c07f2 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -3144,7 +3144,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, if (!S_ISREG(st-st_mode)) ret = index_pipe(sha1, fd, type, path, flags); else if (size = big_file_threshold || type != OBJ_BLOB || -(path would_convert_to_git(path, NULL, 0, 0))) +(path would_convert_to_git(path))) ret = index_core(sha1, fd, size, type, path, flags); else ret = index_stream(sha1, fd, size, type, path, flags); -- 2.1.0.8.gd3b6067 -- 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 v5 4/4] convert: Stream from fd to required clean filter instead of mmap
The data is streamed to the filter process anyway. Better avoid mapping the file if possible. This is especially useful if a clean filter reduces the size, for example if it computes a sha1 for binary data, like git media. The file size that the previous implementation could handle was limited by the available address space; large files for example could not be handled with (32-bit) msysgit. The new implementation can filter files of any size as long as the filter output is small enough. The new code path is only taken if the filter is required. The filter consumes data directly from the fd. The original data is not available to git, so it must fail if the filter fails. The environment variable GIT_MMAP_LIMIT, which has been introduced in the previous commit is used to test that the expected code path is taken. A related test that exercises required filters is modified to verify that the data actually has been modified on its way from the file system to the object store. Signed-off-by: Steffen Prohaska proha...@zib.de --- convert.c | 60 +-- convert.h | 5 + sha1_file.c | 27 ++- t/t0021-conversion.sh | 24 - 4 files changed, 104 insertions(+), 12 deletions(-) diff --git a/convert.c b/convert.c index cb5fbb4..463f6de 100644 --- a/convert.c +++ b/convert.c @@ -312,11 +312,12 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len, struct filter_params { const char *src; unsigned long size; + int fd; const char *cmd; const char *path; }; -static int filter_buffer(int in, int out, void *data) +static int filter_buffer_or_fd(int in, int out, void *data) { /* * Spawn cmd and feed the buffer contents through its stdin. @@ -325,6 +326,7 @@ static int filter_buffer(int in, int out, void *data) struct filter_params *params = (struct filter_params *)data; int write_err, status; const char *argv[] = { NULL, NULL }; + int fd; /* apply % substitution to cmd */ struct strbuf cmd = STRBUF_INIT; @@ -355,7 +357,17 @@ static int filter_buffer(int in, int out, void *data) sigchain_push(SIGPIPE, SIG_IGN); - write_err = (write_in_full(child_process.in, params-src, params-size) 0); + if (params-src) { + write_err = (write_in_full(child_process.in, params-src, params-size) 0); + } else { + /* dup(), because copy_fd() closes the input fd. */ + fd = dup(params-fd); + if (fd 0) + write_err = error(failed to dup file descriptor.); + else + write_err = copy_fd(fd, child_process.in); + } + if (close(child_process.in)) write_err = 1; if (write_err) @@ -371,7 +383,7 @@ static int filter_buffer(int in, int out, void *data) return (write_err || status); } -static int apply_filter(const char *path, const char *src, size_t len, +static int apply_filter(const char *path, const char *src, size_t len, int fd, struct strbuf *dst, const char *cmd) { /* @@ -392,11 +404,12 @@ static int apply_filter(const char *path, const char *src, size_t len, return 1; memset(async, 0, sizeof(async)); - async.proc = filter_buffer; + async.proc = filter_buffer_or_fd; async.data = params; async.out = -1; params.src = src; params.size = len; + params.fd = fd; params.cmd = cmd; params.path = path; @@ -747,6 +760,24 @@ static void convert_attrs(struct conv_attrs *ca, const char *path) } } +int would_convert_to_git_filter_fd(const char *path) +{ + struct conv_attrs ca; + + convert_attrs(ca, path); + if (!ca.drv) + return 0; + + /* Apply a filter to an fd only if the filter is required to succeed. +* We must die if the filter fails, because the original data before +* filtering is not available. +*/ + if (!ca.drv-required) + return 0; + + return apply_filter(path, NULL, 0, -1, NULL, ca.drv-clean); +} + int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst, enum safe_crlf checksafe) { @@ -761,7 +792,7 @@ int convert_to_git(const char *path, const char *src, size_t len, required = ca.drv-required; } - ret |= apply_filter(path, src, len, dst, filter); + ret |= apply_filter(path, src, len, -1, dst, filter); if (!ret required) die(%s: clean filter '%s' failed, path, ca.drv-name); @@ -778,6 +809,23 @@ int convert_to_git(const char *path, const char *src, size_t len, return ret | ident_to_git(path, src, len, dst, ca.ident); } +void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst, +
[PATCH v5 0/4] Stream fd to clean filter; GIT_MMAP_LIMIT, GIT_ALLOC_LIMIT with git_parse_ulong()
Changes since v4: use git_parse_ulong() to parse env vars. Steffen Prohaska (4): convert: Refactor would_convert_to_git() to single arg 'path' Change GIT_ALLOC_LIMIT check to use git_parse_ulong() Introduce GIT_MMAP_LIMIT to allow testing expected mmap size convert: Stream from fd to required clean filter instead of mmap convert.c | 60 +-- convert.h | 10 ++--- sha1_file.c | 50 +++--- t/t0021-conversion.sh | 24 - t/t1050-large.sh | 2 +- wrapper.c | 16 -- 6 files changed, 138 insertions(+), 24 deletions(-) -- 2.1.0.8.gd3b6067 -- 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 v5 3/4] Introduce GIT_MMAP_LIMIT to allow testing expected mmap size
Similar to testing expectations about malloc with GIT_ALLOC_LIMIT (see commit d41489 and previous commit), it can be useful to test expectations about mmap. This introduces a new environment variable GIT_MMAP_LIMIT to limit the largest allowed mmap length. xmmap() is modified to check the limit. Together with GIT_ALLOC_LIMIT tests can now easily confirm expectations about memory consumption. GIT_MMAP_LIMIT will be used in the next commit to test that data will be streamed to an external filter without mmaping the entire file. [commit d41489]: d41489a6424308dc9a0409bc2f6845aa08bd4f7d Add more large blob test cases Signed-off-by: Steffen Prohaska proha...@zib.de --- sha1_file.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index 00c07f2..3204f66 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -663,10 +663,29 @@ void release_pack_memory(size_t need) ; /* nothing */ } +static void mmap_limit_check(size_t length) +{ + static size_t limit = SIZE_MAX; + if (limit == SIZE_MAX) { + const char *var = GIT_MMAP_LIMIT; + unsigned long val = 0; + const char *env = getenv(var); + if (env !git_parse_ulong(env, val)) + die(Failed to parse %s, var); + limit = val; + } + if (limit length limit) + die(attempting to mmap %PRIuMAX over limit %PRIuMAX, + (uintmax_t)length, (uintmax_t)limit); +} + void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset) { - void *ret = mmap(start, length, prot, flags, fd, offset); + void *ret; + + mmap_limit_check(length); + ret = mmap(start, length, prot, flags, fd, offset); if (ret == MAP_FAILED) { if (!length) return NULL; -- 2.1.0.8.gd3b6067 -- 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] Undefine strlcpy if needed.
On 24/08/14 12:13, tsuna wrote: On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Hmm, which version of OS X are we talking about? OS X 10.9.4: $ uname -a Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun 3 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64 Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!) config.mak.uname contains this: ifeq ($(shell expr $(uname_R) : '[15]\.'),2) NO_STRLCPY = YesPlease What does ./configure put in config.mak.autogen for NO_STRLCPY? NO_STRLCPY= OK, so I've got to my limit here! ;-) The conditional shown above (from config.mak.uname) should not have set NO_STRLCPY (assuming that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY is being set somewhere else (command-line, environment), this should just work. puzzled. :( I guess I saw all the warnings because I did just a “git pull —rebase make -j8” without running “make configure ./configure”. Yes, but use of configure is supposed to be optional ... Hopefully, someone who actually knows OS X can solve the mystery. ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Loop index increases monotonically when reading unmerged entries
Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com --- read-cache.c | 1 - 1 file changed, 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index c1a9619..3d70386 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1971,7 +1971,6 @@ int read_index_unmerged(struct index_state *istate) if (add_index_entry(istate, new_ce, 0)) return error(%s: cannot drop to stage #0, new_ce-name); - i = index_name_pos(istate, new_ce-name, len); } return unmerged; } -- 2.0.4.1.g0b8a4f9.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Check order when reading index
Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com --- read-cache.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/read-cache.c b/read-cache.c index 7f5645e..c1a9619 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1438,6 +1438,21 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk, return ce; } +void check_ce_order(struct cache_entry *ce, struct cache_entry *next_ce) +{ + int name_compare = strcmp(ce-name, next_ce-name); + if (0 name_compare) + die(Unordered stage entries in index); + if (!name_compare) { + if (!ce_stage(ce)) + die(Multiple stage entries for merged file '%s', + ce-name); + if (ce_stage(ce) = ce_stage(next_ce)) + die(Unordered stage entries for '%s', + ce-name); + } +} + /* remember to discard_cache() before reading a different cache! */ int read_index_from(struct index_state *istate, const char *path) { @@ -1499,6 +1514,9 @@ int read_index_from(struct index_state *istate, const char *path) ce = create_from_disk(disk_ce, consumed, previous_name); set_index_entry(istate, i, ce); + if (i 0) + check_ce_order(istate-cache[i - 1], ce); + src_offset += consumed; } strbuf_release(previous_name_buf); -- 2.0.4.1.g0b8a4f9.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Loop index increases monotonically when reading unmerged entries
I think this line is dangerous, if add_cache_entry is not able to remove higher-stages it will be looping forever, as happens in the case of this thread. I cannot see why it's even needed, and removing it doesn't break any test. On Sun, Aug 24, 2014 at 7:57 PM, Jaime Soriano Pastor jsorianopas...@gmail.com wrote: Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com --- read-cache.c | 1 - 1 file changed, 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index c1a9619..3d70386 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1971,7 +1971,6 @@ int read_index_unmerged(struct index_state *istate) if (add_index_entry(istate, new_ce, 0)) return error(%s: cannot drop to stage #0, new_ce-name); - i = index_name_pos(istate, new_ce-name, len); } return unmerged; } -- 2.0.4.1.g0b8a4f9.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Undefine strlcpy if needed.
On 2014-08-24 18.18, Ramsay Jones wrote: On 24/08/14 12:13, tsuna wrote: On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Hmm, which version of OS X are we talking about? OS X 10.9.4: $ uname -a Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun 3 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64 Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!) config.mak.uname contains this: ifeq ($(shell expr $(uname_R) : '[15]\.'),2) NO_STRLCPY = YesPlease What does ./configure put in config.mak.autogen for NO_STRLCPY? NO_STRLCPY= OK, so I've got to my limit here! ;-) The conditional shown above (from config.mak.uname) should not have set NO_STRLCPY (assuming that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY is being set somewhere else (command-line, environment), this should just work. puzzled. :( I guess I saw all the warnings because I did just a “git pull —rebase make -j8” without running “make configure ./configure”. Yes, but use of configure is supposed to be optional ... Hopefully, someone who actually knows OS X can solve the mystery. ATB, Ramsay Jones I need to admit that I can not reproduce the warning here, uname -r gives 13.3.0 Could it be that something is special on your machine ? Something in the environment ? Does a fresh clone help ? -- 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] Undefine strlcpy if needed.
On Sun, Aug 24, 2014 at 12:49 PM, Torsten Bögershausen tbo...@web.de wrote: On 2014-08-24 18.18, Ramsay Jones wrote: On 24/08/14 12:13, tsuna wrote: On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Hmm, which version of OS X are we talking about? OS X 10.9.4: $ uname -a Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun 3 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64 Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!) $ uname -r 13.3.0 config.mak.uname contains this: ifeq ($(shell expr $(uname_R) : '[15]\.'),2) NO_STRLCPY = YesPlease What does ./configure put in config.mak.autogen for NO_STRLCPY? NO_STRLCPY= OK, so I've got to my limit here! ;-) The conditional shown above (from config.mak.uname) should not have set NO_STRLCPY (assuming that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY is being set somewhere else (command-line, environment), this should just work. puzzled. :( I guess I saw all the warnings because I did just a “git pull —rebase make -j8” without running “make configure ./configure”. Yes, but use of configure is supposed to be optional ... Hopefully, someone who actually knows OS X can solve the mystery. ATB, Ramsay Jones I need to admit that I can not reproduce the warning here, uname -r gives 13.3.0 Could it be that something is special on your machine ? Something in the environment ? Not that I can think of, the only non-standard” thing I have installed is Homebrew (http://brew.sh/), but otherwise it’s all the standard OS X stuff and Developer tools. I write code on this machine on a daily basis. Does a fresh clone help ? A fresh clone doesn’t even build :-/ $ git clone git://github.com/git/git.git Cloning into 'git'... remote: Counting objects: 176423, done. remote: Compressing objects: 100% (47201/47201), done. remote: Total 176423 (delta 127349), reused 176233 (delta 127209) Receiving objects: 100% (176423/176423), 64.05 MiB | 6.13 MiB/s, done. Resolving deltas: 100% (127349/127349), done. Checking connectivity... done. $ cd git $ make GIT_VERSION = 2.1.0 * new build flags CC credential-store.o In file included from credential-store.c:1: In file included from ./cache.h:8: ./gettext.h:17:11: fatal error: 'libintl.h' file not found # include libintl.h ^ 1 error generated. make: *** [credential-store.o] Error 1 I need to run configure first: $ make configure GEN configure $ ./configure configure: Setting lib to 'lib' (the default) […] $ make tsuna@damogran /tmp/git $ make * new build flags CC credential-store.o * new link flags CC abspath.o […] Then the build succeeds. $ grep NO_STRLCPY config.mak.autogen NO_STRLCPY= $ which cc /usr/bin/cc $ cc --version Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn) Target: x86_64-apple-darwin13.3.0 Thread model: posix -- Benoit tsuna” Sigoure -- 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] bisect: save heap memory. allocate only the required amount
find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..a52631e 100644 --- a/bisect.c +++ b/bisect.c @@ -215,12 +215,16 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n } qsort(array, cnt, sizeof(*array), compare_commit_dist); for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + struct strbuf name = STRBUF_INIT; + struct name_decoration *r; struct object *obj = (array[i].commit-object); - sprintf(r-name, dist=%d, array[i].distance); + strbuf_addf(name, dist=%d, array[i].distance); + r = xmalloc(sizeof(*r) + name.len); + memcpy(r-name, name.buf, name.len + 1); r-next = add_decoration(name_decoration, obj, r); p-item = array[i].commit; + strbuf_release(name); p = p-next; } if (p) -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bisect: save heap memory. allocate only the required amount
On Sun, Aug 24, 2014 at 8:10 AM, Stefan Beller stefanbel...@gmail.com wrote: for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100]; Would it make sense to convert the 'name' into a git strbuf? Please have a look at Documentation/technical/api-strbuf.txt Why not go one step further and format it twice, once only to measure the necessary size to allocate, allocate and then format into it for real? Then you do not need to print into a temporary strbuf, copy the result and free the strbuf, no? BUT. The string will always be dist= followed by decimal representation of a count that fits in int anyway, so I actually think use of strbuf is way overkill (and formatting it twice also is); the patch as posted should be just fine. Or am I missing some uses that require larger/unbounded buffer outside the context of the patch? -- 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] Undefine strlcpy if needed.
On 24/08/14 22:09, tsuna wrote: On Sun, Aug 24, 2014 at 12:49 PM, Torsten Bögershausen tbo...@web.de wrote: On 2014-08-24 18.18, Ramsay Jones wrote: On 24/08/14 12:13, tsuna wrote: On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Hmm, which version of OS X are we talking about? OS X 10.9.4: $ uname -a Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun 3 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64 Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!) $ uname -r 13.3.0 config.mak.uname contains this: ifeq ($(shell expr $(uname_R) : '[15]\.'),2) NO_STRLCPY = YesPlease What does ./configure put in config.mak.autogen for NO_STRLCPY? NO_STRLCPY= OK, so I've got to my limit here! ;-) The conditional shown above (from config.mak.uname) should not have set NO_STRLCPY (assuming that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY is being set somewhere else (command-line, environment), this should just work. puzzled. :( I guess I saw all the warnings because I did just a “git pull —rebase make -j8” without running “make configure ./configure”. Yes, but use of configure is supposed to be optional ... Hopefully, someone who actually knows OS X can solve the mystery. ATB, Ramsay Jones I need to admit that I can not reproduce the warning here, uname -r gives 13.3.0 Could it be that something is special on your machine ? Something in the environment ? Not that I can think of, the only non-standard” thing I have installed is Homebrew (http://brew.sh/), but otherwise it’s all the standard OS X stuff and Developer tools. I write code on this machine on a daily basis. Does a fresh clone help ? A fresh clone doesn’t even build :-/ $ git clone git://github.com/git/git.git Cloning into 'git'... remote: Counting objects: 176423, done. remote: Compressing objects: 100% (47201/47201), done. remote: Total 176423 (delta 127349), reused 176233 (delta 127209) Receiving objects: 100% (176423/176423), 64.05 MiB | 6.13 MiB/s, done. Resolving deltas: 100% (127349/127349), done. Checking connectivity... done. $ cd git $ make GIT_VERSION = 2.1.0 * new build flags CC credential-store.o In file included from credential-store.c:1: In file included from ./cache.h:8: ./gettext.h:17:11: fatal error: 'libintl.h' file not found # include libintl.h ^ 1 error generated. make: *** [credential-store.o] Error 1 Again, I don't have access to an OS X system, so I don't know which package provides libintl/gettext, but it seems to be missing on your system. You can avoid the build failure, without running configure, by setting NO_GETTEXT=YesPlease in your config.mak file. I need to run configure first: $ make configure GEN configure $ ./configure configure: Setting lib to 'lib' (the default) […] So, presumably, configure has set NO_GETEXT=YesPlease in your config.mak.autogen file. ATB, Ramsay Jones -- 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] Undefine strlcpy if needed.
On Sun, Aug 24, 2014 at 5:32 PM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Again, I don't have access to an OS X system, so I don't know which package provides libintl/gettext, but it seems to be missing on your system. Probably yeah, those libraries don’t seem to be provided in standard with OS X or OS X’s development tools, so maybe the Makefile should also default to having NO_GETTEXT=YesPlease when on OS X. You can avoid the build failure, without running configure, by setting NO_GETTEXT=YesPlease in your config.mak file. I need to run configure first: $ make configure GEN configure $ ./configure configure: Setting lib to 'lib' (the default) […] So, presumably, configure has set NO_GETEXT=YesPlease in your config.mak.autogen file. Yes it did. I don’t mind running configure, but so far Git has compiled fine without doing it. Should we fix the default values of NO_STRLCPY / NO_GETEXT on OS X? -- Benoit tsuna Sigoure -- 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