Re: [PATCH v5 02/14] trailer: process trailers from file and arguments
Christian Couder chrisc...@tuxfamily.org writes: Many entries with the same key but distinct values can be configured using: if_exists = add_if_different if_missing = add Many entries with the same key but values that can be the same can be configured using: if_exists = add if_missing = add While the above certainly can express the combinations, I am still puzzled about the value of having under this condition (i.e. if-exists/if-missing) and do this (i.e. add/add-if-different) as two separate concepts. If you do not have an existing entry with the same key, no new entry can be the same as an existing one---therefore, the former allow only distinct values for the same key can just say trailer.Fixes.action = add_if_different or something, without any if_exists/if_missing. In a similar way, the latter duplicated values allowed for the same key can say trailer.Sob.action = add You can throw into the mix other actions like add_if_missing, you can also express only one value allowed for the same key---the first one wins, replace to mean replace if there is one with the same key, and replace_or_add to mean same as 'replace', but add if there is no existing entries with the same key. Will we lose expressiveness by simplifying the semantics, getting rid of this under this condition part and instead making do this part more intelligent? Even if we assume, for the sake of discussion, that it *is* a good idea to separate under this condition part and do this part, I do not think the above is the only or the best way to express distinct values allowed for the same key. How do we argue that this from your example if_exists = add_if_different if_missing = add is a better design than this, for example? if_key_value_exists = ignore ; exact matching key,val exists if_key_exists = add ; otherwise if_missing = add The latter splits remaining conditional logic from do this part (no more add_if_different that causes add action to see if there is the same key and act differently) and moves it to under this condition part. I am trying to illustrate that the line to split the under this condition and do this looks arbitrary and fuzzy here, and because of this arbitrary-ness and fuzziness, it is not very obvious to me why it is a good idea to have under this condition as a separate concept from do this settings. What is the advantage of having such an extra axis? Does it make it easier to manage? Does it allow us to express more? I can see that the location where a new entry (or a duplicated one) is added (i.e. do we prepend? do we append?) can be orthogonal to any of the above; no need to bring up 'where' in the discussion. -- 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 1/4] docs/merge-strategies: remove hyphen from mis-merges
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: [GIT PULL] l10n updates for 1.9.0 round 2
Thansk. -- 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/4] docs/git-clone: clarify use of --no-hardlinks option
Albert L. Lash, IV albert.l...@gmail.com writes: Current text claims optimization, implying the use of hardlinks, when this option ratchets down the level of efficiency. This change explains the difference made by using this option, namely copying instead of hardlinking, and why it may be useful. Signed-off-by: Albert L. Lash, IV ala...@bloomberg.net --- Documentation/git-clone.txt | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index bf3dac0..0363d00 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -55,15 +55,12 @@ repository is specified as a URL, then this flag is ignored (and we never use the local optimizations). Specifying `--no-local` will override the default when `/path/to/repo` is given, using the regular Git transport instead. -+ -To force copying instead of hardlinking (which may be desirable if you -are trying to make a back-up of your repository), but still avoid the -usual Git aware transport mechanism, `--no-hardlinks` can be used. --no-hardlinks:: - Optimize the cloning process from a repository on a - local filesystem by copying files under `.git/objects` - directory. + Force the cloning process from a repository on a local + filesystem to copy the files under the `.git/objects` + directory instead of using hardlinks. This may be desirable + if you are trying to make a back-up of your repository. Makes sense, and it is kind of embarrassing (I have to suspect that this was originally the description of --hardlinks option). [PATCH {1,2}/4] looked trivially correct, too. 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 4/4] docs/git-blame: explain more clearly the example pickaxe use
Albert L. Lash, IV albert.l...@gmail.com writes: We state that the following paragraph mentions the pickaxe interface, but the term pickaxe is not then used. This change clarifies that the example command uses the pickaxe interface and what it is searching for. Signed-off-by: Albert L. Lash, IV ala...@bloomberg.net --- Documentation/git-blame.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt index 8e70a61..ddb88d0 100644 --- a/Documentation/git-blame.txt +++ b/Documentation/git-blame.txt @@ -35,7 +35,8 @@ Apart from supporting file annotation, Git also supports searching the development history for when a code snippet occurred in a change. This makes it possible to track when a code snippet was added to a file, moved or copied between files, and eventually deleted or replaced. It works by searching for -a text string in the diff. A small example: +a text string in the diff. A small example of the pickaxe interface +that searches for `blame_usage`: - $ git log --pretty=oneline -S'blame_usage' Thanks. I cannot shake this nagging feeling that this and the latter half of the previous paragraph may not belong to this page, though. 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: [PATCH 00/11] More preparatory work for multiparent tree-walker
Kirill Smelkov k...@mns.spb.ru writes: Here I'm preparing tree-diff.c to be ready for the new tree-walker, so that the final change is tractable and looks good and non noisy. Some small speedups are gained along the way. The final bits are almost ready, but I don't want to release them in a hurry. No worries. We are not in a hurry to apply non-regression-fix changes during a pre-release feature freeze period anyway. This seems to somehow conflict with other topics and does not cleanly apply on top of your other tree-diff topic, by the way. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a
Duy Nguyen pclo...@gmail.com writes: On Tue, Feb 11, 2014 at 1:46 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/setup.c b/setup.c index 6c3f85f..b09a412 100644 --- a/setup.c +++ b/setup.c @@ -787,3 +787,27 @@ void sanitize_stdfds(void) if (fd 2) close(fd); } + +int daemonize(void) +{ +#ifdef NO_POSIX_GOODIES + errno = -ENOSYS; Negated? Facepalm. I remember I wrote this somewhere but don't remember what topic :( Should I resend? I've already amended it here. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/14] trailer: process trailers from file and arguments
Christian Couder christian.cou...@gmail.com writes: Even if we assume, for the sake of discussion, that it *is* a good idea to separate under this condition part and do this part, I do not think the above is the only or the best way to express distinct values allowed for the same key. How do we argue that this from your example if_exists = add_if_different if_missing = add is a better design than this, for example? if_key_value_exists = ignore ; exact matching key,val exists if_key_exists = add ; otherwise if_missing = add The question is what happens if we want to say if the same key, value pair exists but not near where we would add. With the solution I implemented, that is: ... With the solution you suggest, should we have: ... if_key_value_exists_not_neighbor = add ; exact matching key,val ... or: if_key_value_exists = ignore_if_neighbor ; exact matching key,val exists ... And what happens if we want to say if key exists and value matches regex, how do we express that then? Or what happens when we want to say if key exists and command succeeds? ... With what I suggest, we can still say: ... And then people might ask if they can also use something like this: ... and it will look like we are trying too much to do what should be done in only one script. I think the above illustrates my point very clearly. These numerous questions you have to ask are indications why choosing this condition goes to the left hand side of the equal sign (e.g. exists) and this condition goes to the right hand side (e.g. do-this-if_neighbor) is not working well. The user has to remember which conditions go to the variable name and which conditions go to the action part. And the made-up alternative you listed above is not a solution I suggest to begin with---it is a strawman if we assume such a splitting were a good idea in the first place ;-). What I was wondering if it is an better alternative was the below. The latter splits remaining conditional logic from do this part (no more add_if_different that causes add action to see if there is the same key and act differently) and moves it to under this condition part. ... I am trying to illustrate that the line to split the under this condition and do this looks arbitrary and fuzzy here, and because of this arbitrary-ness and fuzziness, it is not very obvious to me why it is a good idea to have under this condition as a separate concept from do this settings. That is, not splitting the logic into two parts like you did, i.e. if_X = do_Y_if_Z, which invites why is it not if_true = do_Y_if_X_and_Z, if_X_and_Z = do_Y, if_Z = do_Y_if_X? One possible way to avoid the confusion is to say do_Y_if_X_and_Z without making the rule split into conditions and actions---I am NOT saying that is _better_, there may be other solutions to avoid this two-part logic in a cleaner way. -- 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: [RFH] hackday and GSoC topic suggestions
Jeff King p...@peff.net writes: - Branch rename breaks local downstream branches http://article.gmane.org/gmane.comp.version-control.git/241228 If you have a branch B that builds on A, if you are renaming A to C, you may want B to automatically set to build on C in some cases, and in other cases your renaming A is done explicitly in order to severe the tie between A and B and setting the latter to build on C can be a bad thing---after all, the user's intention may be to create a branch A starting at some commit immediately after this rename so that B will keep building on that updated A. So I am not sure if this is a bug. - git stash doesn't use --index as default http://article.gmane.org/gmane.comp.version-control.git/235892 I tend to think git stash was designed to work this way from day one. - using git commit-tree with -F - adds trailing newlines http://article.gmane.org/gmane.comp.version-control.git/236583 According to the initial commit that introduced this option, it deliberately calls strbuf_complete_line() to make sure we do not leave an incomplete line at the end. Many of the other items in your bugs section seem to be worth fixing. 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] bash completion: Add --recurse-submodules
Keshav Kini keshav.k...@gmail.com writes: Sup Yut Sum ch3co...@gmail.com writes: Signed-off-by: Sup Yut Sum ch3co...@gmail.com --- contrib/completion/git-completion.bash | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) Aren't you missing a commit message? The title itself is almost sufficient, I would think. It may need to mention that this is only for fetch, pull and push. I'll tentatively queue the following. Stripping the leftmost constant string with ${var##constant} looks somewhat strange (why wouldn't a single # work?), but that is not a new problem this patch introduces, and can be cleaned up separately if/when somebody wants to. -- 8 -- From: Sup Yut Sum ch3co...@gmail.com Date: Sun, 9 Feb 2014 22:35:31 +0800 Subject: [PATCH] completion: teach --recurse-submodules to fetch, pull and push Signed-off-by: Sup Yut Sum ch3co...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- contrib/completion/git-completion.bash | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8aaf214..c044a68 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1221,14 +1221,20 @@ _git_difftool () __git_complete_revlist_file } +__git_fetch_recurse_submodules=yes on-demand no + __git_fetch_options= --quiet --verbose --append --upload-pack --force --keep --depth= - --tags --no-tags --all --prune --dry-run + --tags --no-tags --all --prune --dry-run --recurse-submodules= _git_fetch () { case $cur in + --recurse-submodules=*) + __gitcomp $__git_fetch_recurse_submodules ${cur##--recurse-submodules=} + return + ;; --*) __gitcomp $__git_fetch_options return @@ -1577,6 +1583,10 @@ _git_pull () __git_complete_strategy return case $cur in + --recurse-submodules=*) + __gitcomp $__git_fetch_recurse_submodules ${cur##--recurse-submodules=} + return + ;; --*) __gitcomp --rebase --no-rebase @@ -1589,6 +1599,8 @@ _git_pull () __git_complete_remote_or_refspec } +__git_push_recurse_submodules=check on-demand + _git_push () { case $prev in @@ -1601,10 +1613,15 @@ _git_push () __gitcomp_nl $(__git_remotes) ${cur##--repo=} return ;; + --recurse-submodules=*) + __gitcomp $__git_push_recurse_submodules ${cur##--recurse-submodules=} + return + ;; --*) __gitcomp --all --mirror --tags --dry-run --force --verbose --receive-pack= --repo= --set-upstream + --recurse-submodules= return ;; -- 1.9.0-rc3-244-g3497008 -- 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] Introduce experimental remote object access mode
Shawn Pearce spea...@spearce.org writes: Why would you do this? Perhaps you need more time in your day to consume tea or coffee. Set GIT_RTT and enjoy a beverage. So the conclusion is that it is not practical to do a lazy fetch if it is done extremely naively at we want this object --- wait a bit and we'll give you level? I am wondering if we can do a bit better, like we want this object --- wait a bit, ah that's a commit, so it is likely that you may want the trees and blobs associated with it, too, if not right now but in a near future, let me push a pack that holds them to you? So-not-signed-off-by: this author or anyone else --- :-) sha1_file.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/sha1_file.c b/sha1_file.c index 6e8c05d..9bdcbc3 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -38,6 +38,7 @@ const unsigned char null_sha1[20]; static const char *no_log_pack_access = no_log_pack_access; static const char *log_pack_access; +static useconds_t rtt; /* * This is meant to hold a *small* number of objects that you would @@ -436,9 +437,20 @@ void prepare_alt_odb(void) read_info_alternates(get_object_directory(), 0); } +static void apply_rtt() +{ + if (!rtt) { + char *rtt_str = getenv(GIT_RTT); + rtt = rtt_str ? strtoul(rtt_str, NULL, 10) * 1000 : 1; + } + if (rtt 1) + usleep(rtt); +} + static int has_loose_object_local(const unsigned char *sha1) { char *name = sha1_file_name(sha1); + apply_rtt(); return !access(name, F_OK); } @@ -1303,6 +1315,7 @@ void prepare_packed_git(void) if (prepare_packed_git_run_once) return; + prepare_packed_git_one(get_object_directory(), 1); prepare_alt_odb(); for (alt = alt_odb_list; alt; alt = alt-next) { @@ -1439,6 +1452,7 @@ static int open_sha1_file(const unsigned char *sha1) struct alternate_object_database *alt; fd = git_open_noatime(name); + apply_rtt(); if (fd = 0) return fd; -- 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] contrib/diff-highlight: multibyte characters diff
Yoshihiro Sugi sugi1...@gmail.com writes: Signed-off-by: Yoshihiro Sugi sugi1...@gmail.com diff-highlight split each hunks and compare them as byte sequences. it causes problems when diff hunks include multibyte characters. This change enable to work on such cases by decoding inputs and encoding output as utf8 string. --- contrib/diff-highlight/diff-highlight | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight index c4404d4..49b4f53 100755 --- a/contrib/diff-highlight/diff-highlight +++ b/contrib/diff-highlight/diff-highlight @@ -2,6 +2,7 @@ use warnings FATAL = 'all'; use strict; +use Encode qw(decode_utf8 encode_utf8); # Highlight by reversing foreground and background. You could do # other things like bold or underline if you prefer. @@ -15,8 +16,9 @@ my @added; my $in_hunk; while () { + $_ = decode_utf8($_); if (!$in_hunk) { - print; + print encode_utf8($_); $in_hunk = /^$COLOR*\@/; } elsif (/^$COLOR*-/) { @@ -30,7 +32,7 @@ while () { @removed = (); @added = (); - print; + print encode_utf8($_); $in_hunk = /^$COLOR*[\@ ]/; } @@ -58,7 +60,8 @@ sub show_hunk { # If one side is empty, then there is nothing to compare or highlight. if (!@$a || !@$b) { - print @$a, @$b; + print encode_utf8($_) for @$a; + print encode_utf8($_) for @$b; return; } @@ -67,17 +70,18 @@ sub show_hunk { # stupid, and only handle multi-line hunks that remove and add the same # number of lines. if (@$a != @$b) { - print @$a, @$b; + print encode_utf8($_) for @$a; + print encode_utf8($_) for @$b; return; } my @queue; for (my $i = 0; $i @$a; $i++) { my ($rm, $add) = highlight_pair($a-[$i], $b-[$i]); - print $rm; + print encode_utf8($rm); push @queue, $add; } - print @queue; + print encode_utf8($_) for @queue; } sub highlight_pair { -- 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: [RFH] hackday and GSoC topic suggestions
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: - Branch rename breaks local downstream branches http://article.gmane.org/gmane.comp.version-control.git/241228 If you have a branch B that builds on A, if you are renaming A to C, you may want B to automatically set to build on C in some cases, and in other cases your renaming A is done explicitly in order to severe the tie between A and B and setting the latter to build on C can be a bad thing---after all, the user's intention may be to create a branch A starting at some commit immediately after this rename so that B will keep building on that updated A. So I am not sure if this is a bug. Having said that, the current behaviour of leaving B half-configured to build on a missing branch is undesirable. If we were to change this so that any branch B that used to build on branch A being renamed to build on the branch under the new name C, the user may have to do an extra --set-upstream-to A on B after recreating A if this was done to save away the current state of A to C and then keep building B on an updated A, so we may have to give _some_ clue what we are doing behind their back when we rename, e.g. $ git branch -m A C warning: branch B is set to build on C now. or something. -- 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] tests: turn on network daemon tests by default
Jeff King p...@peff.net writes: I dug in the history to see if there was any reasoning given for the current off by default setting. It looks like Junio asked for it when the original http-push tests were added, and everything else just followed that. The reasoning there was basically they're heavyweight and we might not be able to run them, but hopefully having the option variable will make that OK. Will queue, thanks. I may have originally asked for it for one reason, thinking that one reason would be enough while having another reason not to run them as well. But there would be countless silent others who have been depending on that choice. Those who run buildfarms may want to disable the networking test if the buildfarms are not isolated well, for example. They have to be told somewhere that now they need to explicitly disable these tests and how. I am in favor of this change but just pointing out possible fallouts might be larger than we think. t/lib-git-daemon.sh | 8 +--- t/lib-httpd.sh | 22 +++--- t/test-lib-functions.sh | 44 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index 1f22de2..e623bef 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -16,9 +16,10 @@ #stop_git_daemon #test_done -if test -z $GIT_TEST_GIT_DAEMON +GIT_TEST_GIT_DAEMON=$(test_tristate $GIT_TEST_GIT_DAEMON) +if test $GIT_TEST_GIT_DAEMON = false then - skip_all=git-daemon testing disabled (define GIT_TEST_GIT_DAEMON to enable) + skip_all=git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to enable) test_done fi @@ -58,7 +59,8 @@ start_git_daemon() { kill $GIT_DAEMON_PID wait $GIT_DAEMON_PID trap 'die' EXIT - error git daemon failed to start + test_skip_or_die $GIT_TEST_GIT_DAEMON \ + git daemon failed to start fi } diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index b43162e..bb900ca 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -30,9 +30,10 @@ # Copyright (c) 2008 Clemens Buchacher dri...@aon.at # -if test -z $GIT_TEST_HTTPD +GIT_TEST_HTTPD=$(test_tristate $GIT_TEST_HTTPD) +if test $GIT_TEST_HTTPD = false then - skip_all=Network testing disabled (define GIT_TEST_HTTPD to enable) + skip_all=Network testing disabled (unset GIT_TEST_HTTPD to enable) test_done fi @@ -76,8 +77,7 @@ GIT_VALGRIND_OPTIONS=$GIT_VALGRIND_OPTIONS; export GIT_VALGRIND_OPTIONS if ! test -x $LIB_HTTPD_PATH then - skip_all=skipping test, no web server found at '$LIB_HTTPD_PATH' - test_done + test_skip_or_die $GIT_TEST_HTTPD no web server found at '$LIB_HTTPD_PATH' fi HTTPD_VERSION=`$LIB_HTTPD_PATH -v | \ @@ -89,19 +89,20 @@ then then if ! test $HTTPD_VERSION -ge 2 then - skip_all=skipping test, at least Apache version 2 is required - test_done + test_skip_or_die $GIT_TEST_HTTPD \ + at least Apache version 2 is required fi if ! test -d $DEFAULT_HTTPD_MODULE_PATH then - skip_all=Apache module directory not found. Skipping tests. - test_done + test_skip_or_die $GIT_TEST_HTTPD \ + Apache module directory not found fi LIB_HTTPD_MODULE_PATH=$DEFAULT_HTTPD_MODULE_PATH fi else - error Could not identify web server at '$LIB_HTTPD_PATH' + test_skip_or_die $GIT_TEST_HTTPD \ + Could not identify web server at '$LIB_HTTPD_PATH' fi prepare_httpd() { @@ -155,9 +156,8 @@ start_httpd() { 3 24 if test $? -ne 0 then - skip_all=skipping test, web server setup failed trap 'die' EXIT - test_done + test_skip_or_die $GIT_TEST_HTTPD web server setup failed fi } diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index aeae3ca..3cc09c0 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -716,6 +716,50 @@ perl () { command $PERL_PATH $@ } +# Normalize the value given in $1 to one of true, false, or auto. The +# result is written to stdout. E.g.: +# +# GIT_TEST_HTTPD=$(test_tristate $GIT_TEST_HTTPD) +# +test_tristate () { + case $1 in + |auto) + echo auto + ;; + *) + # Rely on git-config to handle all the variants of + # true/1/on/etc that we allow. + if ! git -c magic.hack=$1 config --bool magic.hack 2/dev/null + then + # If git-config failed, it was some non-bool value like +
Re: [PATCH 00/11] More preparatory work for multiparent tree-walker
Kirill Smelkov k...@mns.spb.ru writes: Sorry for the confusion. Could you please do the following: Patches should be applied over to ks/tree-diff-walk (74aa4a18). Before applying the patches, please cherry-pick c90483d9(tree-diff: no need to manually verify that there is no mode change for a path) ef4f0928(tree-diff: no need to pass match to skip_uninteresting()) into that branch, and drop them from ks/combine-diff - we'll have one branch reworking the diff tree-walker, and the other taking advantage of it for combine-diff. As long as that does not lose changes to tests and clean-ups, I'm fine with that direction. For example, I do not know if you want to lose e3f62d12 (diffcore-order: export generic ordering interface, 2014-01-20), which is part of the combine-diff topic. -- 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] tests: turn on network daemon tests by default
Jeff King p...@peff.net writes: On Tue, Feb 11, 2014 at 11:51:18AM -0800, Junio C Hamano wrote: Those who run buildfarms may want to disable the networking test if the buildfarms are not isolated well, for example. They have to be told somewhere that now they need to explicitly disable these tests and how. I think they should be OK. The daemons run on the loopback interface, so there is hopefully not a security implication. If multiple buildfarms are sharing the same loopback space (e.g., running in separate directories on the same machine), the auto setting should degrade gracefully. One daemon will win the setup, and the tests will run, and on the other, they will be skipped. I am in favor of this change but just pointing out possible fallouts might be larger than we think. Agreed, but I think the only way to know the size of those fallouts is to try it and see who complains. I would not normally be so cavalier with git itself, but I think for the test infrastructure, we have a small, tech-savvy audience that can help us iterate on it without too much pain. Sure. One immediate complaint is that I would probably need to do something like this in the build automation: if testing a branch without this patch then : do nothing else GIT_TEST_GIT_DAEMON=false fi Arguably, it is the fault of the current/original code that treated *any* non-empty value that is set in the environment variable as true---if it paid attention to GIT_TEST_GIT_DAEMON=NoThanks, we wouldn't have to have a workaround like this. I wonder if GIT_TEST_X=$(test_tristate $GIT_TEST_X) pattern can be made a bit more friendly, though. For example, can we behave differently depending on the reason why $GIT_TEST_X is empty? - People who have *not* been opting in to the expensive tests have not done anything special; GIT_TEST_X environment variable did not exist for them (i.e. unset), and we used to skip when $GIT_TEST_X is an empty string. - We want to encourage people who do not care to run these tests. If people do not do anything, their $GIT_TEST_X will continue to be an empty string without GIT_TEST_X variable in the environment. If we let people who *do* want to opt out of the expensive tests by explicitly setting $GIT_TEST_X to an empty string in the new scheme, wouldn't the transition go a lot smoother? The caller may have to pass the name of the variable: GIT_TEST_DAEMON=$(test_tristate GIT_TEST_DAEMON) and then the callee may become test_tristate () { variable=$1 if eval test x\\${$variable+isset}\ = xisset then # explicitly set eval value=\$$variable case $value in ) echo false ;; false | auto) echo $value ;; *) echo true ;; esac else echo auto fi } so that - Any non-empty string other than the magic strings false and auto continue to mean please I want to test; - Setting the variable explicitly to an empty string will continue to mean no I do not want to test; - Leaving the variable unset will continue to mean I don't really care; just follow the default the project gives 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: git-note -C changes commit type?
Johan Herland jo...@herland.net writes: There is currently no way the git notes commands will allow you to store the 3d7de37 commit object directly as a note. There is also (AFAICS) no easy workaround (git fast-import could've been a workaround if it did not already require the first N/notemodify argument to be a blob object). The best alternative, off the top of my head, would be to write your own program using the notes.h API to manipulate the notes tree directly (or - suboptimally - use other low-level Git operations to do the same). Even worse. I do not think such a non-blob object in the notes tree does not participate in the reachability at all, so you won't be able to fetch refs/notes/whatever and expect to get a useful result. I do not think storing the raw bits of commit object as a blob in the notes tree is useful behaviour, either. The command probably should refuse to get anything non-blob via that option. Perhaps the notes entry should just note the object name of whatever commit it wants to refer to in a *blob*? -- 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 00/11] More preparatory work for multiparent tree-walker
Junio C Hamano gits...@pobox.com writes: Kirill Smelkov k...@mns.spb.ru writes: Sorry for the confusion. Could you please do the following: Patches should be applied over to ks/tree-diff-walk (74aa4a18). Before applying the patches, please cherry-pick c90483d9(tree-diff: no need to manually verify that there is no mode change for a path) ef4f0928(tree-diff: no need to pass match to skip_uninteresting()) into that branch, and drop them from ks/combine-diff - we'll have one branch reworking the diff tree-walker, and the other taking advantage of it for combine-diff. As long as that does not lose changes to tests and clean-ups, I'm fine with that direction. For example, I do not know if you want to lose e3f62d12 (diffcore-order: export generic ordering interface, 2014-01-20), which is part of the combine-diff topic. Ahh, sorry, I misread the drop as salvage these two and drop the rest. The new series does apply cleanly on a commit in master..pu that has both ks/tree-diff-walk and ks/combine-diff, and the latter is not yet in 'next' so we are free to reorganize. Let me flip the latter topic around, also queue these updates and push the result out on 'pu'. 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 v2 2/2] gc: config option for running --auto in background
Duy Nguyen pclo...@gmail.com writes: On Tue, Feb 11, 2014 at 2:11 AM, Junio C Hamano gits...@pobox.com wrote: On Mon, Feb 10, 2014 at 10:43 AM, Junio C Hamano gits...@pobox.com wrote: If --quiet is set, we should not be printing anyway. If not, I thinkg we could only print auto packing in background.. when we actually can do that, else just print the old message. It means an #ifdef NO_POSIX_GOODIES here again though.. Didn't you change it not to die but return nosys or something? Ah, the problem is that it is too late to take back ... will do so in the background when you noticed that daemonize() did not succeed, so you would need a way to see if we can daemonize() before actually doing so if you want to give different messages. int can_daemonize(void) could be an answer that is nicer than NO_POSIX_GOODIES, but I am not sure if it is worth it. Or we could pass the quiet flag to daemonize() and let it print something in the #ifdef NO_POSIX_GOODIES part. Hmph... What would that something say? I was asked to gc in the background but I can't here is not suitable for daemonize() that is not specific to gc. The flow I had in mind was something along the lines of this if (!quiet) { if (detach_auto can_daemonize()) say auto packing in the background; else say auto packing } if (detach_auto can_daemonize()) daemonize(); If we had daemonize(noisy=1) and coded it this way: if (!quiet) say auto packing; if (detach_auto) daemonize(!quiet); we could do something like: daemonize(int noisy) { if (noisy !defined(NO_POSIX_GOODIES)) say , and doing so in the background; ... do the actual daemonizing ... } but that feels ugly. -- 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] tests: turn on network daemon tests by default
Jeff King p...@peff.net writes: Agreed, but I think the only way to know the size of those fallouts is to try it and see who complains. I would not normally be so cavalier with git itself, but I think for the test infrastructure, we have a small, tech-savvy audience that can help us iterate on it without too much pain. There is another. $ GIT_TEST_HTTPD=false sh t5537-fetch-shallow.sh ok 1 - setup ok 2 - setup shallow clone ok 3 - clone from shallow clone ok 4 - fetch from shallow clone ok 5 - fetch --depth from shallow clone ok 6 - fetch --unshallow from shallow clone ok 7 - fetch something upstream has but hidden by clients shallow boundaries ok 8 - fetch that requires changes in .git/shallow is filtered ok 9 - fetch --update-shallow error: Can't use skip_all after running some tests Under 'prove' test target, the way it exits causes: *** prove *** t5537-fetch-shallow.sh .. Dubious, test returned 1 (wstat 256, 0x100) All 9 subtests passed which leads to: Test Summary Report --- t5537-fetch-shallow.sh (Wstat: 256 Tests: 9 Failed: 0) Non-zero exit status: 1 Parse errors: No plan found in TAP output On the 'master' branch without these auto opt-in patches, $ GIT_TEST_HTTPD= sh t5537-fetch-shallow.sh ok 1 - setup ok 2 - setup shallow clone ok 3 - clone from shallow clone ok 4 - fetch from shallow clone ok 5 - fetch --depth from shallow clone ok 6 - fetch --unshallow from shallow clone ok 7 - fetch something upstream has but hidden by clients shallow boundaries ok 8 - fetch that requires changes in .git/shallow is filtered ok 9 - fetch --update-shallow skipping remaining tests, git built without http support # passed all 9 test(s) 1..9 -- 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] Make the global packed_git variable static to sha1_file.c.
Stefan Zager sza...@google.com writes: If anyone has a recommendation for a less labor-intensive way to do this in emacs, I'd be very grateful. This is not do this in emacs, but here is a possible approach. You can ask git diff about what you changed, and actually apply the change while fixing whitespace errors. I.e. git diff sha1_file.c | git apply --cached --whitespace=fix git diff git checkout sha1_file.c The first step will add a cleaned-up version to your index. The second diff (optional) is to see what whitespace errors are introduced when going from that cleaned-up version to what you have in the working tree. With the last step you would update the working tree version to the cleaned-up version from the index. [alias] wsadd = !sh -c 'git diff -- \$@\ | git apply --cached --whitespace=fix;\ git co -- ${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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes: Making a single preparation run for counting the lines will avoid memory fragmentation. Also, fix the allocated memory size which was wrong when sizeof(int *) != sizeof(int), and would have been too small for sizeof(int *) sizeof(int), admittedly unlikely. Signed-off-by: David Kastrup d...@gnu.org --- I think I took sizeof(int*)-sizeof(int) patch to the 'next' branch already, which might have to conflict with this clean-up, but it should be trivial to resolve. Thanks for resending. I was busy elsewhere (i.e. no feedback does not mean silent rejection nor silent agreement at least from me), and such a resend does help prevent patches fall thru cracks. diff --git a/builtin/blame.c b/builtin/blame.c index e44a6bb..1aefedf 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1772,25 +1772,41 @@ static int prepare_lines(struct scoreboard *sb) { const char *buf = sb-final_buf; unsigned long len = sb-final_buf_size; + const char *end = buf + len; + const char *p; + int *lineno; + int num = 0, incomplete = 0; + for (p = buf;;) { + p = memchr(p, '\n', end - p); + if (p) { + p++; num++; + continue; } + break; } + + if (len end[-1] != '\n') + incomplete++; /* incomplete line at the end */ + + sb-lineno = xmalloc(sizeof(*sb-lineno) * (num + incomplete + 1)); + lineno = sb-lineno; + + *lineno++ = 0; + for (p = buf;;) { + p = memchr(p, '\n', end - p); + if (p) { + p++; + *lineno++ = p - buf; + continue; + } + break; + } + + if (incomplete) + *lineno++ = len; + sb-num_lines = num + incomplete; return sb-num_lines; } -- 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: Make the git codebase thread-safe
Stefan Zager sza...@chromium.org writes: ... I used the Very Sleepy profiler to see where all the time was spent on Windows: 55% of the time was spent in OpenFile, and 25% in CloseFile (both in win32). This is somewhat interesting. When we check things out, checkout_paths() has a list of paths to be checked out, and iterates over them and call checkout_entry(). I wonder if you can: - introduce a version of checkout_entry() that takes file descriptors to write to; - have an asynchronous helper threads that pre-open the paths to be written out and feed ce, file descriptor to be written to a queue; - restructure that loop so that it reads the ce, file descriptor to be written from the queue, performs the actual writing out, and then feeds file descriptor to be closed to another queue; and - have another asynchronous helper threads that reads file descriptor to be closed from the queue and close them. Calls to write (and preparation of data to be written) will then remain single-threaded, but it sounds like that codepath is not the bottleneck in your measurement, so -- 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] Make the global packed_git variable static to sha1_file.c.
I'll locally fix up these style issues before commenting on the patch. Thanks. ERROR: space required after that ',' (ctx:VxV) #78: FILE: builtin/count-objects.c:111: + struct pack_data pd = {0,0,0}; ^ ERROR: space required after that ',' (ctx:VxV) #78: FILE: builtin/count-objects.c:111: + struct pack_data pd = {0,0,0}; ^ ERROR: space required after that ',' (ctx:VxV) #171: FILE: builtin/fsck.c:683: + struct verify_packs_data vpd = {0,0,0}; ^ ERROR: space required after that ',' (ctx:VxV) #171: FILE: builtin/fsck.c:683: + struct verify_packs_data vpd = {0,0,0}; ^ ERROR: space required after that ',' (ctx:VxV) #541: FILE: builtin/pack-redundant.c:591: + struct add_pack_data apd = {filename,0,NULL}; ^ ERROR: space required after that ',' (ctx:VxV) #541: FILE: builtin/pack-redundant.c:591: + struct add_pack_data apd = {filename,0,NULL}; ^ ERROR: space required after that ',' (ctx:VxV) #565: FILE: builtin/pack-redundant.c:604: + struct add_pack_data apd = {NULL,0,NULL}; ^ ERROR: space required after that ',' (ctx:VxV) #565: FILE: builtin/pack-redundant.c:604: + struct add_pack_data apd = {NULL,0,NULL}; ^ ERROR: space prohibited after that '-' (ctx:WxW) #726: FILE: pack-revindex.c:47: + num = - 1 - num; ^ ERROR: space required after that ',' (ctx:VxV) #901: FILE: sha1_name.c:195: + struct disambiguate_data d = {len,bin_pfx,ds}; ^ ERROR: space required after that ',' (ctx:VxV) #901: FILE: sha1_name.c:195: + struct disambiguate_data d = {len,bin_pfx,ds}; ^ total: 11 errors, 0 warnings, 781 lines checked -- 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] Make the global packed_git variable static to sha1_file.c.
sza...@chromium.org writes: From 0a59547f3e95ddecf7606c5f259ae6177c5a104f Mon Sep 17 00:00:00 2001 Please drop this line. From: Stefan Zager sza...@chromium.org Please drop this line and instead have it in your e-mail header. Date: Mon, 10 Feb 2014 16:55:12 -0800 The date in your e-mail header, which is the first time general public saw this particular version of the patch, is used by default as the author time. Unless there is a compelling reason to override that with an in-body header, please drop this line. Subject: [PATCH] Make the global packed_git variable static to sha1_file.c. Please drop this line and instead have it in your e-mail header. This patch is a pure refactor with no functional changes,... diff --git a/builtin/count-objects.c b/builtin/count-objects.c index a7f70cb..6554dfe 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -83,14 +83,32 @@ static char const * const count_objects_usage[] = { NULL }; +struct pack_data { + unsigned long packed; + off_t size_pack; + unsigned long num_pack; +}; + +int pack_data_fn(struct packed_git *p, void *data) Can't/shouldn't this be static? Also I'd suggest s/pack_data_fn/collect_pack_data/ or something. _fn may be a good suffix for typedef'ed typename used in a callback function, but for a concrete function, it only adds noise without giving us anything new. Yes, I know there are a few existing violators, but we shouldn't make the codebase worse, using their existence due to past carelessness as an excuse. diff --git a/cache.h b/cache.h index dc040fb..542a9d9 100644 --- a/cache.h +++ b/cache.h ... +/* The 'hint' argument is for the commonly-used 'last found pack' optimization. + * It can be NULL. + */ /* * Please try to have opening slash-asterisk and closing * asterisk-slash in a multi-line comment block on their * own lines by themselves. */ +extern void foreach_packed_git(packed_git_foreach_fn fn, struct packed_git *hint, void *data); + +extern size_t packed_git_count(); +extern size_t packed_git_local_count(); extern size_t packed_git_count(void); extern size_t packed_git_local_count(void); diff --git a/sha1_file.c b/sha1_file.c index 6e8c05d..aeeb7e6 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -60,6 +60,7 @@ static struct cached_object empty_tree = { 0 }; +static struct packed_git *packed_git; static struct packed_git *last_found_pack; static struct cached_object *find_cached_object(const unsigned char *sha1) @@ -468,7 +469,6 @@ static unsigned int pack_open_fds; static unsigned int pack_max_fds; static size_t peak_pack_mapped; static size_t pack_mapped; -struct packed_git *packed_git; Hmm, any particular reason why only this variable and not others are moved up? @@ -1091,6 +1091,37 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local) return p; } +void foreach_packed_git(packed_git_foreach_fn fn, struct packed_git *hint, void *data) +{ + struct packed_git *p; + if (hint ((*fn)(hint, data))) + return; + for (p = packed_git; p; p = p-next) + if (p != hint (*fn)(p, data)) + return; (mental note) In the new API, a non-zero return signals an early return/break from the loop. +} + +size_t packed_git_count() size_t packed_git_count(void) +{ + size_t res = 0; + struct packed_git *p; + + for (p = packed_git; p; p = p-next) + ++res; When pre- or post- increment does not make any difference (i.e. you do not use its value), please stick to post-increment. pre-increment looks unusual in our codebase and becomes distracting while reading it through. Same comments for packed-git-local-count. diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 541667f..bc3074b 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -900,14 +900,45 @@ static int no_try_delta(const char *path) return 0; } +struct find_pack_data { + const unsigned char *sha1; + off_t offset; + struct packed_git *found_pack; + int exclude; + int found_non_local_pack; + int found_pack_keep; +}; + +static int find_pack_fn(struct packed_git *p, void *data) +{ + struct find_pack_data *fpd = (struct find_pack_data *) data; + off_t offset = find_pack_entry_one(fpd-sha1, p); + if (offset) { + if (!fpd-found_pack) { + if (!is_pack_valid(p)) { + warning(packfile %s cannot be accessed, p-pack_name); + return 0; + } + fpd-offset = offset; + fpd-found_pack = p; + } + if (fpd-exclude) + return 1; + if (!p-pack_local) + fpd-found_non_local_pack = 1; + else if
What's cooking in git.git (Feb 2014, #04; Wed, 12)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. As a workaround to make life easier for third-party tools, the upcoming major release will be called Git 1.9.0 (not Git 1.9). The first maintenance release for it will be Git 1.9.1, and the major release after Git 1.9.0 will either be Git 2.0.0 or Git 1.10.0. 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] * al/docs (2014-02-11) 4 commits - docs/git-blame: explain more clearly the example pickaxe use - docs/git-clone: clarify use of --no-hardlinks option - docs/git-remote: capitalize first word of initial blurb - docs/merge-strategies: remove hyphen from mis-merges A handful of documentation updates, all trivially harmless. Will merge to 'next'. * jk/test-ports (2014-02-10) 2 commits - tests: auto-set git-daemon port - tests: auto-set LIB_HTTPD_PORT from test name (this branch is tangled with nd/http-fetch-shallow-fix.) Avoid having to assign port number to be used in tests manually. Will merge to 'next'. * nd/daemonize-gc (2014-02-10) 2 commits - gc: config option for running --auto in background - daemon: move daemonize() to libgit.a Allow running gc --auto in the background. Will merge to 'next'. * nd/gitignore-trailing-whitespace (2014-02-10) 2 commits - dir: ignore trailing spaces in exclude patterns - dir: warn about trailing spaces in exclude patterns Warn and then ignore trailing whitespaces in .gitignore files, unless they are quoted for fnmatch(3), e.g. path\ . * nd/log-show-linear-break (2014-02-10) 1 commit - log: add --show-linear-break to help see non-linear history * ss/completion-rec-sub-fetch-push (2014-02-11) 1 commit - completion: teach --recurse-submodules to fetch, pull and push * ks/tree-diff-more (2014-02-12) 16 commits - tree-diff: reuse base str(buf) memory on sub-tree recursion - tree-diff: no need to call full diff_tree_sha1 from show_path() - tree-diff: rework diff_tree interface to be sha1 based - tree-diff: remove special-case diff-emitting code for empty-tree cases - tree-diff: simplify tree_entry_pathcmp - tree-diff: show_path prototype is not needed anymore - tree-diff: rename compare_tree_entry - tree_entry_pathcmp - tree-diff: move all action-taking code out of compare_tree_entry() - tree-diff: don't assume compare_tree_entry() returns -1,0,1 - FIXUP! - tree-diff: consolidate code for emitting diffs and recursion in one place - tree-diff: show_tree() is not needed - tree-diff: no need to pass match to skip_uninteresting() - tree-diff: no need to manually verify that there is no mode change for a path - combine-diff: move changed-paths scanning logic into its own function - combine-diff: move show_log_first logic/action out of paths scanning (this branch uses ks/combine-diff and ks/tree-diff-walk.) * jh/note-trees-record-blobs (2014-02-12) 1 commit - notes: Disallow reusing non-blob as a note object * jk/run-network-tests-by-default (2014-02-12) 1 commit - tests: turn on network daemon tests by default Teach make test to run networking tests when possible by default. Needs a bit more work. e.g. $gmane/242013 -- [Stalled] * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to be formatted into a manual pages to begin with, and also its contents are a bit stale. It may be a good first step in the right direction, but needs more work to at least get the mark-up right before public consumption. Will hold. * jk/branch-at-publish-rebased (2014-01-17) 5 commits - t1507 (rev-parse-upstream): fix typo in test title - implement @{publish} shorthand - branch_get: provide per-branch pushremote pointers - branch_get: return early on error - sha1_name: refactor upstream_mark Give an easier access to the tracking branches from other side in a triangular workflow by introducing B@{publish} that works in a similar way to how B@{upstream} does. Meant to be used as a basis for whatever Ram wants to build on. Will hold. * rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits - merge: drop unused arg from abort_commit method signature - merge: make prepare_to_commit responsible for write_merge_state - t7505: ensure cleanup after hook blocks merge - t7505: add missing Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that run during git merge. The log message stresses too much on one hook, prepare-commit-msg, but it would equally apply to other hooks like post-merge, I think. Waiting for a reroll. * jl/submodule-recursive-checkout (2013-12-26) 5 commits - Teach checkout to recursively checkout
Re: [PATCH] tests: turn on network daemon tests by default
Jeff King p...@peff.net writes: test_normalize_tristate GIT_TEST_DAEMON Heh, great minds think alike. This is what I am playing with, without committing (because I do like your ask config if this is a kind of various boolean 'false' representations, which I haven't managed to add to it). t/lib-git-daemon.sh | 2 +- t/lib-httpd.sh | 2 +- t/test-lib-functions.sh | 45 +++-- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index 36106de..615bf5d 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -16,7 +16,7 @@ # stop_git_daemon # test_done -GIT_TEST_GIT_DAEMON=$(test_tristate $GIT_TEST_GIT_DAEMON) +test_tristate GIT_TEST_GIT_DAEMON if test $GIT_TEST_GIT_DAEMON = false then skip_all=git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to enable) diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 583fb14..f9c2e22 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -30,7 +30,7 @@ # Copyright (c) 2008 Clemens Buchacher dri...@aon.at # -GIT_TEST_HTTPD=$(test_tristate $GIT_TEST_HTTPD) +test_tristate GIT_TEST_HTTPD if test $GIT_TEST_HTTPD = false then skip_all=Network testing disabled (unset GIT_TEST_HTTPD to enable) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 3cc09c0..21c5214 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -716,27 +716,36 @@ perl () { command $PERL_PATH $@ } -# Normalize the value given in $1 to one of true, false, or auto. The -# result is written to stdout. E.g.: +# Given a variable $1, normalize the value of it to one of true, +# false, or auto and store the result to it. # -# GIT_TEST_HTTPD=$(test_tristate $GIT_TEST_HTTPD) +# test_tristate GIT_TEST_HTTPD # +# A variable set to an empty string is set to 'false'. +# A variable set to 'false' or 'auto' keeps its value. +# Anything else is set to 'true'. +# An unset variable defaults to 'auto'. +# +# The last rule is to allow people to set the variable to an empty +# string and export it to decline testing the particular feature +# for versions both before and after this change. We used to treat +# both unset and empty variable as a signal for do not test and +# took any non-empty string as please test. + test_tristate () { - case $1 in - |auto) - echo auto - ;; - *) - # Rely on git-config to handle all the variants of - # true/1/on/etc that we allow. - if ! git -c magic.hack=$1 config --bool magic.hack 2/dev/null - then - # If git-config failed, it was some non-bool value like - # YesPlease. Default this to true for historical - # compatibility. - echo true - fi - esac + if eval test x\\${$1+isset}\ = xisset + then + # explicitly set + eval + case \\$$1\ in + '') $1=false ;; + false | auto) ;; + *) $1=true ;; + esac + + else + eval $1=auto + fi } # Exit the test suite, either by skipping all remaining tests or by -- 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: Make the git codebase thread-safe
On Wed, Feb 12, 2014 at 12:27 PM, Stefan Zager sza...@chromium.org wrote: On Wed, Feb 12, 2014 at 12:06 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Zager sza...@chromium.org writes: Calls to write (and preparation of data to be written) will then remain single-threaded, but it sounds like that codepath is not the bottleneck in your measurement, so Yes, I considered that as well. At a minimum, that would still require attr.c to implement thread locking, since attribute files must be parsed to look for stream filters. I have already done that work. I would have imagined that use of the attribute system belongs to write and preparation of data to be written category, i.e. the single threaded part of the kludge I outlined. But I'm not sure it's the best long-term approach to add convoluted custom threading solutions to each git operation as it appears on the performance radar. Yeah, it depends on how clean and non-intrusive an abstraction we can make. The kludge I outlined is certainly not very pretty. -- 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 02/11] tree-diff: consolidate code for emitting diffs and recursion in one place
Kirill Smelkov k...@mns.spb.ru writes: +static void show_path(struct strbuf *base, struct diff_options *opt, + struct tree_desc *t1, struct tree_desc *t2) { unsigned mode; const char *path; - const unsigned char *sha1 = tree_entry_extract(desc, path, mode); - int pathlen = tree_entry_len(desc-entry); + const unsigned char *sha1; + int pathlen; int old_baselen = base-len; + int isdir, recurse = 0, emitthis = 1; + + /* at least something has to be valid */ + assert(t1 || t2); + + if (t2) { + /* path present in resulting tree */ + sha1 = tree_entry_extract(t2, path, mode); + pathlen = tree_entry_len(t2-entry); + isdir = S_ISDIR(mode); + } + else { + /* a path was removed - take path from parent. Also take + * mode from parent, to decide on recursion. + */ + tree_entry_extract(t1, path, mode); + pathlen = tree_entry_len(t1-entry); + + isdir = S_ISDIR(mode); + sha1 = NULL; + mode = 0; + } + + if (DIFF_OPT_TST(opt, RECURSIVE) isdir) { + recurse = 1; + emitthis = DIFF_OPT_TST(opt, TREE_IN_RECURSIVE); + } strbuf_add(base, path, pathlen); - if (DIFF_OPT_TST(opt, RECURSIVE) S_ISDIR(mode)) { - if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) - opt-add_remove(opt, *prefix, mode, sha1, 1, base-buf, 0); + if (emitthis) + emit_diff(opt, base, t1, t2); + + if (recurse) { strbuf_addch(base, '/'); - diff_tree_sha1(*prefix == '-' ? sha1 : NULL, -*prefix == '+' ? sha1 : NULL, base-buf, opt); - } else - opt-add_remove(opt, prefix[0], mode, sha1, 1, base-buf, 0); + diff_tree_sha1(t1 ? t1-entry.sha1 : NULL, +t2 ? t2-entry.sha1 : NULL, base-buf, opt); + } After this step, sha1 is assigned but never gets used. Please double-check the fix-up I queued in the series before merging it to 'pu'. 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] tests: turn on network daemon tests by default
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: test_normalize_tristate GIT_TEST_DAEMON Heh, great minds think alike. This is what I am playing with, without committing (because I do like your ask config if this is a kind of various boolean 'false' representations, which I haven't managed to add to it). And this is with the ask config helper. Two tangents. - We may want to do something similar in cvsserver and git-gui to make them more robust. $ git grep -e true --and -e 1 --and -e yes - Do we want to do something similar to GIT_TEST_CREDENTIAL_HELPER? -- 8 -- From: Jeff King p...@peff.net Date: Mon, 10 Feb 2014 16:29:37 -0500 Subject: [PATCH] tests: turn on network daemon tests by default We do not run the httpd nor git-daemon tests by default, as they are rather heavyweight and require network access (albeit over localhost). However, it would be nice if more pepole ran them, for two reasons: 1. We would get more test coverage on more systems. 2. The point of the test suite is to find regressions. It is very easy to change some of the underlying code and break the httpd code without realizing you are even affecting it. Running the httpd tests helps find these problems sooner (ideally before the patches even hit the list). We still want to leave an out, though, for people who really do not want to run them. For that reason, the GIT_TEST_HTTPD and GIT_TEST_GIT_DAEMON variables are now tri-state booleans (true/false/auto), so you can say GIT_TEST_HTTPD=false to turn the tests back off. To support those who want a stable single way to disable these tests across versions of Git before and after this change, an empty string explicitly set to these variables is also taken as false, so the behaviour changes only for those who: a. did not express any preference by leaving these variables unset. They did not test these features before, but now they do; or b. did express that they want to test these features by setting GIT_TEST_FEATURE=false (or any equivalent other ways to tell false to Git, e.g. 0), which has been a valid but funny way to say that they do want to test the feature only because we used to interpret any non-empty string to mean yes please test. They no longer test that feature. In addition, we are forgiving of common setup failures (e.g., you do not have apache installed, or have an old version) when the tri-state is auto (or empty), but report an error when it is true. This makes auto a sane default, as we should not cause failures on setups where the tests cannot run. But it allows people who use true to catch regressions in their system (e.g., they uninstalled apache, but were expecting their automated test runs to test git-httpd, and would want to be notified). Signed-off-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com --- t/lib-git-daemon.sh | 8 --- t/lib-httpd.sh | 22 +-- t/test-lib-functions.sh | 58 + 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index 394b06b..615bf5d 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -16,9 +16,10 @@ # stop_git_daemon # test_done -if test -z $GIT_TEST_GIT_DAEMON +test_tristate GIT_TEST_GIT_DAEMON +if test $GIT_TEST_GIT_DAEMON = false then - skip_all=git-daemon testing disabled (define GIT_TEST_GIT_DAEMON to enable) + skip_all=git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to enable) test_done fi @@ -58,7 +59,8 @@ start_git_daemon() { kill $GIT_DAEMON_PID wait $GIT_DAEMON_PID trap 'die' EXIT - error git daemon failed to start + test_skip_or_die $GIT_TEST_GIT_DAEMON \ + git daemon failed to start fi } diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index bfdff2a..f9c2e22 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -30,9 +30,10 @@ # Copyright (c) 2008 Clemens Buchacher dri...@aon.at # -if test -z $GIT_TEST_HTTPD +test_tristate GIT_TEST_HTTPD +if test $GIT_TEST_HTTPD = false then - skip_all=Network testing disabled (define GIT_TEST_HTTPD to enable) + skip_all=Network testing disabled (unset GIT_TEST_HTTPD to enable) test_done fi @@ -76,8 +77,7 @@ GIT_VALGRIND_OPTIONS=$GIT_VALGRIND_OPTIONS; export GIT_VALGRIND_OPTIONS if ! test -x $LIB_HTTPD_PATH then - skip_all=skipping test, no web server found at '$LIB_HTTPD_PATH' - test_done + test_skip_or_die $GIT_TEST_HTTPD no web server found at '$LIB_HTTPD_PATH' fi HTTPD_VERSION=`$LIB_HTTPD_PATH -v | \ @@ -89,19 +89,20 @@ then then if ! test $HTTPD_VERSION -ge 2 then - skip_all=skipping test, at least Apache version 2 is required
Re: [PATCH 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
Kirill Smelkov k...@mns.spb.ru writes: + /* until we go to it next round, .next holds how many bytes we + * allocated (for faster realloc - we don't need copying old data). + */ + p-next = (struct combine_diff_path *)alloclen; I am getting this here: tree-diff.c: In function '__path_appendnew': tree-diff.c:140:13: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] combine-diff: speed it up, by using multiparent diff tree-walker directly
Kirill Smelkov k...@mns.spb.ru writes: + if (need_generic_pathscan) { + /* NOTE generic case also handles --stat, as it computes + * diff(sha1,parent_i) for all i to do the job, specifically + * for parent0. + */ + paths = find_paths_generic(sha1, parents, diffopts); + } + else { + paths = find_paths_multitree(sha1, parents, diffopts); + + /* show stat against the first parent even + * when doing combined diff. + */ + int stat_opt = (opt-output_format + (DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT)); /* * We see decl-after-stmt here. * Also please have slash-asterisk and asterisk-slash * at the beginning and the end of a multi-line comment * block on their own line. */ -- 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] for-each-ref: add option to omit newlines
Øystein Walle oys...@gmail.com writes: On to the patch itself: I contemplated putting '\n' in the default format and removing it if -n was given, which would get rid of the need to pass an exta argument to show_ref(). But that means we would need to *insert it* when a format is given and -n is not... I would rather see us go in the direction to add -z output option, which is what everybody else that produces NUL terminated entries in our suite of subcommands does. IOW, something along with this line (untested). builtin/for-each-ref.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 51798b4..2c8cac8 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -94,6 +94,7 @@ static const char **used_atom; static cmp_type *used_atom_type; static int used_atom_cnt, need_tagged, need_symref; static int need_color_reset_at_eol; +static int line_termination = '\n'; /* * Used to parse format string and sort specifiers @@ -1023,7 +1024,7 @@ static void show_ref(struct refinfo *info, const char *format, int quote_style) resetv.s = color; print_value(resetv, quote_style); } - putchar('\n'); + putchar(line_termination); } static struct ref_sort *default_sort(void) @@ -1088,6 +1089,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_STRING( 0 , format, format, N_(format), N_(format to use for the output)), OPT_CALLBACK(0 , sort, sort_tail, N_(key), N_(field name to sort on), opt_parse_sort), + OPT_SET_INT('z', NULL, line_termination, + N_(Use NUL instead of LF to end each output records), + '\0'), OPT_END(), }; -- 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] t5537: move http tests out to t5539
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: start_httpd is supposed to be at the beginning of the test file, not the middle of it. The test_seq line in no shallow lines.. test is updated to compensate missing refs that are there in t5537, but not in the new t5539. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- On Thu, Feb 13, 2014 at 8:22 AM, Duy Nguyen pclo...@gmail.com wrote: On Thu, Feb 13, 2014 at 5:12 AM, Jeff King p...@peff.net wrote: lib-httpd was never designed to be included from anywhere except the beginning of the file. But that wouldn't be right for t5537, because it wants to run some of the tests, even if apache setup fails. The right way to do it is probably to have lib-httpd do all of its work in a lazy prereq. I don't know how clunky that will end up, though; it might be simpler to just move the shallow http test into one of the http-fetch scripts. I'll move it out later. Here it is, on top of nd/http-fetch-shallow-fix because the new test in t5537 is picky and a simple merge resolution wouldn't do it. Will queue; thanks. t/t5537-fetch-shallow.sh | 57 --- t/t5539-fetch-http-shallow.sh (new +x) | 82 ++ 2 files changed, 82 insertions(+), 57 deletions(-) diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 098f220..3ae9092 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -173,61 +173,4 @@ EOF ) ' -if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then - say 'skipping remaining tests, git built without http support' - test_done -fi - -. $TEST_DIRECTORY/lib-httpd.sh -start_httpd - -test_expect_success 'clone http repository' ' - git clone --bare --no-local shallow $HTTPD_DOCUMENT_ROOT_PATH/repo.git - git clone $HTTPD_URL/smart/repo.git clone - ( - cd clone - git fsck - git log --format=%s origin/master actual - cat EOF expect -7 -6 -5 -4 -3 -EOF - test_cmp expect actual - ) -' - -# This test is tricky. We need large enough haves that fetch-pack -# will put pkt-flush in between. Then we need a have the server -# does not have, it'll send ACK %s ready -test_expect_success 'no shallow lines after receiving ACK ready' ' - ( - cd shallow - for i in $(test_seq 10) - do - git checkout --orphan unrelated$i - test_commit unrelated$i - git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git \ - refs/heads/unrelated$i:refs/heads/unrelated$i - git push -q ../clone/.git \ - refs/heads/unrelated$i:refs/heads/unrelated$i || - exit 1 - done - git checkout master - test_commit new - git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master - ) - ( - cd clone - git checkout --orphan newnew - test_commit new-too - GIT_TRACE_PACKET=$TRASH_DIRECTORY/trace git fetch --depth=2 - grep fetch-pack ACK .* ready ../trace - ! grep fetch-pack done ../trace - ) -' - -stop_httpd test_done diff --git a/t/t5539-fetch-http-shallow.sh b/t/t5539-fetch-http-shallow.sh new file mode 100755 index 000..94553e1 --- /dev/null +++ b/t/t5539-fetch-http-shallow.sh @@ -0,0 +1,82 @@ +#!/bin/sh + +test_description='fetch/clone from a shallow clone over http' + +. ./test-lib.sh + +if test -n $NO_CURL; then + skip_all='skipping test, git built without http support' + test_done +fi + +. $TEST_DIRECTORY/lib-httpd.sh +start_httpd + +commit() { + echo $1 tracked + git add tracked + git commit -m $1 +} + +test_expect_success 'setup shallow clone' ' + commit 1 + commit 2 + commit 3 + commit 4 + commit 5 + commit 6 + commit 7 + git clone --no-local --depth=5 .git shallow + git config --global transfer.fsckObjects true +' + +test_expect_success 'clone http repository' ' + git clone --bare --no-local shallow $HTTPD_DOCUMENT_ROOT_PATH/repo.git + git clone $HTTPD_URL/smart/repo.git clone + ( + cd clone + git fsck + git log --format=%s origin/master actual + cat EOF expect +7 +6 +5 +4 +3 +EOF + test_cmp expect actual + ) +' + +# This test is tricky. We need large enough haves that fetch-pack +# will put pkt-flush in between. Then we need a have the server +# does not have, it'll send ACK %s ready +test_expect_success 'no shallow lines after receiving ACK ready' ' + ( + cd shallow + for i in $(test_seq 15) + do + git checkout --orphan unrelated$i + test_commit unrelated$i +
Re: [PATCH] release notes: typo fixes
Michael J Gruber g...@drmicha.warpmail.net writes: Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Just a few things I spotted while trying to keep myself informed :) Thanks. Very much appreciated. -- 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
[ANNOUNCE] Git v1.8.5.5
The latest maintenance release Git v1.8.5.5 is now available at the usual places. Hopefully this will be the last update to the 1.8.5.x series. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 7bb4ea883b1f8f6f7f927035f85e8e27b57e0194 git-1.8.5.5.tar.gz 39dd7979c8757d2dc4bc3aaa82741ba93557d566 git-htmldocs-1.8.5.5.tar.gz a4a2aef1440d4751f37c65359da57c9bd51a7beb git-manpages-1.8.5.5.tar.gz The following public repositories all have a copy of the v1.8.5.5 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 Also, http://www.kernel.org/pub/software/scm/git/ has copies of the release tarballs. Git v1.8.5.5 Release Notes == Fixes since v1.8.5.4 * The pathspec matching code, while comparing two trees (e.g. git diff A B -- path1 path2) was too aggressive and failed to match some paths when multiple pathspecs were involved. * git repack --max-pack-size=8g stopped being parsed correctly when the command was reimplemented in C. * A recent update to git send-email broke platforms where /etc/ssl/certs/ directory exists but cannot be used as SSL_ca_path (e.g. Fedora rawhide). * A handful of bugs around interpreting $branch@{upstream} notation and its lookalike, when $branch part has interesting characters, e.g. @, and :, have been fixed. * git clone would fail to clone from a repository that has a ref directly under refs/, e.g. refs/stash, because different validation paths do different things on such a refname. Loosen the client side's validation to allow such a ref. * git log --left-right A...B lost the leftness of commits reachable from A when A is a tag as a side effect of a recent bugfix. This is a regression in 1.8.4.x series. * git merge-base --octopus used to leave cleaning up suboptimal result to the caller, but now it does the clean-up itself. * git mv A B/, when B does not exist as a directory, should error out, but it didn't. Also contains typofixes, documentation updates and trivial code clean-ups. Changes since v1.8.5.4 are as follows: Andy Spencer (1): tree_entry_interesting: match against all pathspecs Jeff King (9): fetch-pack: do not filter out one-level refs interpret_branch_name: factor out upstream handling interpret_branch_name: rename cp variable to at interpret_branch_name: always respect namelen parameter interpret_branch_name: avoid @{upstream} past colon interpret_branch_name: find all possible @-marks repack: fix typo in max-pack-size option repack: make parsed string options const-correct repack: propagate pack-objects options as strings Junio C Hamano (5): merge-base: separate --independent codepath into its own helper merge-base --octopus: reduce the result from get_octopus_merge_bases() revision: mark contents of an uninteresting tree uninteresting revision: propagate flag bits from tags to pointees Git 1.8.5.5 Ruben Kerkhof (1): send-email: /etc/ssl/certs/ directory may not be usable as ca_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: Git GSoC 2014
Thomas Rast t...@thomasrast.ch writes: Downside: not listing code merged as a goal may not make the project as shiny, neither for Git nor for the student. I'd actually view that as an upside. This sounds like a good first step for a feasibility study that is really necessary. I wonder why the handling of storage corruption and replacement could be left broken, though. Is that because libgit2 has known breakages in these areas, or is there some other reason? -- 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] tests: turn on network daemon tests by default
Jeff King p...@peff.net writes: - We may want to do something similar in cvsserver and git-gui to make them more robust. $ git grep -e true --and -e 1 --and -e yes I assume the something here is to respect bool options more consistently? Yeah, mostly by employing your 'git -c magic.var=X config --bool' trick and check only for 'false' and 'true', instead of keeping a hard-coded logic like the lines that hit the above query do. I have no problem with that, but nor do I care too much about those programs (that is partially laziness, but also partially that I do not want to deal with introducing a regression). True, too ;-) - Do we want to do something similar to GIT_TEST_CREDENTIAL_HELPER? No, it is not a boolean. It is a bit of a hack, but it is meant to be used like: GIT_TEST_CREDENTIAL_HELPER=foo ./t0303-* to test some random git-credential-foo you have in your PATH. There is nothing to run by default there. Ah, OK. I was only grepping for test -z .*GIT_TEST_. tri-state is auto (or empty), but report an error when it is You probably want to drop this or empty or change it to or unset, Thanks, I totally missed that. -- 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] notes: Disallow reusing non-blob as a note object
Eric Sunshine sunsh...@sunshineco.com writes: + if (type != OBJ_BLOB) { + free(buf); + die(_(Cannot read note data from non-blob object '%s'.), arg); The way this diagnostic is worded, it sound as if the 'read' failed rather than that the user specified an incorrect object type. Perhaps Object is not a blob '%s' or Expected blob but '%s' has type '%s' or something along those lines? Yeah, sounds good. You also need to say what expects a blob, too. Perhaps something like this? builtin/notes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/notes.c b/builtin/notes.c index c11d6e6..a16bc00 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -272,8 +272,10 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset) die(_(Failed to read object '%s'.), arg); } if (type != OBJ_BLOB) { + struct msg_arg *msg = opt-value; free(buf); - die(_(Cannot read note data from non-blob object '%s'.), arg); + die(_(The -%c option takes a blob, which '%s' is not., + msg-use_editor ? 'c' : 'C', arg)); } strbuf_add((msg-buf), buf, len); free(buf); -- 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] for-each-ref: add option to omit newlines
Øystein Walle oys...@gmail.com writes: However, when specifying a format string it's just a matter of ending the format string in '%00' and you're good to go. But then you get the null byte *and* a newline. And with your proposal there would be no way of saying you want neither. I very well understand that. All other commands that support -z to give you NUL terminated output do not consider that a downside. Why should for-each-ref be special? -- 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: error: src refspec refs/heads/master matches more than one.
Duy Nguyen pclo...@gmail.com writes: On Fri, Feb 14, 2014 at 7:45 PM, Andreas Schwab sch...@linux-m68k.org wrote: Josef Wolf j...@raven.inka.de writes: Notice the refs/heads _within_ refs/heads! Now I wonder how I managed to get into this situation and what's the best way to recover? Probably you did something like git branch refs/heads/master. You can remove it again with git branch -d refs/heads/master. As a porcelain, git branch should prevent (or at least warn) users from creating such refs, I think. warn, possibly, but I do not see a reason to *prevent*. A. You are not allowed to call your branch with a string that begins with 'refs/heads/'. B. Why? A. Because it will confuse you. B. I know what I am doing. A. ??? -- 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 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
Kirill Smelkov k...@mns.spb.ru writes: 8 From: Kirill Smelkov k...@mns.spb.ru Subject: [PATCH v2 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last three do not belong here. Only From, Date and Subject are relevant and taken as in-body headers. For that matter, as the From and Subject above are exactly the same as what you have on the e-mail header, you do not want any of them. I'll strip them out here, so no need to resend. Thanks. Previously diff_tree(), which is now named __diff_tree_sha1(), was That name with two leading underscores is a rather unfortunate, especially for a function that is not a file scope static. No way to rename this to something more sensible? That impedance mismatch *hurts* *performance* *badly* for generating combined diffs - in c839f1bd (combine-diff: optimize combine_diff_path Please avoid referring to a commit that is not in 'master' by its object name. It can be reworked later and get a different name. That slowness comes from the fact that currently, while generating combined diff, a lot of time is spent computing diff(commit,commit^2) just to only then intersect that huge diff to almost small set of files from diff(commit,commit^1). Good observation. |a| |b|a b - a ∉ B - D(A,B) += +aa↓ |-| |-|a b - b ∉ A - D(A,B) += -bb↓ | | | |a = b - investigate δ(a,b)a↓ b↓ In both the n-parallel and diff-tree, when an entry 'a' is a tree, I take this D(A,B) += +a to mean (recursively) adding all the paths within 'a' to the result as addition. Sounds sensible. D(A,B) is by definition the same as combined diff D(A,[B]), so if we could rework the code for common case and make it be not slower for nparent=1 case, usual diff(t1,t2) generation will not be slower, and multiparent diff tree-walker would greatly benefit generating combine-diff. OK. What we do is as follows: 1) diff tree-walker __diff_tree_sha1() is internally reworked to be a paths generator (new name diff_tree_paths()), with each generated path being `struct combine_diff_path` with info for path, new sha1,mode and for every parent which sha1,mode it was in it. 2) From that info, we can still generate usual diff queue with struct diff_filepairs, via exporting generated combine_diff_path, if we know we run for nparent=1 case. (see emit_diff() which is now named emit_diff_p0only()) s/p0/first_parent_/; perhaps? 3) In order for diff_can_quit_early(), which checks DIFF_OPT_TST(opt, HAS_CHANGES)) to work, that exporting have to be happening not in bulk, but incrementally, one diff path at a time. Good thinking. Some notes(*): 1) For loops, i = 0; do { ... } while (++i nparent); is used instead of for (i = 0; i nparent; ++i) ... because for the former case, the compiler have to emit additional prologue code which checks for i = nparent case before entering the loop. As we require nparent must be 0, that additional overhead conflicts with the runs not slower for nparent=1 case than before goal. Unfortunate. I'd rather see us stick to more readable and familiar form for maintainability if this were not measurable. 2) alloca(), for small arrays, is used for the same reason - if we change it to xmalloc()/free() the timings get worse Do you see any use of it outside compat/? I thought we specifically avoid alloca() for portability. Also we do not use variable-length-arrays on the stack either, I think. 3) For every parent tree, we need to keep a tag, whether entry from that parent equals to entry from minimal parent. For performance reasons I'm keeping that tag in entry's mode field in unused bit - see S_IFXMIN_NEQ. Unfortunate, but I do not see another place to keep this information offhand (nor implement this approach without keeping that piece of information). P.S. and combined diff is not some exotic/for-play-only stuff - for No need to convince us about that ;-) example for a program I write to represent Git archives as readonly filesystem, there is initial scan with `git log --reverse --raw --no-abbrev --no-renames -c` to extract log of what was created/changed when, as a result building a map {} sha1- in which commit (and date) a content was added that `-c` means also show combined diff for merges, and without them, if a merge is non-trivial (merges changes from two parents with both having separate changes to a file), or an evil one, the map will not be full, i.e. some valid sha1 would be absent from it. That case was my initial motivation for combined diffs speedup. I wonder if this machinery can be reused for log -m as well (or perhaps you do that already?). After all, by performing a single parallel scan, you are gathering all the necessary
Re: [PATCH] config: git_config_from_file(): handle - filename as stdin
Kirill A. Shutemov kir...@shutemov.name writes: The patch extends git config --file interface to allow read config from stdin. Thanks. The external interface proposed by this change that behaves the way your new test expects is a good addition to the system. I would describe it as: Subject: config: teach git config --file - to read from the standard input I however think the patch implements it at the level that is too low in the callchain. It will affect a lot more than the dash given to git config --file -. Fortunately, it does not make it possible for users to make this mistake [include] path = - and scratch their heads, wondering why git config is not answering until they hit ^D. But that is _only_ because we check if a file whose name is - actually exists in the current directory before falling into this codepath (and usually no such file exists). If such a funnily-named file does exist, we read from that file, not the standard input. So that include codepath happens to be safe, but who knows what dragons lie in other codepaths that call this function. I recall that an earlier implementation of git diff --no-index that made - read one side to be compared from the standard input had exactly the same issue of comparing filename with -, which we had to fix with code reorganization recently. I'd prefer to see this update to git config --file - done the right way from the start. diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 967359344dab..f1a63075e34f 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -484,6 +484,10 @@ test_expect_success 'alternative GIT_CONFIG (--file)' ' test_cmp expect output ' +test_expect_success 'alternative GIT_CONFIG (--file=-)' ' + git config --file - -l other-config output Please leave no SP between redirection operator and its file, i.e. git config --file - --list other-config output 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: error: src refspec refs/heads/master matches more than one.
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: ... A. Because it will confuse you. B. I know what I am doing. A. ??? A. But maybe Git will no longer know what you are doing. Its standard way of resolving references will mean that once a branch refs/heads/wibble exists, referring to a branch wibble will become extra hard. For example, stuff like push origin HEAD:refs/heads/wibble will maybe create or update a new branch wibble, or maybe it will just push to the existing branch refs/heads/wibble. I suspect that is a bad example (do refs on _our_ side influence how RHS of the colon that names refs on the other side is interpreted), but I think I know what you are trying to say. But Git never knows what you are doing---it just does what you tell it to, so it comes back to It will confuse you to the point that you would not be able to guess what Git will do matches your expectation. Whenever I tell them that, a counterpoint B. makes is still I know what I am doing., which is stubborn, rather obvious, and unfortunate. How much of the namespace are we willing to carve out if we were to go this route anyway? Things we use, i.e. refs/heads and refs/tags, would be no-brainers, but do we also forbid refs/review, which we ourselves do not use but some people may have in their repositories? Anything that begins with refs/? Anything that begins with refs/ and has more than two slashes in it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2014, #04; Wed, 12)
brian m. carlson sand...@crustytoothpaste.net writes: Changes to some scripted Porcelains use unsafe variable substitutions and still need to be tightened. Will merge to 'next'. Junio, did you want a reroll with that fixed commit message, or will you fix it up yourself? I haven't merged them yet---if there are need to update any one of them, please reroll a replacement set. 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: diff weirdness (bug?)
Dario Bertini berda...@gmail.com writes: git clone g...@github.com:ansible/ansible.git git revert 3616dffb68badb2b8d56 manually solve the conflict (you can look at the commit here: https://github.com/ansible/ansible/commit/3616dffb68badb2b8d56ef34391d7aae8de79cd6 ) git diff will output: dario@macbook ~/P/ansible (devel*+|REVERTING) git diff diff --cc lib/ansible/constants.py index c055ccf,6eac602..000 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@@ -84,16 -61,8 +84,12 @@@ active_user = pwd.getpwuid(os.geteuid # Needed so the RPM can call setup.py and have modules land in the # correct location. See #1277 for discussion - if getattr(sys, real_prefix, None): - # in a virtualenv - DIST_MODULE_PATH = os.path.join(sys.prefix, 'share/ansible/') - else: - DIST_MODULE_PATH = '/usr/share/ansible/' + DIST_MODULE_PATH = os.path.join(sys.prefix, 'share/ansible/') +# check all of these extensions when looking for yaml files for things like +# group variables +YAML_FILENAME_EXTENSIONS = [ , .yml, .yaml ] + # sections in config file DEFAULTS='defaults' now, it weirdly/incorrectly says that we added the YAML-related lines This is a combined diff, and yaml-related lines are added relative to your _other_ branch you are merging (notice these + are indented by one place). Relative to what you had at the tip of your branch before you started this operation that ended up conflicted, the half-merged result removes if/else that sets DIST_MODULE_PATH and replaces it with a single line (their +/- are on the first column, signifying that these are differences relative to the first parent, i.e. your state before you started the operation). if we remove these 3 lines, we'll get this diff: With that understanding, I think the output after removing these three lines is perfectlyh understandable and correct. You are looking at the three lines that used to exist in the version you started from, that were missing from the other side. If you remoe them, it will show as removal from _your_ version (notice these - that shows what _you_ did manually are on the first column, saying that that is relative to _your_ version). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Feb 2014, #04; Wed, 12)
Andrew Eikum aei...@codeweavers.com writes: On Wed, Feb 12, 2014 at 01:59:41PM -0800, Junio C Hamano wrote: As a workaround to make life easier for third-party tools, the upcoming major release will be called Git 1.9.0 (not Git 1.9). The first maintenance release for it will be Git 1.9.1, and the major release after Git 1.9.0 will either be Git 2.0.0 or Git 1.10.0. Apologies if this ground has been tread before, but has there been a version numbering discussion? A quick google didn't seem to turn anything up. This seems to be an opportune time to drop the useless first digit. Explicitly, the major release numbers would be: 1.8, 1.9, then 2.0, 3.0, 4.0, etc, with the 2nd digit would take the meaning of the current 3rd digit and so on. Considered, and discarded. cf. http://article.gmane.org/gmane.comp.version-control.git/241498 When you see a version number vX.Y.0 next time, think of it as just play vX.Y without the third digit, and you will be fine. People's script cannot learn the think of it as ... part overnight, and that is why we have the .0 there. -- 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] config: git_config_from_file(): handle - filename as stdin
Kirill A. Shutemov kir...@shutemov.name writes: On Fri, Feb 14, 2014 at 10:27:11AM -0800, Junio C Hamano wrote: ... I recall that an earlier implementation of git diff --no-index that made - read one side to be compared from the standard input had exactly the same issue of comparing filename with -, which we had to fix with code reorganization recently. I'd prefer to see this update to git config --file - done the right way from the start. Okay, reworked version is below. It's slightly more invasive then the original. Thanks. It looks more invasive mostly because you renamed check_blob_write() to check_write(). While I think that rename is a right thing to do (it used to be if we are attempting to write to blob, signal an error, but we could have called it check_write(), meaning we are attempting to write; error out if that should not be allowed), it would have been much easier to review and comment if this were done as a separate clean-up preparatory patch. It wouldn't have had to touch this many lines, I would think. And clean-up existing code as a separate step, without changing the behaviour, before starting to work on a new feature is actively encouraged around here. From 199e6a995bb5228578e66189ef586421a4d8d9ba Mon Sep 17 00:00:00 2001 From: Kirill A. Shutemov kir...@shutemov.name Date: Fri, 14 Feb 2014 21:59:39 +0200 Subject: [PATCH] config: teach git config --file - to read from the standard input I do not see a need for these four lines in your e-mail. All the necessary information is already in your e-mail header. Please drop them, perhaps except for the Subject: in-body header. If you are going to have a discussion and then your proposed patch, please do it this way (without the 8-space indentation I added for illustration) using the -- 8 -- scissors line: ... Okay, reworked version is below. -- 8 -- Subject: config: teach git config --file - to read from the standard input Extend git config --file interface to allow reading from the standard input Editing or setting value is an error. Signed-off-by: ... --- diffstat and patch here The in-body header Subject: is there to let you retitle the commit. Editing stdin or setting value in stdin is an error. Good thinking. Even comes with tests to cover these cases. Good. diff --git a/builtin/config.c b/builtin/config.c index 92ebf23f0a9a..625f914c44a0 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -21,6 +21,7 @@ static char key_delim = ' '; static char term = '\n'; static int use_global_config, use_system_config, use_local_config; +static int use_stdin; static const char *given_config_file; static const char *given_config_blob; If we are changing the function signature of git_config_with_options() anyway, would it be even a better idea to introduce something like: struct config_source { int use_stdin; const char *config_file; const char *config_blob; }; static struct config_source given_config_source; and have one _fewer_ parameter to the function, instead of adding one extra parameter? diff --git a/config.c b/config.c index d969a5aefc2b..53dd39f0b9ef 100644 --- a/config.c +++ b/config.c @@ -1030,24 +1030,34 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data) return ret; } -int git_config_from_file(config_fn_t fn, const char *filename, void *data) +static int do_config_from_file(config_fn_t fn, const char *filename, FILE *f, +void *data) { - int ret; - FILE *f = fopen(filename, r); + struct config_source top; - ret = -1; - if (f) { - struct config_source top; + top.u.file = f; + top.name = filename; + top.die_on_error = 1; + top.do_fgetc = config_file_fgetc; + top.do_ungetc = config_file_ungetc; + top.do_ftell = config_file_ftell; + + return do_config_from(top, fn, data); +} - top.u.file = f; - top.name = filename; - top.die_on_error = 1; - top.do_fgetc = config_file_fgetc; - top.do_ungetc = config_file_ungetc; - top.do_ftell = config_file_ftell; +static int git_config_from_stdin(config_fn_t fn, void *data) +{ + return do_config_from_file(fn, stdin, stdin, data); +} So the above callchain will set top.name to the string stdin. How would that interact with the code in handle_path_include() that makes sure that relative config includes are only allowed in file with known location? Other than that, I didn't spot any issue in this round looks. It seems to be almost there. 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: What's cooking in git.git (Feb 2014, #04; Wed, 12)
Andrew Eikum aei...@codeweavers.com writes: My worry is having 2. hang around for another decade or longer. I'd rather see X.0.0 denote a major feature release (currently represented as 1.X.0), with X.Y.0 for minor enhancements and X.Y.Z for bugfix. We need three categories: (1) potentially incompatible, (2) feature, (3) fixes-only. We have been doing two levels of features by having both second and third numbers and we are flattening by removing the second one. It seems reasonable to expect fewer backwards incompatible changes in the future as Git has become more mature. This reduces the utility of reserving X.0.0 for major backwards incompatible changes, especially considering it's already been eight years for the first increment. We are not done yet, far from it. If we can stay at 2.X longer, that is a very good thing. If we followed your numbering scheme, you rob from the users a way to learn about a rare event, a potentially backward-incompatible change. How would you tell your users when the version gap really matters? After hearing You need to plan carefully when you update to version 47 and then updating to version 47 (or the user may skip that version), the user will learn about a new version 48 and does not hear such a you need to be careful. What should he think? No news is a good news? He should refrain from updating because the last one was a big one? What if the last time he updated was to version 43, stayed at that version for a long time without paying much attention (as Git grows more and more mature), and now we have version 50 after having a large compatibility gap at version 47 he did not pay much attention because he was skipping? The rarer the important event is, the more necessary that the importance is communicated clearly. -- 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 v5 02/14] trailer: process trailers from file and arguments
Christian Couder chrisc...@tuxfamily.org writes: For example some people might want: if_exists = overwrite if_missing = add while others might want: if_exists = overwrite if_missing = do_nothing and I don't see how we can say that with just: action = do_Y_if_X_and_Z Yes, but then we go back to my original question: why exists and missing are so special, and why there aren't two kinds of exists (i.e. there exists an entry with the same key, value vs there exists an entry with the same key). I would have understood your this is not too hard to understand for users if you had three (i.e. missing, in addition to these two flavours of exists), but with only two, I do not see how it is useful in a hypothetical situation like above. For example, how would you express something like this only with if-exists vs if-missing? if_exists_exactly = ignore if_exists_with_different_value = append if_missng = prepend_to_the_beginning -- 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
[ANNOUNCE] Git v1.9.0
The latest feature release Git v1.9.0 is now available at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: e60667fc16e5a5f1cde46616b0458cc802707743 git-1.9.0.tar.gz 65eb3f411f4699695c7081a7c716cabb9ce23d75 git-htmldocs-1.9.0.tar.gz cff590c92b4d1c8a143c078473140b653cc5d56a git-manpages-1.9.0.tar.gz The following public repositories all have a copy of the v1.9.0 tag and the master branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Also, http://www.kernel.org/pub/software/scm/git/ has copies of the release tarballs. Git v1.9.0 Release Notes Backward compatibility notes git submodule foreach $cmd $args used to treat $cmd $args the same way ssh did, concatenating them into a single string and letting the shell unquote. Careless users who forget to sufficiently quote $args get their argument split at $IFS whitespaces by the shell, and got unexpected results due to this. Starting from this release, the command line is passed directly to the shell, if it has an argument. Read-only support for experimental loose-object format, in which users could optionally choose to write their loose objects for a short while between v1.4.3 and v1.5.3 era, has been dropped. The meanings of the --tags option to git fetch has changed; the command fetches tags _in addition to_ what is fetched by the same command line without the option. The way git push $there $what interprets the $what part given on the command line, when it does not have a colon that explicitly tells us what ref at the $there repository is to be updated, has been enhanced. A handful of ancient commands that have long been deprecated are finally gone (repo-config, tar-tree, lost-found, and peek-remote). Backward compatibility notes (for Git 2.0.0) When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics, which pushes: - only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch, if you are pushing to the same remote as you fetch from; or - only the current branch to the branch with the same name, if you are pushing to a remote that is not where you usually fetch from. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. The default prefix for git svn will change in Git 2.0. For a long time, git svn created its remote-tracking branches directly under refs/remotes, but it will place them under refs/remotes/origin/ unless it is told otherwise with its --prefix option. Updates since v1.8.5 Foreign interfaces, subsystems and ports. * The HTTP transport, when talking GSS-Negotiate, uses 100 Continue response to avoid having to rewind and resend a large payload, which may not be always doable. * Various bugfixes to remote-bzr and remote-hg (in contrib/). * The build procedure is aware of MirBSD now. * Various git p4, git svn and gitk updates. UI, Workflows Features * Fetching from a shallowly-cloned repository used to be forbidden, primarily because the codepaths involved were
What's cooking in git.git (Feb 2014, #05; Fri, 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'. The tip of 'master' is v1.9.0. The first maintenance release for it will be Git 1.9.1, and the major release after Git 1.9.0 will either be Git 2.0.0 or Git 1.10.0. 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] * ks/tree-diff-nway (2014-02-14) 2 commits - combine-diff: speed it up, by using multiparent diff - tree-diff: rework diff_tree() to generate diffs for multiparent cases as well (this branch uses ks/combine-diff, ks/tree-diff-more and ks/tree-diff-walk.) Instead of running N pair-wise diff-trees when inspecting a N-parent merge, find the set of paths that were touched by walking N+1 trees in parallel. These set of paths can then be turned into N pair-wise diff-tree results to be processed through rename detections and such. And N=2 case nicely degenerates to the usual 2-way diff-tree, which is very nice. Promising, but unfortunately the implementation seems a bit too unportable for such a core part of the system. -- [Stalled] * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to be formatted into a manual pages to begin with, and also its contents are a bit stale. It may be a good first step in the right direction, but needs more work to at least get the mark-up right before public consumption. Will hold. * jk/branch-at-publish-rebased (2014-01-17) 5 commits - t1507 (rev-parse-upstream): fix typo in test title - implement @{publish} shorthand - branch_get: provide per-branch pushremote pointers - branch_get: return early on error - sha1_name: refactor upstream_mark Give an easier access to the tracking branches from other side in a triangular workflow by introducing B@{publish} that works in a similar way to how B@{upstream} does. Meant to be used as a basis for whatever Ram wants to build on. Will hold. * rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits - merge: drop unused arg from abort_commit method signature - merge: make prepare_to_commit responsible for write_merge_state - t7505: ensure cleanup after hook blocks merge - t7505: add missing Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that run during git merge. The log message stresses too much on one hook, prepare-commit-msg, but it would equally apply to other hooks like post-merge, I think. Waiting for a reroll. * jl/submodule-recursive-checkout (2013-12-26) 5 commits - Teach checkout to recursively checkout submodules - submodule: teach unpack_trees() to update submodules - submodule: teach unpack_trees() to repopulate submodules - submodule: teach unpack_trees() to remove submodule contents - submodule: prepare for recursive checkout of submodules Expecting a reroll. * jc/graph-post-root-gap (2013-12-30) 3 commits - WIP: document what we want at the end - graph: remove unused code a bit - graph: stuff the current commit into graph-columns[] This was primarily a RFH ($gmane/239580). * fc/transport-helper-fixes (2013-12-09) 6 commits - remote-bzr: support the new 'force' option - test-hg.sh: tests are now expected to pass - transport-helper: check for 'forced update' message - transport-helper: add 'force' to 'export' helpers - transport-helper: don't update refs in dry-run - transport-helper: mismerge fix Updates transport-helper, fast-import and fast-export to allow the ref mapping and ref deletion in a way similar to the natively supported transports. Reported to break t5541, and has been stalled for a while without fixes. Will discard. * fc/completion (2013-12-09) 1 commit - completion: fix completion of certain aliases SZEDER Gábor noticed that this breaks git -c var=val alias and also suggested a better description of the change. Has been stalled for a while without much comments from anybody interested. Will discard. * mo/subtree-split-updates (2013-12-10) 3 commits - subtree: add --edit option - subtree: allow --squash and --message with push - subtree: support split --rejoin --squash Has been stalled for a while without much comments from anybody interested. Will discard. * hv/submodule-ignore-fix (2013-12-06) 4 commits - disable complete ignorance of submodules for index - HEAD diff - always show committed submodules in summary after commit - teach add -f option for ignored submodules - fix 'git add' to skip submodules configured as ignored Teach git add to be consistent with git status when changes to submodules are set to be ignored, to avoid surprises after checking with git status to see there isn't any
Re: [PATCH] for-each-ref: add option to omit newlines
Øystein Walle oys...@gmail.com writes: Maybe it's all subjective... I'm okay with just leaving things as they are. Lack of -z in for-each-ref can be called an inconsistency that already exists you may want to fix in any case. As an extension to that, I would not be fundamentally against a new option, e.g. --terminiator=7, that causes us to use putchar(7) instead of putchar('\n') or putchar('\0') to terminate each records. At that point, -z becomes a synonym for --terminator=0. And --terminator='' might even be a natural extension to that option to cause us not to call any putchar() there. If we were to do that, we should do them for all commands that let you use -z, not just for-each-ref, for consistency reasons, I would think. -- 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 v5 02/14] trailer: process trailers from file and arguments
For example, how would you express something like this only with if-exists vs if-missing? if_exists_exactly = ignore if_exists_with_different_value = append if_missng = prepend_to_the_beginning First, previously in the discussion you said that you didn't want us to talk about the where = (after | before) part, because you could see that it was orthogonal to the other stuff, but now it looks like you want again to put that on the table. Oh, then replace both append and prepend with append (it was a mistake). Can you express that without having two kinds of if-exists? But it could be possible with only if-exists vs if-missing like this: if_exists = append_if_different if_missing = prepend ... because we can still easily express things like: if_exists = append_if_different_neighbor The proliferation of these random if_X on the action part _is_ exactly what I find the proposal confusing. -- 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 v5 02/14] trailer: process trailers from file and arguments
Christian Couder chrisc...@tuxfamily.org writes: but we also want to say: action = do_Y_if_X_and_Z AND do_U_if_V For example some people might want: if_exists = overwrite if_missing = add while others might want: if_exists = overwrite if_missing = do_nothing and I don't see how we can say that with just: action = do_Y_if_X_and_Z That is a very relevant illustration that makes me realize why I found your if-exists/if-missing = do-Y-if-Z somewhat distasteful. Your if_exists = add_if_different says if the same key is there, add it if the value is different, but it also implicitly says donothing if the value is the same. That is, you are saying with the above if_exists = add_if_different AND ignore_if_same So you already have to support more than one actions depending on the condition, unless you want to limit the actions for all the cases other than X to be only ignore when you invent your next do_Y_if_X, X being different in this case, but you support (and need to support) different-neighbour and other random collections of conditions, I think. Which is essentially the same as saying that you need this: action = do_Y_if_X_and_Z AND do_U_if_V Again, unless all the U's are limited to ignore, that is. -- 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 v5 02/14] trailer: process trailers from file and arguments
Junio C Hamano gits...@pobox.com writes: That is, you are saying with the above if_exists = add_if_different AND ignore_if_same So you already have to support more than one actions depending on the condition, ... of conditions, I think. Which is essentially the same as saying that you need this: action = do_Y_if_X_and_Z AND do_U_if_V Again, unless all the U's are limited to ignore, that is. Oh by the way, don't get me wrong. I am not saying that the last one is necessarily better or worse. I am only saying that the semantics proposed seems to be hard to explain and we need to do find a way to do better. If you have these existing ones: Sob: A Sob: B Sob: C Sob: D and you have Sob: B at hand, Sob.if-missing would not fire (because if-exists/if-missing is only about keys) ans Sob.if-exists will. What happens is now up to the action part (i.e. what follows if_exists =, e.g. add_if_different). The conditional part of add_if_different action is explainable as a conditon on the value (as opposed to condition on keys, which is the left-hand-side). But what does a condition with neighbour in its name really mean? It is not a condition about the value, but also is a condition about the order of the existing records. What is the right mental model the end-user needs to form when understanding these? Conditions on keys go on the left, and any other random conditions can come as a modifier after action e.g. add_if_same_value_is_not_at_the_end? -- 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: Make the git codebase thread-safe
Duy Nguyen pclo...@gmail.com writes: On Sat, Feb 15, 2014 at 8:15 AM, Zachary Turner ztur...@chromium.org wrote: ... 2) Use TLS as you suggest and have one fd per pack thread. Probably the most complicated code change (at least for me, being a first-time contributor) It's not so complicated. I suggested a patch [1] before (surprise!). ... [1] http://article.gmane.org/gmane.comp.version-control.git/196042 That message is at the tail end of the discussion. I wonder why nothing came out of it back then. While I do not see anything glaringly wrong with the change from a quick glance over it, it would be nice to hear how well it performs on their platform from Windows folks. 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: error: src refspec refs/heads/master matches more than one.
Duy Nguyen pclo...@gmail.com writes: Prevent is a strong word. I meant we only do it if they force it. Something like this.. -- 8 -- diff --git a/branch.c b/branch.c index 723a36b..3f0540f 100644 --- a/branch.c +++ b/branch.c @@ -251,6 +251,11 @@ void create_branch(const char *head, forcing = 1; } + if (!force dwim_ref(name, strlen(name), sha1, real_ref)) + die(_(creating ref refs/heads/%s makes %s ambiguous.\n + Use -f to create it anyway.), + name, name); Does this check still allow you to create a branch refs/heads/next and then later create a branch next? The latter will introduce an ambiguity without any prevention, even though the prevention would trigger if the order in which these two branches are created is swapped--- the end result has ambiguity but the safety covers only one avenue to the confusing situation. And the only way I can think of to avoid that kind of confusion is to forbid creation of a subset of possible names by reserving a set of known (but arbitrary) prefixes---which I am not sure is a good way to go. At least not yet. So... -- 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: diff weirdness (bug?)
Dario Bertini berda...@gmail.com writes: On 02/14/2014 09:03 PM, Junio C Hamano wrote: This is a combined diff, and yaml-related lines are added relative to your _other_ branch you are merging (notice these + are indented by one place). Relative to what you had at the tip of your branch before you started this operation that ended up conflicted, the half-merged result removes if/else that sets DIST_MODULE_PATH and replaces it with a single line (their +/- are on the first column, signifying that these are differences relative to the first parent, i.e. your state before you started the operation). if we remove these 3 lines, we'll get this diff: With that understanding, I think the output after removing these three lines is perfectlyh understandable and correct. You are looking at the three lines that used to exist in the version you started from, that were missing from the other side. If you remoe them, it will show as removal from _your_ version (notice these - that shows what _you_ did manually are on the first column, saying that that is relative to _your_ version). Thank you, I was completely unaware of combined diffs. Still: I can't see how this would explain the empty diff when deleting 4 lines instead of 3. Also: in the diff output I get 2 hashes, but these are not the hashes of the commits, but the contents of the files apparently. One should be HEAD (but if I run sha1sum over the file the hash doesn't match), but A blob object name (or for that matter, names of any type of object) is not the same as the hash over its contents alone. See combined diff format section of git diff --help if you are interested in reading what the output format is telling you. the other can't be the commit which I reverted: the diff is too small... or at least this is what I understand By the way, in the man of git diff there's the briefly mentioned '-m' flag. If anyone else reading this mail in the archives is confused by the combined diff output, just use git diff -m HEAD... I'll probably add this in my git aliases If you are primarily interested in what a merge (or other mergy-operation like revert) did to your working tree state, relative to the state it operated on, git diff HEAD is most likely what you want. -- 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] config: teach git config --file - to read from the standard input
Jeff King p...@peff.net writes: +} else { +if (cf-name) +return error(bad config file line %d in %s, +cf-linenr, cf-name); +else +return error(bad config file line %d, cf-linenr); +} I think I preferred the earlier version where you had stdin in the name field, and this hunk could just go away. I know you switched it to NULL here to avoid making bogus relative filenames in includes. Exactly the same comment here. I really like the way how this series becomes more polished every time we see it. But that would be pretty straightforward to fix by splitting the name field into an additional path field. It looks like git config --blob has the same problem (it will try relative lookups in the filesystem, even though that is nonsensical from the blob). So we could fix the existing bug at the same time. :) Perhaps this could go at the start of your series? Sounds like the right thing to do. Thanks. -- 8 -- Subject: config: disallow relative include paths from blobs When we see a relative config include like: [include] path = foo we make it relative to the containing directory of the file that contains the snippet. This makes no sense for config read from a blob, as it is not on the filesystem. Something like HEAD:some/path could have a relative path within the tree, but: 1. It would not be part of include.path, which explicitly refers to the filesystem. 2. It would need different parsing rules anyway to determine that it is a tree path. The current code just uses the name field, which is wrong. Let's split that into name and path fields, use the latter for relative includes, and fill in only the former for blobs. Signed-off-by: Jeff King p...@peff.net --- I don't think we considered includes at all when adding --blob. I cannot think of any good reason to have even an absolute filesystem include from a blob. And I wonder if it is possible to cause security mischief by convincing git to look at /etc/passwd or similar (it would not parse, of course, but you might be able to glean information from the error messages or something). It's probably OK, though, because you would generally not read real config from an untrusted source (there are many bad things they could set), and other code which uses the config reader explicitly does not turn on includes. config.c | 10 ++ t/t1305-config-include.sh | 16 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/config.c b/config.c index d969a5a..b295310 100644 --- a/config.c +++ b/config.c @@ -21,6 +21,7 @@ struct config_source { } buf; } u; const char *name; + const char *path; int die_on_error; int linenr; int eof; @@ -97,12 +98,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (!is_absolute_path(path)) { char *slash; - if (!cf || !cf-name) + if (!cf || !cf-path) return error(relative config includes must come from files); - slash = find_last_dir_sep(cf-name); + slash = find_last_dir_sep(cf-path); if (slash) - strbuf_add(buf, cf-name, slash - cf-name + 1); + strbuf_add(buf, cf-path, slash - cf-path + 1); strbuf_addstr(buf, path); path = buf.buf; } @@ -1040,7 +1041,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) struct config_source top; top.u.file = f; - top.name = filename; + top.name = top.path = filename; top.die_on_error = 1; top.do_fgetc = config_file_fgetc; top.do_ungetc = config_file_ungetc; @@ -1062,6 +1063,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf, top.u.buf.len = len; top.u.buf.pos = 0; top.name = name; + top.path = NULL; top.die_on_error = 0; top.do_fgetc = config_buf_fgetc; top.do_ungetc = config_buf_ungetc; diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index a707076..6edd38b 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -122,6 +122,22 @@ test_expect_success 'relative includes from command line fail' ' test_must_fail git -c include.path=one config test.one ' +test_expect_success 'absolute includes from blobs work' ' + echo [test]one = 1 one + echo [include]path=$(pwd)/one blob + blob=$(git hash-object -w blob) + echo 1 expect + git config --blob=$blob test.one actual + test_cmp expect actual +' + +test_expect_success 'relative includes from blobs fail' ' + echo [test]one = 1 one +
Re: [PATCH 0/4] Good bye fnmatch
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Long story short, we wanted globbing wildcard ** so I ripped wildmatch library from rsync to do it. And it opened a possibility to replace fnmatch completely, which would provide consistent behavior across platforms (native fnmatch behaves differently on many corner cases), and some performance gains. I started fnmatch replacement with 4917e1e (Makefile: promote wildmatch to be the default fnmatch implementation - 2013-05-30). This is the final step. Nice. Nguyễn Thái Ngọc Duy (4): Use wildmatch() directly without fnmatch() wrapper Revert test-wildmatch: add perf command to compare wildmatch and fnmatch Stop using fnmatch (either native or compat) Actually remove compat fnmatch source code Makefile| 22 -- builtin/apply.c | 2 +- builtin/branch.c| 2 +- builtin/describe.c | 2 +- builtin/for-each-ref.c | 2 +- builtin/ls-remote.c | 2 +- builtin/name-rev.c | 2 +- builtin/reflog.c| 2 +- builtin/replace.c | 2 +- builtin/show-branch.c | 2 +- builtin/tag.c | 2 +- compat/fnmatch/fnmatch.c (gone) | 494 compat/fnmatch/fnmatch.h (gone) | 84 --- config.mak.uname| 10 - configure.ac| 28 --- diffcore-order.c| 2 +- dir.c | 11 +- git-compat-util.h | 12 - refs.c | 2 +- revision.c | 2 +- t/t3070-wildmatch.sh| 13 -- test-wildmatch.c| 79 --- 22 files changed, 20 insertions(+), 759 deletions(-) -- 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/RESEND] attr: allow pattern escape using backslashes
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Patterns in .gitattributes are separated by whitespaces, which makes it impossible to specify exact spaces in the pattern. '?' can be used as a workaround, but it matches other characters too. This patch makes a space following a backslash part of the pattern, not a pattern separator. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Last discussion is [1] although the thread went off topic, so no actual discussion. The only people who could get hurt with this patch are those who do have a path that ends with a backslash (e.g. 'hello\') and have been matching it with a pattern that literally match with it, but even then they should have been quoting it at the end of the pattern (e.g. as 'hello\\'), so the new parsing rule would not be confused, I would think. So, I like it. Thanks. [1] http://thread.gmane.org/gmane.comp.version-control.git/212631 Documentation/gitattributes.txt | 6 +++--- attr.c | 8 +++- t/t0003-attributes.sh | 5 + 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 643c1ba..5d4d386 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -20,9 +20,9 @@ Each line in `gitattributes` file is of form: pattern attr1 attr2 ... -That is, a pattern followed by an attributes list, -separated by whitespaces. When the pattern matches the -path in question, the attributes listed on the line are given to +That is, a pattern followed by an attributes list, separated by +whitespaces that are not quoted by a backslash. When the pattern matches +the path in question, the attributes listed on the line are given to the path. Each attribute can be in one of these states for a given path: diff --git a/attr.c b/attr.c index 8d13d70..699716d 100644 --- a/attr.c +++ b/attr.c @@ -209,7 +209,13 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, if (!*cp || *cp == '#') return NULL; name = cp; - namelen = strcspn(name, blank); + namelen = 0; + while (name[namelen] != '\0' !strchr(blank, name[namelen])) { + if (name[namelen] == '\\' name[namelen + 1] != '\0') + namelen += 2; + else + namelen++; + } if (strlen(ATTRIBUTE_MACRO_PREFIX) namelen starts_with(name, ATTRIBUTE_MACRO_PREFIX)) { if (!macro_ok) { diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index b9d7947..2f16805 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -23,6 +23,7 @@ test_expect_success 'setup' ' echo offon -test test echo no notest echo A/e/F test=A/e/F + echo A\\ b test=space ) .gitattributes ( echo g test=a/g @@ -195,6 +196,10 @@ test_expect_success 'root subdir attribute test' ' attr_check subdir/a/i unspecified ' +test_expect_success 'quoting in pattern' ' + attr_check A b space +' + test_expect_success 'negative patterns' ' echo !f test=bar .gitattributes git check-attr test -- '''!f''' 2errors -- 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: error: src refspec refs/heads/master matches more than one.
John Keeping j...@keeping.me.uk writes: There's already the arbitrary set of prefixes in refs.c::prettify_refname() and refs.c::ref_rev_parse_rules(). I can see how a user might think that since git log refs/heads/name is equivalent to git log master then git branch refs/heads/name should be equivalent to git branch name. Not quite, I am afraid. Branch names used for git branch name and git checkout name are like the Lvalue of an assignment, as opposed to extended SHA-1 expressions to express any commit (e.g. 'master^0', 'refs/heads/master', or 'master') that correspond to the Rvalues used in an expression. Because git checkout can take a branch name or an arbitrary commit object name, there needs a way for the users to disambiguate. Saying that git checkout refs/heads/name must be equivalent to git checkout name is like arguing that assignment value+0 = x should be valid because value+0 is a valid value. For the first parameter to git branch, there is no ambiguity---it must be the name of a branch and cannot be an arbitrary commit object name, so special casing git branch refs/heads/master to mean git branch master may not be too bad. But then we need to either start rejecting git branch refs/tags/v1.0 or keep allowing it to create a ref refs/heads/refs/tags/v1.0 to denote the branch of that exact name given---neither of which is more consistent, so... -- 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 5/5] implement @{publish} shorthand
Jeff King p...@peff.net writes: In that sense, publish is not the best word, either, as it describes only the first two, but not the third case (and those are just examples; there may be other setups beyond that, even). Perhaps @{push} would be the most direct word. Hmph, then the other one would be @{pull}. Which does not sound too bad, IMHO. -- 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] Provide a 'git help user-manual' route to the docbook
Philip Oakley philipoak...@iee.org writes: diff --git a/Documentation/gituser-manual.txt b/Documentation/gituser-manual.txt new file mode 100644 index 000..9fd4744 --- /dev/null +++ b/Documentation/gituser-manual.txt @@ -0,0 +1,34 @@ +gituser-manual(7) += + +NAME + +gituser-manual - a link to the user-manual docbook + + +SYNOPSIS + +[verse] +'git help user-manual' + +link:user-manual.html[Git User's Manual] Is it just me, or is typing $ git help user-manual and not seeing the manual itself, but only a link you have to click to get there a worthwhile addition? I would not mind having a clickable link in the output from $ git help git or something that does already have other useful information, though. + +DESCRIPTION +--- +Git is a fast, scalable, distributed revision control system with an +unusually rich command set that provides both high-level operations +and full access to internals. + +The link:user-manual.html[Git User's Manual] provides an +in-depth introduction to Git. + +SEE ALSO + +linkgit:gittutorial[7], +linkgit:giteveryday[7], +linkgit:gitcli[7], +linkgit:gitworkflows[7] + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/builtin/help.c b/builtin/help.c index 1fdefeb..be7c39d 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -427,6 +427,7 @@ static struct { { modules, N_(Defining submodule properties) }, { revisions, N_(Specifying revisions and ranges for Git) }, { tutorial, N_(A tutorial introduction to Git (for version 1.5.1 or newer)) }, + { user-manual, N_(A link to the user-manual docbook) }, { workflows, N_(An overview of recommended workflows with Git) }, }; -- 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] Fix documentation AsciiDoc links for external urls
Roberto Tyley roberto.ty...@gmail.com writes: Turns out that putting 'link:' before the 'http' is actually superfluous in AsciiDoc, as there's already a predefined macro to handle it. http, https, [etc] URLs are rendered using predefined inline macros. http://www.methods.co.nz/asciidoc/userguide.html#_urls Hypertext links to files on the local file system are specified using the link inline macro. http://www.methods.co.nz/asciidoc/userguide.html#_linking_to_local_documents Despite being superfluous, the reference implementation of AsciiDoc tolerates the extra 'link:' and silently removes it, giving a functioning link in the generated HTML. However, AsciiDoctor (the Ruby implementation of AsciiDoc used to render the http://git-scm.com/ site) does /not/ have this behaviour, and so generates broken links, as can be seen here: http://git-scm.com/docs/git-cvsimport (links to cvs2git parsecvs) http://git-scm.com/docs/git-filter-branch (link to The BFG) It's worth noting that after this change, the html generated by 'make html' in the git project is identical, and all links still work. --- Sign-off? The overall reasoning sounds good, and the patch also looks sensible. Thanks. Documentation/git-cvsimport.txt | 4 ++-- Documentation/git-filter-branch.txt | 4 ++-- Documentation/gitcore-tutorial.txt| 2 +- Documentation/gitcvs-migration.txt| 2 +- Documentation/gitweb.txt | 2 +- Documentation/technical/http-protocol.txt | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt index 2df9953..260f39f 100644 --- a/Documentation/git-cvsimport.txt +++ b/Documentation/git-cvsimport.txt @@ -21,8 +21,8 @@ DESCRIPTION *WARNING:* `git cvsimport` uses cvsps version 2, which is considered deprecated; it does not work with cvsps version 3 and later. If you are performing a one-shot import of a CVS repository consider using -link:http://cvs2svn.tigris.org/cvs2git.html[cvs2git] or -link:https://github.com/BartMassey/parsecvs[parsecvs]. +http://cvs2svn.tigris.org/cvs2git.html[cvs2git] or +https://github.com/BartMassey/parsecvs[parsecvs]. Imports a CVS repository into Git. It will either create a new repository, or incrementally import into an existing one. diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt index 2eba627..09535f2 100644 --- a/Documentation/git-filter-branch.txt +++ b/Documentation/git-filter-branch.txt @@ -436,7 +436,7 @@ git-filter-branch allows you to make complex shell-scripted rewrites of your Git history, but you probably don't need this flexibility if you're simply _removing unwanted data_ like large files or passwords. For those operations you may want to consider -link:http://rtyley.github.io/bfg-repo-cleaner/[The BFG Repo-Cleaner], +http://rtyley.github.io/bfg-repo-cleaner/[The BFG Repo-Cleaner], a JVM-based alternative to git-filter-branch, typically at least 10-50x faster for those use-cases, and with quite different characteristics: @@ -455,7 +455,7 @@ characteristics: _is_ possible to write filters that include their own parallellism, in the scripts executed against each commit. -* The link:http://rtyley.github.io/bfg-repo-cleaner/#examples[command options] +* The http://rtyley.github.io/bfg-repo-cleaner/#examples[command options] are much more restrictive than git-filter branch, and dedicated just to the tasks of removing unwanted data- e.g: `--strip-blobs-bigger-than 1M`. diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt index 058a352..d2d7c21 100644 --- a/Documentation/gitcore-tutorial.txt +++ b/Documentation/gitcore-tutorial.txt @@ -1443,7 +1443,7 @@ Although Git is a truly distributed system, it is often convenient to organize your project with an informal hierarchy of developers. Linux kernel development is run this way. There is a nice illustration (page 17, Merges to Mainline) in -link:http://www.xenotime.net/linux/mentor/linux-mentoring-2006.pdf[Randy Dunlap's presentation]. +http://www.xenotime.net/linux/mentor/linux-mentoring-2006.pdf[Randy Dunlap's presentation]. It should be stressed that this hierarchy is purely *informal*. There is nothing fundamental in Git that enforces the chain of diff --git a/Documentation/gitcvs-migration.txt b/Documentation/gitcvs-migration.txt index 5ea94cb..5f4e890 100644 --- a/Documentation/gitcvs-migration.txt +++ b/Documentation/gitcvs-migration.txt @@ -117,7 +117,7 @@ Importing a CVS archive --- First, install version 2.1 or higher of cvsps from -link:http://www.cobite.com/cvsps/[http://www.cobite.com/cvsps/] and make +http://www.cobite.com/cvsps/[http://www.cobite.com/cvsps/] and make sure it is in your path. Then cd to a checked out CVS working directory of the project you are
Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling
John Keeping j...@keeping.me.uk writes: There are two problems here: 1) If no argument is provided, then the command segfaults 2) The argument is not consumed, so there will be excess output Fix both of these in one go by restructuring the handler for this option. Reported-by: Daniel Hahler genml+git-2...@thequod.de Signed-off-by: John Keeping j...@keeping.me.uk --- Looks sensible; thanks. builtin/rev-parse.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index aaeb611..645cc4a 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -738,9 +738,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, --resolve-git-dir)) { - const char *gitdir = resolve_gitdir(argv[i+1]); + const char *gitdir; + if (++i = argc) + die(--resolve-git-dir requires an argument); + gitdir = resolve_gitdir(argv[i]); if (!gitdir) - die(not a gitdir '%s', argv[i+1]); + die(not a gitdir '%s', argv[i]); puts(gitdir); continue; } -- 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: git gc --aggressive led to about 40 times slower git log --raw
Jonathan Nieder jrnie...@gmail.com writes: David Kastrup wrote: Duy Nguyen pclo...@gmail.com writes: Likely because --aggressive passes --depth=250 to pack-objects. Long delta chains could reduce pack size and increase I/O as well as zlib processing signficantly. [...] Compression should reduce rather than increase the total amount of reads. --depth=250 means to allow chains of To get this object, first inflate this object, then apply this delta of length 250. That's absurdly long, and doesn't even help compression much in practice (many short chains referring to the same objects tends to work fine). We probably shouldn't make --aggressive do that. Something like --depth=10 would make more sense. Yes, my thinking indeed. I didn't know --agressive was so aggressive myself, as I personally never use it. git repack -a -d -f --depth=32 window=4000 is what I often use, but I suspect most people would not be patient enough for that 4k window. Let's do something like this first and then later make --depth configurable just like --width, perhaps? For aggressive, I think the default width (hardcoded to 250 but configurable) is a bit too narrow. builtin/gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index 6be6c8d..0d010f0 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -204,7 +204,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (aggressive) { argv_array_push(repack, -f); - argv_array_push(repack, --depth=250); + argv_array_push(repack, --depth=20); if (aggressive_window 0) argv_array_pushf(repack, --window=%d, aggressive_window); } -- 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] diff: do not reuse_worktree_file for submodules
Thomas Rast t...@thomasrast.ch writes: The GIT_EXTERNAL_DIFF calling code attempts to reuse existing worktree files for the worktree side of diffs, for performance reasons. However, that code also tries to do the same with submodules. This results in calls to $GIT_EXTERNAL_DIFF where the old-file is a file of the form Submodule commit $sha1, but the new-file is a directory in the worktree. Fix it by never reusing a worktree file in the submodule case. Reported-by: Grégory Pakosz gregory.pak...@gmail.com Signed-off-by: Thomas Rast t...@thomasrast.ch --- diff.c | 5 +++-- t/t4020-diff-external.sh | 30 +- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/diff.c b/diff.c index 7c59bfe..e9a8874 100644 --- a/diff.c +++ b/diff.c @@ -2845,8 +2845,9 @@ static struct diff_tempfile *prepare_temp_file(const char *name, remove_tempfile_installed = 1; } - if (!one-sha1_valid || - reuse_worktree_file(name, one-sha1, 1)) { + if (!S_ISGITLINK(one-mode) + (!one-sha1_valid || + reuse_worktree_file(name, one-sha1, 1))) { I agree with the goal/end result, but I have to wonder if the reuse_worktree_file() be the helper function that ought to encapsulate such a logic? Instead of feeding it an object name and a path, if we passed a diff_filespec to the helper, it would have access to the mode as well. It would result in a more intrusive change, so I'd prefer to see your patch applied first and then build such a refactor on top, perhaps like the attached. diff.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/diff.c b/diff.c index a96992a..74eec80 100644 --- a/diff.c +++ b/diff.c @@ -2582,11 +2582,13 @@ void fill_filespec(struct diff_filespec *spec, const unsigned char *sha1, * the work tree has that object contents, return true, so that * prepare_temp_file() does not have to inflate and extract. */ -static int reuse_worktree_file(const char *name, const unsigned char *sha1, int want_file) +static int reuse_worktree_file(const struct diff_filespec *spec, int want_file) { const struct cache_entry *ce; struct stat st; int pos, len; + const char *name = spec-path; + const unsigned char *sha1 = spec-sha1; /* * We do not read the cache ourselves here, because the @@ -2698,7 +2700,7 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only) return diff_populate_gitlink(s, size_only); if (!s-sha1_valid || - reuse_worktree_file(s-path, s-sha1, 0)) { + reuse_worktree_file(s, 0)) { struct strbuf buf = STRBUF_INIT; struct stat st; int fd; @@ -2844,17 +2846,17 @@ static struct diff_tempfile *prepare_temp_file(const char *name, if (!S_ISGITLINK(one-mode) (!one-sha1_valid || -reuse_worktree_file(name, one-sha1, 1))) { +reuse_worktree_file(one, 1))) { struct stat st; - if (lstat(name, st) 0) { + if (lstat(one-path, st) 0) { if (errno == ENOENT) goto not_a_valid_file; - die_errno(stat(%s), name); + die_errno(stat(%s), one-path); } if (S_ISLNK(st.st_mode)) { struct strbuf sb = STRBUF_INIT; - if (strbuf_readlink(sb, name, st.st_size) 0) - die_errno(readlink(%s), name); + if (strbuf_readlink(sb, one-path, st.st_size) 0) + die_errno(readlink(%s), one-path); prep_temp_blob(name, temp, sb.buf, sb.len, (one-sha1_valid ? one-sha1 : null_sha1), @@ -2864,7 +2866,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name, } else { /* we can borrow from the file in the work tree */ - temp-name = name; + temp-name = one-path; if (!one-sha1_valid) strcpy(temp-hex, sha1_to_hex(null_sha1)); else -- 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 1/2] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
Kirill Smelkov k...@navytux.spb.ru writes: 2) alloca(), for small arrays, is used for the same reason - if we change it to xmalloc()/free() the timings get worse Do you see any use of it outside compat/? I thought we specifically avoid alloca() for portability. Also we do not use variable-length-arrays on the stack either, I think. No, no usage outside compat/ and I knew alloca and VLAs are not used in Git codebase for portability, and I understand alloca will be criticized, but wanted to start the discussion rolling. I've actually started without alloca, and used xmalloc/free for [nparent] vectors, but the impact was measurable, so it just had to be changed to something more optimal. For me, personally, alloca is ok, but I understand there could be portability issues (by the way, what compiler/system Git cares about does not have working alloca?). Thats why I propose we do the following 1. at configure time, determine, do we have working alloca, and define #define HAVE_ALLOCA if yes. 2. in code #ifdef HAVE_ALLOCA # define xalloca(size) (alloca(size)) # define xalloca_free(p)do {} while(0) #else # define xalloca(size) (xmalloc(size)) # define xalloca_free(p)(free(p)) #endif and use it like func() { p = xalloca(size); ... xalloca_free(p); } This way, for systems, where alloca is available, we'll have optimal on-stack allocations with fast executions. On the other hand, on systems, where alloca is not available, this gracefully fallbacks to xmalloc/free. Please tell me what you think. I guess the above is clean enough, and we cannot do better than that, if we want to use alloca() on platforms where we can. -- 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] Rename read_replace_refs to check_replace_refs
Michael Haggerty mhag...@alum.mit.edu writes: The semantics of this flag was changed in commit ecef23 inline lookup_replace_object() calls but wasn't renamed at the time to minimize code churn. Rename it now, and add a comment explaining its use. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This change doesn't conflict with anything in pu; perhaps we can squeeze it in now? I think it is a good time to do this kind of clean-up once the post-release dusts settle. 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: error: src refspec refs/heads/master matches more than one.
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: Duy Nguyen pclo...@gmail.com writes: + if (!force dwim_ref(name, strlen(name), sha1, real_ref)) + die(_(creating ref refs/heads/%s makes %s ambiguous.\n + Use -f to create it anyway.), + name, name); Does this check still allow you to create a branch refs/heads/next and then later create a branch next? The latter will introduce an ambiguity without any prevention, even though the prevention would trigger if the order in which these two branches are created is swapped--- the end result has ambiguity but the safety covers only one avenue to the confusing situation. And the only way I can think of to avoid that kind of confusion is to forbid creation of a subset of possible names by reserving a set of known (but arbitrary) prefixes---which I am not sure is a good way to go. At least not yet. Just for the record: after seeing the respective arguments, I consider it the sanest way. It's conceivable to give a configuration option for augmenting the set of reserved prefixes. That would allow to adapt the arbitrariness to match the policies or ref name choices of a particular project while keeping the set of references addressed by the standard git commands in check automagically. I am inclined to say that we should start by just giving a warning whenever git branch, git checkout -b, etc. tries to create a branch whose name begins with refs/ and other potentially ambiguous ones that match ref_rev_parse_rules[]. I personally do not think people name their branch with a name that begins with refs/ on purpose; I am not sure about other ones, like heads or tags. Personally I think it also is unlikely to want to have these words immediately followed by a slash in the branch name, so it may not even be necessary to give them any way to turn off the warning, which in turn would mean we can promote that warning to an die() that can be overridable with --force, but by going that first warn and see if people are annoyed route, we would hopefully find out soon enough that there may be some people who do want to name their branches in a funny way ;-) -- 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 release notes man page
Philip Oakley philipoak...@iee.org writes: A few days too late for the 1.9.0 release cycle :( This responds to Stefan Nwe's request for a 'git help' command that would access the release notes. ($gmane/240595 17 Jan 2014). I've used the full name release-notes for the help guide rather than Stefan's original 'git help relnotes'. The release-notes man page lists just the notes for the current release. The combined notes for all releases is nearing 15k lines. RelNotes are incremental and only useful for those who know what the immediately previous release contained, but for most people who get their Git from distros, I have this impression that the versions of Git they get skip versions, and seeing the notable changes since the previous source release will not give them wrong information---they may have this warm fuzzy feeling that they know what is going on, but they are missing information on all the accumulated changes that were added in earlier versions their distro skipped---these changes are still in the version they are running. I do not understand why it is even a good idea to show release notes from the command line git interface. -- 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] config: teach git config --file - to read from the standard input
Junio C Hamano gits...@pobox.com writes: I think I preferred the earlier version where you had stdin in the name field, and this hunk could just go away. I know you switched it to NULL here to avoid making bogus relative filenames in includes. Exactly the same comment here. I really like the way how this series becomes more polished every time we see it. But that would be pretty straightforward to fix by splitting the name field into an additional path field. It looks like git config --blob has the same problem (it will try relative lookups in the filesystem, even though that is nonsensical from the blob). So we could fix the existing bug at the same time. :) Perhaps this could go at the start of your series? Sounds like the right thing to do. Thanks. And [PATCH 3/3] rebased on the others may look like this. builtin/config.c | 11 +++ cache.h | 1 + config.c | 42 -- t/t1300-repo-config.sh| 17 +++-- t/t1305-config-include.sh | 16 +++- 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index de41ba5..5677c94 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -360,6 +360,9 @@ static int get_colorbool(int print) static void check_write(void) { + if (given_config_source.use_stdin) + die(writing to stdin is not supported); + if (given_config_source.blob) die(writing config blobs is not supported); } @@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + if (given_config_source.file + !strcmp(given_config_source.file, -)) { + given_config_source.file = NULL; + given_config_source.use_stdin = 1; + } + if (use_global_config) { char *user_config = NULL; char *xdg_config = NULL; @@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_argc(argc, 0, 0); if (!given_config_source.file nongit) die(not in a git directory); + if (given_config_source.use_stdin) + die(editing stdin is not supported); if (given_config_source.blob) die(editing blobs is not supported); git_config(git_default_config, NULL); diff --git a/cache.h b/cache.h index 9d94bd6..4db19b5 100644 --- a/cache.h +++ b/cache.h @@ -1147,6 +1147,7 @@ extern int update_server_info(int); #define CONFIG_GENERIC_ERROR 7 struct git_config_source { + unsigned int use_stdin:1; const char *file; const char *blob; }; diff --git a/config.c b/config.c index a21b0dd..9dd0bdb 100644 --- a/config.c +++ b/config.c @@ -1031,24 +1031,36 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data) return ret; } -int git_config_from_file(config_fn_t fn, const char *filename, void *data) +static int do_config_from_file(config_fn_t fn, + const char *filename, FILE *f, + const char *label, void *data) { - int ret; - FILE *f = fopen(filename, r); + struct config_source top; - ret = -1; - if (f) { - struct config_source top; + top.u.file = f; + top.name = label; + top.path = filename; + top.die_on_error = 1; + top.do_fgetc = config_file_fgetc; + top.do_ungetc = config_file_ungetc; + top.do_ftell = config_file_ftell; + + return do_config_from(top, fn, data); +} - top.u.file = f; - top.name = top.path = filename; - top.die_on_error = 1; - top.do_fgetc = config_file_fgetc; - top.do_ungetc = config_file_ungetc; - top.do_ftell = config_file_ftell; +static int git_config_from_stdin(config_fn_t fn, void *data) +{ + return do_config_from_file(fn, NULL, stdin, stdin, data); +} - ret = do_config_from(top, fn, data); +int git_config_from_file(config_fn_t fn, const char *filename, void *data) +{ + int ret = -1; + FILE *f; + f = fopen(filename, r); + if (f) { + ret = do_config_from_file(fn, filename, f, filename, data); fclose(f); } return ret; @@ -1190,7 +1202,9 @@ int git_config_with_options(config_fn_t fn, void *data, * If we have a specific filename, use it. Otherwise, follow the * regular lookup sequence. */ - if (config_source config_source-file) + if (config_source config_source-use_stdin) + return git_config_from_stdin(fn, data); + else if (config_source config_source-file
Re: [PATCH] Git release notes man page
Junio C Hamano gits...@pobox.com writes: Philip Oakley philipoak...@iee.org writes: A few days too late for the 1.9.0 release cycle :( This responds to Stefan Nwe's request for a 'git help' command that would access the release notes. ($gmane/240595 17 Jan 2014). I've used the full name release-notes for the help guide rather than Stefan's original 'git help relnotes'. The release-notes man page lists just the notes for the current release. The combined notes for all releases is nearing 15k lines. RelNotes are incremental and only useful for those who know what the immediately previous release contained, but for most people who get their Git from distros, I have this impression that the versions of Git they get skip versions, and seeing the notable changes since the previous source release will not give them wrong information---they Ehh,, s/will not give them/will give them/; obviously... may have this warm fuzzy feeling that they know what is going on, but they are missing information on all the accumulated changes that were added in earlier versions their distro skipped---these changes are still in the version they are running. I do not understand why it is even a good idea to show release notes from the command line git interface. -- 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-contacts: do not fail parsing of good diffs
lar...@gullik.org (Lars Gullik Bjønnes) writes: If a line in a patch starts with --- it will be deemed malformed unless it also contains the proper diff header format. This situation can happen with a valid patch if it has a line starting with -- and that line is removed. This patch just removes the check in git-contacts. Signed-off-by: Lars Gullik Bjønnes lar...@gullik.org --- If the script wanted to be more correct, it should be paying attention to the $len it already parses out of the hunk headers to make sure it does not mistake removal of a line that begins with -- as the beginning of a patch to a different path, but as the original does not seem to aim to be so careful anyway, this change should be OK, I would say. The patch was whitespace damaged, by the way. It was easy to hand tweak so there is no need to resend this particular patch, but if you are planning to send more patches, please check your MUA and tell it not to. Thanks. contrib/contacts/git-contacts | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts index 428cc1a..dbe2abf 100755 --- a/contrib/contacts/git-contacts +++ b/contrib/contacts/git-contacts @@ -96,8 +96,6 @@ sub scan_patches { next unless $id; if (m{^--- (?:a/(.+)|/dev/null)$}) { $source = $1; - } elsif (/^--- /) { - die Cannot parse hunk source: $_\n; } elsif (/^@@ -(\d+)(?:,(\d+))?/ $source) { my $len = defined($2) ? $2 : 1; push @{$sources-{$source}{$id}}, [$1, $len] if $len; -- 1.9.0 -- 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 5/5] streaming: simplify attaching a filter
John Keeping j...@keeping.me.uk writes: We are guaranteed that 'nst' is non-null because it is allocated with xmalloc(), and in fact we rely on this three lines later by unconditionally dereferencing it. The intent of the original code is for attach_stream_filter() to detect an error condition and return NULL, in which case it closes the istream it allocated and signal error to the caller, I think, and falling thru to use st-anything and return st when that happens is *not* a guarantee that a-s-f will not detect an error ever, but rather is a bug in the error codepath. Signed-off-by: John Keeping j...@keeping.me.uk --- streaming.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/streaming.c b/streaming.c index d7c9f32..8a7135d 100644 --- a/streaming.c +++ b/streaming.c @@ -151,10 +151,7 @@ struct git_istream *open_istream(const unsigned char *sha1, } if (filter) { /* Add !is_null_stream_filter(filter) for performance */ - struct git_istream *nst = attach_stream_filter(st, filter); - if (!nst) - close_istream(st); - st = nst; + st = attach_stream_filter(st, filter); } *size = st-size; -- 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 5/5] streaming: simplify attaching a filter
Junio C Hamano gits...@pobox.com writes: John Keeping j...@keeping.me.uk writes: We are guaranteed that 'nst' is non-null because it is allocated with xmalloc(), and in fact we rely on this three lines later by unconditionally dereferencing it. The intent of the original code is for attach_stream_filter() to detect an error condition and return NULL, in which case it closes the istream it allocated and signal error to the caller, I think, and falling thru to use st-anything and return st when that happens is *not* a guarantee that a-s-f will not detect an error ever, but rather is a bug in the error codepath. In other words, something like this instead. -- 8 -- Subject: [PATCH] open_istream(): do not dereference NULL in the error case When stream-filter cannot be attached, it is expected to return NULL, and we should close the stream we opened and signal an error by returning NULL ourselves from this function. However, we attempted to dereference that NULL pointer between the point we detected the error and returned from the function. Brought-to-attention-by: John Keeping j...@keeping.me.uk Signed-off-by: Junio C Hamano gits...@pobox.com --- streaming.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/streaming.c b/streaming.c index d7c9f32..2ff036a 100644 --- a/streaming.c +++ b/streaming.c @@ -152,8 +152,10 @@ struct git_istream *open_istream(const unsigned char *sha1, if (filter) { /* Add !is_null_stream_filter(filter) for performance */ struct git_istream *nst = attach_stream_filter(st, filter); - if (!nst) + if (!nst) { close_istream(st); + return NULL; + } st = nst; } -- 1.9.0-279-gdc9e3eb -- 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: git gc --aggressive led to about 40 times slower git log --raw
Duy Nguyen pclo...@gmail.com writes: Lower depth than default (50) does not sound aggressive to me, at least from disk space utilization. I agree it should be configurable though. Do you mean you want to keep --aggressive to mean too aggressive in resulting size, to the point that it is not useful to anybody? Shallow and wide will give us, with a large window, the most aggressively efficient packfiles that are useful, and we would rather want to fix it to be usable, I would think. -- 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] rev-parse: fix --resolve-git-dir argument handling
Junio C Hamano gits...@pobox.com writes: John Keeping j...@keeping.me.uk writes: There are two problems here: 1) If no argument is provided, then the command segfaults 2) The argument is not consumed, so there will be excess output Fix both of these in one go by restructuring the handler for this option. Reported-by: Daniel Hahler genml+git-2...@thequod.de Signed-off-by: John Keeping j...@keeping.me.uk --- Looks sensible; thanks. Ehh, I spoke too fast. Don't we already have this queued as a43219f2 (rev-parse: check i before using argv[i] against argc, 2014-01-28)? -- 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 1/3] introduce GIT_INDEX_VERSION environment variable
Thomas Gummerer t.gumme...@gmail.com writes: diff --git a/Documentation/git.txt b/Documentation/git.txt index aec3726..bc9eeea 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -712,6 +712,11 @@ Git so take care if using Cogito etc. index file. If not specified, the default of `$GIT_DIR/index` is used. +'GIT_INDEX_VERSION':: + This environment variable allows the specification of an index + version for new repositories. It won't affect existing index + files. By default index file version 3 is used. + This is half-correct, isn't it? In-code variable may say version 3 but we demote it to version 2 unless we absolutely need to use the version 3 ugliness. diff --git a/read-cache.c b/read-cache.c index 3f735f3..3993e12 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1223,6 +1223,22 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall #define INDEX_FORMAT_DEFAULT 3 +static unsigned int get_index_format_default() +{ + char *envversion = getenv(GIT_INDEX_VERSION); + if (!envversion) { + return INDEX_FORMAT_DEFAULT; + } else { + unsigned int version = strtol(envversion, NULL, 10); If we are using strtol() to parse it carefully, we should make sure it parses to the end by giving a non-NULL second argument and checking where the parsing stopped. diff --git a/t/t1600-index.sh b/t/t1600-index.sh new file mode 100755 index 000..37fd84d --- /dev/null +++ b/t/t1600-index.sh @@ -0,0 +1,24 @@ +#!/bin/sh + +test_description='index file specific tests' + +. ./test-lib.sh + +test_expect_success 'setup' ' + echo 1 a +' + +test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' ' + ( + GIT_INDEX_VERSION=1 + export GIT_INDEX_VERSION + git add a 21 | sed s/[0-9]// actual.err + sed -e s/ Z$/ / -\EOF expect.err + warning: GIT_INDEX_VERSION set, but the value is invalid. + Using version Z + EOF + test_i18ncmp expect.err actual.err + ) +' If we already have an index in version N when this test starts, what should happen? Will queue on 'pu', with some suggested fixups. 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 2/3] test-lib: allow setting the index format version
Thomas Gummerer t.gumme...@gmail.com writes: Allow adding a TEST_GIT_INDEX_VERSION variable to config.mak to set the index version with which the test suite should be run. ... diff --git a/Makefile b/Makefile index 287e6f8..c98d28f 100644 --- a/Makefile +++ b/Makefile @@ -342,6 +342,10 @@ all:: # Define DEFAULT_HELP_FORMAT to man, info or html # (defaults to man) if you want to have a different default when # git help is called without a parameter specifying the format. +# +# Define TEST_GIT_INDEX_FORMAT to 2, 3 or 4 to run the test suite s/_FORMAT/_VERSION/, I would think. Will queue on 'pu' with a fix-up 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] Git release notes man page
Philip Oakley philipoak...@iee.org writes: RelNotes are incremental and only useful for those who know what the immediately previous release contained, but for most people who get their Git from distros, I have this impression that the versions of Git they get skip versions, and seeing the notable changes since the previous source release will not give them wrong information---they may have this warm fuzzy feeling that they know what is going on, but they are missing information on all the accumulated changes that were added in earlier versions their distro skipped---these changes are still in the version they are running. That's a reasonable argument. I am not making an argument in order to reject the notion of making release-notes available, though. I am only raising concerns, pointing out why showing a single release notes as if that were the only one that matter is misleading. I am not opposed to the idea of making release notes available to those who do not install from the source, by the way. Being able to grep the release notes through may help people who are writing scripts using Git to learn when a feature they want to use appeared to make sure that they do not depend on something their users may not have yet. But for that kind of users, it would be more helpful to point them at the file location they can find the plain text version of release notes, instead of giving them a bunch of web links they need to read through page by page. I did look at trying to get the stalenotes to work as an alternative, that is extract the stalenotes section from the git.txt, and create a release notes man page from that. I am not sure if stale-notes section meshes well with this; the primary purpose of it was to point at the whole documentation set, not just release-notes, for versions previously released, so those who came to a website that hosts them can pick the version, possibly a stale one, that they are using and read the manual pages for that version, without seeing newer features that are not available to them. I do not think it is very useful in the context of your You received this single version of software, and you can access its documentation off-line feature---you cannot reasonably expect that such a software release to contain all the past documentation sets, but even if you could do so, that is not how normal people use the installed documentation, i.e. to learn about older releases that they no longer have. -- 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: git gc --aggressive led to about 40 times slower git log --raw
Philippe Vaucher philippe.vauc...@gmail.com writes: fwiw this is the thread that added --depth=250 http://thread.gmane.org/gmane.comp.gcc.devel/94565/focus=94626 This post is quite interesting: http://article.gmane.org/gmane.comp.gcc.devel/94637 Yes, it most clearly says that --depth=250 was *not* a recommendation, with technical background to explain why such a long delta chain is a bad idea. -- 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] gitweb: Avoid overflowing page body frame with large images
Andrew Keller and...@kellerfarm.com writes: When displaying a blob in gitweb, if it's an image, specify constraints for maximum display width and height to prevent the image from overflowing the frame of the enclosing page_body div. This change assumes that it is more desirable to see the whole image without scrolling (new behavior) than it is to see every pixel without zooming (previous behavior). Signed-off-by: Andrew Keller and...@kellerfarm.com --- This is an updated copy of this patch. Could I request a thumbs up, thumbs down, or thumbs sideways from those who develop gitweb? I do not develop gitweb, but the change looks reasonable to me. gitweb/gitweb.perl |2 +- gitweb/static/gitweb.css |5 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 3bc0f0b..79057b7 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -7094,7 +7094,7 @@ sub git_blob { git_print_page_path($file_name, blob, $hash_base); print div class=\page_body\\n; if ($mimetype =~ m!^image/!) { - print qq!img type=!.esc_attr($mimetype).qq!!; + print qq!img class=blob type=!.esc_attr($mimetype).qq!!; if ($file_name) { print qq! alt=!.esc_attr($file_name).qq! title=!.esc_attr($file_name).qq!!; } diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css index 3b4d833..3212601 100644 --- a/gitweb/static/gitweb.css +++ b/gitweb/static/gitweb.css @@ -32,6 +32,11 @@ img.avatar { vertical-align: middle; } +img.blob { + max-height: 100%; + max-width: 100%; +} + a.list img.avatar { border-style: none; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] revert.c: Allow to specify -x via git-config
Guido Günther a...@sigxcpu.org writes: Without this when maintaining stable branches it's easy to forget to use -x to track where a patch was cherry-picked from. --- Documentation/config.txt | 4 Documentation/git-cherry-pick.txt | 8 builtin/revert.c | 14 +- 3 files changed, 25 insertions(+), 1 deletion(-) Hmph. Does this round address the issues raised in the previous discussion in any way? How does it affect people's existing scripts that use cherry-pick and rely on it not doing the unwanted -x thing if such a configuration variable is introduced as the first step in the series, without even giving them to override the configured default from the command line? For that matter, I do not think a new override option from the command line is a great solution, as that approach forces people's existing script to be adjusted. I personally found the way Jonathan explained why git backport alias is the best solution (not just a usable workaround) very compelling, especially his point (3): (3) The caller explicitly specifies their intent by running git backport. It doesn't affect unrelated uses of cherry-pick on other branches. I do not even mind throwing something like this: #!/bin/sh # git backport - cherry-pick with -x always on. exec git cherry-pick -x $@ in contrib/ somewhere, which feels like a far more appropriate solution to your easy to forget problem, 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: release-notes could be clearer on git-fetch changes
Jan Engelhardt jeng...@inai.de writes: The release notes for 1.9.0 read: * The --tags option to git fetch no longer tells the command to fetch _only_ the tags. It instead fetches tags _in addition to_ what are fetched by the same command line without the option. I think the release notes should also say -- like it was done extensively for git add -- how to get back the old behavior (perhaps through now-different commands). Perhaps, but the release note is not a place to repeat what the documentation already teaches---it primarily is to enumerate the changed areas, to highlight the things users may want to look up in the documentation, to give them a starting point. You would do something like this, I would think: git fetch $there 'refs/tags/*:refs/tags/*' -- 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 14/25] setup.c: convert is_git_directory() to use strbuf
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- (Only minor nits first during this round of review) diff --git a/strbuf.h b/strbuf.h index 73e80ce..aec9fdb 100644 --- a/strbuf.h +++ b/strbuf.h @@ -116,6 +116,10 @@ extern void strbuf_add(struct strbuf *, const void *, size_t); static inline void strbuf_addstr(struct strbuf *sb, const char *s) { strbuf_add(sb, s, strlen(s)); } +static inline void strbuf_addstr_at(struct strbuf *sb, size_t len, const char *s) { Please have the opening { on its own line. Surrounding existing functions are all offenders, but that is not an excuse to make it worse (cleaning them up will need to be done in a separate patch). + strbuf_setlen(sb, len); + strbuf_add(sb, s, strlen(s)); I am not sure addstr_at() gives us a good abstraction, or at least the name conveys what it does well not to confuse readers. At first after only seeing its name, I would have expected that it would splice the given string into an existing strbuf at the location, not chopping the existing strbuf at the location and appending. +} static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2) { strbuf_grow(sb, sb2-len); strbuf_add(sb, sb2-buf, sb2-len); -- 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 18/25] setup.c: support multi-checkout repo setup
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: (Only nitpicks during this round of review). diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh index 8f36aa9..d8bdaf4 100755 --- a/t/t1501-worktree.sh +++ b/t/t1501-worktree.sh @@ -346,4 +346,80 @@ test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' ' test_cmp expected actual ' +test_expect_success 'Multi-worktree setup' ' + mkdir work + mkdir -p repo.git/repos/foo + cp repo.git/HEAD repo.git/index repo.git/repos/foo + unset GIT_DIR GIT_CONFIG GIT_WORK_TREE Are these known to be set always when we get to this point? Otherwise please use sane_unset. -- 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 23/25] checkout: detach if the branch is already checked out elsewhere
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: The normal rule is anything outside refs/heads/ is detached. This strictens the rule a bit more: if the branch is checked out (either in $GIT_COMMON_DIR/HEAD or any $GIT_DIR/repos/.../HEAD) then it's detached as well. A hint is given so the user knows where to go and do something there if they still want to checkout undetached here. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com (Only nitpicks during this round of review). diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index 76eae4a..f6a5c47 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -13,13 +13,14 @@ test_expect_success 'checkout --to not updating paths' ' ' test_expect_success 'checkout --to a new worktree' ' + git rev-parse HEAD expect git checkout --to here master ( cd here test_cmp ../init.t init.t - git symbolic-ref HEAD actual - echo refs/heads/master expect - test_cmp expect actual + ! git symbolic-ref HEAD test_must_fail? -- 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 25/25] gc: support prune --repos
(Only nitpicks during this round of review). @@ -274,6 +284,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) argv_array_pushl(reflog, reflog, expire, --all, NULL); argv_array_pushl(repack, repack, -d, -l, NULL); argv_array_pushl(prune, prune, --expire, NULL ); + argv_array_pushl(prune_repos, prune, --repos, --expire, NULL ); No SP before closing ). -- 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 01/25] path.c: make get_pathname() return strbuf instead of static buffer
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: (Only nitpicks during this round of review). -static char *get_pathname(void) +static struct strbuf *get_pathname() static struct strbuf *get_pathname(void) -- 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 24/25] prune: strategies for linked checkouts
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: (Only nitpicks during this round of review). + if (get_device_or_die(path) != get_device_or_die(get_git_dir())) { + strbuf_reset(sb); + strbuf_addf(sb, %s/locked, sb_repo.buf); + write_file(sb.buf, 1, located on a different file system\n); + keep_locked = 1; + } else { + strbuf_reset(sb); + strbuf_addf(sb, %s/link, sb_repo.buf); + link(sb_git.buf, sb.buf); /* it's ok to fail */ If so, perhaps tell that to the code by saying something like (void) link(...); ? But why is it OK to fail in the first place? If we couldn't link, don't you want to fall back to the lock codepath? After all, the are we on the same device? check is to cope with the case where we cannot link and that alternate codepath is supposed to be prepared to handle the ah, we cannot link case correctly, 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: [PATCH v3 24/25] prune: strategies for linked checkouts
Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: (Only nitpicks during this round of review). +if (get_device_or_die(path) != get_device_or_die(get_git_dir())) { +strbuf_reset(sb); +strbuf_addf(sb, %s/locked, sb_repo.buf); +write_file(sb.buf, 1, located on a different file system\n); +keep_locked = 1; +} else { +strbuf_reset(sb); +strbuf_addf(sb, %s/link, sb_repo.buf); +link(sb_git.buf, sb.buf); /* it's ok to fail */ If so, perhaps tell that to the code by saying something like (void) link(...); ? But why is it OK to fail in the first place? If we couldn't link, don't you want to fall back to the lock codepath? After all, the are we on the same device? check is to cope with the case where we cannot link and that alternate codepath is supposed to be prepared to handle the ah, we cannot link case correctly, no? In other words, couldn't that part more like this? That is, we optimisiticly try the link(2) first and if it fails for whatever reason fall back to whatever magic the lock codepath implements. + strbuf_reset(sb); + strbuf_addf(sb, %s/link, sb_repo.buf); + if (link(sb_git.buf, sb.buf) 0) { + strbuf_reset(sb); + strbuf_addf(sb, %s/locked, sb_repo.buf); + write_file(sb.buf, 1, located on a different file system\n); + keep_locked = 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 v3 00/25] Support multiple checkouts
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: In short you can attach multiple worktrees to the same git repository with git checkout --to somewhere. This is basically what git-new-workdir is for. This is exciting. I'll be pushing this out on 'pu' (with trivial fix-ups squashed in and/or queued on-top) for people to play with. I'm still slowly chewing thru these patches, though. Haven't had a chance to read them carefully yet. 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
What's cooking in git.git (Feb 2014, #06; Wed, 19)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The tip of 'next' hasn't been rewound, and none of the topics that have been cooking there has graduated, yet. Hopefully that can start happening to open the new development cycle later this week, at which time I'll be also updating tinyurl.com/gitCal and also annotate the topics listed in this file with the course of action (i.e. if you run Meta/cook -w, many are listed as Undecided right now, which I need to fix). There are many stalled topics; I think some of them do deserve to be discarded as marked, but others do solve (or at least attempt to) real issues and it would be nice to see people help unblock them (one reason for blockage would be that they introduce regressions). 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] * bc/blame-crlf-test (2014-02-18) 1 commit - blame: add a failing test for a CRLF issue. * jk/http-no-curl-easy (2014-02-18) 1 commit - http: never use curl_easy_perform * jk/janitorial-fixes (2014-02-18) 5 commits - open_istream(): do not dereference NULL in the error case - builtin/mv: don't use memory after free - utf8: use correct type for values in interval table - utf8: fix iconv error detection - notes-utils: handle boolean notes.rewritemode correctly * ks/config-file-stdin (2014-02-18) 4 commits - config: teach git config --file - to read from the standard input - config: change git_config_with_options() interface - builtin/config.c: rename check_blob_write() - check_write() - config: disallow relative include paths from blobs * lb/contrib-contacts-looser-diff-parsing (2014-02-18) 1 commit - git-contacts: do not fail parsing of good diffs * mh/replace-refs-variable-rename (2014-02-18) 1 commit - Rename read_replace_refs to check_replace_refs * nd/commit-editor-cleanup (2014-02-18) 3 commits - commit: add --cleanup=scissors - wt-status.c: move cut-line print code out to wt_status_add_cut_line - wt-status.c: make cut_line[] const to shrink .data section a bit * nd/no-more-fnmatch (2014-02-18) 4 commits - Actually remove compat fnmatch source code - Stop using fnmatch (either native or compat) - Revert test-wildmatch: add perf command to compare wildmatch and fnmatch - Use wildmatch() directly without fnmatch() wrapper * nd/reset-setup-worktree (2014-02-18) 1 commit - reset: optionally setup worktree and refresh index on --mixed * po/git-help-user-manual (2014-02-18) 1 commit - Provide a 'git help user-manual' route to the docbook * rt/links-for-asciidoctor (2014-02-18) 1 commit - Fix documentation AsciiDoc links for external urls * tg/index-v4-format (2014-02-18) 5 commits - read-cache: add index.version config variable - FIXUP - test-lib: allow setting the index format version - FIXUP - introduce GIT_INDEX_VERSION environment variable * tr/diff-submodule-no-reuse-worktree (2014-02-18) 2 commits - diff: refactor reuse_worktree_file() - diff: do not reuse_worktree_file for submodules * nd/multiple-work-trees (2014-02-19) 26 commits - FIXUP??? - gc: support prune --repos - prune: strategies for linked checkouts - checkout: detach if the branch is already checked out elsewhere - checkout: clean up half-prepared directories in --to mode - checkout: support checking out into a new working directory - use new wrapper write_file() for simple file writing - wrapper.c: wrapper to open a file, fprintf then close - setup.c: support multi-checkout repo setup - setup.c: detect $GIT_COMMON_DIR check_repository_format_gently() - setup.c: convert check_repository_format_gently to use strbuf - setup.c: detect $GIT_COMMON_DIR in is_git_directory() - setup.c: convert is_git_directory() to use strbuf - git-stash: avoid hardcoding $GIT_DIR/logs/ - *.sh: avoid hardcoding $GIT_DIR/hooks/... - git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects - Add new environment variable $GIT_COMMON_DIR - commit: use SEQ_DIR instead of hardcoding sequencer - fast-import: use git_path() for accessing .git dir instead of get_git_dir() - reflog: avoid constructing .lock path with git_path - *.sh: respect $GIT_INDEX_FILE - Make git_path() aware of file relocation in $GIT_DIR - path.c: group git_path(), git_pathdup() and strbuf_git_path() together - path.c: rename vsnpath() to do_git_path() - Convert git_snpath() to strbuf_git_path() - path.c: make get_pathname() return strbuf instead of static buffer -- [Stalled] * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to be formatted into a manual pages to begin with, and also its contents
Re: [PATCH v3 01/25] path.c: make get_pathname() return strbuf instead of static buffer
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: We've been avoiding PATH_MAX whenever possible. This patch makes get_pathname() return a strbuf and updates the callers to take advantage of this. The code is simplified as we no longer need to worry about buffer overflow. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- path.c | 119 +++-- 1 file changed, 50 insertions(+), 69 deletions(-) Nice. char *git_pathdup(const char *fmt, ...) { - char path[PATH_MAX], *ret; + struct strbuf *path = get_pathname(); va_list args; va_start(args, fmt); - ret = vsnpath(path, sizeof(path), fmt, args); + vsnpath(path, fmt, args); va_end(args); - return xstrdup(ret); + return strbuf_detach(path, NULL); } This feels somewhat wrong. This function is for callers who are willing to take ownership of the path buffer and promise to free the returned buffer when they are done, because you are returning strbuf_detach()'ed piece of memory, giving the ownership away. The whole point of using get_pathname() is to allow callers not to care about allocation issues on the paths they scribble on during their short-and-simple codepaths that do not have too many uses of similar temporary path buffers. Why borrow from that round-robin pool (which may now cause some codepaths to overflow the number of such active temporary path buffers---have they been all audited)? Is there a reason not to do just an equivalent of #define git_pathdup mkpathdup and be done with it? Am I missing something? char *mkpathdup(const char *fmt, ...) { - char *path; struct strbuf sb = STRBUF_INIT; va_list args; - va_start(args, fmt); strbuf_vaddf(sb, fmt, args); va_end(args); - path = xstrdup(cleanup_path(sb.buf)); - - strbuf_release(sb); - return path; + strbuf_cleanup_path(sb); + return strbuf_detach(sb, NULL); } char *mkpath(const char *fmt, ...) { va_list args; - unsigned len; - char *pathname = get_pathname(); - + struct strbuf *pathname = get_pathname(); va_start(args, fmt); - len = vsnprintf(pathname, PATH_MAX, fmt, args); + strbuf_vaddf(pathname, fmt, args); va_end(args); - if (len = PATH_MAX) - return bad_path; - return cleanup_path(pathname); + return cleanup_path(pathname-buf); } On the other hand, this one does seem correct. char *git_path(const char *fmt, ...) { - char *pathname = get_pathname(); + struct strbuf *pathname = get_pathname(); va_list args; - char *ret; - va_start(args, fmt); - ret = vsnpath(pathname, PATH_MAX, fmt, args); + vsnpath(pathname, fmt, args); va_end(args); - return ret; + return pathname-buf; } So does this. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/25] Convert git_snpath() to strbuf_git_path()
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: @@ -651,14 +653,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts, new-name); } } - if (old-path old-name) { - char log_file[PATH_MAX], ref_file[PATH_MAX]; - - git_snpath(log_file, sizeof(log_file), logs/%s, old-path); - git_snpath(ref_file, sizeof(ref_file), %s, old-path); - if (!file_exists(ref_file) file_exists(log_file)) - remove_path(log_file); - } + if (old-path old-name + !file_exists(git_path(%s, old-path)) + file_exists(git_path(logs/%s, old-path))) + remove_path(git_path(logs/%s, old-path)); Hmph. Is this conversion safe? This adds three uses of the round-robin path buffer; if a caller of this function used two or more path buffers obtained from get_pathname() and expected their contents to remain stable across the call to this, it will silently break. -- 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 02/25] Convert git_snpath() to strbuf_git_path()
Duy Nguyen pclo...@gmail.com writes: - } + if (old-path old-name + !file_exists(git_path(%s, old-path)) + file_exists(git_path(logs/%s, old-path))) + remove_path(git_path(logs/%s, old-path)); Hmph. Is this conversion safe? This adds three uses of the round-robin path buffer; if a caller of this function used two or more path buffers obtained from get_pathname() and expected their contents to remain stable across the call to this, it will silently break. three round-robin buffers but not all required at the same time, once the first file_exists() returns the first round-robin buffer could be free, and remove_path() calls git_path again, not reusing the result from the second file_exists, so the second buffer is free to go too. I know these three callers to git_path() will not step on each other's toes. But that is not the question I asked. -- 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: error: src refspec refs/heads/master matches more than one.
Michael Haggerty mhag...@alum.mit.edu writes: I wonder whether we could give a way to specify a reference in an unambiguous, canonical fashion like I expected, for example by using a leading slash: /refs/heads/mybranch. This could be a way for the user to ask for DWIMming to be turned off without having to resort to plumbing commands like update-ref. This wouldn't necessarily solve the problem, but it would at least lead the new user to type git branch /refs/heads/mybranch instead of the ambiguous command above, which Git could either accept or reject in good conscience rather than having to speculate about what the user *really* meant. I think that supporting absolute reference names like this would also be useful for scripts, which otherwise probably often have subtle failure modes if the user has defined reference names that are ambiguous, modulo DWIM, with the reference that the script intended. I do agree that things start to become confusing to the end users when we tell refnames and object names apart and behave differently, e.g. git checkout master vs git checkout master^0 (this example uses a disambiguation syntax that is related to but different from what you brought up). For the name in git branch name [commit] (but not commit), I do not see much value in allowing the users to say refs/heads/ in the first place---all the local branch refs are to be created in refs/heads/ anyway and git branch /refs/tags/bar (if we were to allow your notation to name an absolute ref) will have to be checked and signaled as an error. Even though there is no reason to forbid a ref to be named in such a way at the lowest machinery level (read: at the sha1_name.c layer) [*1*], I would say it would be better to at least warn users when they create such a ref with Porcelain commands like branch, checkout -b, etc., or even outright forbid. In other contexts, however, it _might_ make sense, but I am somewhat skeptical. For example, if you have a branch 'foo' (whose ref being refs/heads/foo) and a branch 'refs/heads/foo' (whose ref being refs/heads/refs/heads/foo) at the same time, you need some way to clarify that you mean the former, and one way to do so may be git branch newfoo /refs/heads/foo and removing the latter would be git branch -D /refs/heads/refs/heads/foo perhaps. But this starts to sound like a workaround to a problem that the user ended up having such a strangely named branch in the first place, not a useful feature. [Footnote] *1* The way refs are used and the specific meanings given to some of the hierarchies under refs/ by the core-git Porcelain is not fundamental to the data model of Git. Git the SCM by convention uses refs/heads/ for branches, and it is perfectly fine for Git the SCM to enforce its own policy like that to its end users and forbid creating and using any ref outside that hierarchy as a local branch (e.g. checking it out), but I'd prefer it if we can keep the lower level a general filesystem to build SCM on top layer as separate from such policy decision as possible. -- 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