Re: [PATCH 2/2] describe: Add documentation for --abbrev=+

2014-08-24 Thread brian m. carlson
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.

2014-08-24 Thread Ramsay Jones
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.

2014-08-24 Thread tsuna
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 Thread Jonh Wendell
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

2014-08-24 Thread Heiko Voigt
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

2014-08-24 Thread Gábor Szeder
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

2014-08-24 Thread Arjun Sreedharan
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

2014-08-24 Thread Stefan Beller
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.

2014-08-24 Thread Mr. Larry Markson
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

2014-08-24 Thread Ramsay Jones
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

2014-08-24 Thread Steffen Prohaska
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()

2014-08-24 Thread Steffen Prohaska
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'

2014-08-24 Thread Steffen Prohaska
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

2014-08-24 Thread Steffen Prohaska
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()

2014-08-24 Thread Steffen Prohaska
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

2014-08-24 Thread Steffen Prohaska
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.

2014-08-24 Thread Ramsay Jones
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

2014-08-24 Thread Jaime Soriano Pastor
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

2014-08-24 Thread Jaime Soriano Pastor
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

2014-08-24 Thread Jaime Soriano Pastor
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.

2014-08-24 Thread Torsten Bögershausen
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.

2014-08-24 Thread tsuna
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

2014-08-24 Thread Arjun Sreedharan
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

2014-08-24 Thread Junio C Hamano
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.

2014-08-24 Thread Ramsay Jones
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.

2014-08-24 Thread tsuna
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