GSoC 2015. Microprojects
As I understood there are four unsolved microprojects. I think I can deal with Make git diff --no-index $directory $file DWIM better but I don't understand what exactly should I do. I tried to run 'git diff --no-index ~/git/ diff.h' cmd on git sources but it says 'error: file/directory conflict: /home/localhost/git/, diff.h'. I have used git many times, but I'm not familiar with git diff. Thanks in advance! -- 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
[GSoC][PATCH v2] log: forbid log --graph --no-walk
In git-log, --graph shows a graphical representation of a continuous commit history, and --no-walk shows discrete specified commits without continuity. Using both doesn't make sense, so we forbid the combined use of these flags. Signed-off-by: Manos Pitsidianakis epi...@norn.io --- builtin/log.c | 2 ++ t/t4202-log.sh | 4 2 files changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..0194133 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); +if (rev-graph rev-no_walk) +die(--graph and --no-walk are incompatible); if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..4dd939b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' +test_must_fail git log --graph --no-walk +' + test_done -- 2.1.4 -- 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] log: forbid log --graph --no-walk
On 03/15/2015 03:08 AM, Dongcan Jiang wrote: it seems that your patch could not pass t4052-stat-output.sh. I think it would be better if you could improve the specification for this change in Document/rev-list-options.txt Can't grok why this happens. What exactly is happening in t4052-stat-output.sh? Is it testing every possible combination of git commands and arguments? -- 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] log: forbid log --graph --no-walk
Because revs-no_walk gets set when it comes to git show. You can find more information on [1]. [1] http://article.gmane.org/gmane.comp.version-control.git/264921 2015-03-15 9:24 GMT+08:00 Manos Pitsidianakis epi...@norn.io: On 03/15/2015 03:08 AM, Dongcan Jiang wrote: it seems that your patch could not pass t4052-stat-output.sh. I think it would be better if you could improve the specification for this change in Document/rev-list-options.txt Can't grok why this happens. What exactly is happening in t4052-stat-output.sh? Is it testing every possible combination of git commands and arguments? -- 江东灿(Dongcan Jiang) Team of Search Engine Web Mining School of Electronic Engineering Computer Science Peking University, Beijing, 100871, P.R.China -- 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] forbid log --graph --no-walk
In git-log, --graph shows a graphical representation of a continuous commit history, and --no-walk shows discrete specified commits without continuity. This doesn't make sense, so we forbid the combined use of these flags. Signed-off-by: Manos Pitsidianakis epi...@norn.io --- builtin/log.c | 2 ++ t/t4202-log.sh | 4 2 files changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..0194133 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); +if (rev-graph rev-no_walk) +die(--graph and --no-walk are incompatible); if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..4dd939b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' +test_must_fail git log --graph --no-walk +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags
Dave Olszewski cx...@pobox.com writes: On Fri, 13 Mar 2015, Junio C Hamano wrote: Jeff King p...@peff.net writes: From: Dave Olszewski cx...@pobox.com Signed-off-by: Dave Olszewski cx...@pobox.com --- Again, this is just a preview. Dave should send the final when he thinks it is good. Dave? I do not see anything wrong with this version that builds on top of the previous 2 clean-up. Personally I find that these clean-up changes more valuable than I care about this particular feature, and it is unfortunate that waiting an Ack or reroll of this one kept them stalled. I am tempted to throw Helped-by: Peff into the log message and merge the result to 'next', unless I hear otherwise in a few days. Sorry, work has kept me very busy lately, I haven't had time to re-visit this. Jeff's version looks great to me, please go ahead with it. Thanks everyone. Dave 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/GSoC/RFC] fetch: git fetch --deepen
Duy Nguyen pclo...@gmail.com writes: On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano gits...@pobox.com wrote: Dongcan Jiang dongcan.ji...@gmail.com writes: What does (you) exactly mean in [1]? The local branch or the local remote ref? As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. That would make it harder to implement fetch --deepen than the version that moves refs if they are updated. The comment you are responding to was in the context of What does the illustrated 'your' history mean? Is it a history of a single ref (if so in which repository)? to clarify that the example was not fetching _new_ history (and with the RFC/POC design that was posted, my understanding is that deepen was meant to only deepen). The paragraph you are reacting to is not an endorsement for that only deepen, never advance design. It merely is a clarification of the explanation that was in the paragraph that follows. And I think what Dongcan implemented moves refs. From the user point of view, I think it's ok with either version, so the one that's easier to implement wins. The you does not explicitly depict any ref, because the picture is meant to illustrate _everything_ the repository at the receiving end of fetch has. It used to have two commits, A and B (and the tree and blob objects necessary to complete these two commits). After deepening the history by one commit, it then will have commit A^ and its trees and blobs. [1] http://article.gmane.org/gmane.comp.version-control.git/212950 -- 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
[no subject]
-- 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: [GSoC][PATCH v2] log: forbid log --graph --no-walk
On Sun, Mar 15, 2015 at 01:33:07AM +0200, epilys wrote: diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..0194133 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); +if (rev-graph rev-no_walk) +die(--graph and --no-walk are incompatible); It looks like you indented here with four spaces instead of a tab. We prefer tabs in Git. if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..4dd939b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' +test_must_fail git log --graph --no-walk And here as well. -- 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 v3] log: forbid log --graph --no-walk
it seems that your patch could not pass t4052-stat-output.sh. I think it would be better if you could improve the specification for this change in Document/rev-list-options.txt 2015-03-15 8:43 GMT+08:00 Manos Pitsidianakis epi...@norn.io: In git-log, --graph shows a graphical representation of a continuous commit history, and --no-walk shows discrete specified commits without continuity. Using both doesn't make sense, so we forbid the combined use of these flags. Signed-off-by: Manos Pitsidianakis epi...@norn.io --- This is a microproject intended to complement my GSoC application. builtin/log.c | 2 ++ t/t4202-log.sh | 4 2 files changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..5aaf964 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); + if (rev-graph rev-no_walk) + die(--graph and --no-walk are incompatible); if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..5d72713 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' + test_must_fail git log --graph --no-walk +' + test_done -- 2.1.4 -- 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 -- 江东灿(Dongcan Jiang) Team of Search Engine Web Mining School of Electronic Engineering Computer Science Peking University, Beijing, 100871, P.R.China -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] Define a structure for object IDs.
Duy Nguyen pclo...@gmail.com writes: Notice that the first time pack-obj[] is filled using lookup_object(). So yes, the hash table has all the pointers that pack-obj[] has. Are we talking about the same thing? By the hash table, I mean **obj_hash that is a hashtable that uses sha-1 form of identifier as the key. So the hash table has all the pointers sounds like all the objects you instantiate from a v4 packfile needs their object name known in the sha-1 form anyway when it gets instantiated (or more importantly, when you realize there is another copy of it already in the **obj_hash hashtable, you may have to free or refrain from creating it in the first place). As long as we use **obj_hash hashtable as all the objects this process cares about in the world and struct object_id.hash as the key into it, I do not think pack,nth representation belongs to that layer. I do think pack,nth representation has its use as a short-cut mechanism to reach an indirectly referenced object directly without instantiating intermediate objects when dealing with an extended SHA-1 expression (cf. v1.0^0:t example in a few messages upthread). I just think it does not belong to struct object_id that are used to refer to in-core objects. -- 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] forbid log --graph --no-walk
In git-log, --graph shows a graphical representation of a continuous commit history, and --no-walk shows discrete specified commits without continuity. This doesn't make sense, so we forbid the combined use of these flags. Signed-off-by: Manos Pitsidianakis epi...@norn.io --- builtin/log.c | 2 ++ t/t4202-log.sh | 4 2 files changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..0194133 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); +if (rev-graph rev-no_walk) +die(--graph and --no-walk are incompatible); if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..4dd939b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' +test_must_fail git log --graph --no-walk +' + test_done -- 2.1.4 -- 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 1/4] git-credential-store: support multiple credential files
Paul Tan pyoka...@gmail.com writes: I think you could even get away without passing default_fn here, and just use the rule the first file in the list is the default. Unless you are anticipating ever passing something else, but I couldn't think of a case where that would be useful. Even though in this case the store_credential() function is not used anywhere else, from my personal API design experience I think that cementing the rule of the first file in the list is the default in the behavior of the function is not a good thing. For example, in the future, we may wish to keep the precedence ordering the same, but if none of the credential files exist, we create the XDG file by default instead. I am not sure if this is not a premature over-engineering---I am not convinced that such a future need will be fulfilled by passing just a single default_fn this version already passes, or it needs even more parameters that this version does not pass yet, and the interface to the function needs to be updated at that point when you need it _anyways_. One thing that we all agree is that we don't need the extra parameter within the context of what the current code does. So, it smells like a case of YAGNI a bit, at least to me. -- 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] forbid log --graph --no-walk
Subject is usually prefixed with the part that is changed, in this case log. For example: log: forbid log --graph --no-walk On Sat, Mar 14, 2015 at 11:31:59PM +0200, epilys wrote: In git-log, --graph shows a graphical representation of a continuous commit history, and --no-walk shows discrete specified commits without continuity. This doesn't make sense, so we forbid the combined use of these flags. Signed-off-by: Manos Pitsidianakis epi...@norn.io --- builtin/log.c | 2 ++ t/t4202-log.sh | 4 2 files changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..0194133 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); +if (rev-graph rev-no_walk) Patch got corrupted here (Space before +) +die(--graph and --no-walk are incompatible); if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..4dd939b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' Here also +test_must_fail git log --graph --no-walk +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC][PATCH v2] log: forbid log --graph --no-walk
On 03/15/2015 01:47 AM, brian m. carlson wrote: It looks like you indented here with four spaces instead of a tab. We prefer tabs in Git. Messed that up. Do you think I should resubmit a v3 or am I hogging the mailing list too much? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] log: forbid log --graph --no-walk
In git-log, --graph shows a graphical representation of a continuous commit history, and --no-walk shows discrete specified commits without continuity. Using both doesn't make sense, so we forbid the combined use of these flags. Signed-off-by: Manos Pitsidianakis epi...@norn.io --- This is a microproject intended to complement my GSoC application. builtin/log.c | 2 ++ t/t4202-log.sh | 4 2 files changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..5aaf964 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, memset(w, 0, sizeof(w)); userformat_find_requirements(NULL, w); + if (rev-graph rev-no_walk) + die(--graph and --no-walk are incompatible); if (!rev-show_notes_given (!rev-pretty_given || w.notes)) rev-show_notes = 1; if (rev-show_notes) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..5d72713 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep ^| | gpg: Good signature actual ' +test_expect_success 'forbid log --graph --no-walk' ' + test_must_fail git log --graph --no-walk +' + test_done -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] Define a structure for object IDs.
Duy Nguyen pclo...@gmail.com writes: Anyway, wouldn't this be all academic? I do not see how you would keep the object name in the pack, nth format in-core, as the obj_hash[] is a hashtable keyed by sha-1, and even when we switch to a different hash, I cannot see how such a table to ensure the singleton-ness of in-core objects can be keyed sometimes by hash and by pack, nth in some other time. I'm implementing something to see how much we gain by avoiding object lookup. The current approach is having struct object ** obj in struct packed_git, indexed by nth. So when you have pack, nth and pack-obj[nth] is valid, you'll get to struct object * without hashing. But do you realize that the hashtable serves two purposes? Grab the object from its name is one thing, and the other one I am not seeing how you will make it work with sometimes sha-1 sometimes pack,nth is to ensure that we will have only one in-core copy for the same object. We even walk the hashtable when we want to drop the flag bits from all in-core objects, so even if you instanciated an in-core object without going through the object name layer, the hashtable needs to have a pointer to such a pointer, no? -- 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: [GSoC][PATCH v2] log: forbid log --graph --no-walk
On Sun, Mar 15, 2015 at 01:55:28AM +0200, epilys wrote: On 03/15/2015 01:47 AM, brian m. carlson wrote: It looks like you indented here with four spaces instead of a tab. We prefer tabs in Git. Messed that up. Do you think I should resubmit a v3 or am I hogging the mailing list too much? You're going to want to submit a v3. While you're at it, you probably want to drop the [GSoC] from the patch header, as it will be part of the commit message when applied, and we don't want that. Finally, you probably want to use your full name in the From: line, as that will be used to create the commit, and we prefer full names over aliases as well. It can be helpful to try formatting the patch with git format-patch and then checking over it with less and applying it with git am to see how it will look to other Git developers. This is a small patch, so it's not as big a deal, but for larger series I generally try to wait two or three days (at least one of which is a weekday) before posting a new version so that people have time to read, test, and comment on it. -- 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] contrib/completion: escape the forward slash in __git_match_ctag
John Szakmeister j...@szakmeister.net writes: The current definition results in an incorrect expansion of the term under zsh. For instance /^${1\\/}/ under zsh with the argument hi results in: /^/\/h/\/i/ This results in an output similar to this when trying to complete `git grep chartab` under zsh: :: git grep chartabawk: cmd. line:1: /^/\/c/\/h/\/a/\/r/\/t/\/a/\/b/ { print $1 } awk: cmd. line:1:^ backslash not last character on line awk: cmd. line:1: /^/\/c/\/h/\/a/\/r/\/t/\/a/\/b/ { print $1 } awk: cmd. line:1:^ syntax error Leaving the prompt in a goofy state until the user hits a key. Escaping the literal / in the parameter expansion (using /^${1//\//\\/}/) results in: /^chartab/ allowing the completion to work correctly. This formulation also works under bash. Signed-off-by: John Szakmeister j...@szakmeister.net --- I've been bit by this bug quite a bit, but didn't have time to track it down until today. I hope the proposed solution is acceptable. contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c21190d..a899234 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1305,7 +1305,7 @@ _git_gitk () } __git_match_ctag() { - awk /^${1\\/}/ { print \$1 } $2 + awk /^${1//\//\\/}/ { print \$1 } $2 The updated pattern look sensible to me. / to start the pattern part, extra / to say repeatedly replace all, \/ to say a single slash is what is to be replaced, / to say here is where the pattern ends, and then \\/ to say replace with backslash-slash. In fact, I have to suspect that the original working by pure accident or a bug. Thanks. } _git_grep () -- 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: GSoC 2015. Microprojects
Yurii Shevtsov unge...@gmail.com writes: As I understood there are four unsolved microprojects. I think I can deal with Make git diff --no-index $directory $file DWIM better but I don't understand what exactly should I do. OK. I tried to run 'git diff --no-index ~/git/ diff.h' cmd on git sources but it says 'error: file/directory conflict: /home/localhost/git/, diff.h'. There you have it. Do you think erroring out is useful? Imagine a user who is used to the way ordinary diff command (not git diff) operates. What would be the behaviour that may match the expectation of such a user better? In other words, if you have the source tree of git in ~/git and then you have diff.h in your current directory, perhaps prepared by doing something like this: $ cd $HOME $ git clone git://git.kernel.org/pub/scm/git/git.git git $ mkdir junk $ cp git/diff.h junk $ cd junk $ echo hello diff.h what would the ordinary diff say when you do these commands (still in that ~/junk directory)? $ diff -u ~/git diff.h $ diff -u diff.h ~/git Wouldn't it be wonderful if git diff --no-index worked the same way as these? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] Define a structure for object IDs.
On Sun, Mar 15, 2015 at 5:19 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: Anyway, wouldn't this be all academic? I do not see how you would keep the object name in the pack, nth format in-core, as the obj_hash[] is a hashtable keyed by sha-1, and even when we switch to a different hash, I cannot see how such a table to ensure the singleton-ness of in-core objects can be keyed sometimes by hash and by pack, nth in some other time. I'm implementing something to see how much we gain by avoiding object lookup. The current approach is having struct object ** obj in struct packed_git, indexed by nth. So when you have pack, nth and pack-obj[nth] is valid, you'll get to struct object * without hashing. But do you realize that the hashtable serves two purposes? Grab the object from its name is one thing, and the other one I am not seeing how you will make it work with sometimes sha-1 sometimes pack,nth is to ensure that we will have only one in-core copy for the same object. We even walk the hashtable when we want to drop the flag bits from all in-core objects, so even if you instanciated an in-core object without going through the object name layer, the hashtable needs to have a pointer to such a pointer, no? Notice that the first time pack-obj[] is filled using lookup_object(). So yes, the hash table has all the pointers that pack-obj[] has. If somebody wants to remove an object out of hash table, then all pack-obj[] are invalidated, but I don't think we ever delete objects from hash table. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git.c: two memory leak fixes
Alexander Kuleshov kuleshovm...@gmail.com writes: This patch provides fixes for two minor memory leaks in the handle_alias function: 1. We allocate memory for alias_argv with the xmalloc function call, after run_command_v_opt function will be executed we no need in this variable anymore, so let's free it. This is technically correct, but do we really care about it? We have finished running the command and all that is left is either to exit(ret) or to die(); either will let the operating system clear the memory together with the entire process for us. The normal exit codepath still holds a live alias_string when it calls exist(ret), but you do not free it even after this patch, which is an inconsistent stance to take. I think it is OK not to bother freeing alias_string just before exit(), and I would apply the same reasoning to alias_argv[], too. 2. Memory allocated for alias_string variable in the alias_lookup function, need to free it. I think it is wrong to free alias_string after passing it to split_cmdline() and while you still need to use the new_argv. Reading the split_cmdline() implementation again, the new argv which is an array of pointers is newly allocated, but I think the pointers in the array point into the cmdline parameter. From the caller's point of view, new_argv[0] points at the beginning of alias_string, new_argv[1] points at the beginning of the second word of alias_string, etc., all of which will become invalid if you free alias_string, no? Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- git.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git.c b/git.c index 086fac1..501e5bd 100644 --- a/git.c +++ b/git.c @@ -252,6 +252,7 @@ static int handle_alias(int *argcp, const char ***argv) alias_argv[argc] = NULL; ret = run_command_v_opt(alias_argv, RUN_USING_SHELL); + free(alias_argv); if (ret = 0) /* normal exit */ exit(ret); @@ -259,6 +260,7 @@ static int handle_alias(int *argcp, const char ***argv) alias_command, alias_string + 1); } count = split_cmdline(alias_string, new_argv); + free(alias_string); if (count 0) die(Bad alias.%s string: %s, alias_command, split_cmdline_strerror(count)); -- 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
What's cooking in git.git (Mar 2015, #05; Sat, 14)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. On the 'maint' front is the latest maintenance release v2.3.3. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * jk/tag-h-column-is-a-listing-option (2015-03-12) 1 commit - tag: fix some mis-organized options in -h listing git tag -h used to show the --column and --sort options that are about listing in a wrong section. Will merge to 'next'. * as/userdiff-sh (2015-03-13) 1 commit - userdiff: funcname and word patterns for sh * bc/object-id (2015-03-13) 10 commits - apply: convert threeway_stage to object_id - patch-id: convert to use struct object_id - commit: convert parts to struct object_id - diff: convert struct combine_diff_path to object_id - bulk-checkin.c: convert to use struct object_id - zip: use GIT_SHA1_HEXSZ for trailers - archive.c: convert to use struct object_id - bisect.c: convert leaf functions to use struct object_id - define utility functions for object IDs - define a structure for object IDs * ct/prompt-untracked-fix (2015-03-13) 1 commit - git prompt: use toplevel to find untracked files The prompt script (in contrib/) did not show the untracked sign when working in a subdirectory without any untracked files. * jk/smart-http-hide-refs (2015-03-12) 2 commits - upload-pack: do not check NULL return of lookup_unknown_object - upload-pack: fix transfer.hiderefs over smart-http The transfer.hiderefs support did not quite work for smart-http transport. Will merge to 'next'. * jk/test-annoyances (2015-03-12) 5 commits - t5551: make EXPENSIVE test cheaper - t5541: move run_with_cmdline_limit to test-lib.sh - t: pass GIT_TRACE through Apache - t: redirect stderr GIT_TRACE to descriptor 4 - t: translate SIGINT to an exit Will merge to 'next'. * nd/config-doc-camelCase (2015-03-13) 1 commit - *config.txt: stick to camelCase naming convention Will merge to 'next'. -- [Stalled] * jk/push-config (2015-02-17) 4 commits - [NEEDSACK] push: allow --follow-tags to be set by config push.followTags - cmd_push: pass flags pointer to config callback - cmd_push: set atomic bit directly - git_push_config: drop cargo-culted wt_status pointer Waiting for Ack and/or update for the tip one from Dave Olszewski ($gmane/263880, $gmane/263991). * nd/untracked-cache (2015-03-12) 24 commits - git-status.txt: advertisement for untracked cache - untracked cache: guard and disable on system changes - mingw32: add uname() - t7063: tests for untracked cache - update-index: test the system before enabling untracked cache - update-index: manually enable or disable untracked cache - status: enable untracked cache - untracked-cache: temporarily disable with $GIT_DISABLE_UNTRACKED_CACHE - untracked cache: mark index dirty if untracked cache is updated - untracked cache: print stats with $GIT_TRACE_UNTRACKED_STATS - untracked cache: avoid racy timestamps - read-cache.c: split racy stat test to a separate function - untracked cache: invalidate at index addition or removal - untracked cache: load from UNTR index extension - untracked cache: save to an index extension - ewah: add convenient wrapper ewah_serialize_strbuf() - untracked cache: don't open non-existent .gitignore - untracked cache: mark what dirs should be recursed/saved - untracked cache: record/validate dir mtime and reuse cached output - untracked cache: make a wrapper around {open,read,close}dir() - untracked cache: invalidate dirs recursively if .gitignore changes - untracked cache: initial untracked cache validation - untracked cache: record .gitignore information and dir hierarchy - dir.c: optionally compute sha-1 of a .gitignore file Need extra sets of eyes to review this. * ak/stash-store-create-help (2015-01-13) 1 commit - stash: show create and store subcommands in usage-help Will discard. * bp/diff-relative-config (2015-01-07) 2 commits - diff: teach diff.relative to give default to --relative=value - diff: teach --no-relative to override earlier --relative No comments? No interests? * jc/diff-b-m (2015-02-23) 5 commits . WIPWIP . WIP: diff-b-m - diffcore-rename: allow easier debugging - diffcore-rename.c: add locate_rename_src() - diffcore-break: allow debugging git diff -B -M produced incorrect patch when the postimage of a completely rewritten file is similar to the preimage of a removed file; such a resulting file must not be expressed as a rename from other place. The fix in this patch is broken, unfortunately. * pw/remote-set-url-fetch (2014-11-26) 1 commit - remote: add --fetch and --both options to set-url Expecting a reroll. * je/quiltimport-no-fuzz
[ANNOUNCE] Git v2.3.3
The latest maintenance release Git v2.3.3 is now available at the usual places. It is comprised of 26 non-merge commits since v2.3.2, contributed by 11 people, 1 of which is a new contributor. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.3.3' tag and the 'maint' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git New contributors whose contributions weren't in v2.3.2 are as follows. Welcome to the Git development community! Mikko Rapeli. Returning contributors who helped this release are as follows. Thanks for your continued support. Anders Kaseorg, Ben Walton, Jeff King, Johannes Sixt, Junio C Hamano, Mårten Kongstad, Michael J Gruber, Michal Sojka, Nguyễn Thái Ngọc Duy, and René Scharfe. Git v2.3.3 Release Notes Fixes since v2.3.2 -- * A corrupt input to git diff -M used cause us to segfault. * The borrowed code in kwset API did not follow our usual convention to use unsigned char to store values that range from 0-255. * Description given by grep -h for its --exclude-standard option was phrased poorly. * Documentaton for git remote add mentioned --tags and --no-tags and it was not clear that fetch from the remote in the future will use the default behaviour when neither is given to override it. * git diff --shortstat --dirstat=changes showed a dirstat based on lines that was never asked by the end user in addition to the dirstat that the user asked for. * The interaction between git submodule update and the submodule.*.update configuration was not clearly documented. * git apply was not very careful about reading from, removing, updating and creating paths outside the working tree (under --index/--cached) or the current directory (when used as a replacement for GNU patch). * git daemon looked up the hostname even when %CH and %IP interpolations are not requested, which was unnecessary. * The interpolated-path option of git daemon inserted any string client declared on the host= capability request without checking. Sanitize and limit %H and %CH to a saner and a valid DNS name. Also contains typofixes, documentation updates and trivial code clean-ups. Changes since v2.3.2 are as follows: Anders Kaseorg (1): t5516: correct misspelled pushInsteadOf Ben Walton (1): kwset: use unsigned char to store values with high-bit set Jeff King (5): git_connect: let user override virtual-host we send to daemon t5570: test git-daemon's --interpolated-path option daemon: sanitize incoming virtual hostname diffcore-rename: split locate_rename_dst into two functions diffcore-rename: avoid processing duplicate destinations Johannes Sixt (1): test_ln_s_add: refresh stat info of fake symbolic links Junio C Hamano (11): apply: reject input that touches outside the working area apply: do not read from the filesystem under --index apply: do not read from beyond a symbolic link apply: do not touch a file beyond a symbolic link t4008: correct stale comments t9300: correct expected object names t4010: correct expected object names tests: do not borrow from COPYING and README from the real source t/diff-lib: check exact object names in compare_diff_raw t4008: modernise style Git 2.3.3 Michael J Gruber (1): git-remote.txt: describe behavior without --tags and --no-tags Michal Sojka (1): submodule: improve documentation of update subcommand Mikko Rapeli (1): Documentation/git-clean.txt: document that -f may need to be given twice Mårten Kongstad (1): diff --shortstat --dirstat: remove duplicate output Nguyễn Thái Ngọc Duy (1): grep: correct help string for --exclude-standard René Scharfe (2): daemon: look up client-supplied hostname lazily daemon: use callback to build interpolated path -- 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/GSoC/RFC] fetch: git fetch --deepen
On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano gits...@pobox.com wrote: Dongcan Jiang dongcan.ji...@gmail.com writes: What does (you) exactly mean in [1]? The local branch or the local remote ref? As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. That would make it harder to implement fetch --deepen than the version that moves refs if they are updated. And I think what Dongcan implemented moves refs. From the user point of view, I think it's ok with either version, so the one that's easier to implement wins. The you does not explicitly depict any ref, because the picture is meant to illustrate _everything_ the repository at the receiving end of fetch has. It used to have two commits, A and B (and the tree and blob objects necessary to complete these two commits). After deepening the history by one commit, it then will have commit A^ and its trees and blobs. [1] http://article.gmane.org/gmane.comp.version-control.git/212950 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags
Jeff King p...@peff.net writes: From: Dave Olszewski cx...@pobox.com Signed-off-by: Dave Olszewski cx...@pobox.com --- Again, this is just a preview. Dave should send the final when he thinks it is good. Dave? I do not see anything wrong with this version that builds on top of the previous 2 clean-up. Personally I find that these clean-up changes more valuable than I care about this particular feature, and it is unfortunate that waiting an Ack or reroll of this one kept them stalled. I am tempted to throw Helped-by: Peff into the log message and merge the result to 'next', unless I hear otherwise in a few days. The if/else I added to the config callback is kind of ugly. I wonder if we should have git_config_bit, or even just a function to set/clear a bit. Then the OPT_BIT code could use it, too. Something like: munge_bit(flags, TRANSPORT_PUSH_FOLLOW_TAGS, git_config_bool(k, v)); Or maybe that is getting too fancy and obfuscated for a simple bit set/clear. I dunno. I think we agreed that the code we have in this series is good. Documentation/config.txt | 6 ++ Documentation/git-push.txt | 5 - builtin/push.c | 9 + contrib/completion/git-completion.bash | 1 + 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ae6791d..e01d21c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2079,6 +2079,12 @@ new default). -- +push.followTags:: + If set to true enable '--follow-tags' option by default. You + may override this configuration at time of push by specifying + '--no-follow-tags'. + + rebase.stat:: Whether to show a diffstat of what changed upstream since the last rebase. False by default. diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index ea97576..caa187b 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -128,7 +128,10 @@ already exists on the remote side. Push all the refs that would be pushed without this option, and also push annotated tags in `refs/tags` that are missing from the remote but are pointing at commit-ish that are - reachable from the refs being pushed. + reachable from the refs being pushed. This can also be specified + with configuration variable 'push.followTags'. For more + information, see 'push.followTags' in linkgit:git-config[1]. + --signed:: GPG-sign the push request to update refs on the receiving diff --git a/builtin/push.c b/builtin/push.c index c25108f..6831c2d 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -482,6 +482,7 @@ static int option_parse_recurse_submodules(const struct option *opt, static int git_push_config(const char *k, const char *v, void *cb) { + int *flags = cb; int status; status = git_gpg_config(k, v, NULL); @@ -511,6 +512,14 @@ static int git_push_config(const char *k, const char *v, void *cb) return 0; } + if (!strcmp(k, push.followtags)) { + if (git_config_bool(k, v)) + *flags |= TRANSPORT_PUSH_FOLLOW_TAGS; + else + *flags = ~TRANSPORT_PUSH_FOLLOW_TAGS; + return 0; + } + return git_default_config(k, v, NULL); } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c21190d..cffb2b8 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2188,6 +2188,7 @@ _git_config () pull.octopus pull.twohead push.default + push.followTags rebase.autosquash rebase.stat receive.autogc -- 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 1/4] git-credential-store: support multiple credential files
On Fri, Mar 13, 2015 at 2:15 PM, Jeff King p...@peff.net wrote: +static void store_credential(const struct string_list *fns, struct credential *c, + const char *default_fn) I think you could even get away without passing default_fn here, and just use the rule the first file in the list is the default. Unless you are anticipating ever passing something else, but I couldn't think of a case where that would be useful. Even though in this case the store_credential() function is not used anywhere else, from my personal API design experience I think that cementing the rule of the first file in the list is the default in the behavior of the function is not a good thing. For example, in the future, we may wish to keep the precedence ordering the same, but if none of the credential files exist, we create the XDG file by default instead. It's a balance of flexibility, but in this case I think putting the default filename in a separate argument is a good thing. + struct string_list fns = STRING_LIST_INIT_NODUP; This is declared NODUP... - if (!file) + if (file) + string_list_append_nodup(fns, file); So you can just call the regular string_list_append here (the idea of declaring the list as DUP or NODUP is that the callers do not have to care; string_list_append does the right thing). Actually, thinking about it again from a memory management perspective, STRING_LIST_INIT_DUP should be used instead as the string_list `fns` should own the memory of the strings inside it. Thus, in this case since `file` is pulled from the argv array, string_list_append() should be used to duplicate the memory, and for expand_user_path(), string_list_append_nodup() should be used because the returned path is newly-allocated memory. Finally, string_list_clear() should be called at the end to release memory. Thanks for taking the time to review the patches, I will work on v4 now. (Really keen on getting this to pu) Regards, Paul -- 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/GSoC/RFC] fetch: git fetch --deepen
On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote: @@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args, if (is_repository_shallow()) write_shallow_commits(req_buf, 1, NULL); if (args-depth 0) - packet_buf_write(req_buf, deepen %d, args-depth); + packet_buf_write(req_buf, depth %d, args-depth); packet_buf_flush(req_buf); state_len = req_buf.len; Junio already questioned about your replacing starts_with(deepen ..) with starts_with(depth ..), this is part of that question. If you run make test you should find some breakages, or we need to improve our test suite. I think you just need to keep the old if (args-depth 0) send depth unchanged and add two new statements that check args-depth_deepen and sends depth . BTW, I think deepen-more or deepen-relative would be better than depth (*), which is a noun. But that's a minor point at this stage. And because --depth and --deepen are mutually exclusive, you need to make sure that the user must not specify that. The user includes the client from the server perspective, we need to make sure that upload-pack barf if some client sends both deepen and depth. (*) I was about to suggest you use deepen for both --depth and --deepen: --depth sends deepen NUM while --deepen sends deepen +NUM. The protocol as described in pack-protocol.txt may allow this extension. Unfortunately the current implementation is too relaxed. We use strtol() to parse NUM and it would accept the leading + instead of barfing out. So old clients would mistakes --deepen as --depth, not good. And it even accepts base 8 and 16!! We should fix this. -- Duy -- 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 01/16] list-files: command skeleton
On Fri, Mar 13, 2015 at 4:02 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: list-files is supposed to be the user friendly version of ls-files, or an alternative to git-status. Nothing fancy in this patch yet. The result of applying this patch alone will not give us anything fancy, but the patch itself is interesting ;-) That's the point. Fancy stuff comes as separate, bite-size patches. +static void populate_cached_entries(struct string_list *result, + const struct index_state *istate) +{ + int i; + + for (i = 0; i istate-cache_nr; i++) { + const struct cache_entry *ce = istate-cache[i]; + + if (!match_pathspec(pathspec, ce-name, ce_namelen(ce), + 0, NULL, + S_ISDIR(ce-ce_mode) || + S_ISGITLINK(ce-ce_mode))) Because we won't tell the user You gave me Mkaefile but that did not match when git list-files Mkaefile does not produce anything, we do not need to pass seen[] down from here. We probably want that feature back. Not implemented yet though. + prefix = cmd_prefix; + if (prefix) + prefix_length = strlen(prefix); + + if (read_cache() 0) + die(_(index file corrupt)); + + git_config(ls_config, NULL); + + argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0); + + parse_pathspec(pathspec, 0, +PATHSPEC_PREFER_CWD | +PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP, +cmd_prefix, argv); + pathspec.max_depth = 0; + pathspec.recursive = 1; + + refresh_index(the_index, REFRESH_QUIET | REFRESH_UNMERGED, + pathspec, NULL, NULL); It would be better to do read-cache-preload, instead of read-cache, if you are going to immediately refresh. That is what git status does. parse_options(), or *_SLASH_CHEAP to be exact, needs the index being loaded (but not necesarily refreshed), so we can't simply move it closer to refresh_index() after we have got the pathspec. And doing read-cache-preload() without pathspec where read_cache() is seems a waste of lstat() because the default mode is only look at cwd and subdirs, not full worktree like git-status. I have a patch (not sent yet) that adds read-cache-preload back. But this 01/16 will likely stay simple and unoptimized. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/10] Define a structure for object IDs.
On Fri, Mar 13, 2015 at 1:03 PM, Junio C Hamano gits...@pobox.com wrote: To me, pack, offset information smells to belong more to a struct object (or its subclass) as an optional annotation---when a caller is asked to parse_object(), you would bypass the sha1_read_file() that goes and looks the object name up from the list of pack .idx and instead go there straight using that annotation. For pack v4, commits and trees can be encoded this way. Even if your in-pack representation of a commit object allowed to store the tree pointer in pack, nth format, its object name must be computed as if you have the commit object in the traditional format and computed the hash of that (together with the standard type size\0 header), and at that point, you need the contained object's name in sha-1 format (imagine how you would implement the commit-tree command). True. But commit-tree is not often executed as, say, rev-list, where the hash is used as 20-byte key to some pieces somewhere (pack data, struct object *). If we keep pack, nth in object_id (the union approach), v4-aware code has the chance to go on a fast path. Hence, I do not think the v4 encoding changes the discussion very much. I see the primary value of v4 encoding is to shorten the length of various fields take on-disk and in-pack. If it were pack, offset, it could be argued that it would also be faster to get to the packed data in the packfile, and going from pack, nth to the .idx file and then going to the location in the data in the packfile would be faster than going from sha-1 to a particular pack and its in-pack offset, with the difference of cost around log(n)*m where n is the number of objects in a pack and m is the total number of packs in the repository. It is true that nth format (force that the referred-to object lives in the same pack as the referrer) can help speed up interpretation of extended SHA-1 expression, e.g. v1.0^0:t, which can read v1.0 tag in v4 format, find the nth info for the commit pointed by the tag and get to that data in the pack, find the nth info for the top-tree recorded in that commit and directly get to the data of that tree, and then find the entry for t, which will give the object name for that subtree again in nth format, and at that point you can find the sha-1 of that final object, without having to know any object names of the intermediate objects (i.e. you must start from sha-1 of the tag you obtain from the refs API, but you didn't use the object name of the commit and its top-level tree). So for such a codepath, I would say it would be sufficient to use a union in struct people have been envisioning and convert pack, nth to sha-1 when the latter form becomes necessary for the first time for the object. It's the point: faster access (to either pack data, or struct object *). Using a separate union from struct object_id (iow, object_id only contains SHA-1) works. But imagine to independent code islands can perform this type of fast access. In order to pass this union from one island to another, we need to update pretty much every function call between them. The change to using object_id has a similar impact. If we go with union in object_id, we may be able to avoid the second mass change to make use of pack, nth. Anyway, wouldn't this be all academic? I do not see how you would keep the object name in the pack, nth format in-core, as the obj_hash[] is a hashtable keyed by sha-1, and even when we switch to a different hash, I cannot see how such a table to ensure the singleton-ness of in-core objects can be keyed sometimes by hash and by pack, nth in some other time. I'm implementing something to see how much we gain by avoiding object lookup. The current approach is having struct object ** obj in struct packed_git, indexed by nth. So when you have pack, nth and pack-obj[nth] is valid, you'll get to struct object * without hashing. If pack-obj[nth] is NULL, hashing and looking up are required the first time using SHA-1, then the result is cached in pack-obj[]. So no, it's not academic :) -- Duy -- 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] contrib/completion: escape the forward slash in __git_match_ctag
The current definition results in an incorrect expansion of the term under zsh. For instance /^${1\\/}/ under zsh with the argument hi results in: /^/\/h/\/i/ This results in an output similar to this when trying to complete `git grep chartab` under zsh: :: git grep chartabawk: cmd. line:1: /^/\/c/\/h/\/a/\/r/\/t/\/a/\/b/ { print $1 } awk: cmd. line:1:^ backslash not last character on line awk: cmd. line:1: /^/\/c/\/h/\/a/\/r/\/t/\/a/\/b/ { print $1 } awk: cmd. line:1:^ syntax error Leaving the prompt in a goofy state until the user hits a key. Escaping the literal / in the parameter expansion (using /^${1//\//\\/}/) results in: /^chartab/ allowing the completion to work correctly. This formulation also works under bash. Signed-off-by: John Szakmeister j...@szakmeister.net --- I've been bit by this bug quite a bit, but didn't have time to track it down until today. I hope the proposed solution is acceptable. -John contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c21190d..a899234 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1305,7 +1305,7 @@ _git_gitk () } __git_match_ctag() { - awk /^${1\\/}/ { print \$1 } $2 + awk /^${1//\//\\/}/ { print \$1 } $2 } _git_grep () -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/10] Use a structure for object IDs.
On Fri, Mar 13, 2015 at 11:39:26PM +, brian m. carlson wrote: Changes since v1: I just realized that I sent this out as v2 instead of v3. It really is v3. My apologies for the error. -- 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 02/16] list-files: make :(glob) pathspec default
On Fri, Mar 13, 2015 at 4:10 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/list-files.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/list-files.c b/builtin/list-files.c index ac33f13..b99f2b7 100644 --- a/builtin/list-files.c +++ b/builtin/list-files.c @@ -65,6 +65,8 @@ int cmd_list_files(int argc, const char **argv, const char *cmd_prefix) { struct string_list result = STRING_LIST_INIT_NODUP; + setenv(GIT_GLOB_PATHSPECS_ENVIRONMENT, 1, 0); + if (argc == 2 !strcmp(argv[1], -h)) usage_with_options(ls_usage, ls_options); Yikes. I do not have enough info at this step in the series to judge if it is sensible to force the :(glob) interpretation as default, but is it something we would want to do commonly to flip the default per Git subcommand? As this new 'ls' imitates the unix version, I try to hide this subtle difference of git. Perhaps this is a premature move because we may want to make :(glob) default for most commands at 3.0 or something, except plumbing commands. If so, using the environment feels like a clunkyway to do that. How about a two-patch clean-up before this step? (1) remove the handling of literal_global and friends that peek into various environment variables from prefix_pathspec(), which is a function that is repeatedly called for each pathspec element given from the command line, and move that logic to parse_pathspec(); pass necessary information down to prefix_pathspec() as parameter(s); (2) allow parse_pathspec() so that the caller can say the default, when there is no environment variable given by the end user to tell us otherwise, is to :(glob). Sounds like a good thing to do anyway. Will do. -- Duy -- 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 03/16] list-files: show paths relative to cwd
On Fri, Mar 13, 2015 at 4:28 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: static struct pathspec pathspec; static const char *prefix; @@ -22,7 +23,7 @@ static void add_one(struct string_list *result, const char *name) struct strbuf sb = STRBUF_INIT; struct string_list_item *item; -strbuf_addstr(sb, name); +quote_path_relative(name, prefix_length ? prefix : NULL, sb); item = string_list_append(result, strbuf_detach(sb, NULL)); item-util = (char *)name; } Hmph, wouldn't it make it more cumbersome and problematic to do things like this here in add_one() phase? I am imagining that the endgame of this program will be - populate_cached_entries() reads from the data source (at this step, there is just the index), calling add_one() whose responsibility is to record the paths that are interesting to an in-core structure; - perform some interesting filtering, annotating, ordering, etc. (at this step, there is none) yet to come; - display() will iterate over the result and then format the result. For example, if you do the quote thing too early, don't codepaths in the later phases have to worry about item-string not matching the original pathname anymore? If you want to do something like /bin/ls -t, you may have to lstat() the paths for each item, but if these store a path relative to the prefix, wouldn't you have to prepend the prefix again before running lstat()? I am just wondering if this prefix-stripping and quoting belongs to the output phase, not the input phase. Hmph, another interpretation of this patch is that your item-string are not the true filenames but the result of applying some interesting processing to the filenames and the true filenames are kept in item-util. Is that what is going on? Exactly. We would need to sort and stuff later on, so true filenames are preserved in util-item. A cleaner way is perhaps carry all metadata in item-util and item-string remains true filename, then do all the formatting, coloring for all strings just before displaying. It seems a lot varying data to carry around. If that is the case, it sort of makes sense to me, even though it would feel a bit unusual way to use the string-list. Thanks. -- Duy -- 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
Want to work on Git in GSoc 2015
Sir I am a third year undergraduate student in Computer Science .I have good knowledge in C. I read all the micro projects ideas and interested to work on it . So kindly tell me how should I start . -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] userdiff: funcname and word patterns for sh
On overall, the patch looks good. Some suggestions to improve the tests and a minor nitpick below. Adrien Schildknecht adrien+...@schischi.me writes: +++ b/t/t4034/sh/post @@ -0,0 +1,36 @@ +foo() {lsecho} This part is unchanged here and in the pre file. What does it test? +$((x++)) +$((x--)) +$((--x)) +$((++x)) +$((x*y)) +$((xy)) +$((x**y)) +$((x/y)) +$((x%y)) +$((x+y)) +$((x-y)) +[ x=y ] +[ x=y ] +[ x==y ] +[ x!=y ] Not sure what the last ones are testing. If it's [ as the test command, spelled as [, then spaces are mandatory around the operators (and equality should be written =, not == in POSIX). +xy xy x-y xy xy x|y xy xy xy +xy +xy +x|y +x||y +x=y +$((x+=y)) +$((x-=y)) +$((x*=y)) +$((x/=y)) +$((x%=y)) +$((x=y)) +$((x=y)) +$((x=y)) +$((x^=y)) +$((x|=y)) I think you should test the case of multiple-letters identifiers. One of the benefit of having a proper word-diff pattern is that e.g. - pre=foo + post=bar will consider the change pre - post, and not an unmodified p with the change re - ost (otherwise, --color-words=. just works). +PATTERNS(sh, + ^([ \t]*(function[ \t]+)?[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\).*)$, + /* -- */ + [a-zA-Z0-9_]+ Nitpick: the indentation is not homogeneous. You should add a space after the tab on the first two lines to get a correct alignment. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 1/4] git-credential-store: support multiple credential files
On Sat, Mar 14, 2015 at 04:15:53PM +0800, Paul Tan wrote: Even though in this case the store_credential() function is not used anywhere else, from my personal API design experience I think that cementing the rule of the first file in the list is the default in the behavior of the function is not a good thing. For example, in the future, we may wish to keep the precedence ordering the same, but if none of the credential files exist, we create the XDG file by default instead. It's a balance of flexibility, but in this case I think putting the default filename in a separate argument is a good thing. Yeah, I see your line of reasoning. I think this is probably a case of YAGNI, but it is really a matter of personal preference. It's not a big deal either way. So you can just call the regular string_list_append here (the idea of declaring the list as DUP or NODUP is that the callers do not have to care; string_list_append does the right thing). Actually, thinking about it again from a memory management perspective, STRING_LIST_INIT_DUP should be used instead as the string_list `fns` should own the memory of the strings inside it. Thus, in this case since `file` is pulled from the argv array, string_list_append() should be used to duplicate the memory, and for expand_user_path(), string_list_append_nodup() should be used because the returned path is newly-allocated memory. Finally, string_list_clear() should be called at the end to release memory. Yeah, I had the same thought, but I noticed that you don't call string_list_clear. Nor is it really necessary to do so. Since git programs are generally short-lived, we often let the OS take care of deallocating variables like this (it's not appropriate for all variables, of course, but quite frequently there are variables that are effectively allocated at program startup and used until program end). Again, I think this is a matter of taste. I don't mind if you want to string_list_clear at the end, and I agree that using nodup() is the right thing in that case. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags
On Fri, Mar 13, 2015 at 11:06:22PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: From: Dave Olszewski cx...@pobox.com Signed-off-by: Dave Olszewski cx...@pobox.com --- Again, this is just a preview. Dave should send the final when he thinks it is good. Dave? I do not see anything wrong with this version that builds on top of the previous 2 clean-up. Personally I find that these clean-up changes more valuable than I care about this particular feature, and it is unfortunate that waiting an Ack or reroll of this one kept them stalled. I am tempted to throw Helped-by: Peff into the log message and merge the result to 'next', unless I hear otherwise in a few days. FWIW, as the author of the leadup patches, that would be fine with me. I think the end patch is in fine shape. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] git-credential-store: support multiple credential files
On Sat, Mar 14, 2015 at 01:33:28PM -0400, Jeff King wrote: On Sat, Mar 14, 2015 at 04:15:53PM +0800, Paul Tan wrote: Even though in this case the store_credential() function is not used anywhere else, from my personal API design experience I think that cementing the rule of the first file in the list is the default in the behavior of the function is not a good thing. For example, in the future, we may wish to keep the precedence ordering the same, but if none of the credential files exist, we create the XDG file by default instead. It's a balance of flexibility, but in this case I think putting the default filename in a separate argument is a good thing. Yeah, I see your line of reasoning. I think this is probably a case of YAGNI, but it is really a matter of personal preference. It's not a big deal either way. By the way, I hope this (and the other comment) do not come off as you are wrong, but I do not feel like arguing with you. I really do think these are a matter of taste, and while we often express issues of taste in reviews, it is ultimately up to the patch submitter (who is, after all, doing most of the work) to have the final say on minor issues like this. Sometimes the response to taste issue is oh, I didn't think to do that, thanks for the suggestion and sometimes it is nah, I like it better my way. And both are OK. Of course there are also taste issues where we insist (e.g., consistent whitespace), but I do not think this is one of them. :) Maybe that was all obvious, but since you are new to the list, I wanted to make sure I was clear. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags
On Fri, 13 Mar 2015, Junio C Hamano wrote: Jeff King p...@peff.net writes: From: Dave Olszewski cx...@pobox.com Signed-off-by: Dave Olszewski cx...@pobox.com --- Again, this is just a preview. Dave should send the final when he thinks it is good. Dave? I do not see anything wrong with this version that builds on top of the previous 2 clean-up. Personally I find that these clean-up changes more valuable than I care about this particular feature, and it is unfortunate that waiting an Ack or reroll of this one kept them stalled. I am tempted to throw Helped-by: Peff into the log message and merge the result to 'next', unless I hear otherwise in a few days. Sorry, work has kept me very busy lately, I haven't had time to re-visit this. Jeff's version looks great to me, please go ahead with it. Thanks everyone. Dave -- 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
protect branches against deletion from remote
Hi, If you try to delete a local branch feature which is not merged into another branch, you will get the following warning: . error: The branch 'feature' is not fully merged. . If you are sure you want to delete it, run 'git branch -D feature'. If a local branch is worth to be protected in such a way, why not the remote tracking one: $ git push origin :feature no warning, just deleted. Well i can set receive.denyDeletes on the remote repository, but this completely removes the ability to delete remote branches. How to achieve a similar behavior as with local branches? (BTW. with git push . :feature it is also possible to force deletion of local, unmerged branches) Best regards Henning -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git.c: two memory leak fixes
This patch provides fixes for two minor memory leaks in the handle_alias function: 1. We allocate memory for alias_argv with the xmalloc function call, after run_command_v_opt function will be executed we no need in this variable anymore, so let's free it. 2. Memory allocated for alias_string variable in the alias_lookup function, need to free it. Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- git.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git.c b/git.c index 086fac1..501e5bd 100644 --- a/git.c +++ b/git.c @@ -252,6 +252,7 @@ static int handle_alias(int *argcp, const char ***argv) alias_argv[argc] = NULL; ret = run_command_v_opt(alias_argv, RUN_USING_SHELL); + free(alias_argv); if (ret = 0) /* normal exit */ exit(ret); @@ -259,6 +260,7 @@ static int handle_alias(int *argcp, const char ***argv) alias_command, alias_string + 1); } count = split_cmdline(alias_string, new_argv); + free(alias_string); if (count 0) die(Bad alias.%s string: %s, alias_command, split_cmdline_strerror(count)); -- 2.3.3.469.g69a3822.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