Re: [RFD PATCH 0/3] Free all the memory!

2016-05-16 Thread David Turner
On Mon, 2016-05-16 at 20:22 -0700, Stefan Beller wrote: > When using automated tools to find memory leaks, it is hard to > distinguish > between actual leaks and intentional non-cleanups at the end of the > program, > such that the actual leaks hide in the noise. valgrind on git rev-parse HEAD sho

Re: [PATCH v2 09/12] attr: (re)introduce git_check_attr() and struct git_attr_check

2016-05-16 Thread Junio C Hamano
Eric Sunshine writes: > This field is unused, and git_attr_check_initl() neglects to > initialize it (if it is intended to be used in the future). There are two patterns to use the new API: - initl() variant is "initialize it once, never extend"; it is very much deliberate. This is used by

Re: [RFC-PATCHv6 4/4] pathspec: allow querying for attributes

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > +static struct git_attr_check *check; > +static int match_attrs(const char *name, int namelen, > +const struct pathspec_item *item) > +{ > + char *path; > + int i; > + > + if (!check) { > + check = git_attr_check_alloc(); > +

Re: [RFC-PATCHv6 4/4] pathspec: allow querying for attributes

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > + * attr:+val to find value set to true > + * attr:-val to find a value set to false > + * attr:!val to find a value that is not set > + * (i.e. it is neither set as "val", "val=", nor unset as "-val") > + * attr:val=value: to find value that have at least a and b set.

Re: [PATCH v2 09/12] attr: (re)introduce git_check_attr() and struct git_attr_check

2016-05-16 Thread Eric Sunshine
On Mon, May 16, 2016 at 5:05 PM, Junio C Hamano wrote: > A common pattern to check N attributes for many paths is to > > (1) prepare an array A of N git_attr_check_elem items; > (2) call git_attr() to intern the N attribute names and fill A; > (3) repeatedly call git_check_attrs() for path with

Re: [PATCH 1/3] mv: free memory at the end if desired

2016-05-16 Thread Torsten Bögershausen
>[PATCH 1/3] mv: free memory at the end if desired s/desired/DEVELOPER/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCHv6 0/4] pathspec attrs [WAS pathspec labels [WAS submodule groups]]

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > I wanted to understand Junios series, so I built on top. 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 v3 0/1] CRLF-Handling: bug fix around ce_compare_data()

2016-05-16 Thread Torsten Bögershausen
On 05/16/2016 06:13 PM, Junio C Hamano wrote: Wait a minute, please. I only asked the reason why you did it that way and mentioned that the end result seemed equivalent. "The end result seems equivalent" does not automatically mean "therefore you must avoid changing the code." If you still pre

Re: [PATCH v1 2/2] Ignore dirty submodule states during stash

2016-05-16 Thread Eric Sunshine
On Mon, May 16, 2016 at 11:40 PM, Vasily Titskiy wrote: > It checks if 'stash pop' does not trigger merge conflics > in submodules. Missing sign-off. Also, it would be best to combine these two patches so that the fix and patch reside in a single patch. More below... > --- > diff --git a/t/t39

Re: [PATCH 2/3] pack-redundant: free all memory

2016-05-16 Thread Eric Sunshine
On Mon, May 16, 2016 at 11:22 PM, Stefan Beller wrote: > Signed-off-by: Stefan Beller > --- > diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c > @@ -223,6 +223,18 @@ static inline size_t pack_list_size(struct pack_list *pl) > +static inline void pack_list_free(struct pack_list *pl

Re: [RFD PATCH 0/3] Free all the memory!

2016-05-16 Thread Eric Sunshine
On Mon, May 16, 2016 at 11:22 PM, Stefan Beller wrote: > When using automated tools to find memory leaks, it is hard to distinguish > between actual leaks and intentional non-cleanups at the end of the program, > such that the actual leaks hide in the noise. > > The end goal of this (unfinished) s

Re: [PATCH v1 2/2] Ignore dirty submodule states during stash

2016-05-16 Thread Vasily Titskiy
It checks if 'stash pop' does not trigger merge conflics in submodules. --- t/t3903-stash.sh | 35 +++ 1 file changed, 35 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 2142c1f..e48a5b5 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -

Re: [PATCH v1 1/2] Ignore dirty submodule states during stash

2016-05-16 Thread Vasily Titskiy
As stash does not know how to deal with submodules, it should not try to save/restore their states as it leads to redundant merge conflicts. Signed-off-by: Vasily Titskiy --- git-stash.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-stash.sh b/git-stash.sh index c7c65e

[PATCH v1 0/2] Ignore dirty submodule states during stash

2016-05-16 Thread Vasily Titskiy
Impoved description + added test git-stash.sh | 2 +- t/t3903-stash.sh | 35 +++ 2 files changed, 36 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More major

[PATCH 1/3] mv: free memory at the end if desired

2016-05-16 Thread Stefan Beller
From: Stefan Beller Signed-off-by: Stefan Beller --- Makefile | 7 ++- builtin/mv.c | 7 +++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3f03366..e533257 100644 --- a/Makefile +++ b/Makefile @@ -389,7 +389,8 @@ CFLAGS += -Werror \

[PATCH 2/3] pack-redundant: free all memory

2016-05-16 Thread Stefan Beller
From: Stefan Beller Signed-off-by: Stefan Beller --- builtin/pack-redundant.c | 17 + 1 file changed, 17 insertions(+) diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index 72c8158..c75c5c9 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@

[PATCH 3/3] rev-parse: free all memory

2016-05-16 Thread Stefan Beller
From: Stefan Beller Signed-off-by: Stefan Beller --- builtin/rev-parse.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index c961b74..3296d22 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -371,7 +371,7

[RFD PATCH 0/3] Free all the memory!

2016-05-16 Thread Stefan Beller
When using automated tools to find memory leaks, it is hard to distinguish between actual leaks and intentional non-cleanups at the end of the program, such that the actual leaks hide in the noise. The end goal of this (unfinished) series is to close all intentional memory leaks when enabling the

Re: [PATCH] Ignore dirty submodule states during stash

2016-05-16 Thread Vasily Titskiy
Hi Stefan, > > So, this is the issue. Instead of getting my local changes, I got a > > conflict (and stash is not > > poped out). The root cause is the 'stash' command does not know how to deal > > with submodules, > > but currently it tries to save the state of submodules, and even tries to >

[RFC-PATCHv6 2/4] pathspec: move long magic parsing out of prefix_pathspec

2016-05-16 Thread Stefan Beller
`prefix_pathspec` is quite a lengthy function and we plan on adding more. Split it up for better readability. As we want to add code into the inner loop of the long magic parsing, we also benefit from lower indentation. Signed-off-by: Stefan Beller --- pathspec.c | 84 +++

[RFC-PATCHv6 4/4] pathspec: allow querying for attributes

2016-05-16 Thread Stefan Beller
The pathspec mechanism is extended via the new ":(attr:eol=input)pattern/to/match" syntax to filter paths so that it requires paths to not just match the given pattern but also have the specified attrs attached for them to be chosen. Signed-off-by: Stefan Beller --- attr.c | 2 +- attr.h

[RFC-PATCHv6 0/4] pathspec attrs [WAS pathspec labels [WAS submodule groups]]

2016-05-16 Thread Stefan Beller
This goes on top of origin/jc/attr and is RFC as I did not write tests nor documentation, yet. I wanted to understand Junios series, so I built on top. The meat is in the last patch, which allows for git ls-files :(attr:-text)path/pattern # (ATTR_FALSE) git ls-files :(attr:+

[RFC-PATCHv6 1/4] Documentation: fix a typo

2016-05-16 Thread Stefan Beller
Signed-off-by: Stefan Beller --- Documentation/gitattributes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index e3b1de8..af2c682 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattribu

[RFC-PATCHv6 3/4] pathspec: move prefix check out of the inner loop

2016-05-16 Thread Stefan Beller
The prefix check is not related the check of pathspec magic; also there is no code that is relevant after we'd break the loop on a match for "prefix:". So move the check before the loop and shortcircuit the outer loop. Signed-off-by: Stefan Beller --- pathspec.c | 19 ++- 1 file

[PATCH v6 4/9] connect: make parse_connect_url() return separated host and port

2016-05-16 Thread Mike Hommey
Now that nothing besides CONNECT_DIAG_URL is using hostandport, we can have parse_connect_url() itself do the host and port splitting. This still leaves "user@" part of the host, if there is one, which will be addressed in a subsequent change. This however does add /some/ handling of the "user@" p

[PATCH v6 9/9] connect: move ssh command line preparation to a separate function

2016-05-16 Thread Mike Hommey
Signed-off-by: Mike Hommey --- connect.c | 108 +- 1 file changed, 58 insertions(+), 50 deletions(-) diff --git a/connect.c b/connect.c index 7360d57..38cdffc 100644 --- a/connect.c +++ b/connect.c @@ -690,6 +690,61 @@ static enum proto

[PATCH v6 6/9] connect: make parse_connect_url() return the user part of the url as a separate value

2016-05-16 Thread Mike Hommey
Signed-off-by: Mike Hommey --- connect.c | 33 + 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/connect.c b/connect.c index 3a77b2f..2659b40 100644 --- a/connect.c +++ b/connect.c @@ -602,11 +602,13 @@ static char *get_port(char *host) * Extract p

[PATCH v6 2/9] connect: only match the host with core.gitProxy

2016-05-16 Thread Mike Hommey
Currently, core.gitProxy doesn't actually match purely on domain names as documented: it also matches ports. So a core.gitProxy value like "script for kernel.org" doesn't make the script called for an url like git://kernel.org:port/path, while it is called for git://kernel.org/path. This per-port

[PATCH v6 3/9] connect: fill the host header in the git protocol with the host and port variables

2016-05-16 Thread Mike Hommey
The last use of the hostandport variable, besides being strdup'ed before being split into host and port, is to fill the host header in the git protocol. Instead of relying on parse_connect_url() to return a host:port string that makes sense there, construct one from the host and port variables. S

[PATCH v6 7/9] connect: change the --diag-url output to separate user and host

2016-05-16 Thread Mike Hommey
Signed-off-by: Mike Hommey --- connect.c | 6 ++ t/t5500-fetch-pack.sh | 14 -- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/connect.c b/connect.c index 2659b40..dcaf32f 100644 --- a/connect.c +++ b/connect.c @@ -720,10 +720,8 @@ struct child_proces

[PATCH v6 8/9] connect: actively reject git:// urls with a user part

2016-05-16 Thread Mike Hommey
Currently, urls of the for git://user@host don't work because user@host is not resolving at the DNS level, but we shouldn't be relying on it being an invalid host name, and actively reject it for containing a username in the first place. Signed-off-by: Mike Hommey --- connect.c | 3 +++ 1 file c

[PATCH v6 1/9] connect: call get_host_and_port() earlier

2016-05-16 Thread Mike Hommey
Currently, get_host_and_port() is called in git_connect() for the ssh protocol, and in git_tcp_connect_sock() for the git protocol. Instead of doing this, just call it from a single place, right after parse_connect_url(), and pass the host and port separately to git_*_connect() functions. We howev

[PATCH v6 0/9] connect: various cleanups

2016-05-16 Thread Mike Hommey
The main difference here is patch 2/9 now throwing an error in user's face when they have a core.gitProxy configuration with a port number. There might be some bikeshedding to do on the content of the error message. Mike Hommey (9): connect: call get_host_and_port() earlier connect: only match

[PATCH v6 5/9] connect: group CONNECT_DIAG_URL handling code

2016-05-16 Thread Mike Hommey
Previous changes made both branches handling CONNECT_DIAG_URL identical. We can now remove one of those branches and have CONNECT_DIAG_URL be handled in one place. Signed-off-by: Mike Hommey --- connect.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/connec

Re: git push --quiet option does not seem to work

2016-05-16 Thread Chris B
>> Once I included the whole email in my reply, but otherwise I deleted it >> all. > Both are bad practice. If you are considerate with the reader's time, this > consideration is typically reprocicated. So it is a good idea to save the > reader time by giving them the precise context they need. T

Re: [PATCH v2 01/12] commit.c: use strchrnul() to scan for one line

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 4:41 PM, Junio C Hamano wrote: > On Mon, May 16, 2016 at 4:19 PM, Stefan Beller wrote: >>> if (*p) { >>> p += 2; >>> - for (eol = p; *eol && *eol != '\n'; eol++) >>> - ; /* do nothing */ >>> + eol =

Re: [PATCH v2 02/12] attr.c: use strchrnul() to scan for one line

2016-05-16 Thread Junio C Hamano
On Mon, May 16, 2016 at 4:28 PM, Stefan Beller wrote: >> for (sp = buf; *sp; ) { >> char *ep; >> int more; >> - for (ep = sp; *ep && *ep != '\n'; ep++) >> - ; >> + >> + ep = strchrnul(sp, '\n'); > > (Even les

Re: [PATCH v2 01/12] commit.c: use strchrnul() to scan for one line

2016-05-16 Thread Junio C Hamano
On Mon, May 16, 2016 at 4:19 PM, Stefan Beller wrote: >> if (*p) { >> p += 2; >> - for (eol = p; *eol && *eol != '\n'; eol++) >> - ; /* do nothing */ >> + eol = strchrnul(p, '\n'); > > Nit: > You could include the +2 into th

[Bug] git-log prints wrong unixtime with --date=format:%s

2016-05-16 Thread Michael Heerdegen
Hello, the command git log --pretty=format:%ad --date=format:%s displays wrong unixtime values; apparently how much the printed value differs from the expected value depends on the system's time zone and whether daylight savings time is enabled or not. Here is a reproducible recipe compiled

Re: [PATCH v2 03/12] attr.c: update a stale comment on "struct match_attr"

2016-05-16 Thread Junio C Hamano
On Mon, May 16, 2016 at 4:34 PM, Stefan Beller wrote: >> - * If is_macro is false, then u.pattern points at the filename pattern >> - * to which the rule applies. (The memory pointed to is part of the >> - * memory block allocated for the match_attr instance.) >> + * If is_macro is false, then u.

Re: [PATCH v2 03/12] attr.c: update a stale comment on "struct match_attr"

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 2:05 PM, Junio C Hamano wrote: > When 82dce998 (attr: more matching optimizations from .gitignore, > 2012-10-15) changed a pointer to a string "*pattern" into an > embedded "struct pattern" in struct match_attr, it forgot to update > the comment that describes the structure

Re: [PATCH v2 02/12] attr.c: use strchrnul() to scan for one line

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 2:05 PM, Junio C Hamano wrote: > Signed-off-by: Junio C Hamano > --- > attr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/attr.c b/attr.c > index eec5d7d..45aec1b 100644 > --- a/attr.c > +++ b/attr.c > @@ -402,8 +402,8 @@ static struct att

Re: [PATCH v2 01/12] commit.c: use strchrnul() to scan for one line

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 2:05 PM, Junio C Hamano wrote: > Signed-off-by: Junio C Hamano > --- > commit.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/commit.c b/commit.c > index 3f4f371..1f9ee8a 100644 > --- a/commit.c > +++ b/commit.c > @@ -415,8 +415,7 @@ int find_

Re: [PATCH v5 4/9] connect: make parse_connect_url() return separated host and port

2016-05-16 Thread Mike Hommey
On Mon, May 16, 2016 at 03:39:08PM -0700, Junio C Hamano wrote: > Mike Hommey writes: > > > + get_host_and_port(&host, &port); > > + > > + if (*host && !port) { > > + /* The host might contain a user:password string, ignore it > > +* when searching for the port again */

Re: [PATCH v5 2/9] connect: only match the host with core.gitProxy

2016-05-16 Thread Junio C Hamano
Mike Hommey writes: > The gitProxy script gets the port passed. Why would you need different > scripts for different ports if the port is passed as an argument? Also, > if it's deliberate, it's widely undocumented. Fair enough. A user who has been working around thsi "oversight", would have rel

Re: [PATCH v5 8/9] connect: actively reject git:// urls with a user part

2016-05-16 Thread Junio C Hamano
Mike Hommey writes: > Currently, urls of the for git://user@host don't work because user@host > is not resolving at the DNS level, but we shouldn't be relying on it > being an invalid host name, and actively reject it for containing a > username in the first place. Makes sense. Connecting to ho

Re: [PATCH v5 4/9] connect: make parse_connect_url() return separated host and port

2016-05-16 Thread Junio C Hamano
Mike Hommey writes: > + get_host_and_port(&host, &port); > + > + if (*host && !port) { > + /* The host might contain a user:password string, ignore it > + * when searching for the port again */ > + char *end_user = strrchr(host, '@'); > + p

Re: [PATCH v5 2/9] connect: only match the host with core.gitProxy

2016-05-16 Thread Mike Hommey
On Mon, May 16, 2016 at 03:30:01PM -0700, Junio C Hamano wrote: > Mike Hommey writes: > > > Currently, core.gitProxy doesn't actually match purely on domain names > > as documented: it also matches ports. > > ... > > This per-port behavior seems like an oversight rather than a deliberate > > choi

Re: [PATCH v5 2/9] connect: only match the host with core.gitProxy

2016-05-16 Thread Junio C Hamano
Mike Hommey writes: > Currently, core.gitProxy doesn't actually match purely on domain names > as documented: it also matches ports. > ... > This per-port behavior seems like an oversight rather than a deliberate > choice, so, make git://kernel.org:port/path call the gitProxy script in Hmph. Th

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: >> Ah, of course. I thought that you were trying to limit ":(attr:)" >> magic only to attributes that begin with "label-", which is where my >> "why not?" comes from. > > And going by the logic you presented before, we would > need to error out for the given pathspec ":()"

Re: [PATCH v6 00/17] Port branch.c to use ref-filter's printing options

2016-05-16 Thread Junio C Hamano
Karthik Nayak writes: > This is part of unification of the commands 'git tag -l, git branch -l > and git for-each-ref'. This ports over branch.c to use ref-filter's > printing options. > > Initially posted here: $(gmane/279226). It was decided that this series > would follow up after refactoring

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 3:02 PM, Junio C Hamano wrote: > Stefan Beller writes: > >>> Hmph, why not? >> >> We need a namespace for which >> * we can guarantee that it is for labeling purposes only (even in the future) >> * is obvious to the user to be a labeling name space >> >> Starting with "lab

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: >> Hmph, why not? > > We need a namespace for which > * we can guarantee that it is for labeling purposes only (even in the future) > * is obvious to the user to be a labeling name space > > Starting with "label" offers both? Ah, of course. I thought that you were trying t

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 2:50 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> And we want to have both "label=A B C" and attr:label=A B C" or *just* the >> attr query? > > I think the choice at this point is between supporting just "label=A > B C" or supporting just "attr:eol=crlf text=auto

Re: [PATCH 2/2] difftool: handle unmerged files in dir-diff mode

2016-05-16 Thread Junio C Hamano
Thanks, will queue. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > And we want to have both "label=A B C" and attr:label=A B C" or *just* the > attr query? I think the choice at this point is between supporting just "label=A B C" or supporting just "attr:eol=crlf text=auto !diff". I think "attr:label=A" is merely a degenerated case of t

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 2:18 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> This is another case for using ':' instead of '='. >> So I think ':' is better for this future enhancement. >> >> Also this future enhancement may ask for >> >> git ls-files ":(attr:label=foo)" >> >> or >> >

Re: bug: autostash is lost after aborted rebase

2016-05-16 Thread Eugene Yarmash
The bug still persists when you abort the rebase by using :cq in Vim (exit with an error code). See also http://stackoverflow.com/q/37252108/244297 -- View this message in context: http://git.661346.n2.nabble.com/bug-autostash-is-lost-after-aborted-rebase-tp7611141p7656556.html Sent from the gi

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > This is another case for using ':' instead of '='. > So I think ':' is better for this future enhancement. > > Also this future enhancement may ask for > > git ls-files ":(attr:label=foo)" > > or > > git ls-files ":(attr:-label)" > > so the user can circumvent

[PATCH v2 12/12] attr: retire git_check_attrs() API

2016-05-16 Thread Junio C Hamano
Since nobody uses the old API, make it file-scope static, and update the documentation to describe the new API. Signed-off-by: Junio C Hamano --- Documentation/technical/api-gitattributes.txt | 62 +++ attr.c| 3 +- attr.h

[PATCH v2 11/12] attr: convert git_check_attrs() callers to use the new API

2016-05-16 Thread Junio C Hamano
The remaining callers are all simple "I have N attributes I am interested in. I'll ask about them with various paths one by one". After this step, no caller to git_check_attrs() remains. After removing it, we can extend "struct git_attr_check" struct with data that can be used in optimizing the

[PATCH v2 10/12] attr: convert git_all_attrs() to use "struct git_attr_check"

2016-05-16 Thread Junio C Hamano
This updates the other two ways the attribute check is done via an array of "struct git_attr_check_elem" elements. These two niches appear only in "git check-attr". * The caller does not know offhand what attributes it wants to ask about and cannot use git_attr_check_initl() to prepare the

[PATCH v2 07/12] attr.c: simplify macroexpand_one()

2016-05-16 Thread Junio C Hamano
The double-loop wants to do an early return immediately when one matching macro is found. Eliminate the extra variable 'a' used for that purpose and rewrite the "assign the found item to 'a' to make it non-NULL and force the loop(s) to terminate" with a direct return from there. Signed-off-by: Ju

[PATCH v2 08/12] attr: rename function and struct related to checking attributes

2016-05-16 Thread Junio C Hamano
The traditional API to check attributes is to prepare an N-element array of "struct git_attr_check" and pass N and the array to the function "git_check_attr()" as arguments. In preparation to revamp the API to pass a single structure, in which these N elements are held, rename the type used for th

[PATCH v2 06/12] attr.c: mark where #if DEBUG ends more clearly

2016-05-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano --- attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/attr.c b/attr.c index a7f2c3f..95416d3 100644 --- a/attr.c +++ b/attr.c @@ -469,7 +469,7 @@ static void debug_set(const char *what, const char *match, struct git_attr *attr #define debug

[PATCH v2 09/12] attr: (re)introduce git_check_attr() and struct git_attr_check

2016-05-16 Thread Junio C Hamano
A common pattern to check N attributes for many paths is to (1) prepare an array A of N git_attr_check_elem items; (2) call git_attr() to intern the N attribute names and fill A; (3) repeatedly call git_check_attrs() for path with N and A; A look-up for these N attributes for a single path P s

[PATCH v2 05/12] attr.c: complete a sentence in a comment

2016-05-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano --- attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/attr.c b/attr.c index 05db667..a7f2c3f 100644 --- a/attr.c +++ b/attr.c @@ -300,7 +300,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, * directory (aga

[PATCH v2 04/12] attr.c: explain the lack of attr-name syntax check in parse_attr()

2016-05-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano --- attr.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/attr.c b/attr.c index 4ae7801..05db667 100644 --- a/attr.c +++ b/attr.c @@ -183,6 +183,12 @@ static const char *parse_attr(const char *src, int lineno, const char *cp, retur

[PATCH v2 03/12] attr.c: update a stale comment on "struct match_attr"

2016-05-16 Thread Junio C Hamano
When 82dce998 (attr: more matching optimizations from .gitignore, 2012-10-15) changed a pointer to a string "*pattern" into an embedded "struct pattern" in struct match_attr, it forgot to update the comment that describes the structure. Signed-off-by: Junio C Hamano --- attr.c | 5 ++--- 1 file

[PATCH v2 02/12] attr.c: use strchrnul() to scan for one line

2016-05-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano --- attr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/attr.c b/attr.c index eec5d7d..45aec1b 100644 --- a/attr.c +++ b/attr.c @@ -402,8 +402,8 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok) for

[PATCH v2 00/12] revamping git_check_attr() API

2016-05-16 Thread Junio C Hamano
A common pattern to check N attributes for many paths is to (1) prepare an array A of N git_attr_check_elem items; (2) call git_attr() to intern the N attribute names and fill A; (3) repeatedly call git_check_attrs() for path with N and A; A look-up for these N attributes for a single path P s

[PATCH v2 01/12] commit.c: use strchrnul() to scan for one line

2016-05-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano --- commit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/commit.c b/commit.c index 3f4f371..1f9ee8a 100644 --- a/commit.c +++ b/commit.c @@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const char **subject)

Re: Best Practices with code/build fixes post-merge?

2016-05-16 Thread Junio C Hamano
Robert Dailey writes: > Sometimes, I merge 2 branches that have deviated quite a bit. A > worst-case example would be some API change. The topic branch > (long-lived) may start using the old API. However, once I merge the > topic back to master, that API no longer exists. As such, every place > t

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > "Path '%s': Ignoring label set to false; This may behave > differently in future versions of Git." I agree that mentioning "ignoring" is good enough. I think our recent messages begin with lowercase, though. -- To unsubscribe from this list: send the line "unsubscri

Best Practices with code/build fixes post-merge?

2016-05-16 Thread Robert Dailey
Sometimes, I merge 2 branches that have deviated quite a bit. A worst-case example would be some API change. The topic branch (long-lived) may start using the old API. However, once I merge the topic back to master, that API no longer exists. As such, every place that introduces a usage of the old

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 12:08 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> "... but for now Git treats it as if it is not set at all" is a valuable >> information to the user, still? > > Not at all. "What you are using is wrong and there is no guarantee > what behaviour you would get"

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > "... but for now Git treats it as if it is not set at all" is a valuable > information to the user, still? Not at all. "What you are using is wrong and there is no guarantee what behaviour you would get" is the message we need to convey. -- To unsubscribe from this list:

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 11:52 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> So "warn and ignore" for data from .gitattributes and die for >> commandline arguments? That makes sense. > > Yes. > > On the "command line" front, because we may want to give different > meanings to these two en

Re: [PATCH 0/6] modernize t1500

2016-05-16 Thread Junio C Hamano
Eric Sunshine writes: > On Tue, May 10, 2016 at 2:26 PM, Junio C Hamano wrote: >> Junio C Hamano writes: >>> Junio C Hamano writes: >>> Subject: [PATCH 7/6] t1500: finish preparation upfront >>> >>> The earlier tests do not attempt to modify the contents of .git (by >>> creating objects or ref

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > So "warn and ignore" for data from .gitattributes and die for > commandline arguments? That makes sense. Yes. On the "command line" front, because we may want to give different meanings to these two entries in the future: :(label=-doc)Documentation/ :(la

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: >> I am NOT suggesting to make this enhancement in the prototype to >> allow us experiment with submodule selection use case, but this is >> an obvious place to allow >> >> :(label=A B):(label=C D) >> >> to mean ((A & B) | (C & D)) by making item->labels an array of

Re: [PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error

2016-05-16 Thread Christian Couder
On Mon, May 16, 2016 at 5:04 AM, Eric Sunshine wrote: > On Wed, May 11, 2016 at 9:17 AM, Christian Couder > wrote: >> To libify `git apply` functionality we have to signal errors to the >> caller instead of die()ing or exit()ing. >> >> To do that in a compatible manner with the rest of the error

[PATCHv4 5/5] pathspec: record labels

2016-05-16 Thread Stefan Beller
The "label" attribute can be attached to paths, and the pathspec mechanism is extended via the new ":(label=X)pattern/to/match" syntax to filter paths so that it requires paths to not just match the given pattern but also have the specified labels attached for them to be chosen. Labels are meant t

[PATCH 2/2] difftool: handle unmerged files in dir-diff mode

2016-05-16 Thread David Aguilar
When files are unmerged they can show up as both unmerged and modified in the output of `git diff --raw`. This causes difftool's dir-diff to create filesystem entries for the same path twice, which fails when it encounters a duplicate path. Ensure that each worktree path is only processed once. A

[PATCHv4 1/5] Documentation: fix a typo

2016-05-16 Thread Stefan Beller
Signed-off-by: Stefan Beller --- Documentation/gitattributes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index e3b1de8..af2c682 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattribu

[PATCHv4 4/5] pathspec: move prefix check out of the inner loop

2016-05-16 Thread Stefan Beller
The prefix check is not related the check of pathspec magic; also there is no code that is relevant after we'd break the loop on a match for "prefix:". So move the check before the loop and shortcircuit the outer loop. Signed-off-by: Stefan Beller --- pathspec.c | 19 ++- 1 file

[PATCH 1/2] difftool: initialize variables for readability

2016-05-16 Thread David Aguilar
The code always goes into one of the two conditional blocks but make it clear that not doing so is an error condition by setting $ok to 0. Signed-off-by: David Aguilar --- git-difftool.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-difftool.perl b/git-difftool.p

[PATCHv4 3/5] pathspec: move long magic parsing out of prefix_pathspec

2016-05-16 Thread Stefan Beller
`prefix_pathspec` is quite a lengthy function and we plan on adding more. Split it up for better readability. As we want to add code into the inner loop of the long magic parsing, we also benefit from lower indentation. Signed-off-by: Stefan Beller --- pathspec.c | 84 +++

[PATCHv4 0/5] pathspec labels

2016-05-16 Thread Stefan Beller
Thanks Junio for the review! Thanks Duy for suggestions to think about in the not-submodule case :) * when invalid labels are found -> in the .gitattributes "warn and ignore" -> in command line args die(..) * treat labels set to false as unset. * fixes in documentation/ reworded the commit mes

[PATCHv4 2/5] Documentation: correct typo in example for querying attributes

2016-05-16 Thread Stefan Beller
The introductory text of the example has a typo in the specification which makes it harder to follow that example. Signed-off-by: Stefan Beller --- Documentation/technical/api-gitattributes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/api-gitatt

Re: [PATCH v10 20/20] untracked-cache: config option

2016-05-16 Thread David Turner
On Sun, 2016-05-15 at 16:43 +0700, Duy Nguyen wrote: > On Fri, May 13, 2016 at 3:20 AM, David Turner < > dtur...@twopensource.com> wrote: > > Add a config option to populate the untracked cache. > > > > For installations that have centrally-managed configuration, it's > > easier to set a config on

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Stefan Beller
> I am NOT suggesting to make this enhancement in the prototype to > allow us experiment with submodule selection use case, but this is > an obvious place to allow > > :(label=A B):(label=C D) > > to mean ((A & B) | (C & D)) by making item->labels an array of set > of labels. This is what

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 10:39 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> On Sun, May 15, 2016 at 3:06 AM, Duy Nguyen wrote: >>> Instead of putting everything in under the same attribute name >>> "label", make one attribute per label? Would this work? >>> >>> *.[ch] c-group code-group

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Stefan Beller
On Mon, May 16, 2016 at 10:38 AM, Junio C Hamano wrote: > Stefan Beller writes: > >>> Let's avoid "are meant to", which is merely to give you an excuse to >>> say "it was meant to ... but the implementation did not quite achieve >>> that goal". >>> >>> The "label" attribute can be attached to

Re: [PATCH 0/6] modernize t1500

2016-05-16 Thread Eric Sunshine
On Tue, May 10, 2016 at 2:26 PM, Junio C Hamano wrote: > Junio C Hamano writes: >> Junio C Hamano writes: >> Subject: [PATCH 7/6] t1500: finish preparation upfront >> >> The earlier tests do not attempt to modify the contents of .git (by >> creating objects or refs, for example), which means tha

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: > On Sun, May 15, 2016 at 3:06 AM, Duy Nguyen wrote: >> Instead of putting everything in under the same attribute name >> "label", make one attribute per label? Would this work? >> >> *.[ch] c-group code-group >> >> And the pathspec magic name can be simply "attr", any git

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Junio C Hamano
Stefan Beller writes: >> Let's avoid "are meant to", which is merely to give you an excuse to >> say "it was meant to ... but the implementation did not quite achieve >> that goal". >> >> The "label" attribute can be attached to paths, and the pathspec >> mechanism is extended via the new

Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

2016-05-16 Thread Stefan Beller
On Sun, May 15, 2016 at 3:06 AM, Duy Nguyen wrote: > Instead of putting everything in under the same attribute name > "label", make one attribute per label? Would this work? > > *.[ch] c-group code-group > > And the pathspec magic name can be simply "attr", any git attribute > can be used with it,

Re: [PATCH v2 52/94] builtin/apply: read_patch_file() return -1 instead of die()ing

2016-05-16 Thread Christian Couder
On Mon, May 16, 2016 at 3:56 AM, Eric Sunshine wrote: > On Wed, May 11, 2016 at 9:17 AM, Christian Couder > wrote: >> To libify `git apply` functionality we have to signal errors to the >> caller instead of die()ing. Let's do that by using error() instead >> of die()ing in read_patch_file(). >> >

Re: [PATCH 5/5] pathspec: record labels

2016-05-16 Thread Stefan Beller
On Sat, May 14, 2016 at 12:23 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> Labels were originally designed to manage large amount of >> submodules, the discussion steered this in a more general >> direction, such that other files can be labeled as well. > > s/other files/any path/. > >>

  1   2   >