[PATCH 0/6] Re-roll rr/triangle
There are two important changes in this round: 1. Rebased on latest master, resolving all conflicts (all in t5516). I ran the testsuite immediately after resolving the conflicts, so everything should be good. 2. Peff's suggestion to avoid using test_must_fail on compound statements. I've changed the meaning of the tests slightly to (approximately) invert the tests. The inter-diff follows. diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 13028a4..3281ff1 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -521,14 +521,14 @@ git config --remove-section remote.there git config --remove-section branch.master test_expect_success 'push with remote.pushdefault' ' - mk_test up_repo heads/frotz + mk_test up_repo heads/master mk_test down_repo heads/master test_config remote.up.url up_repo test_config remote.down.url down_repo test_config branch.master.remote up test_config remote.pushdefault down git push - test_must_fail check_push_result up_repo $the_commit heads/master + check_push_result up_repo $the_first_commit heads/master check_push_result down_repo $the_commit heads/master ' @@ -543,8 +543,8 @@ test_expect_success 'push with config remote.*.pushurl' ' ' test_expect_success 'push with config branch.*.pushremote' ' - mk_test up_repo heads/frotz - mk_test side_repo heads/quux + mk_test up_repo heads/master + mk_test side_repo heads/master mk_test down_repo heads/master test_config remote.up.url up_repo test_config remote.pushdefault side_repo @@ -552,8 +552,8 @@ test_expect_success 'push with config branch.*.pushremote' ' test_config branch.master.remote up test_config branch.master.pushremote down git push - test_must_fail check_push_result up_repo $the_commit heads/master - test_must_fail check_push_result side_repo $the_commit heads/master + check_push_result up_repo $the_first_commit heads/master + check_push_result side_repo $the_first_commit heads/master check_push_result down_repo $the_commit heads/master ' Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.name.pushremote Documentation/config.txt | 24 +++- builtin/push.c | 2 +- remote.c | 41 -- remote.h | 1 + t/t5516-fetch-push.sh| 322 +++ 5 files changed, 241 insertions(+), 149 deletions(-) -- 1.8.2.363.g901f5bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] remote.c: simplify a bit of code using git_config_string()
A small segment where handle_config() parses the branch.remote configuration variable can be simplified using git_config_string(). Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- remote.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/remote.c b/remote.c index ca1edd9..34ddc5b 100644 --- a/remote.c +++ b/remote.c @@ -357,9 +357,8 @@ static int handle_config(const char *key, const char *value, void *cb) return 0; branch = make_branch(name, subkey - name); if (!strcmp(subkey, .remote)) { - if (!value) - return config_error_nonbool(key); - branch-remote_name = xstrdup(value); + if (git_config_string(branch-remote_name, key, value)) + return -1; if (branch == current_branch) { default_remote_name = branch-remote_name; explicit_default_remote_name = 1; -- 1.8.2.363.g901f5bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] t5516 (fetch-push): update test description
The file was originally created in bcdb34f (Test wildcard push/fetch, 2007-06-08), and only contained tests that exercised wildcard functionality at the time. In subsequent commits, many other tests unrelated to wildcards were added but the test description was never updated. Fix this. Helped-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t5516-fetch-push.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 383a2eb..f394271 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1,6 +1,17 @@ #!/bin/sh -test_description='fetching and pushing, with or without wildcard' +test_description='Basic fetch/push functionality. + +This test checks the following functionality: + +* command-line syntax +* refspecs +* fast-forward detection, and overriding it +* configuration +* hooks +* --porcelain output format +* hiderefs +' . ./test-lib.sh -- 1.8.2.363.g901f5bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] t5516 (fetch-push): drop implicit arguments from helper functions
From: Jeff King p...@peff.net Many of the tests in t5516 look like: mk_empty git push testrepo ... check_push_result $commit heads/master It's reasonably easy to see what is being tested, with the exception that testrepo is a magic global name (it is implicitly used in the helpers, but we have to name it explicitly when calling git directly). Let's make it explicit when call the helpers, too. This is slightly more typing, but makes the test snippets read more naturally. It also makes it easy for future tests to use an alternate or multiple repositories, without a proliferation of helper functions. [rr: fixed sloppy quoting] Signed-off-by: Jeff King p...@peff.net Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t5516-fetch-push.sh | 282 ++ 1 file changed, 145 insertions(+), 137 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index f394271..b800a8e 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -18,10 +18,11 @@ This test checks the following functionality: D=`pwd` mk_empty () { - rm -fr testrepo - mkdir testrepo + repo_name=$1 + rm -fr $repo_name + mkdir $repo_name ( - cd testrepo + cd $repo_name git init git config receive.denyCurrentBranch warn mv .git/hooks .git/hooks-disabled @@ -29,14 +30,17 @@ mk_empty () { } mk_test () { - mk_empty + repo_name=$1 + shift + + mk_empty $repo_name ( for ref in $@ do - git push testrepo $the_first_commit:refs/$ref || + git push $repo_name $the_first_commit:refs/$ref || exit done - cd testrepo + cd $repo_name for ref in $@ do echo $the_first_commit expect @@ -49,9 +53,10 @@ mk_test () { } mk_test_with_hooks() { + repo_name=$1 mk_test $@ ( - cd testrepo + cd $repo_name mkdir .git/hooks cd .git/hooks @@ -83,13 +88,16 @@ mk_test_with_hooks() { } mk_child() { - rm -rf $1 - git clone testrepo $1 + rm -rf $2 + git clone $1 $2 } check_push_result () { + repo_name=$1 + shift + ( - cd testrepo + cd $repo_name echo $1 expect shift for ref in $@ @@ -119,7 +127,7 @@ test_expect_success setup ' ' test_expect_success 'fetch without wildcard' ' - mk_empty + mk_empty testrepo ( cd testrepo git fetch .. refs/heads/master:refs/remotes/origin/master @@ -131,7 +139,7 @@ test_expect_success 'fetch without wildcard' ' ' test_expect_success 'fetch with wildcard' ' - mk_empty + mk_empty testrepo ( cd testrepo git config remote.up.url .. @@ -145,7 +153,7 @@ test_expect_success 'fetch with wildcard' ' ' test_expect_success 'fetch with insteadOf' ' - mk_empty + mk_empty testrepo ( TRASH=$(pwd)/ cd testrepo @@ -161,7 +169,7 @@ test_expect_success 'fetch with insteadOf' ' ' test_expect_success 'fetch with pushInsteadOf (should not rewrite)' ' - mk_empty + mk_empty testrepo ( TRASH=$(pwd)/ cd testrepo @@ -177,7 +185,7 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' ' ' test_expect_success 'push without wildcard' ' - mk_empty + mk_empty testrepo git push testrepo refs/heads/master:refs/remotes/origin/master ( @@ -189,7 +197,7 @@ test_expect_success 'push without wildcard' ' ' test_expect_success 'push with wildcard' ' - mk_empty + mk_empty testrepo git push testrepo refs/heads/*:refs/remotes/origin/* ( @@ -201,7 +209,7 @@ test_expect_success 'push with wildcard' ' ' test_expect_success 'push with insteadOf' ' - mk_empty + mk_empty testrepo TRASH=$(pwd)/ test_config url.$TRASH.insteadOf trash/ git push trash/testrepo refs/heads/master:refs/remotes/origin/master @@ -214,7 +222,7 @@ test_expect_success 'push with insteadOf' ' ' test_expect_success 'push with pushInsteadOf' ' - mk_empty + mk_empty testrepo TRASH=$(pwd)/ test_config url.$TRASH.pushInsteadOf trash/ git push trash/testrepo refs/heads/master:refs/remotes/origin/master @@ -227,7 +235,7 @@ test_expect_success 'push with pushInsteadOf' ' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' - mk_empty + mk_empty testrepo TRASH=$(pwd)/
[PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
Currently, do_push() in push.c calls remote_get(), which gets the configured remote for fetching and pushing. Replace this call with a call to pushremote_get() instead, a new function that will return the remote configured specifically for pushing. This function tries to work with the string pushremote_name, before falling back to the codepath of remote_get(). This patch has no visible impact, but serves to enable future patches to introduce configuration variables to set pushremote_name. For example, you can now do the following in handle_config(): if (!strcmp(key, remote.pushdefault)) git_config_string(pushremote_name, key, value); Then, pushes will automatically go to the remote specified by remote.pushdefault. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/push.c | 2 +- remote.c | 25 + remote.h | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index 5e4a0e9..909c34d 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags) static int do_push(const char *repo, int flags) { int i, errs; - struct remote *remote = remote_get(repo); + struct remote *remote = pushremote_get(repo); const char **url; int url_nr; diff --git a/remote.c b/remote.c index 34ddc5b..2b06e22 100644 --- a/remote.c +++ b/remote.c @@ -49,6 +49,7 @@ static int branches_nr; static struct branch *current_branch; static const char *default_remote_name; +static const char *pushremote_name; static int explicit_default_remote_name; static struct rewrites rewrites; @@ -670,17 +671,21 @@ static int valid_remote_nick(const char *name) return !strchr(name, '/'); /* no slash */ } -struct remote *remote_get(const char *name) +static struct remote *remote_get_1(const char *name, const char *pushremote_name) { struct remote *ret; int name_given = 0; - read_config(); if (name) name_given = 1; else { - name = default_remote_name; - name_given = explicit_default_remote_name; + if (pushremote_name) { + name = pushremote_name; + name_given = 1; + } else { + name = default_remote_name; + name_given = explicit_default_remote_name; + } } ret = make_remote(name, 0); @@ -699,6 +704,18 @@ struct remote *remote_get(const char *name) return ret; } +struct remote *remote_get(const char *name) +{ + read_config(); + return remote_get_1(name, NULL); +} + +struct remote *pushremote_get(const char *name) +{ + read_config(); + return remote_get_1(name, pushremote_name); +} + int remote_is_configured(const char *name) { int i; diff --git a/remote.h b/remote.h index 8743d6e..cf56724 100644 --- a/remote.h +++ b/remote.h @@ -51,6 +51,7 @@ struct remote { }; struct remote *remote_get(const char *name); +struct remote *pushremote_get(const char *name); int remote_is_configured(const char *name); typedef int each_remote_fn(struct remote *remote, void *priv); -- 1.8.2.363.g901f5bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] remote.c: introduce remote.pushdefault
This new configuration variable defines the default remote to push to, and overrides `branch.name.remote` for all branches. It is useful in the typical triangular-workflow setup, where the remote you're fetching from is different from the remote you're pushing to. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/config.txt | 13 ++--- remote.c | 7 +++ t/t5516-fetch-push.sh| 12 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index f79184c..9c6fd4a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -727,9 +727,12 @@ branch.autosetuprebase:: This option defaults to never. branch.name.remote:: - When in branch name, it tells 'git fetch' and 'git push' which - remote to fetch from/push to. It defaults to `origin` if no remote is - configured. `origin` is also used if you are not on any branch. + When on branch name, it tells 'git fetch' and 'git push' + which remote to fetch from/push to. The remote to push to + may be overridden with `remote.pushdefault` (for all branches). + If no remote is configured, or if you are not on any branch, + it defaults to `origin` for fetching and `remote.pushdefault` + for pushing. branch.name.merge:: Defines, together with branch.name.remote, the upstream branch @@ -1898,6 +1901,10 @@ receive.updateserverinfo:: If set to true, git-receive-pack will run git-update-server-info after receiving data from git-push and updating refs. +remote.pushdefault:: + The remote to push to by default. Overrides + `branch.name.remote` for all branches. + remote.name.url:: The URL of a remote repository. See linkgit:git-fetch[1] or linkgit:git-push[1]. diff --git a/remote.c b/remote.c index 2b06e22..6337e11 100644 --- a/remote.c +++ b/remote.c @@ -389,9 +389,16 @@ static int handle_config(const char *key, const char *value, void *cb) add_instead_of(rewrite, xstrdup(value)); } } + if (prefixcmp(key, remote.)) return 0; name = key + 7; + + /* Handle remote.* variables */ + if (!strcmp(name, pushdefault)) + return git_config_string(pushremote_name, key, value); + + /* Handle remote.name.* variables */ if (*name == '/') { warning(Config remote shorthand cannot begin with '/': %s, name); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index b800a8e..797b537 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -497,6 +497,18 @@ test_expect_success 'push with config remote.*.push = HEAD' ' check_push_result testrepo $the_first_commit heads/local ' +test_expect_success 'push with remote.pushdefault' ' + mk_test up_repo heads/master + mk_test down_repo heads/master + test_config remote.up.url up_repo + test_config remote.down.url down_repo + test_config branch.master.remote up + test_config remote.pushdefault down + git push + check_push_result up_repo $the_first_commit heads/master + check_push_result down_repo $the_commit heads/master +' + test_expect_success 'push with config remote.*.pushurl' ' mk_test testrepo heads/master -- 1.8.2.363.g901f5bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] remote.c: introduce branch.name.pushremote
This new configuration variable overrides `remote.pushdefault` and `branch.name.remote` for pushes. When you pull from one place (e.g. your upstream) and push to another place (e.g. your own publishing repository), you would want to set `remote.pushdefault` to specify the remote to push to for all branches, and use this option to override it for a specific branch. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/config.txt | 19 +++ remote.c | 4 t/t5516-fetch-push.sh| 15 +++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9c6fd4a..3d750e0 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -730,9 +730,19 @@ branch.name.remote:: When on branch name, it tells 'git fetch' and 'git push' which remote to fetch from/push to. The remote to push to may be overridden with `remote.pushdefault` (for all branches). - If no remote is configured, or if you are not on any branch, - it defaults to `origin` for fetching and `remote.pushdefault` - for pushing. + The remote to push to, for the current branch, may be further + overridden by `branch.name.pushremote`. If no remote is + configured, or if you are not on any branch, it defaults to + `origin` for fetching and `remote.pushdefault` for pushing. + +branch.name.pushremote:: + When on branch name, it overrides `branch.name.remote` for + pushing. It also overrides `remote.pushdefault` for pushing + from branch name. When you pull from one place (e.g. your + upstream) and push to another place (e.g. your own publishing + repository), you would want to set `remote.pushdefault` to + specify the remote to push to for all branches, and use this + option to override it for a specific branch. branch.name.merge:: Defines, together with branch.name.remote, the upstream branch @@ -1903,7 +1913,8 @@ receive.updateserverinfo:: remote.pushdefault:: The remote to push to by default. Overrides - `branch.name.remote` for all branches. + `branch.name.remote` for all branches, and is overridden by + `branch.name.pushremote` for specific branches. remote.name.url:: The URL of a remote repository. See linkgit:git-fetch[1] or diff --git a/remote.c b/remote.c index 6337e11..68eb99b 100644 --- a/remote.c +++ b/remote.c @@ -364,6 +364,10 @@ static int handle_config(const char *key, const char *value, void *cb) default_remote_name = branch-remote_name; explicit_default_remote_name = 1; } + } else if (!strcmp(subkey, .pushremote)) { + if (branch == current_branch) + if (git_config_string(pushremote_name, key, value)) + return -1; } else if (!strcmp(subkey, .merge)) { if (!value) return config_error_nonbool(key); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 797b537..7bf1555 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -519,6 +519,21 @@ test_expect_success 'push with config remote.*.pushurl' ' check_push_result testrepo $the_commit heads/master ' +test_expect_success 'push with config branch.*.pushremote' ' + mk_test up_repo heads/master + mk_test side_repo heads/master + mk_test down_repo heads/master + test_config remote.up.url up_repo + test_config remote.pushdefault side_repo + test_config remote.down.url down_repo + test_config branch.master.remote up + test_config branch.master.pushremote down + git push + check_push_result up_repo $the_first_commit heads/master + check_push_result side_repo $the_first_commit heads/master + check_push_result down_repo $the_commit heads/master +' + test_expect_success 'push with dry-run' ' mk_test testrepo heads/master -- 1.8.2.363.g901f5bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] send-email: use the three-arg form of open in recipients_cmd
Junio C Hamano wrote: we can silence Perlcritique, even though we do not gain much safety by doing so. Nit: it's perlcritic; critique is used to refer to the output of a critic. -- 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: clarify documentation for the --verify option
On 04/01/2013 06:56 PM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Because the primary use case of this option is to implement end-user input validation, I think it would be helpful to clarify use of the peeler here. Perhaps ... A SQUASH??? patch on top of your original is queued on 'pu', together with the earlier ^{object} peeler patch. Comments, improvements, etc. would be nice. Yes, your version is better. I would make one change, though. In your + Make sure the single given parameter can be turned into a + raw 20-byte SHA-1 that can be used to access the object + database, and emit it to the standard output. If it can't, + error out. it could be made clearer that exactly one parameter should be provided. Maybe + Verify that exactly one parameter is provided, and that it + can be turned into a raw 20-byte SHA-1 that can be used to + access the object database. If so, emit the SHA-1 to the + standard output; otherwise, error out. But this makes it sound a little like the raw 20-byte SHA-1 will be output to stdout, whereas both the input and the output are in fact 40-character hex-encoded SHA-1s. Perhaps a further change s/raw 20-byte SHA-1/full SHA-1/ would avoid the false implication? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] send-email: use return; not return undef; on error codepaths
Junio C Hamano wrote: Note that we leave return undef; in validate_address on purpose, even though Perlcritic may complain. The primary return site of the function returns whatever is in the scaler variable $address, so it is pointless to change only the other return undef; to return. The caller must be prepared to see an array with a single undef as the return value from this subroutine anyway. The way I understood it: All callsites only ever call validate_address via validate_address_list, which in turn maps a list of addresses by calling validate_address on each address. Therefore, validate_address returning an undef in one codepath is correct. -- 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] peel_onion(): teach $foo^{object} peeler
While I was in the middle of suggesting documentation for this new syntax, I discovered that you already added documentation to your repo but didn't mention the new version on the mailing list (or maybe I overlooked it). It would be helpful if you would submit your own changes to the mailing list to make it harder for the rest of us to overlook them--and easier to look over them :-) The documentation looks fine to me. Off topic: Your patch reminds me of something else that surprised me: there is no $userstring^{tag}. I suppose it would be a bit ambiguous, given that tags can point at tags, and it would also be less useful than the other suffixes. But its absence irked the completionist in me :-) Michael On 04/01/2013 12:38 AM, Junio C Hamano wrote: A string that names an object can be suffixed with ^{type} peeler to say I have this object name; peel it until you get this type. If you cannot do so, it is an error. v1.8.2^{commit} asks for a commit that is pointed at an annotated tag v1.8.2; v1.8.2^{tree} unwraps it further to the top-level tree object. A special suffix ^{} (i.e. no type specified) means I do not care what it unwraps to; just peel annotated tag until you get something that is not a tag. When you have a random user-supplied string, you can turn it to a bare 40-hex object name, and cause it to error out if such an object does not exist, with: git rev-parse --verify $userstring^{} for most objects, but this does not yield the tag object name when $userstring refers to an annotated tag. Introduce a new suffix, ^{object}, that only makes sure the given name refers to an existing object. Then git rev-parse --verify $userstring^{object} becomes a way to make sure $userstring refers to an existing object. This is necessary because the plumbing rev-parse --verify is only about make sure the argument is something we can feed to get_sha1() and turn it into a raw 20-byte object name SHA-1 and is not about make sure that 20-byte object name SHA-1 refers to an object that exists in our object store. When the given $userstring is already a 40-hex, by definition rev-parse --verify $userstring can turn it into a raw 20-byte object name. With $userstring^{object}, we can make sure that the 40-hex string names an object that exists in our object store before --verify kicks in. Signed-off-by: Junio C Hamano gits...@pobox.com --- sha1_name.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sha1_name.c b/sha1_name.c index 45788df..85b6e75 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -594,7 +594,7 @@ struct object *peel_to_type(const char *name, int namelen, while (1) { if (!o || (!o-parsed !parse_object(o-sha1))) return NULL; - if (o-type == expected_type) + if (expected_type == OBJ_ANY || o-type == expected_type) return o; if (o-type == OBJ_TAG) o = ((struct tag*) o)-tagged; @@ -645,6 +645,8 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) expected_type = OBJ_TREE; else if (!strncmp(blob_type, sp, 4) sp[4] == '}') expected_type = OBJ_BLOB; + else if (!prefixcmp(sp, object})) + expected_type = OBJ_ANY; else if (sp[0] == '}') expected_type = OBJ_NONE; else if (sp[0] == '/') -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: Move hard-coded colors to .gitk
On Thu, Mar 28, 2013 at 4:23 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Would we consider shipping some themes with gitk, in contrib/ perhaps? It does not seem like anyone is against it. I am not sure how gitk development integrates into git's, but I guess we will have to wait until the patch is integrated not only to gitk but also to git, before having themes that require it. Other than that, I think it's a good 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
[PATCH 0/3] Some small fixes to glossary-content.txt
While proof-reading the user-manual I noticed some issues with glossary-content.txt: - There is some outdated, misleading or irrelevant information which might only confuse new Git users and should therefore be removed. - The entries for object, object name and SHA1 lacked a little bit of consistency. - The glossary contains partial definitions for refspec and pathspec. The refspec definition was replaced by a link to the git-push man-page. I also removed the definition for pathspec but didn't find a single place where pathspecs are defined. The glossary surely is the wrong place for a complete definition, but is there something missing here or is it sufficient to spread the definition of pathspecs to the relevant man-pages? [PATCH 1/3] Remove outdated/missleading/irrelevant entries from glossary-content.txt [PATCH 2/3] Improve description of SHA1 related topics in glossary-content.txt [PATCH 3/3] Remove definition of refspec and pathspec from glossary-content.txt --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] Remove outdated/missleading/irrelevant entries from glossary-content.txt
Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/glossary-content.txt | 28 +--- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index eb7ba84..ab02238 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -5,7 +5,7 @@ [[def_bare_repository]]bare repository:: A bare repository is normally an appropriately - named def_directory,directory with a `.git` suffix that does not + named directory with a `.git` suffix that does not have a locally checked-out copy of any of the files under revision control. That is, all of the Git administrative and control files that would normally be present in the @@ -79,7 +79,7 @@ to point at the new commit. An def_object,object which contains the information about a particular def_revision,revision, such as def_parent,parents, committer, author, date and the def_tree_object,tree object which corresponds - to the top def_directory,directory of the stored + to the top directory of the stored revision. [[def_core_git]]core Git:: @@ -104,26 +104,11 @@ to point at the new commit. an arbitrary def_commit,commit that isn't necessarily the tip of any particular branch. In this case HEAD is said to be detached. -[[def_dircache]]dircache:: - You are *way* behind. See def_index,index. - -[[def_directory]]directory:: - The list you get with ls :-) - [[def_dirty]]dirty:: A def_working_tree,working tree is said to be dirty if it contains modifications which have not been def_commit,committed to the current def_branch,branch. -[[def_ent]]ent:: - Favorite synonym to def_tree-ish,tree-ish by some total geeks. See - http://en.wikipedia.org/wiki/Ent_(Middle-earth) for an in-depth - explanation. Avoid this term, not to confuse people. - -[[def_evil_merge]]evil merge:: - An evil merge is a def_merge,merge that introduces changes that - do not appear in any def_parent,parent. - [[def_fast_forward]]fast-forward:: A fast-forward is a special type of def_merge,merge where you have a def_revision,revision and you are merging another @@ -257,8 +242,7 @@ This commit is referred to as a merge commit, or sometimes just a def_object,object. [[def_octopus]]octopus:: - To def_merge,merge more than two def_branch,branches. Also denotes an - intelligent predator. + To def_merge,merge more than two def_branch,branches. [[def_origin]]origin:: The default upstream def_repository,repository. Most projects have @@ -468,9 +452,7 @@ should not be combined with other pathspec. object of an arbitrary type (typically a tag points to either a def_tag_object,tag or a def_commit_object,commit object). In contrast to a def_head,head, a tag is not updated by - the `commit` command. A Git tag has nothing to do with a Lisp - tag (which would be called an def_object_type,object type - in Git's context). A tag is most typically used to mark a particular + the `commit` command. A tag is most typically used to mark a particular point in the commit ancestry def_chain,chain. [[def_tag_object]]tag object:: @@ -494,7 +476,7 @@ should not be combined with other pathspec. [[def_tree_object]]tree object:: An def_object,object containing a list of file names and modes along with refs to the associated blob and/or tree objects. A - def_tree,tree is equivalent to a def_directory,directory. + def_tree,tree is equivalent to a directory. [[def_tree-ish]]tree-ish:: A def_ref,ref pointing to either a def_commit_object,commit -- 1.8.1.msysgit.1 --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Improve description of SHA1 related topics in glossary-content.txt
Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/glossary-content.txt | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index ab02238..05bfebc 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -146,7 +146,7 @@ to point at the new commit. created. Configured via the `.git/info/grafts` file. [[def_hash]]hash:: - In Git's context, synonym to def_object_name,object name. + In Git's context, synonym for def_object_name,object name. [[def_head]]head:: A def_ref,named reference to the def_commit,commit at the tip of a @@ -230,10 +230,9 @@ This commit is referred to as a merge commit, or sometimes just a Synonym for def_object_name,object name. [[def_object_name]]object name:: - The unique identifier of an def_object,object. The def_hash,hash - of the object's contents using the Secure Hash Algorithm - 1 and usually represented by the 40 character hexadecimal encoding of - the def_hash,hash of the object. + The unique identifier of an def_object,object: The def_SHA1,SHA1 hash + of the object's contents. The object name is usually represented by the + 40 character hexadecimal encoding of the hash value. [[def_object_type]]object type:: One of the identifiers def_commit_object,commit, @@ -426,7 +425,8 @@ should not be combined with other pathspec. Source code management (tool). [[def_SHA1]]SHA1:: - Synonym for def_object_name,object name. + Secure Hash Algorithm 1; a cryptographic hash function. + In the context of Git used as a synonym for def_object_name,object name. [[def_shallow_repository]]shallow repository:: A shallow def_repository,repository has an incomplete -- 1.8.1.msysgit.1 --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] Remove definition of refspec and pathspec from glossary-content.txt
Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/glossary-content.txt | 65 ++ 1 file changed, 3 insertions(+), 62 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index 05bfebc..1c9c522 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -261,59 +261,7 @@ This commit is referred to as a merge commit, or sometimes just a pack. [[def_pathspec]]pathspec:: - Pattern used to specify paths. -+ -Pathspecs are used on the command line of git ls-files, git -ls-tree, git add, git grep, git diff, git checkout, -and many other commands to -limit the scope of operations to some subset of the tree or -worktree. See the documentation of each command for whether -paths are relative to the current directory or toplevel. The -pathspec syntax is as follows: - -* any path matches itself -* the pathspec up to the last slash represents a - directory prefix. The scope of that pathspec is - limited to that subtree. -* the rest of the pathspec is a pattern for the remainder - of the pathname. Paths relative to the directory - prefix will be matched against that pattern using fnmatch(3); - in particular, '*' and '?' _can_ match directory separators. -+ -For example, Documentation/*.jpg will match all .jpg files -in the Documentation subtree, -including Documentation/chapter_1/figure_1.jpg. - -+ -A pathspec that begins with a colon `:` has special meaning. In the -short form, the leading colon `:` is followed by zero or more magic -signature letters (which optionally is terminated by another colon `:`), -and the remainder is the pattern to match against the path. The optional -colon that terminates the magic signature can be omitted if the pattern -begins with a character that cannot be a magic signature and is not a -colon. -+ -In the long form, the leading colon `:` is followed by a open -parenthesis `(`, a comma-separated list of zero or more magic words, -and a close parentheses `)`, and the remainder is the pattern to match -against the path. -+ -The magic signature consists of an ASCII symbol that is not -alphanumeric. -+ --- -top `/`;; - The magic word `top` (mnemonic: `/`) makes the pattern match - from the root of the working tree, even when you are running - the command from inside a subdirectory. --- -+ -Currently only the slash `/` is recognized as the magic signature, -but it is envisioned that we will support more types of magic in later -versions of Git. -+ -A pathspec with only a colon means there is no pathspec. This form -should not be combined with other pathspec. + Pattern used to specify paths in Git commands. [[def_parent]]parent:: A def_commit_object,commit object contains a (possibly empty) list @@ -382,15 +330,8 @@ should not be combined with other pathspec. [[def_refspec]]refspec:: A refspec is used by def_fetch,fetch and def_push,push to describe the mapping between remote - def_ref,ref and local ref. They are combined with a colon in - the format src:dst, preceded by an optional plus sign, +. - For example: `git fetch $URL - refs/heads/master:refs/heads/origin` means grab the master - def_branch,branch def_head,head from the $URL and store - it as my origin branch head. And `git push - $URL refs/heads/master:refs/heads/to-upstream` means publish my - master branch head as to-upstream branch at $URL. See also - linkgit:git-push[1]. + def_ref,ref and local ref. + See linkgit:git-push[1] for details. [[def_remote_tracking_branch]]remote-tracking branch:: A regular Git def_branch,branch that is used to follow changes from -- 1.8.1.msysgit.1 --- Thomas -- 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/4] transport-helper: check if remote helper is alive
On Mon, Apr 1, 2013 at 11:19 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Apr 1, 2013 at 11:01 PM, Jeff King p...@peff.net wrote: On Mon, Apr 01, 2013 at 10:51:20PM -0600, Felipe Contreras wrote: So in fetch_with_import, we have a remote-helper, and we have a bidirectional pipe to it. We then call get_importer, which starts fast-import, whose stdin is connected to the stdout of the remote helper. We tell the remote-helper to run the import, then we wait for fast-import to finish (and complain if it fails). Then what? We seem to do some more work, which I think is what causes the errors you see; but should we instead be reaping the helper at this point unconditionally? Its stdout has presumably been flushed out to fast-import; is there anything else for us to get from it besides its exit code? The problem is not with import, since fast-import would generally wait properly for a 'done' status, the problem is with export. Your patch modified fetch_with_import. Are you saying that it isn't necessary to do so? It's not, I added it for symmetry. But that's the case *if* the remote-helper is properly using the done feature. Actually, it is a problem, because without this check the transport-helper just goes on without realizing the whole thing has failed and doesn't produce a proper error message: fatal: bad object error: testgit::/home/felipec/dev/git/t/trash directory.t5801-remote-helpers/server did not send all necessary objects It's possible to send a ping command to the remote-helper, but doing so triggers a SIGPIPE. I would rather show a proper error message as my patch suggests by just checking if the command is running. Also, the design is such that the remote-helper stays alive, even after fast-export has finished. So if we expect to be able to communicate with the remote-helper after fast-export has exited, is it a protocol failure that the helper does not say yes, I finished the export or similar? If so, can we fix that? I am not too familiar with this protocol, but it looks like we read from helper-out right after closing the exporter, to get the ref statuses. Shouldn't we be detecting the error if the helper hangs up there? I guess that should be possible, I'll give that a try. I gave this a try and it does work, but it seems rather convoluted to me: diff --git a/transport-helper.c b/transport-helper.c index f0d28aa..10b7842 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -25,7 +25,8 @@ struct helper_data { option : 1, push : 1, connect : 1, - no_disconnect_req : 1; + no_disconnect_req : 1, + done_export : 1; char *export_marks; char *import_marks; /* These go from remote name (as in list) to private name */ @@ -46,7 +47,7 @@ static void sendline(struct helper_data *helper, struct strbuf *buffer) die_errno(Full write to remote helper failed); } -static int recvline_fh(FILE *helper, struct strbuf *buffer) +static int recvline_fh(FILE *helper, struct strbuf *buffer, int safe) { strbuf_reset(buffer); if (debug) @@ -54,7 +55,10 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer) if (strbuf_getline(buffer, helper, '\n') == EOF) { if (debug) fprintf(stderr, Debug: Remote helper quit.\n); - exit(128); + if (safe) + return -1; + else + exit(128); } if (debug) @@ -64,7 +68,12 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer) static int recvline(struct helper_data *helper, struct strbuf *buffer) { - return recvline_fh(helper-out, buffer); + return recvline_fh(helper-out, buffer, 0); +} + +static int recvline_safe(struct helper_data *helper, struct strbuf *buffer) +{ + return recvline_fh(helper-out, buffer, 1); } static void xchgline(struct helper_data *helper, struct strbuf *buffer) @@ -201,6 +210,8 @@ static struct child_process *get_helper(struct transport *transport) strbuf_addstr(arg, --import-marks=); strbuf_addstr(arg, capname + strlen(import-marks )); data-import_marks = strbuf_detach(arg, NULL); + } else if (!strcmp(capname, done-export)) { + data-done_export = 1; } else if (mandatory) { die(Unknown mandatory capability %s. This remote helper probably needs newer version of Git., @@ -540,7 +551,7 @@ static int process_connect_service(struct transport *transport, goto exit; sendline(data, cmdbuf); - recvline_fh(input, cmdbuf); + recvline_fh(input, cmdbuf, 0); if (!strcmp(cmdbuf.buf, )) {
[PATCH v2 0/4] run-command: new check_command helper
Hi, Here's the second version of the patches incorporating input from Junio and Peff. The first patch does all the work, the second patch uses it; basically, this is needed so the transport-helper code is able to check if the remote-helper child is stilll running. Without this support, the status of the remote-helper files and configuration can end up very badly when errors occur, to the point where the user is unable to use it any more. The rest of the patches are for testing purposes only. I ran all the tests with these, and I didn't see any problems. Felipe Contreras (4): run-command: add new check_command helper transport-helper: check if remote helper is alive tmp: remote-helper: add timers to catch errors tmp: run-command: code to exercise check_command git-remote-testgit| 12 ++ run-command.c | 58 ++- run-command.h | 5 t/t5801-remote-helpers.sh | 19 transport-helper.c| 11 + 5 files changed, 99 insertions(+), 6 deletions(-) -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] run-command: add new check_command helper
And persistent_waitpid() to recover the information from the last run. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- run-command.c | 53 +++-- run-command.h | 5 + 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/run-command.c b/run-command.c index 07e27ff..b900c6e 100644 --- a/run-command.c +++ b/run-command.c @@ -226,14 +226,27 @@ static inline void set_cloexec(int fd) fcntl(fd, F_SETFD, flags | FD_CLOEXEC); } -static int wait_or_whine(pid_t pid, const char *argv0) +static pid_t persistent_waitpid(struct child_process *cmd, pid_t pid, int *status) +{ + pid_t waiting; + + if (cmd-last_status.valid) { + *status = cmd-last_status.status; + return pid; + } + + while ((waiting = waitpid(pid, status, 0)) 0 errno == EINTR) + ; /* nothing */ + return waiting; +} + +static int wait_or_whine(struct child_process *cmd, pid_t pid, const char *argv0) { int status, code = -1; pid_t waiting; int failed_errno = 0; - while ((waiting = waitpid(pid, status, 0)) 0 errno == EINTR) - ; /* nothing */ + waiting = persistent_waitpid(cmd, pid, status); if (waiting 0) { failed_errno = errno; @@ -276,6 +289,8 @@ int start_command(struct child_process *cmd) int failed_errno = failed_errno; char *str; + cmd-last_status.valid = 0; + /* * In case of errors we must keep the promise to close FDs * that have been passed in via -in and -out. @@ -437,7 +452,7 @@ fail_pipe: * At this point we know that fork() succeeded, but execvp() * failed. Errors have been reported to our stderr. */ - wait_or_whine(cmd-pid, cmd-argv[0]); + wait_or_whine(cmd, cmd-pid, cmd-argv[0]); failed_errno = errno; cmd-pid = -1; } @@ -542,7 +557,7 @@ fail_pipe: int finish_command(struct child_process *cmd) { - return wait_or_whine(cmd-pid, cmd-argv[0]); + return wait_or_whine(cmd, cmd-pid, cmd-argv[0]); } int run_command(struct child_process *cmd) @@ -553,6 +568,32 @@ int run_command(struct child_process *cmd) return finish_command(cmd); } +int check_command(struct child_process *cmd) +{ + int status; + pid_t waiting; + + if (cmd-last_status.valid) + return 0; + + while ((waiting = waitpid(cmd-pid, status, WNOHANG)) 0 errno == EINTR) + ; /* nothing */ + + if (!waiting) + return 1; + + if (waiting == cmd-pid) { + cmd-last_status.valid = 1; + cmd-last_status.status = status; + return 0; + } + + if (waiting 0) + die(BUG: waitpid reported a random pid?); + + return 0; +} + static void prepare_run_command_v_opt(struct child_process *cmd, const char **argv, int opt) @@ -729,7 +770,7 @@ error: int finish_async(struct async *async) { #ifdef NO_PTHREADS - return wait_or_whine(async-pid, child process); + return wait_or_whine(cmd, async-pid, child process); #else void *ret = (void *)(intptr_t)(-1); diff --git a/run-command.h b/run-command.h index 221ce33..74a733d 100644 --- a/run-command.h +++ b/run-command.h @@ -39,11 +39,16 @@ struct child_process { unsigned stdout_to_stderr:1; unsigned use_shell:1; unsigned clean_on_exit:1; + struct last_status { + unsigned valid:1; + int status; + } last_status; }; int start_command(struct child_process *); int finish_command(struct child_process *); int run_command(struct child_process *); +int check_command(struct child_process *); extern char *find_hook(const char *name); extern int run_hook(const char *index_file, const char *name, ...); -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] transport-helper: check if remote helper is alive
Otherwise transport-helper will continue checking for refs and other things what will confuse the user more. --- git-remote-testgit| 11 +++ t/t5801-remote-helpers.sh | 19 +++ transport-helper.c| 8 3 files changed, 38 insertions(+) diff --git a/git-remote-testgit b/git-remote-testgit index b395c8d..ca0cf09 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -61,12 +61,23 @@ do echo feature import-marks=$gitmarks echo feature export-marks=$gitmarks fi + + if test -n $GIT_REMOTE_TESTGIT_FAILURE + then + exit -1 + fi + echo feature done git fast-export ${testgitmarks_args[@]} $refs | sed -e s#refs/heads/#${prefix}/heads/#g echo done ;; export) + if test -n $GIT_REMOTE_TESTGIT_FAILURE + then + exit -1 + fi + before=$(git for-each-ref --format='%(refname) %(objectname)') git fast-import ${testgitmarks_args[@]} --quiet diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f387027..efe67e2 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -166,4 +166,23 @@ test_expect_success 'push ref with existing object' ' compare_refs local dup server dup ' +test_expect_success 'proper failure checks for fetching' ' + (GIT_REMOTE_TESTGIT_FAILURE=1 + export GIT_REMOTE_TESTGIT_FAILURE + cd local + test_must_fail git fetch 2 error + grep Error while running helper error + ) +' + +# We sleep to give fast-export a chance to catch the SIGPIPE +test_expect_failure 'proper failure checks for pushing' ' + (GIT_REMOTE_TESTGIT_FAILURE=1 + export GIT_REMOTE_TESTGIT_FAILURE + cd local + test_must_fail git push --all 2 error + grep Error while running helper error + ) +' + test_done diff --git a/transport-helper.c b/transport-helper.c index cb3ef7d..dfdfa7a 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -460,6 +460,10 @@ static int fetch_with_import(struct transport *transport, if (finish_command(fastimport)) die(Error while running fast-import); + + if (!check_command(data-helper)) + die(Error while running helper); + argv_array_free_detached(fastimport.argv); /* @@ -818,6 +822,10 @@ static int push_refs_with_export(struct transport *transport, if (finish_command(exporter)) die(Error while running fast-export); + + if (!check_command(data-helper)) + die(Error while running helper); + push_update_refs_status(data, remote_refs); return 0; } -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] tmp: remote-helper: add timers to catch errors
This way the test reliably succeeds (in catching the failure). Not sure what's the proper way to do this, but here it is for the record. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-remote-testgit| 1 + t/t5801-remote-helpers.sh | 2 +- transport-helper.c| 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/git-remote-testgit b/git-remote-testgit index ca0cf09..6ae1b7f 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -75,6 +75,7 @@ do export) if test -n $GIT_REMOTE_TESTGIT_FAILURE then + sleep 1 # don't let fast-export get SIGPIPE exit -1 fi diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index efe67e2..d6702b4 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -176,7 +176,7 @@ test_expect_success 'proper failure checks for fetching' ' ' # We sleep to give fast-export a chance to catch the SIGPIPE -test_expect_failure 'proper failure checks for pushing' ' +test_expect_success 'proper failure checks for pushing' ' (GIT_REMOTE_TESTGIT_FAILURE=1 export GIT_REMOTE_TESTGIT_FAILURE cd local diff --git a/transport-helper.c b/transport-helper.c index dfdfa7a..f0d28aa 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -823,6 +823,9 @@ static int push_refs_with_export(struct transport *transport, if (finish_command(exporter)) die(Error while running fast-export); + if (getenv(GIT_REMOTE_TESTGIT_FAILURE)) + sleep(2); + if (!check_command(data-helper)) die(Error while running helper); -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] tmp: run-command: code to exercise check_command
Do not merge! Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- run-command.c | 5 + 1 file changed, 5 insertions(+) diff --git a/run-command.c b/run-command.c index b900c6e..2bb3dcf 100644 --- a/run-command.c +++ b/run-command.c @@ -246,6 +246,11 @@ static int wait_or_whine(struct child_process *cmd, pid_t pid, const char *argv0 pid_t waiting; int failed_errno = 0; + do { + if (!check_command(cmd)) + break; + } while(1); + waiting = persistent_waitpid(cmd, pid, status); if (waiting 0) { -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] count-objects: output KiB instead of kilobytes
The code uses division by 1024. Also, the manual uses KiB. Signed-off-by: Mihai Capotă mi...@mihaic.ro --- builtin/count-objects.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/count-objects.c b/builtin/count-objects.c index 9afaa88..ecc13b0 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) printf(garbage: %lu\n, garbage); } else - printf(%lu objects, %lu kilobytes\n, + printf(%lu objects, %lu KiB\n, loose, (unsigned long) (loose_size / 1024)); return 0; } -- 1.7.9.5 -- 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
Bug in filter-branch -d option, new files are dumped into parent
Hi, I think I have stumbled on a bug in the -d option of git filter-branch. It seems like in the final stage of filter-branch, regardless of where -d is set, it will make updates to the working directory as being the parent of the -d directory, and the actual working directory is left as it were before the filtering. For example if using -d /tmp/git-rewrite, the new checkout files are dumped into /tmp. A simple test scenario: ### mkdir test cd test git init echo foo foo git add foo git commit -mfoo echo bar bar git add bar git commit -mbar git checkout -b rewrite git filter-branch -d /tmp/git-rewrite --tree-filter 'echo baz baz' -- rewrite # git status # shows 'baz' as unstaged-deleted in the working directory # # ls /tmp/ # shows the 'baz' file created in the root, above git-rewrite # # git log --stat --oneline # does show expected result though # # if you run it again you'll get an error because of the /tmp/baz file git reset --hard master git filter-branch -f -d /tmp/git-rewrite --tree-filter 'echo baz baz' -- rewrite # Rewrite afac18542ac3d432b647866f5ac6918b81b3bb78 (2/2) # Ref 'refs/heads/rewrite' was rewritten # error: Untracked working tree file 'baz' would be overwritten by merge. # # At this point, 'baz' is instead staged-deleted in the working directory ### -- Martin Erik Werner martinerikwer...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: zsh completion broken for file completion
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 01/04/2013 11:29, Felipe Contreras ha scritto: On Thu, Feb 28, 2013 at 12:59 PM, Manlio Perillo manlio.peri...@gmail.com mailto:manlio.peri...@gmail.com wrote: [1] Basically, on my system I need the following change at the end of the file: -_git +autoload -U +X compinit compinit +compdef _git git gitk I don't know the reason, however; and it seems that it is a problem only for me Are you sourcing this script? If so, don't; do what is suggested at the top: use fpath to load it automatically. I'm using fpath as documented. However I tested the script again, and now seems to work correctly. It is possible that in the past I was using an incorrect configuration. Thanks Manlio Perillo -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlFa01IACgkQscQJ24LbaUQOmACghDC30GqXXPIExHOPl9HrrO1y BYgAn2QPAYvtsSAAiPpgMnmMRI3z0LE8 =kmm0 -END PGP SIGNATURE- -- 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 0/6] Re-roll rr/triangle
On Tue, Apr 02, 2013 at 01:10:28PM +0530, Ramkumar Ramachandra wrote: Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.name.pushremote Thanks; I didn't see any problems in this round. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] status: show commit sha1 in You are currently reverting message
Signed-off-by: Matthieu Moy matthieu@imag.fr --- t/t7512-status-help.sh | 7 --- wt-status.c| 8 ++-- wt-status.h| 1 + 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index d745cf4..bf08d4e 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -686,10 +686,11 @@ test_expect_success 'status while reverting commit (conflicts)' ' test_commit old to-revert.txt echo new to-revert.txt test_commit new to-revert.txt - test_must_fail git revert HEAD^ + TO_REVERT=$(git rev-parse --short HEAD^) + test_must_fail git revert $TO_REVERT cat expected -EOF # On branch master - # You are currently reverting a commit. + # You are currently reverting commit $TO_REVERT. # (fix conflicts and run git revert --continue) # (use git revert --abort to cancel the revert operation) # @@ -710,7 +711,7 @@ test_expect_success 'status while reverting commit (conflicts resolved)' ' git add to-revert.txt cat expected -EOF # On branch master - # You are currently reverting a commit. + # You are currently reverting commit $TO_REVERT. # (all conflicts fixed: run git revert --continue) # (use git revert --abort to cancel the revert operation) # diff --git a/wt-status.c b/wt-status.c index 5123c71..e956910 100644 --- a/wt-status.c +++ b/wt-status.c @@ -969,7 +969,8 @@ static void show_revert_in_progress(struct wt_status *s, struct wt_status_state *state, const char *color) { - status_printf_ln(s, color, _(You are currently reverting a commit.)); + status_printf_ln(s, color, _(You are currently reverting commit %s.), +find_unique_abbrev(state-revert_head_sha1, DEFAULT_ABBREV)); if (advice_status_hints) { if (has_unmerged(s)) status_printf_ln(s, color, @@ -1105,6 +1106,7 @@ void wt_status_get_state(struct wt_status_state *state, int get_detached_from) { struct stat st; + unsigned char sha1[20]; if (!stat(git_path(MERGE_HEAD), st)) { state-merge_in_progress = 1; @@ -1132,8 +1134,10 @@ void wt_status_get_state(struct wt_status_state *state, state-bisect_in_progress = 1; state-branch = read_and_strip_branch(BISECT_START); } - if (!stat(git_path(REVERT_HEAD), st)) { + if (!stat(git_path(REVERT_HEAD), st) + !get_sha1(REVERT_HEAD, sha1)) { state-revert_in_progress = 1; + hashcpy(state-revert_head_sha1, sha1); } if (get_detached_from) diff --git a/wt-status.h b/wt-status.h index 35cd6cb..4121bc2 100644 --- a/wt-status.h +++ b/wt-status.h @@ -85,6 +85,7 @@ struct wt_status_state { char *onto; char *detached_from; unsigned char detached_sha1[20]; + unsigned char revert_head_sha1[20]; }; void wt_status_prepare(struct wt_status *s); -- 1.8.2.359.g6e2e2c6.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] filter-branch: return to original dir after filtering
The first thing filter-branch does is to create a temporary directory, either .git-rewrite in the current directory (which may be the working tree or the repository if bare), or in a directory specified by -d. We then chdir to $tempdir/t as our temporary working directory in which to run tree filters. After finishing the filter, we then attempt to go back to the original directory with cd ../... This works in the .git-rewrite case, but if -d is used, we end up in a random directory. The only thing we do after this chdir is to run git-read-tree, but that means that: 1. The working directory is not updated to reflect the filtered history. 2. We dump random files into $tempdir/.. (e.g., if you use -d /tmp/foo, we dump junk into /tmp). Fix it by recording the full path to the original directory and returning there explicitly. Signed-off-by: Jeff King p...@peff.net --- On Tue, Apr 02, 2013 at 02:32:21PM +0200, Martin Erik Werner wrote: I think I have stumbled on a bug in the -d option of git filter-branch. It seems like in the final stage of filter-branch, regardless of where -d is set, it will make updates to the working directory as being the parent of the -d directory, and the actual working directory is left as it were before the filtering. Yep, definitely a bug. Thanks for reporting. git-filter-branch.sh | 5 +++-- t/t7003-filter-branch.sh | 14 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 5314249..ac2a005 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -199,6 +199,7 @@ esac test -d $tempdir die $tempdir already exists, please remove it esac +orig_dir=$(pwd) mkdir -p $tempdir/t tempdir=$(cd $tempdir; pwd) cd $tempdir/t @@ -206,7 +207,7 @@ die die # Remove tempdir on exit -trap 'cd ../..; rm -rf $tempdir' 0 +trap 'cd $orig_dir; rm -rf $tempdir' 0 ORIG_GIT_DIR=$GIT_DIR ORIG_GIT_WORK_TREE=$GIT_WORK_TREE @@ -469,7 +470,7 @@ fi done fi -cd ../.. +cd $orig_dir rm -rf $tempdir trap - 0 diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index 1e7a209..9496736 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -64,6 +64,20 @@ test_expect_success 'correct GIT_DIR while using -d' ' grep drepo $TRASHDIR/backup-refs ' +test_expect_success 'tree-filter works with -d' ' + git init drepo-tree + ( + cd drepo-tree + test_commit one + git filter-branch -d $TRASHDIR/dfoo \ + --tree-filter echo changed one.t + echo changed expect + git cat-file blob HEAD:one.t actual + test_cmp expect actual + test_cmp one.t actual + ) +' + test_expect_success 'Fail if commit filter fails' ' test_must_fail git filter-branch -f --commit-filter exit 1 HEAD ' -- 1.8.2.rc0.33.gd915649 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] status: show 'revert' state and status hint
This is the logical equivalent for git status of 3ee4452 (bash: teach __git_ps1 about REVERT_HEAD). Signed-off-by: Matthieu Moy matthieu@imag.fr --- --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -282,6 +282,8 @@ __git_ps1 () r=|MERGING elif [ -f $g/CHERRY_PICK_HEAD ]; then r=|CHERRY-PICKING + elif [ -f $g/REVERT_HEAD ]; then + r=|REVERTING Good. It makes sense to also teach git status about REVERT_HEAD. t/t7512-status-help.sh | 57 ++ wt-status.c| 24 + wt-status.h| 1 + 3 files changed, 82 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 06749a6..d745cf4 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -678,4 +678,61 @@ test_expect_success 'status showing detached from a tag' ' test_i18ncmp expected actual ' +test_expect_success 'status while reverting commit (conflicts)' ' + git checkout master + echo before to-revert.txt + test_commit before to-revert.txt + echo old to-revert.txt + test_commit old to-revert.txt + echo new to-revert.txt + test_commit new to-revert.txt + test_must_fail git revert HEAD^ + cat expected -EOF + # On branch master + # You are currently reverting a commit. + # (fix conflicts and run git revert --continue) + # (use git revert --abort to cancel the revert operation) + # + # Unmerged paths: + # (use git reset HEAD file... to unstage) + # (use git add file... to mark resolution) + # + # both modified: to-revert.txt + # + no changes added to commit (use git add and/or git commit -a) + EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status while reverting commit (conflicts resolved)' ' + echo reverted to-revert.txt + git add to-revert.txt + cat expected -EOF + # On branch master + # You are currently reverting a commit. + # (all conflicts fixed: run git revert --continue) + # (use git revert --abort to cancel the revert operation) + # + # Changes to be committed: + # (use git reset HEAD file... to unstage) + # + # modified: to-revert.txt + # + # Untracked files not listed (use -u option to show untracked files) + EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status after reverting commit' ' + git revert --continue + cat expected -\EOF + # On branch master + nothing to commit (use -u to show untracked files) + EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + test_done diff --git a/wt-status.c b/wt-status.c index cea8e55..5123c71 100644 --- a/wt-status.c +++ b/wt-status.c @@ -965,6 +965,25 @@ static void show_cherry_pick_in_progress(struct wt_status *s, wt_status_print_trailer(s); } +static void show_revert_in_progress(struct wt_status *s, + struct wt_status_state *state, + const char *color) +{ + status_printf_ln(s, color, _(You are currently reverting a commit.)); + if (advice_status_hints) { + if (has_unmerged(s)) + status_printf_ln(s, color, + _( (fix conflicts and run \git revert --continue\))); + else + status_printf_ln(s, color, + _( (all conflicts fixed: run \git revert --continue\))); + } + if (advice_status_hints) + status_printf_ln(s, color, + _( (use \git revert --abort\ to cancel the revert operation))); + wt_status_print_trailer(s); +} + static void show_bisect_in_progress(struct wt_status *s, struct wt_status_state *state, const char *color) @@ -1113,6 +1132,9 @@ void wt_status_get_state(struct wt_status_state *state, state-bisect_in_progress = 1; state-branch = read_and_strip_branch(BISECT_START); } + if (!stat(git_path(REVERT_HEAD), st)) { + state-revert_in_progress = 1; + } if (get_detached_from) wt_status_get_detached_from(state); @@ -1130,6 +1152,8 @@ static void wt_status_print_state(struct wt_status *s, show_rebase_in_progress(s, state, state_color); else if (state-cherry_pick_in_progress) show_cherry_pick_in_progress(s, state, state_color); + else if
Re: check-attr doesn't respect recursive definitions
On Sat, Mar 30, 2013 at 09:45:51AM +, Jan Larres wrote: I am trying to write a custom archiving script that checks the export-ignore attribute to know which files from an ls-files output it should skip. Through this I noticed that for files in directories for which the export-ignore (or any other) attribute is set, check-attr still reports 'unspecified'. More precisely: $ git init test Initialized empty Git repository in /home/jan/test/.git/ $ cd test $ mkdir foo $ touch foo/bar $ echo foo export-ignore .gitattributes $ git check-attr export-ignore foo foo: export-ignore: set $ git check-attr export-ignore foo/bar foo/bar: export-ignore: unspecified I would expect the last command to also report 'set'. I've also tried other patterns like 'foo/' and 'foo*', but it didn't make any difference. Is this expected behaviour? It does make checking the attributes of single files somewhat more difficult. Yes, it is the expected behavior, though I cannot offhand think of anything that would break if we did apply it recursively. git-archive ignores the directory as expected, but unfortunately it doesn't have an option to just list the files it would archive instead of actually archiving them. Yes, git-archive feeds the directories into the attribute machinery as it traverses the tree, so it actually checks for attributes of foo before recursing. You can do the same, but I agree it is quite a bit more annoying than just piping ls-files -z into check-attr --stdin -z. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ZSH segmentation fault while completing git mv dir/
Felipe Contreras felipe.contre...@gmail.com writes: And this is a workaround: --- a/contrib/completion/git-completion.zsh +++ b/contrib/completion/git-completion.zsh @@ -66,7 +66,7 @@ __gitcomp_file () local IFS=$'\n' compset -P '*[=:]' - compadd -Q -p ${2-} -f -- ${=1} _ret=0 + compadd -Q -p ${2-} -- ${=1} _ret=0 } OK, not something we want to apply to git.git, but this means a workaround for users is to create a _git file with this content instead instead of copying/symlinking git-completion.zsh as _git (replace $GIT_ROOT_PATH with the appropriate value): . $GIT_ROOT_PATH/contrib/completion/git-completion.zsh # Work around ZSH bug on zsh 4.3.10-dev-1-cvs0720, which segfaults # when completing git mv subdir/tab by redefining __gitcomp_file __gitcomp_file () { emulate -L zsh local IFS=$'\n' compset -P '*[=:]' # The original had a -f here to indicate file completion. compadd -Q -p ${2-} -- ${=1} _ret=0 } (Hope this helps in case someone has the same problem and finds this thread ...) Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
Sorry for the late reply, Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: When the commands give an actual output (e.g. when ran with -v), the output is visually mixed with the warning. The newline makes the actual output more visible. Signed-off-by: Matthieu Moy matthieu@imag.fr --- It would have been easier to immediately understand what is going on if you said blank line instead of newline ;-) Indeed. An obvious issues is what if user does not run with -v or if -v produces no results. We will be left with an extra blank line at the end. Right, but displaying the blank line only when there's an actual output does not seem easy, and I'd rather avoid too much damage in the code for a warning which is only temporary. I suspect that the true reason why the warning does not stand out and other output looks mixed in it may be because we only prefix the first line with the warning: label. In the longer term, I have a feeling that we should be showing something like this instead: $ cd t echo t*.sh git add -u -v warning: The behavior of 'git add --update (or -u)' with no path ar... warning: subdirectory of the tree will change in Git 2.0 and should... warning: To add content for the whole tree, run: I personnally do not like this kind of output, the warning: on the 2nd and 3rd lines break the flow reading the message. But that's probably a matter of taste. using a logic similar to what strbuf_add_commented_lines() and strbuf_add_lines() use. This would mean changing the warning() function, which would change all warnings. I'm fine with either dropping my patch or applying it as-is (with s/newline/blank line/ in the commit message); a bit reluctant to changing the output of warning(...), but that's an option if other people like it. builtin/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index ab1c9e8..620bf00 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -344,7 +344,7 @@ static void warn_pathless_add(const char *option_name, const char *short_name) { git add %s .\n (or git add %s .)\n \n - With the current Git version, the command is restricted to the current directory.), + With the current Git version, the command is restricted to the current directory.\n), option_name, short_name, option_name, short_name, option_name, short_name); -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: check-attr doesn't respect recursive definitions
On Tue, Apr 02, 2013 at 10:31:30AM -0400, Jeff King wrote: On Sat, Mar 30, 2013 at 09:45:51AM +, Jan Larres wrote: I am trying to write a custom archiving script that checks the export-ignore attribute to know which files from an ls-files output it should skip. Through this I noticed that for files in directories for which the export-ignore (or any other) attribute is set, check-attr still reports 'unspecified'. More precisely: $ git init test Initialized empty Git repository in /home/jan/test/.git/ $ cd test $ mkdir foo $ touch foo/bar $ echo foo export-ignore .gitattributes $ git check-attr export-ignore foo foo: export-ignore: set $ git check-attr export-ignore foo/bar foo/bar: export-ignore: unspecified I would expect the last command to also report 'set'. I've also tried other patterns like 'foo/' and 'foo*', but it didn't make any difference. Is this expected behaviour? It does make checking the attributes of single files somewhat more difficult. Yes, it is the expected behavior, though I cannot offhand think of anything that would break if we did apply it recursively. Naively, such a patch might look something like this: diff --git a/attr.c b/attr.c index de170ff..ac04188 100644 --- a/attr.c +++ b/attr.c @@ -791,8 +791,18 @@ static void collect_all_attrs(const char *path) check_all_attr[i].value = ATTR__UNKNOWN; rem = attr_nr; - for (stk = attr_stack; 0 rem stk; stk = stk-prev) + for (stk = attr_stack; 0 rem stk; stk = stk-prev) { + last_slash = path; + for (cp = path; *cp; cp++) { + if (*cp == '/') { + rem = fill(path, cp - path + 1, + last_slash - path, + stk, rem); + last_slash = cp; + } + } rem = fill(path, pathlen, basename_offset, stk, rem); + } } int git_check_attr(const char *path, int num, struct git_attr_check *check) which is based on current next (because it has some other related fixes for handling directories). It seems to work for me, but I suspect we could do it more optimally. If you have N files in foo/, this will check foo/ against the attribute stack N times, and we should be able to amortize that work. Hmm. Actually, thinking on it more, I'm not sure this is even going to give the right answer anyway, as it will check foo against the foo/.gitattributes, which is not right. I think we'd need to collect attributes as we walk the stack in prepare_attr_stack. So take this as a this is not how to do it patch. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] send-email: use return; not return undef; on error codepaths
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: Note that we leave return undef; in validate_address on purpose, even though Perlcritic may complain. The primary return site of the function returns whatever is in the scaler variable $address, so it is pointless to change only the other return undef; to return. The caller must be prepared to see an array with a single undef as the return value from this subroutine anyway. The way I understood it: All callsites only ever call validate_address via validate_address_list, which in turn maps a list of addresses by calling validate_address on each address. Therefore, validate_address returning an undef in one codepath is correct. I think we are in agreement then. The implication of what you just said is that anybody that calls this subroutine _must_ be (and indeed is) prepared to see a single 'undef' returned from it, hence the use pattern, @foo = validate_address(...); if (@foo) { ... we got a valid one ... } else { ... we did not ... } cannot be used for the subroutine _anyway_. @foo may get an invalid address, even with the don't say 'return undef;' say 'return;' wisdom is applied to the other return site in the subroutine. Now, the subroutine's body essentially is: while (!extract_valid($address)) { ... if (...) { return undef; # or just return; } } return $address; You can prove that the return $address on the last line will never return 'undef' by proving that extract_valid() never returns true for 'undef' input. With that, we can safely change the error return to do return; in the loop. When it is done, the subroutine's new interface becomes Single address goes in, either a single valid address comes out and that will never be an undef, or nothing (not even an undef) comes out upon error. Which means the sole caller of the subroutine needs to be updated, which currently does this: return grep { defined $_ } map { validate_address($_) } @_; As the subroutine never returns an undef as no this is not valid, but acts more like a filter to cull bad ones from the input, the outer grep { defined $_ } becomes unnecessary. And I think that change logically belongs to the same patch as the one that inspects how validate_address behaves and updates its sometimes we return undef to signal a bad input to return nothing. That is why validate_adress was excluded from [1/3] which deals with other simpler cases. -- 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: clarify documentation for the --verify option
Michael Haggerty mhag...@alum.mit.edu writes: On 04/01/2013 06:56 PM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Because the primary use case of this option is to implement end-user input validation, I think it would be helpful to clarify use of the peeler here. Perhaps ... A SQUASH??? patch on top of your original is queued on 'pu', together with the earlier ^{object} peeler patch. Comments, improvements, etc. would be nice. Yes, your version is better. I would make one change, though. In your + Make sure the single given parameter can be turned into a + raw 20-byte SHA-1 that can be used to access the object + database, and emit it to the standard output. If it can't, + error out. it could be made clearer that exactly one parameter should be provided. Maybe + Verify that exactly one parameter is provided, and that it That is probably better (I was hoping the single would mean the same to the reader, though). Thanks. + can be turned into a raw 20-byte SHA-1 that can be used to + access the object database. If so, emit the SHA-1 to the + standard output; otherwise, error out. But this makes it sound a little like the raw 20-byte SHA-1 will be output to stdout,... I did consider that point, wrote and outputs 40-hex in my earlier draft, and then rejected it because it was even more misleading. The output follows the usual rules for rev parameters, e.g. git rev-parse --short --verify HEAD git rev-parse --symbolic --verify v1.8.2^{tree} and --verify does not mean 40-hex output. That is why I left it vague as emit it. I agree that the wording incorrectly hints that you may be able to get 20-byte raw output. I didn't find a satisfactory phrasing. -- 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] merge: a random object may not necssarily be a commit
Jeff King p...@peff.net writes: On Mon, Apr 01, 2013 at 12:57:17PM -0700, Junio C Hamano wrote: The user could have said git merge $(git rev-parse v1.0.0); we shouldn't mark it as Merge commit '1598fb...' as the merge name, even though such an invocation might be crazy. We could even read the tag header from the tag object and replace the object name the user gave us, but let's not lose the information by doing so, at least not yet. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/merge.c | 13 + 1 file changed, 13 insertions(+) diff --git a/builtin/merge.c b/builtin/merge.c index 0ec8f0d..990e90c 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf *msg) strbuf_release(line); goto cleanup; } + +if (remote_head-util) { +struct merge_remote_desc *desc; +desc = merge_remote_util(remote_head); +if (desc desc-obj desc-obj-type == OBJ_TAG) { +strbuf_addf(msg, %s\t\t%s '%s'\n, +sha1_to_hex(desc-obj-sha1), +typename(desc-obj-type), +remote); +goto cleanup; +} +} + strbuf_addf(msg, %s\t\tcommit '%s'\n, sha1_to_hex(remote_head-object.sha1), remote); I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT that would yield something we could merge, but it feels weird that you check only for OBJ_TAG here, and otherwise still say commit. Would the intent be more clear if it just said: if (desc desc-obj desc-obj-type != OBJ_COMMIT) { ... ? I suspect not. The point of the added code is that it knows we want to special case merging a tag object, and it wants to keep any other case behaving the same as before. -- 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] merge: a random object may not necssarily be a commit
On Tue, Apr 02, 2013 at 08:02:13AM -0700, Junio C Hamano wrote: + if (remote_head-util) { + struct merge_remote_desc *desc; + desc = merge_remote_util(remote_head); + if (desc desc-obj desc-obj-type == OBJ_TAG) { + strbuf_addf(msg, %s\t\t%s '%s'\n, + sha1_to_hex(desc-obj-sha1), + typename(desc-obj-type), + remote); + goto cleanup; + } + } + strbuf_addf(msg, %s\t\tcommit '%s'\n, sha1_to_hex(remote_head-object.sha1), remote); I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT that would yield something we could merge, but it feels weird that you check only for OBJ_TAG here, and otherwise still say commit. Would the intent be more clear if it just said: if (desc desc-obj desc-obj-type != OBJ_COMMIT) { ... ? I suspect not. The point of the added code is that it knows we want to special case merging a tag object, and it wants to keep any other case behaving the same as before. Ah. I read it as if we have a random object, we do not want to just say commit X, because it is not a commit. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] peel_onion(): teach $foo^{object} peeler
Michael Haggerty mhag...@alum.mit.edu writes: Off topic: Your patch reminds me of something else that surprised me: there is no $userstring^{tag}. I suppose it would be a bit ambiguous, given that tags can point at tags, and it would also be less useful than the other suffixes. But its absence irked the completionist in me :-) Yes, unfortunately, foo^{type} means start from foo, and until what you are looking at it is of type, repeatedly peel to see if you can get to an object of that type, or stop and report an error. If a tag A points at another tag B, which in turn points at an object C, you will never see B by applying usual peeling operator. Also, v1.8.2^{tag} would be give the tag itself, while master^{tag} would not report the commit master but would error out, which would be useless. You are better off doing `git cat-file -t foo` and seeing if it is a tag object at that point. -- 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 0/3] Some small fixes to glossary-content.txt
Thomas Ackermann th.ac...@arcor.de writes: While proof-reading the user-manual I noticed some issues with glossary-content.txt: - There is some outdated, misleading or irrelevant information which might only confuse new Git users and should therefore be removed. - The entries for object, object name and SHA1 lacked a little bit of consistency. - The glossary contains partial definitions for refspec and pathspec. Only on the partial bit, and not limited to refspec and pathspec. I think ideally for any key concepts: - The glossary should mention what it is and what it is used for; - Individual manual pages should mention how it is constructed and how it can be used. To be complete, it may need to start by describing what it is and what it is used for, which may have to duplicate what we say in the glossary. -- 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
git https transport and wrong password
Hi, I have a problem with git (1.7.9 and 1.8.2.357.gcc3e4eb) and https transport to gerrit server (2.5.1-3-g719dfc7). I'm producing the problem on Cygwin but my colleagues have same issue on Linux as well. Gerrit server is matching corporate policies with single sign on, so after three failed login attempts the account gets locked until a password reset. Git amplifies this problem by asking for users password only once, and if user made a typo git is still re-using the wrong password enough times to get an account immediately locked. I have client side logs with GIT_CURL_VERBOSE=1 but from intranet so can't publish them directly. Here's roughly what the log shows: --- $ GIT_CURL_VERBOSE=1 git fetch ... GET /gerrit/.../info/refs?service=git-upload-pack HTTP/1.1 ... HTTP/1.1 401 Authorization Required ... -- I guess git prompts for password here. -- * Issue another request to this URL: 'https://..info/refs?service=git-upload-pack' ... * Re-using existing connection! ... ... * Server auth using Basic with user '...' GET /gerrit/.../info/refs?service=git-upload-pack HTTP/1.1 Authorization: Basic ... ... HTTP/1.1 401 Authorization Required Date: ... * Authentication problem. Ignoring this. ... * The requested URL returned error: 401 * Closing connection 0 ... * About to connect() to ... ... * Connected to ... ... * STATE: PROTOCONNECT = DO handle... * Server auth using Basic with user '...' GET /gerrit/.../info/refs?service=git-upload-pack HTTP/1.1 Authorization: Basic ... ... * STATE: DO = DO_DONE handle... * STATE: DO_DONE = WAITPERFORM handle... * STATE: WAITPERFORM = PERFORM handle... ... HTTP/1.1 302 Found ... Location: ...funnylongurl ... * Ignoring the response-body * Connection #1 to host ... left intact * Issue another request to this URL: '...funnylongurl' ... * Server auth using Basic with user '...' GET ...funnylongurl Authorization: Basic ... ... * The requested URL returned error: 500 Internal Server Error * Closing connection 1 ... * About to connect()... ... * Server auth using Basic with user '...' GET /gerrit/.../info/refs HTTP/1.1 Authorization: Basic ... ... HTTP/1.1 302 Found Date... Set-Cookie... Cache-Control: no-store Location: ...funnylongurl ... * Re-using existing connection! (#2)... GET ...funnylongurl ... * The requested URL returned error: 500 Internal Server Error * Closing connection 2 ... error: The requested URL returned error: 500 Internal Server Error while accessing ... fatal: HTTP request failed --- Any idea what could be wrong here? Is git client really retrying with the bad password? Regards, -Mikko -- 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] Remove outdated/missleading/irrelevant entries from glossary-content.txt
Thomas Ackermann th.ac...@arcor.de writes: -[[def_directory]]directory:: - The list you get with ls :-) - The text indeed has a room for improvement, but it probably makes sense to have an entry for `directory` here, as folks who are used to say Folders may not know what it is. -[[def_evil_merge]]evil merge:: - An evil merge is a def_merge,merge that introduces changes that - do not appear in any def_parent,parent. Which one of outdated, misleading or irrelevant category does this fall into? It certainly is not outdated (diff --cc/-c is often a way to view evil merges), the text defines what an evil merge is precisely and I do not think it is misleading. Is it irrelevant? @@ -468,9 +452,7 @@ should not be combined with other pathspec. object of an arbitrary type (typically a tag points to either a def_tag_object,tag or a def_commit_object,commit object). In contrast to a def_head,head, a tag is not updated by - the `commit` command. A Git tag has nothing to do with a Lisp - tag (which would be called an def_object_type,object type - in Git's context). A tag is most typically used to mark a particular + the `commit` command. A tag is most typically used to mark a particular point in the commit ancestry def_chain,chain. Even though I personally am slightly in favor of removal, I suspect that is primarily because I already know what Git tag is, and it is different from the type tag in the Lisp-speak. It's similar in spirit why I would prefer to keep `directory` to help people who speak of Folders. Other changes in this patch look OK to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Improve description of SHA1 related topics in glossary-content.txt
Thomas Ackermann th.ac...@arcor.de writes: Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/glossary-content.txt | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index ab02238..05bfebc 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -146,7 +146,7 @@ to point at the new commit. created. Configured via the `.git/info/grafts` file. [[def_hash]]hash:: - In Git's context, synonym to def_object_name,object name. + In Git's context, synonym for def_object_name,object name. OK. [[def_object_name]]object name:: - The unique identifier of an def_object,object. The def_hash,hash - of the object's contents using the Secure Hash Algorithm - 1 and usually represented by the 40 character hexadecimal encoding of - the def_hash,hash of the object. + The unique identifier of an def_object,object: The def_SHA1,SHA1 hash + of the object's contents. The object name is usually represented by the + 40 character hexadecimal encoding of the hash value. I am torn on this one. When you have a file A on the filesystem, the object name of the blob that records the contents of that file is *not* the same as output from sha1sum A. I doubt we should spell out _how_ it is computed. In the glossary, it is better to say what it is and what it is used for; the tutorial and Documentation/technical/ give better details that the readers who refer to the glossary do not need. The unique identifier of an def_object,object. The object name is usually represented by a 40 character hexadecimal string. Also colloquially called def_SHA1,SHA-1. might be sufficient. @@ -426,7 +425,8 @@ should not be combined with other pathspec. Source code management (tool). [[def_SHA1]]SHA1:: - Synonym for def_object_name,object name. + Secure Hash Algorithm 1; a cryptographic hash function. + In the context of Git used as a synonym for def_object_name,object name. We should spell it as SHA-1 with a dash. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Remove definition of refspec and pathspec from glossary-content.txt
Thomas Ackermann th.ac...@arcor.de writes: Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/glossary-content.txt | 65 ++ 1 file changed, 3 insertions(+), 62 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index 05bfebc..1c9c522 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -261,59 +261,7 @@ This commit is referred to as a merge commit, or sometimes just a pack. [[def_pathspec]]pathspec:: + Pattern used to specify paths in Git commands. Strictly speaking, it is used to limit, but in the context of this document it shouldn't matter either way. [[def_parent]]parent:: A def_commit_object,commit object contains a (possibly empty) list @@ -382,15 +330,8 @@ should not be combined with other pathspec. [[def_refspec]]refspec:: A refspec is used by def_fetch,fetch and def_push,push to describe the mapping between remote + def_ref,ref and local ref. + See linkgit:git-push[1] for details. I think we can just drop See ... for details from here. Besides, why just push and not fetch? -- 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: check-attr doesn't respect recursive definitions
Jeff King p...@peff.net writes: Yes, it is the expected behavior, though I cannot offhand think of anything that would break if we did apply it recursively. Conceptually that breaks our brain. All files in doc/ directories are text and doc/ directory is text are two different things, 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: git https transport and wrong password
On Tue, Apr 02, 2013 at 06:54:40PM +0300, Mikko Rapeli wrote: I have client side logs with GIT_CURL_VERBOSE=1 but from intranet so can't publish them directly. Here's roughly what the log shows: Maybe this is simpler summary: $ grep HTTP\/1.1 log.txt GET ...info/refs?service=git-upload-pack HTTP/1.1 401 Authorization required password prompt here, and ctrl-c does not work in Cygwin, sigh. GET ...info/refs?service=git-upload-pack HTTP/1.1 401 Authorization required GET ...info/refs?service=git-upload-pack HTTP/1.1 302 Found account locked I presume GET longredirecturl GET ...info/refs HTTP/1.1 302 Found GET longredirecturl I was not able reproduce this issue using curl directly to get the info/refs page. -Mikko -- 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: clarify documentation for the --verify option
On 04/02/2013 04:57 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: On 04/01/2013 06:56 PM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Because the primary use case of this option is to implement end-user input validation, I think it would be helpful to clarify use of the peeler here. Perhaps ... A SQUASH??? patch on top of your original is queued on 'pu', together with the earlier ^{object} peeler patch. Comments, improvements, etc. would be nice. Yes, your version is better. I would make one change, though. In your +Make sure the single given parameter can be turned into a +raw 20-byte SHA-1 that can be used to access the object +database, and emit it to the standard output. If it can't, +error out. it could be made clearer that exactly one parameter should be provided. Maybe +Verify that exactly one parameter is provided, and that it That is probably better (I was hoping the single would mean the same to the reader, though). Thanks. + can be turned into a raw 20-byte SHA-1 that can be used to +access the object database. If so, emit the SHA-1 to the +standard output; otherwise, error out. But this makes it sound a little like the raw 20-byte SHA-1 will be output to stdout,... I did consider that point, wrote and outputs 40-hex in my earlier draft, and then rejected it because it was even more misleading. The output follows the usual rules for rev parameters, e.g. git rev-parse --short --verify HEAD git rev-parse --symbolic --verify v1.8.2^{tree} and --verify does not mean 40-hex output. That is why I left it vague as emit it. I agree that the wording incorrectly hints that you may be able to get 20-byte raw output. I didn't find a satisfactory phrasing. It's the explicit mention of raw 20-byte that puts the reader in mind of 20-byte binary data. I think any version that omitted that phrase would let the reader make the assumption that the SHA-1s are expressed as 40-byte hex numbers just they are everywhere else in the command-line interface. But I'm OK with any of the variations that we have discussed. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: check-attr doesn't respect recursive definitions
On Tue, Apr 02, 2013 at 09:11:02AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Yes, it is the expected behavior, though I cannot offhand think of anything that would break if we did apply it recursively. Conceptually that breaks our brain. All files in doc/ directories are text and doc/ directory is text are two different things, no? In some systems, yes, but git does not have any notion of doc/ as an item (after all, we track content in files, not directories), so I do not see what it means to specify a directory except to say everything under it has this property. We already treat foo/ in gitignore this way (i.e., everything under foo is ignored). And we treat export-ignore the same way in git-archive. I agree that saying Documentation diff=text is not something I would expect people to do, but what _should_ it do? Recursively applying under the directory seems a sane option. I would not want to paint us into a corner, but I cannot think of any other reasonable thing to do with it (which is why I phrased it as I cannot think of anything that would break, and not this is a great idea; counterpoints are welcome). I am sympathetic to Jan's issue, though. If git were consistent about applying attributes only to paths, then the repo owner would have written dir/* export-ignore, not dir. But it is not, and git-archive does ask for attributes on directories, but without providing a good mechanism for doing those attribute lookups from external programs. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
Matthieu Moy matthieu@grenoble-inp.fr writes: I'm fine with either dropping my patch or applying it as-is (with s/newline/blank line/ in the commit message). OK; let's insert it immediately after e24afab09137 (add: make pathless 'add [-u|-A]' warning a file-global function, 2013-03-19), like the attached. I'd prefer to spell this die(...\n ); over die(...\n); as the latter _looks_ as if the author didn't know die() gives us line termination. The former hopefully is more explicit that we do want an empty line at the end. commit a8ed21a59219e8fe81b76ed0961641555f25cdad Author: Matthieu Moy matthieu@imag.fr Date: Mon Mar 11 09:01:33 2013 +0100 add: add a blank line at the end of pathless 'add [-u|-A]' warning When the commands give an actual output (e.g. when ran with -v), the output is visually mixed with the warning. An additional blank line makes the actual output more visible. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com diff --git a/builtin/add.c b/builtin/add.c index a4028ee..db02233 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -57,7 +57,9 @@ static void warn_pathless_add(void) git add %s .\n (or git add %s .)\n \n - With the current Git version, the command is restricted to the current directory.), + With the current Git version, the command is restricted to + the current directory.\n, + ), option_with_implicit_dot, short_option_with_implicit_dot, option_with_implicit_dot, short_option_with_implicit_dot, option_with_implicit_dot, short_option_with_implicit_dot); -- 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] peel_onion(): teach $foo^{object} peeler
On 04/02/2013 05:45 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Off topic: Your patch reminds me of something else that surprised me: there is no $userstring^{tag}. I suppose it would be a bit ambiguous, given that tags can point at tags, and it would also be less useful than the other suffixes. But its absence irked the completionist in me :-) Yes, unfortunately, foo^{type} means start from foo, and until what you are looking at it is of type, repeatedly peel to see if you can get to an object of that type, or stop and report an error. If a tag A points at another tag B, which in turn points at an object C, you will never see B by applying usual peeling operator. Also, v1.8.2^{tag} would be give the tag itself, while master^{tag} would not report the commit master but would error out, which would be useless. You are better off doing `git cat-file -t foo` and seeing if it is a tag object at that point. All correct, of course. But the user would never use master^{tag} unless he wants a tag and nothing else, so erroring out would be exactly the thing he wants in that case. This is no different than the ^{commit} part of master^{tree}^{commit}, which correctly errors out because a commit cannot be inferred from a tree. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: check-attr doesn't respect recursive definitions
Jeff King p...@peff.net writes: On Tue, Apr 02, 2013 at 09:11:02AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Yes, it is the expected behavior, though I cannot offhand think of anything that would break if we did apply it recursively. Conceptually that breaks our brain. All files in doc/ directories are text and doc/ directory is text are two different things, no? In some systems, yes, but git does not have any notion of doc/ as an item (after all, we track content in files, not directories), so I do not see what it means to specify a directory except to say everything under it has this property. That was true back when gitattributes (and ignore) was defined to apply only to the paths we track. But export-ignore abuses the attrtibute system, allows a directory to be specified in the match pattern, and we declared that is a kosher use by the patch that caused 1.8.1.X regression, no? So Git does not have any notion of doc/' as an item is no longer true, I 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: ZSH segmentation fault while completing git mv dir/
Matthieu Moy matthieu@grenoble-inp.fr writes: Felipe Contreras felipe.contre...@gmail.com writes: And this is a workaround: --- a/contrib/completion/git-completion.zsh +++ b/contrib/completion/git-completion.zsh @@ -66,7 +66,7 @@ __gitcomp_file () local IFS=$'\n' compset -P '*[=:]' - compadd -Q -p ${2-} -f -- ${=1} _ret=0 + compadd -Q -p ${2-} -- ${=1} _ret=0 } OK, not something we want to apply to git.git, but this means a workaround for users is to ... Would it help users more to have that as part of the instruction at the beginning of contrib/completion/git-completion.zsh where it already says here is how you use it via fpath, than leaving it here in the list archive? (Hope this helps in case someone has the same problem and finds this thread ...) -- 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: check-attr doesn't respect recursive definitions
On Tue, Apr 02, 2013 at 09:43:30AM -0700, Junio C Hamano wrote: In some systems, yes, but git does not have any notion of doc/ as an item (after all, we track content in files, not directories), so I do not see what it means to specify a directory except to say everything under it has this property. That was true back when gitattributes (and ignore) was defined to apply only to the paths we track. But export-ignore abuses the attrtibute system, allows a directory to be specified in the match pattern, and we declared that is a kosher use by the patch that caused 1.8.1.X regression, no? So Git does not have any notion of doc/' as an item is no longer true, I think. Yes, but as I explained later, the meaning of apply an attribute to dir in such cases is always equivalent to apply attribute recursively to dir/*. So I do not think we are violating that rule to recursively apply all attributes. But let's take a step back. I think Jan is trying to do a very reasonable thing: come up with the same set of paths that git-archive would. What's the best way to solve that? Recursive application of attributes is one way, but is there another way we could help with solving that? Using: git ls-tree --name-only -zrt HEAD | git check-attr --stdin -z export-ignore means we can find out that foo/ is ignored. But he would have to manually post-process the output to see that foo/bar is below foo. Not impossible, but I just wonder if git can be more helpful in figuring this out. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] remote-helpers: fix the run of all tests
On 02.04.13 01:40, Jeff King wrote: On Mon, Apr 01, 2013 at 11:46:00PM +0200, Antoine Pelisse wrote: On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras felipe.contre...@gmail.com wrote: +export TEST_LINT := I think test-lint-executable still makes sense here. Also test-lint-shell-syntax, which finds a problem with the current code: $ cd contrib/remote-helpers $ make test TEST_LINT=test-lint-shell-syntax make -e -C ../../t test make[1]: Entering directory `/home/peff/compile/git/t' rm -f -r test-results /home/peff/compile/git/contrib/remote-helpers/test-bzr.sh:139: error: echo -n is not portable (please use printf): echo -n content expected make[1]: *** [test-lint-shell-syntax] Error 1 make[1]: Leaving directory `/home/peff/compile/git/t' make: *** [test] Error 2 I think the check for duplicate-numbers is the only one that does not make sense. [] Not sure about that, I send a suggestion of a patch in a minute. Highlights: 1) - rename the contrib test cases and assigns real TC numbers 2) - Forward the numbers into the main test Makefile 1) Will probably collide with Felipe's changes, so we just can pick up the idea. 2) Is for only review. If we agree on the re-numbering of TC's in contrib, we can apply a second round of the patch later. /Torsten -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test-lint-duplicates: check numbering in contrib/remote-helpers
Running make inside contrib/remote-helpers failes in test-lint-duplicates This was because the regexp to check for duplicate numbers strips everything after the first - in the filename, including the prefix. As a result, 2 pathnames like /contrib/remote-helpers/test-bzr.sh and /contrib/remote-helpers/test-hg-bidi.sh are both converted into /contrib/remote, and reported as duplicate. Improve the regexp: Remove everything after t- (where X stand for a digit) Rename the tests in contrib/remote-helpers into t5810-test-bzr.sh, t5820-test-hg-bidi.sh, t5821-test-hg-hg-git.sh, t5830-test-hg.sh Feed the numbers used in contrib/remote-helpers into t/Makefile and check that numbering is unique across both directories Signed-off-by: Torsten Bögershausen tbo...@web.de --- contrib/remote-helpers/Makefile| 3 +- contrib/remote-helpers/t5810-test-bzr.sh | 143 +++ contrib/remote-helpers/t5820-test-hg-bidi.sh | 243 +++ contrib/remote-helpers/t5821-test-hg-hg-git.sh | 534 + contrib/remote-helpers/t5830-test-hg.sh| 121 ++ contrib/remote-helpers/test-bzr.sh | 143 --- contrib/remote-helpers/test-hg-bidi.sh | 243 --- contrib/remote-helpers/test-hg-hg-git.sh | 534 - contrib/remote-helpers/test-hg.sh | 121 -- t/Makefile | 2 +- 10 files changed, 1044 insertions(+), 1043 deletions(-) create mode 100755 contrib/remote-helpers/t5810-test-bzr.sh create mode 100755 contrib/remote-helpers/t5820-test-hg-bidi.sh create mode 100755 contrib/remote-helpers/t5821-test-hg-hg-git.sh create mode 100755 contrib/remote-helpers/t5830-test-hg.sh delete mode 100755 contrib/remote-helpers/test-bzr.sh delete mode 100755 contrib/remote-helpers/test-hg-bidi.sh delete mode 100755 contrib/remote-helpers/test-hg-hg-git.sh delete mode 100755 contrib/remote-helpers/test-hg.sh diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile index 9a76575..012ad50 100644 --- a/contrib/remote-helpers/Makefile +++ b/contrib/remote-helpers/Makefile @@ -1,6 +1,7 @@ -TESTS := $(wildcard test*.sh) +TESTS := $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)) export T := $(addprefix $(CURDIR)/,$(TESTS)) +export TDUP := $(sort $(wildcard ../../t/t[0-9][0-9][0-9][0-9]-*.sh)) export MAKE := $(MAKE) -e export PATH := $(CURDIR):$(PATH) diff --git a/contrib/remote-helpers/t5810-test-bzr.sh b/contrib/remote-helpers/t5810-test-bzr.sh new file mode 100755 index 000..70aa8a0 --- /dev/null +++ b/contrib/remote-helpers/t5810-test-bzr.sh @@ -0,0 +1,143 @@ +#!/bin/sh +# +# Copyright (c) 2012 Felipe Contreras +# + +test_description='Test remote-bzr' + +. ./test-lib.sh + +if ! test_have_prereq PYTHON; then + skip_all='skipping remote-bzr tests; python not available' + test_done +fi + +if ! $PYTHON_PATH -c 'import bzrlib'; then + skip_all='skipping remote-bzr tests; bzr not available' + test_done +fi + +cmd=' +import bzrlib +bzrlib.initialize() +import bzrlib.plugin +bzrlib.plugin.load_plugins() +import bzrlib.plugins.fastimport +' + +if ! $PYTHON_PATH -c $cmd; then + echo consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins 12 + skip_all='skipping remote-bzr tests; bzr-fastimport not available' + test_done +fi + +check () { + (cd $1 + git log --format='%s' -1 + git symbolic-ref HEAD) actual + (echo $2 + echo refs/heads/$3) expected + test_cmp expected actual +} + +bzr whoami A U Thor aut...@example.com + +test_expect_success 'cloning' ' + (bzr init bzrrepo + cd bzrrepo + echo one content + bzr add content + bzr commit -m one + ) + + git clone bzr::$PWD/bzrrepo gitrepo + check gitrepo one master +' + +test_expect_success 'pulling' ' + (cd bzrrepo + echo two content + bzr commit -m two + ) + + (cd gitrepo git pull) + + check gitrepo two master +' + +test_expect_success 'pushing' ' + (cd gitrepo + echo three content + git commit -a -m three + git push + ) + + echo three expected + cat bzrrepo/content actual + test_cmp expected actual +' + +test_expect_success 'roundtrip' ' + (cd gitrepo + git pull + git log --format=%s -1 origin/master actual) + echo three expected + test_cmp expected actual + + (cd gitrepo git push git pull) + + (cd bzrrepo + echo four content + bzr commit -m four + ) + + (cd gitrepo git pull git push) + + check gitrepo four master + + (cd gitrepo + echo five content + git commit -a -m five + git push git pull + ) + + (cd bzrrepo bzr revert) + + echo five expected + cat bzrrepo/content actual + test_cmp expected actual +' + +cat expected EOF +100644 blob 54f9d6da5c91d556e6b54340b1327573073030af content +100755 blob 68769579c3eaadbe555379b9c3538e6628bae1eb executable +12 blob 6b584e8ece562ebffc15d38808cd6b98fc3d97ea link +EOF +
Re: [PATCH 2/2] peel_onion(): teach $foo^{object} peeler
Michael Haggerty mhag...@alum.mit.edu writes: On 04/02/2013 05:45 PM, Junio C Hamano wrote: Also, v1.8.2^{tag} would be give the tag itself, while master^{tag} would not report the commit master but would error out, which would be useless. You are better off doing `git cat-file -t foo` and seeing if it is a tag object at that point. All correct, of course. But the user would never use master^{tag} unless he wants a tag and nothing else, so erroring out would be exactly the thing he wants in that case. This is no different than the ^{commit} part of master^{tree}^{commit}, which correctly errors out because a commit cannot be inferred from a tree. Correct; I was only saying adding it is not something that solves a problem that cannot be solved with the current system (i.e. no added value from feature point-of-view). I would not object to a patch to allow git rev-parse v1.8.2^{tag} for completeness. We may want to rethink if we can lose the hardcoded lengths like 6, 3, 4, 4 from here, but I didn't bother ;-). sha1_name.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 85b6e75..47f39a8 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -639,16 +639,18 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) return -1; sp++; /* beginning of type name, or closing brace for empty */ - if (!strncmp(commit_type, sp, 6) sp[6] == '}') + if (!strncmp(tag_type, sp, 3) sp[3] == '}') + expected_type = OBJ_TAG; + else if (!strncmp(commit_type, sp, 6) sp[6] == '}') expected_type = OBJ_COMMIT; else if (!strncmp(tree_type, sp, 4) sp[4] == '}') expected_type = OBJ_TREE; else if (!strncmp(blob_type, sp, 4) sp[4] == '}') expected_type = OBJ_BLOB; else if (!prefixcmp(sp, object})) - expected_type = OBJ_ANY; + expected_type = OBJ_ANY; /* ok as long as it exists */ else if (sp[0] == '}') - expected_type = OBJ_NONE; + expected_type = OBJ_NONE; /* unwrap until we get a non-tag */ else if (sp[0] == '/') expected_type = OBJ_COMMIT; 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 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
Junio C Hamano gits...@pobox.com writes: diff --git a/builtin/add.c b/builtin/add.c index a4028ee..db02233 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -57,7 +57,9 @@ static void warn_pathless_add(void) git add %s .\n (or git add %s .)\n \n - With the current Git version, the command is restricted to the current directory.), + With the current Git version, the command is restricted to + the current directory.\n, + ), Looks good. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aw: Re: [PATCH 3/3] Remove definition of refspec and pathspec from glossary-content.txt
I think we can just drop See ... for details from here. Besides, why just push and not fetch? I just kept the original reference which only linked to git-push. --- Thomas -- 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
[BUG] inconsistent behavior with --set-upstream vs --set-upstream-to
Hi! It looks like git branch --set-upstream-to doesn't function with 1.8.1.3 until I run --set-upstream. Is this a known bug? root@fuji-current:/usr/src # git branch --set-upstream-to origin/pjdfstest-onefs pjdfstest-onefs fatal: Not a valid object name: 'origin/pjdfstest-onefs'. root@fuji-current:/usr/src # git branch --set-upstream origin/pjdfstest-onefs pjdfstest-onefs The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to Branch origin/pjdfstest-onefs set up to track local branch pjdfstest-onefs. root@fuji-current:/usr/src # git branch --set-upstream-to origin/pjdfstest-onefs pjdfstest-onefs Branch pjdfstest-onefs set up to track local branch origin/pjdfstest-onefs. root@fuji-current:/usr/src # git --version git version 1.8.1.3 Thanks! -Garrett PS Please CC me as I'm not subscribed to the list.-- 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] test-lint-duplicates: check numbering in contrib/remote-helpers
On Tue, Apr 2, 2013 at 6:53 PM, Torsten Bögershausen tbo...@web.de wrote: --- contrib/remote-helpers/Makefile| 3 +- contrib/remote-helpers/t5810-test-bzr.sh | 143 +++ contrib/remote-helpers/t5820-test-hg-bidi.sh | 243 +++ contrib/remote-helpers/t5821-test-hg-hg-git.sh | 534 + contrib/remote-helpers/t5830-test-hg.sh| 121 ++ contrib/remote-helpers/test-bzr.sh | 143 --- contrib/remote-helpers/test-hg-bidi.sh | 243 --- contrib/remote-helpers/test-hg-hg-git.sh | 534 - contrib/remote-helpers/test-hg.sh | 121 -- t/Makefile | 2 +- 10 files changed, 1044 insertions(+), 1043 deletions(-) create mode 100755 contrib/remote-helpers/t5810-test-bzr.sh create mode 100755 contrib/remote-helpers/t5820-test-hg-bidi.sh create mode 100755 contrib/remote-helpers/t5821-test-hg-hg-git.sh create mode 100755 contrib/remote-helpers/t5830-test-hg.sh delete mode 100755 contrib/remote-helpers/test-bzr.sh delete mode 100755 contrib/remote-helpers/test-hg-bidi.sh delete mode 100755 contrib/remote-helpers/test-hg-hg-git.sh delete mode 100755 contrib/remote-helpers/test-hg.sh Don't hesitate to format patches with -M :) -- 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: ZSH segmentation fault while completing git mv dir/
Junio C Hamano gits...@pobox.com writes: Would it help users more to have that as part of the instruction at the beginning of contrib/completion/git-completion.zsh where it already says here is how you use it via fpath, than leaving it here in the list archive? Juging from the answers I got, I do not think many people are hit by the bug, and the problem will anyway disappear as people update their ZSH, so I'm not sure it's worth the trouble. This thread is already #1 on google with git zsh segmentation fault so it's not terribly difficult to find. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Add a note in the on-line Git book about installing the man pages?
Hi, It seems the git man pages (or HTML pages) are not installed by default when you install git as described in the http://git-scm.com/book I propose to add a note to http://git-scm.com/book/en/Getting-Started-Installing-Git along those lines: Getting the git manpages from https://code.google.com/p/git-core/downloads/list; and unpack them (supposing you installed your git into /opt/git): TARGET=/opt/git VERSION=`$TARGET/bin/git --version | awk '{print $3}' | cut -d '.' -f1-3` wget http://git-core.googlecode.com/files/git-manpages-${VERSION}.tar.gz; mkdir $TARGET/share/man tar xz -C $TARGET/share/man -f git-manpages-${VERSION}.tar.gz -- 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: check-attr doesn't respect recursive definitions
On Tue, Apr 02, 2013 at 12:51:28PM -0400, Jeff King wrote: But let's take a step back. I think Jan is trying to do a very reasonable thing: come up with the same set of paths that git-archive would. What's the best way to solve that? Recursive application of attributes is one way, but is there another way we could help with solving that? Using: git ls-tree --name-only -zrt HEAD | git check-attr --stdin -z export-ignore means we can find out that foo/ is ignored. But he would have to manually post-process the output to see that foo/bar is below foo. Not impossible, but I just wonder if git can be more helpful in figuring this out. One way to solve the problem is something like the patch below, which allows git archive --format=lstree to give an lstree-like listing, but with export-ignore attributes applied. It feels weirdly specific, though, like there should be a more general solution. --- Makefile | 1 + archive-lstree.c | 43 +++ archive.c| 1 + archive.h| 1 + quote.c | 4 ++-- quote.h | 1 + 6 files changed, 49 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index e9749ca..5e63d72 100644 --- a/Makefile +++ b/Makefile @@ -772,6 +772,7 @@ LIB_OBJS += archive.o LIB_OBJS += alias.o LIB_OBJS += alloc.o LIB_OBJS += archive.o +LIB_OBJS += archive-lstree.o LIB_OBJS += archive-tar.o LIB_OBJS += archive-zip.o LIB_OBJS += argv-array.o diff --git a/archive-lstree.c b/archive-lstree.c new file mode 100644 index 000..5ca1bbc --- /dev/null +++ b/archive-lstree.c @@ -0,0 +1,43 @@ +#include cache.h +#include archive.h +#include quote.h +#include commit.h +#include blob.h + +static int write_lstree_entry(struct archiver_args *args, + const unsigned char *sha1, + const char *path, size_t pathlen, + unsigned int mode) +{ + const char *type; + + if (S_ISDIR(mode)) + return 0; + else if (S_ISGITLINK(mode)) + type = commit_type; + else + type = blob_type; + + printf(%06o %s %s\t, mode, type, sha1_to_hex(sha1)); + quote_c_style_counted(path, pathlen, NULL, stdout, 0); + printf(\n); + + return 0; +} + +static int write_lstree_archive(const struct archiver *ar, + struct archiver_args *args) +{ + return write_archive_entries(args, write_lstree_entry); +} + +static struct archiver lstree_archiver = { + lstree, + write_lstree_archive, + ARCHIVER_REMOTE +}; + +void init_lstree_archiver(void) +{ + register_archiver(lstree_archiver); +} diff --git a/archive.c b/archive.c index 3f00da6..4966db7 100644 --- a/archive.c +++ b/archive.c @@ -420,6 +420,7 @@ int write_archive(int argc, const char **argv, const char *prefix, git_config(git_default_config, NULL); init_tar_archiver(); init_zip_archiver(); + init_lstree_archiver(); argc = parse_archive_args(argc, argv, ar, args, name_hint, remote); if (nongit) { diff --git a/archive.h b/archive.h index 895afcd..1428fc4 100644 --- a/archive.h +++ b/archive.h @@ -27,6 +27,7 @@ extern void init_zip_archiver(void); extern void init_tar_archiver(void); extern void init_zip_archiver(void); +extern void init_lstree_archiver(void); typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, diff --git a/quote.c b/quote.c index 911229f..da6b7e4 100644 --- a/quote.c +++ b/quote.c @@ -206,8 +206,8 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen) * of name, enclosed with double quotes if asked and needed only. * Return value is the same as in (1). */ -static size_t quote_c_style_counted(const char *name, ssize_t maxlen, -struct strbuf *sb, FILE *fp, int no_dq) +size_t quote_c_style_counted(const char *name, ssize_t maxlen, +struct strbuf *sb, FILE *fp, int no_dq) { #undef EMIT #define EMIT(c) \ diff --git a/quote.h b/quote.h index 133155a..f2b8acb 100644 --- a/quote.h +++ b/quote.h @@ -55,6 +55,7 @@ extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp); extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq); +extern size_t quote_c_style_counted(const char *name, ssize_t len, struct strbuf *, FILE *, int no_dq); extern void quote_two_c_style(struct strbuf *, const char *, const char *, int); extern void write_name_quoted(const char *name, FILE *, int terminator); -- 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: check-attr doesn't respect recursive definitions
Jeff King p...@peff.net writes: On Tue, Apr 02, 2013 at 09:43:30AM -0700, Junio C Hamano wrote: In some systems, yes, but git does not have any notion of doc/ as an item (after all, we track content in files, not directories), so I do not see what it means to specify a directory except to say everything under it has this property. That was true back when gitattributes (and ignore) was defined to apply only to the paths we track. But export-ignore abuses the attrtibute system, allows a directory to be specified in the match pattern, and we declared that is a kosher use by the patch that caused 1.8.1.X regression, no? So Git does not have any notion of doc/' as an item is no longer true, I think. Yes, but as I explained later, the meaning of apply an attribute to dir in such cases is always equivalent to apply attribute recursively to dir/*. So I do not think we are violating that rule to recursively apply all attributes. I think this is where we disagree. Attribute does not recursively apply in general. It was _never_ designed to (and that is the authoritative answer, as I was who designed it in Apr 2007 ;-)). It is not even true to say that archive applies export-ignore to the directory recursively, with or without the recent change. Would it allow everything but dir/file to be excluded and still dir/file to be included in the archive if you have a .gitattribute file like this? dir/ export-ignore dir/file !export-ignore I do not think so. If we _were_ living in an alternate universe where we did not have the .gitignore file and instead expressed what it does with an ignore attribute, then having dir in the top-level .gitignore file and !file in the .gitignore file in dir/ directory may be equivalent to having dir/ ignore dir/file -ignore in your .gitattribute, and we would want to ignore dir/other while including dir/file in the may be interesting paths. The point is, yes, we could choose to define individual attribute keys to apply recursively, but in general attributes were designed not to recurse, and no existing attribute recurses. But let's take a step back. I think Jan is trying to do a very reasonable thing: come up with the same set of paths that git-archive would. What's the best way to solve that? Because the attribute does not recursively apply in general, and it is entirely up to the application and a particular attribute key to decide how the key is applied in the context of the application, check-attr by itself cannot know. You need to know how archive treats export-ignore attribute and then use check-attr with that knowledge. Using: git ls-tree --name-only -zrt HEAD | git check-attr --stdin -z export-ignore means we can find out that foo/ is ignored. Yes, and after learning that, we need to reject everything foo/* using the knowledge that git archive cuts all subpaths of a path that has export-ignore attribute is attached to. -- 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: Aw: Re: [PATCH 3/3] Remove definition of refspec and pathspec from glossary-content.txt
Thomas Ackermann th.ac...@arcor.de writes: I think we can just drop See ... for details from here. Besides, why just push and not fetch? I just kept the original reference which only linked to git-push. That is only because the last example the See ... for details refers to is about 'git push', isn't 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: [BUG] inconsistent behavior with --set-upstream vs --set-upstream-to
On Tue, Apr 02, 2013 at 10:07:01AM -0700, Garrett Cooper wrote: It looks like git branch --set-upstream-to doesn't function with 1.8.1.3 until I run --set-upstream. Is this a known bug? No, but I do not think that is exactly what is going on. root@fuji-current:/usr/src # git branch --set-upstream-to origin/pjdfstest-onefs pjdfstest-onefs fatal: Not a valid object name: 'origin/pjdfstest-onefs'. This is complaining that origin/pjdfstest-onefs does not actually exist Does it? If the pjdfstest-onefs branch exists on the remote, do you need to do a git fetch to make sure we have a local refs/remotes/origin/pjdfstest-onefs tracking branch locally? root@fuji-current:/usr/src # git branch --set-upstream origin/pjdfstest-onefs pjdfstest-onefs The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to Branch origin/pjdfstest-onefs set up to track local branch pjdfstest-onefs. This did _not_ create the remote-tracking branch refs/remotes/origin/pjdfstest-onefs. It created a new local branch called origin/pjdfstest-onefs (i.e., refs/heads/origin/pjdfstest-onefs), whose upstream is another local branch pjdfstest-onefs. That backwards order to the arguments is why --set-upstream is deprecated; many people have made the same mistake. root@fuji-current:/usr/src # git branch --set-upstream-to origin/pjdfstest-onefs pjdfstest-onefs Branch pjdfstest-onefs set up to track local branch origin/pjdfstest-onefs. Note how it says local branch here; you are not tracking anything at the origin. You are tracking a local branch that happens to have origin/ in the name. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aw: Re: [PATCH 1/3] Remove outdated/missleading/irrelevant entries from glossary-content.txt
The text indeed has a room for improvement, but it probably makes sense to have an entry for `directory` here, as folks who are used to say Folders may not know what it is. I assumed the number of such people so low that it's not worth to keep this - to most people obvious - explanation. Which one of outdated, misleading or irrelevant category does this fall into? It certainly is not outdated (diff --cc/-c is often a way to view evil merges), the text defines what an evil merge is precisely and I do not think it is misleading. Is it irrelevant? I considered it irrelevant because it tries to define evil merge which is - at least to my experience - not used as some kind of well known notion. But I might of course be wrong. Even though I personally am slightly in favor of removal, I suspect that is primarily because I already know what Git tag is, and it is different from the type tag in the Lisp-speak. I assumed the cardinality of the set of Lisp users is so small that this addition will confuse more people than help somebody. --- Thomas -- 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: Add a note in the on-line Git book about installing the man pages?
David Tonhofer d.tonho...@m-plify.com writes: It seems the git man pages (or HTML pages) are not installed by default when you install git as described in the http://git-scm.com/book I propose to add a note to http://git-scm.com/book/en/Getting-Started-Installing-Git along those lines: Getting the git manpages from https://code.google.com/p/git-core/downloads/list; and unpack them (supposing you installed your git into /opt/git): I suspect that most end-users would install not from the source but install Git from distro as binary packages. I do not know what the page of that website you refer to talks about offhand, but if it is about installing from the source (so that they can help us improve the software and its documentation), it may be a better idea to let the users build the documentation pages from the source, not from the above URL, which serves pre-formatted documentation we cannot take a patch for. -- 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 0/6] Re-roll rr/triangle
Jeff King p...@peff.net writes: On Tue, Apr 02, 2013 at 01:10:28PM +0530, Ramkumar Ramachandra wrote: Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.name.pushremote Thanks; I didn't see any problems in this round. Can I turn that into 5 Reviewed-by's? -- 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: [BUG] inconsistent behavior with --set-upstream vs --set-upstream-to
On Apr 2, 2013, at 10:23 AM, Jeff King p...@peff.net wrote: On Tue, Apr 02, 2013 at 10:07:01AM -0700, Garrett Cooper wrote: It looks like git branch --set-upstream-to doesn't function with 1.8.1.3 until I run --set-upstream. Is this a known bug? No, but I do not think that is exactly what is going on. root@fuji-current:/usr/src # git branch --set-upstream-to origin/pjdfstest-onefs pjdfstest-onefs fatal: Not a valid object name: 'origin/pjdfstest-onefs'. This is complaining that origin/pjdfstest-onefs does not actually exist Does it? If the pjdfstest-onefs branch exists on the remote, do you need to do a git fetch to make sure we have a local refs/remotes/origin/pjdfstest-onefs tracking branch locally? root@fuji-current:/usr/src # git branch --set-upstream origin/pjdfstest-onefs pjdfstest-onefs The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to Branch origin/pjdfstest-onefs set up to track local branch pjdfstest-onefs. This did _not_ create the remote-tracking branch refs/remotes/origin/pjdfstest-onefs. It created a new local branch called origin/pjdfstest-onefs (i.e., refs/heads/origin/pjdfstest-onefs), whose upstream is another local branch pjdfstest-onefs. That backwards order to the arguments is why --set-upstream is deprecated; many people have made the same mistake. root@fuji-current:/usr/src # git branch --set-upstream-to origin/pjdfstest-onefs pjdfstest-onefs Branch pjdfstest-onefs set up to track local branch origin/pjdfstest-onefs. Note how it says local branch here; you are not tracking anything at the origin. You are tracking a local branch that happens to have origin/ in the name. I push the branch to origin/ and then things tend to work, but since I obviously had been doing things wrong what's the correct order of operations for creating a branch and setting the upstream appropriately? Thanks! -Garrett PS I love git as a tool, but I really wish the workflows were simpler or more straightforward, and error messages were clearer. It seems like this would help prevent usage errors like this..-- 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
Aw: Re: Aw: Re: [PATCH 3/3] Remove definition of refspec and pathspec from glossary-content.txt
That is only because the last example the See ... for details refers to is about 'git push', isn't it? This is correct but there was no direct link to git-fetch in the first example and I did not check which combination of man pages gives the complete definition of refspecs ... --- Thomas -- 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] remote-helpers: fix the run of all tests
On Tue, Apr 02, 2013 at 06:53:28PM +0200, Torsten Bögershausen wrote: I think the check for duplicate-numbers is the only one that does not make sense. [] Not sure about that, I send a suggestion of a patch in a minute. Highlights: 1) - rename the contrib test cases and assigns real TC numbers 2) - Forward the numbers into the main test Makefile I'm not sure if this is a good idea or not. It puts the contrib/remote-helpers into the same number namespace as the rest of the test scripts, and enforces uniqueness with test-lint-duplicates, when make test is run from contrib/remote-helpers. But people working on the main test scripts would not get any such check, and would happily break contrib/remote-helpers by adding duplicate test numbers. It makes sense to me to either: 1. Have the contrib/remote-helpers test live in their own test namespace completely, with their own numbers and test-results, and pull in relevant bits from the main test harness. We do this already with contrib/subtree. I suggested this when the tests first appeared, but there was some argument, and I don't remember the details. 2. Just integrate contrib test scripts into the main repository, but leave them off by default. For example, add: if test -z $GIT_TEST_REMOTE_HELPERS; then skip_all=Remote helper tests disabled (define GIT_TEST_REMOTE_HELPERS) test_done fi to the top of the scripts, and then set GIT_TEST_REMOTE_HELPERS in contrib/remote-helpers/Makefile before chaining to the test Makefile. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] count-objects: output KiB instead of kilobytes
Mihai Capotă mi...@mihaic.ro writes: The code uses division by 1024. Also, the manual uses KiB. Signed-off-by: Mihai Capotă mi...@mihaic.ro --- builtin/count-objects.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/count-objects.c b/builtin/count-objects.c index 9afaa88..ecc13b0 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix) printf(garbage: %lu\n, garbage); } else - printf(%lu objects, %lu kilobytes\n, + printf(%lu objects, %lu KiB\n, loose, (unsigned long) (loose_size / 1024)); return 0; } I guess nobody reads this in scripts, so it should be OK. Will queue. 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: Composing git repositories
Jonathan Nieder wrote: Elated is probably not the right word. More annoyed at being told their work is ugly without an accompanying concrete and actionable bug report. :) If I had an actionable report, I'd have started hammering patches instead of wasting everyone's time here. I'm was presenting fragments of my thoughts, hoping that it turn into concrete actionable work after exchanging a few emails. I'm also annoyed that it didn't happen. If you are curious, at a quieter time it might be useful to ask for pointers to the discussions that led to the current design, and folks on the list might be glad to help. Will do. The search on GMane is no good, and taking a local dump to search using real tools is just too painful; does someone already have a local dump? -- 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
RFC: allowing multiple parallel sequencers
Hey- I've recently started looking into the possibility of having git support multiple in-progress sequencers, and wanted to solicit opinions for how best to do it. The use case is primarily for cherry-pick - i.e. I often have need to cherry pick a large set of commits to an older kernel, and I may do this for several work efforts in parallel. As such, it would be great if I could be able to have multiple sequencer states in progress that could be swapped out with one another. I know this could be done manually by saving the sequencer directory to another name and moving it back, but it would be really nice if there was something a bit more polished and integrated. The thoughts I had were: 1) A per branch sequence directory - when creating the sequence directory, prepend the name of the branch that you are on to the sequencer directory name, and lookup the sequencer using that prefix. This would fit quite well I think. It would allow us to maintain a sequencer per branch, and would be relatively easy to implement (we would need to have a generic function to return the current branch name, and some extra check to delete sequencers when branches are deleted, but nothing too difficult). It would be problematic however, in that working in detached head state would preclude the use of the mechanism (we could work around that by using a global sequencer in detached head mode, or we could add an option to specify a sequencer prefix). 2) Augment the git-stash command to save sequencer state optionally. This would be somewhat more difficult to implement I think (we would need to add .git/sequencer/* to the untracked file list when creating the stash commit). It would however allow arbitrary sequencers to be used on arbitrary branches (including detached head mode, if thats useful). So, before I went implementing, I wanted to solicit opinions here. Does anyone have any thoughts (including completely different directions to move in for this feature)? Thanks! Neil -- 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: [BUG] inconsistent behavior with --set-upstream vs --set-upstream-to
On Tue, Apr 02, 2013 at 10:30:35AM -0700, Garrett Cooper wrote: I push the branch to origin/ and then things tend to work, but since I obviously had been doing things wrong what's the correct order of operations for creating a branch and setting the upstream appropriately? Once you have pushed it, the push creates the refs/remotes/origin/foo tracking branch automatically. You are then free to reference it wherever you like, including in set-upstream-to. However, you can also just ask push to do it for you with --set-upstream or -u. So the workflow is something like: $ git checkout -b my-topic $ hack hack hack $ git commit -m looking good, time to publish $ git push -u origin HEAD PS I love git as a tool, but I really wish the workflows were simpler or more straightforward, and error messages were clearer. It seems like this would help prevent usage errors like this.. Things slowly improve as people make suggestions. I think the thing that might have helped here is better advice when set-upstream-to is pointed to a ref that does not exist. Patches coming in a minute. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Re-roll rr/triangle
On Tue, Apr 02, 2013 at 10:28:20AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Tue, Apr 02, 2013 at 01:10:28PM +0530, Ramkumar Ramachandra wrote: Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.name.pushremote Thanks; I didn't see any problems in this round. Can I turn that into 5 Reviewed-by's? Yes, please do. Six if you want me to review my own patch. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Composing git repositories
On Tue, Apr 02, 2013 at 11:14:49PM +0530, Ramkumar Ramachandra wrote: If you are curious, at a quieter time it might be useful to ask for pointers to the discussions that led to the current design, and folks on the list might be glad to help. Will do. The search on GMane is no good, and taking a local dump to search using real tools is just too painful; does someone already have a local dump? Yes, I have a maildir with the complete list archive, which I index using mairix (or sometimes grep if I'm doing something particularly tricky). I find the search on gmane to be abysmal. I'm happy to make my dump available to anyone who wants it, but it's kind of big (about 1.4G uncompressed). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Composing git repositories
Jonathan Nieder jrnie...@gmail.com writes: If you are curious, at a quieter time it might be useful to ask for pointers to the discussions that led to the current design, and folks on the list might be glad to help. Not on the current design but the discussion before that round that influenced the outcome greatly was this: http://thread.gmane.org/gmane.comp.version-control.git/14486/focus=14492 where we discussed a separate gitlink type of object. And obviously this discussion is also a must read: http://thread.gmane.org/gmane.comp.version-control.git/44106 I vaguely recall asking (or seeing somebody ask) why Linus ended up with using commit in index without introducing a separate gitlink type, but I didn't find it. IIRC, the answer was it turned out that we didn't need it or something like that, which I tend to agree. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: allowing multiple parallel sequencers
Neil Horman nhor...@tuxdriver.com writes: I've recently started looking into the possibility of having git support multiple in-progress sequencers, and wanted to solicit opinions for how best to do it The thoughts I had were: 1) A per branch sequence directory... 2) Augment the git-stash command... 3) A per branch working tree. That is how I would do this myself, anyway ;-) -- 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: Aw: Re: [PATCH 1/3] Remove outdated/missleading/irrelevant entries from glossary-content.txt
Thomas Ackermann th.ac...@arcor.de writes: Even though I personally am slightly in favor of removal, I suspect that is primarily because I already know what Git tag is, and it is different from the type tag in the Lisp-speak. I assumed the cardinality of the set of Lisp users is so small that this addition will confuse more people than help somebody. The text indeed has a room for improvement, but it probably makes sense to have an entry for `directory` here, as folks who are used to say Folders may not know what it is. I assumed the number of such people so low that it's not worth to keep this - to most people obvious - explanation. For the above two (they are of the same theme) to help one audience, I tend to be cautious and try not to say I don't fall into the target audience, and to me it is misleading/irrelevant, so let's remove it. Which one of outdated, misleading or irrelevant category does this fall into? It certainly is not outdated (diff --cc/-c is often a way to view evil merges), the text defines what an evil merge is precisely and I do not think it is misleading. Is it irrelevant? I considered it irrelevant because it tries to define evil merge which is - at least to my experience - not used as some kind of well known notion. But I might of course be wrong. In a merge-heavy workflow, evil merges have to happen from time to time, and it is a good concept to know about. I however think the description is too literal and it does not lead to the understanding of what it is used for. I see a few questions on the stackoverflow with unsatisfactory literal answers, too. -- 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: Composing git repositories
Jens Lehmann wrote: But I think we recently learned to support that use case with submodules. I think there are two floating models: - Tracked: [...] - Untracked: Some people just want the newest tip of a branch checked out in the submodule and update that from time to time (I suspect this is because they are used to SVN externals, which I believe work that way). You throw away reproducibility, which I think is not good and not the way I expect Git to work. [...] Nope, it has nothing to do with SVN externals; I've never used them. And no, all repositories aren't created equal. I should be able to add in magit.git into my dotfiles repository without worrying about which commits the other repositories were at a particular commit. If my project depends on the bleeding edge of poppler and girarra, I should always be able to tell what commits in each subproject the build was passing in. In other words, I should be able to freely mixed floating and fixed submodules. There's no reason for one to be Right, and the other to be a shunned second-class citizen. -- 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: Composing git repositories
Ramkumar Ramachandra wrote: I should be able to add in magit.git into my dotfiles repository For this, ideally what we'd want is a file that lists repositories that should be cloned at the same time as cloning the dotfiles repository, to reconstitute your dotfiles on a new machine. From then on, when acting on the dotfiles repository (with git status, git commit, etc) git should not pay attention to the magit subdirectory at all, because there is no coupling between the two. Did I get that right? That sounds similar to what Junio does with the Meta subdirectory in his git development worktree. I don't think submodules are a good fit, but it might make sense to start respecting a .motd file to allow the following in a hypothetical world where everyone who clones git uses the same scripts Junio does: $ git clone git://repo.or.cz/git.git Cloning into 'git'... remote: Counting objects: 151283, done. remote: Compressing objects: 100% (38546/38546), done. remote: Total 151283 (delta 111004), reused 151073 (delta 110797) Receiving objects: 100% (151283/151283), 36.39 MiB | 7.66 MiB/s, done. Resolving deltas: 100% (111004/111004), done. Don't forget to git clone -b todo git://repo.or.cz/git.git git/Meta for maintenance scripts. $ That would allow you to include an arbitrary setup script (including cloning dependencies as well as running autoreconf or whatever) and give people cloning a quick reminder to inspect it if paranoid and then run it. Jonathan -- 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
[RFC/PATCH 0/5] branch --set-upstream-to error-message improvements
On Tue, Apr 02, 2013 at 01:51:13PM -0400, Jeff King wrote: Things slowly improve as people make suggestions. I think the thing that might have helped here is better advice when set-upstream-to is pointed to a ref that does not exist. Patches coming in a minute. Or 60 minutes. :) I'm not decided on whether the last patch is overkill or not (or even if it is not, whether it may end up confusing people who do not fit into one of the slots it suggests). [1/5]: t3200: test --set-upstream-to with bogus refs [2/5]: branch: factor out upstream is not a branch error messages [3/5]: branch: improve error message for missing --set-upstream-to ref [4/5]: branch: mention start_name in set-upstream error messages [5/5]: branch: give advice when tracking start-point is missing -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] t3200: test --set-upstream-to with bogus refs
These tests pass with the current code, but let's make sure we don't accidentally break the behavior in the future. Note that our tests expect failure when we try to set the upstream to or from a missing branch. Technically we are just munging config here, so we do not need the refs to exist. But seeing that they do exist is a good check that the user has not made a typo. Signed-off-by: Jeff King p...@peff.net --- t/t3200-branch.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index b08c9f2..09f65f8 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -409,6 +409,18 @@ test_expect_success '--set-upstream-to fails on detached HEAD' ' git checkout - ' +test_expect_success '--set-upstream-to fails on a missing dst branch' ' + test_must_fail git branch --set-upstream-to master does-not-exist +' + +test_expect_success '--set-upstream-to fails on a missing src branch' ' + test_must_fail git branch --set-upstream-to does-not-exist master +' + +test_expect_success '--set-upstream-to fails on a non-ref' ' + test_must_fail git branch --set-upstream-to HEAD^{} +' + test_expect_success 'use --set-upstream-to modify HEAD' ' test_config branch.master.remote foo test_config branch.master.merge foo -- 1.8.2.rc0.33.gd915649 -- 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] rerere forget: grok files containing NUL
Am 02.04.2013 00:48, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Using 'git rerere forget .' after a merge that involved binary files runs into an infinite loop if the binary file contains a zero byte. Replace a strchrnul by memchr because the former does not make progress as soon as the NUL is encountered. Hmph, thanks. Is it the right behaviour for rerere to even attempt to interfere with a merge that involves binary files in the first place? Why not? And how could rerere tell the difference? It would have to do the same check for binary-ness as the merge driver before it even starts looking closer at the files. Does the three-way merge machinery replay recorded resolution for such a binary file correctly (after your fix, that is)? Yes, it does. It recognizes the binary-ness and picks 'our' side. Only then comes rerere_mem_getline into play. diff --git a/rerere.c b/rerere.c index a6a5cd5..4d940cd 100644 --- a/rerere.c +++ b/rerere.c @@ -284,8 +284,10 @@ static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_) strbuf_release(sb); if (!io-input.len) return -1; -ep = strchrnul(io-input.buf, '\n'); -if (*ep == '\n') +ep = memchr(io-input.buf, '\n', io-input.len); +if (!ep) +ep = io-input.buf + io-input.len; +else if (*ep == '\n') ep++; len = ep - io-input.buf; strbuf_add(sb, io-input.buf, 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
[PATCH 00/13] remote-hg: general updates
Hi, Here is the next round of patches for remote-hg, some which have been contributed through github. Fortunately it seems to be working for the most part, but there are some considerable issues while pushing branches and tags. Dusty Phillips (1): remote-hg: add missing config variable in doc Felipe Contreras (11): remote-hg: trivial cleanups remote-hg: properly report errors on bookmark pushes remote-hg: make sure fake bookmarks are updated remote-hg: trivial test cleanups remote-hg: redirect buggy mercurial output remote-hg: split bookmark handling remote-hg: refactor export remote-hg: update remote bookmarks remote-hg: force remote push remote-hg: don't update bookmarks unnecessarily remote-hg: update tags globally Peter van Zetten (1): remote-hg: fix for files with spaces contrib/remote-helpers/git-remote-hg | 73 contrib/remote-helpers/test-hg-bidi.sh | 6 +-- contrib/remote-helpers/test-hg-hg-git.sh | 4 +- 3 files changed, 61 insertions(+), 22 deletions(-) -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] branch: factor out upstream is not a branch error messages
This message is duplicated, and is quite long. Let's factor it out, which avoids the repetition and the long lines. It will also make future patches easier as we tweak the message. While we're at it, let's also mark it for translation. Signed-off-by: Jeff King p...@peff.net --- branch.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/branch.c b/branch.c index 2bef1e7..1acbd4e 100644 --- a/branch.c +++ b/branch.c @@ -197,6 +197,9 @@ int validate_new_branchname(const char *name, struct strbuf *ref, return 1; } +static const char upstream_not_branch[] = +N_(Cannot setup tracking information; starting point is not a branch.); + void create_branch(const char *head, const char *name, const char *start_name, int force, int reflog, int clobber_head, @@ -231,14 +234,14 @@ void create_branch(const char *head, case 0: /* Not branching from any existing branch */ if (explicit_tracking) - die(Cannot setup tracking information; starting point is not a branch.); + die(_(upstream_not_branch)); break; case 1: /* Unique completion -- good, only if it is a real branch */ if (prefixcmp(real_ref, refs/heads/) prefixcmp(real_ref, refs/remotes/)) { if (explicit_tracking) - die(Cannot setup tracking information; starting point is not a branch.); + die(_(upstream_not_branch)); else real_ref = NULL; } -- 1.8.2.rc0.33.gd915649 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/13] remote-hg: trivial cleanups
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 328c2dc..d0dfb1e 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -531,7 +531,6 @@ def parse_blob(parser): data = parser.get_data() blob_marks[mark] = data parser.next() -return def get_merge_files(repo, p1, p2, files): for e in repo[p1].files(): @@ -542,7 +541,7 @@ def get_merge_files(repo, p1, p2, files): files[e] = f def parse_commit(parser): -global marks, blob_marks, bmarks, parsed_refs +global marks, blob_marks, parsed_refs global mode from_mark = merge_mark = None @@ -647,10 +646,11 @@ def parse_commit(parser): rev = repo[node].rev() parsed_refs[ref] = node - marks.new_mark(rev, commit_mark) def parse_reset(parser): +global parsed_refs + ref = parser[1] parser.next() # ugh @@ -715,11 +715,11 @@ def do_export(parser): continue print ok %s % ref -print - if peer: parser.repo.push(peer, force=False) +print + def fix_path(alias, repo, orig_url): repo_url = util.url(repo.url()) url = util.url(orig_url) -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/13] remote-hg: add missing config variable in doc
From: Dusty Phillips du...@linux.ca Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 4 1 file changed, 4 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index d0dfb1e..844ec50 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -23,6 +23,10 @@ import urllib # If you want to switch to hg-git compatibility mode: # git config --global remote-hg.hg-git-compat true # +# If you are not in hg-git-compat mode and want to disable the tracking of +# named branches: +# git config --global remote-hg.track-branches false +# # git: # Sensible defaults for git. # hg bookmarks are exported as git branches, hg branches are prefixed -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/13] remote-hg: properly report errors on bookmark pushes
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 844ec50..19eb4db 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -710,6 +710,7 @@ def do_export(parser): else: old = '' if not bookmarks.pushbookmark(parser.repo, bmark, old, node): +print error %s % ref continue elif ref.startswith('refs/tags/'): tag = ref[len('refs/tags/'):] -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/13] remote-hg: fix for files with spaces
From: Peter van Zetten peter.van.zet...@cgi.com Set the maximum number of splits to make when dividing the diff stat lines based on space characters. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 19eb4db..c6a1a47 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -578,7 +578,7 @@ def parse_commit(parser): mark = int(mark_ref[1:]) f = { 'mode' : hgmode(m), 'data' : blob_marks[mark] } elif parser.check('D'): -t, path = line.split(' ') +t, path = line.split(' ', 1) f = { 'deleted' : True } else: die('Unknown file command: %s' % line) @@ -625,7 +625,7 @@ def parse_commit(parser): i = data.find('\n--HG--\n') if i = 0: tmp = data[i + len('\n--HG--\n'):].strip() -for k, v in [e.split(' : ') for e in tmp.split('\n')]: +for k, v in [e.split(' : ', 1) for e in tmp.split('\n')]: if k == 'rename': old, new = v.split(' = ', 1) files[new]['rename'] = old -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/13] remote-hg: make sure fake bookmarks are updated
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 7 +++ contrib/remote-helpers/test-hg-bidi.sh | 1 + contrib/remote-helpers/test-hg-hg-git.sh | 1 + 3 files changed, 9 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index c6a1a47..b200e60 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -709,9 +709,16 @@ def do_export(parser): old = bmarks[bmark].hex() else: old = '' + +if bmark == 'master' and 'master' not in parser.repo._bookmarks: +# fake bookmark +print ok %s % ref +continue + if not bookmarks.pushbookmark(parser.repo, bmark, old, node): print error %s % ref continue + elif ref.startswith('refs/tags/'): tag = ref[len('refs/tags/'):] parser.repo.tag([tag], node, None, True, None, {}) diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh index 1d61982..fe38e49 100755 --- a/contrib/remote-helpers/test-hg-bidi.sh +++ b/contrib/remote-helpers/test-hg-bidi.sh @@ -30,6 +30,7 @@ git_clone () { hg_clone () { ( hg init $2 + hg -R $2 bookmark -i master cd $1 git push -q hg::$PWD/../$2 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' ) diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh index 3f253b7..e116cb0 100755 --- a/contrib/remote-helpers/test-hg-hg-git.sh +++ b/contrib/remote-helpers/test-hg-hg-git.sh @@ -35,6 +35,7 @@ git_clone_git () { hg_clone_git () { ( hg init $2 + hg -R $2 bookmark -i master cd $1 git push -q hg::$PWD/../$2 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' ) -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/13] remote-hg: trivial test cleanups
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/test-hg-bidi.sh | 5 ++--- contrib/remote-helpers/test-hg-hg-git.sh | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh index fe38e49..a3c88f6 100755 --- a/contrib/remote-helpers/test-hg-bidi.sh +++ b/contrib/remote-helpers/test-hg-bidi.sh @@ -22,7 +22,6 @@ fi # clone to a git repo git_clone () { - hg -R $1 bookmark -f -r tip master git clone -q hg::$PWD/$1 $2 } @@ -201,8 +200,8 @@ test_expect_success 'hg branch' ' hg_push hgrepo gitrepo hg_clone gitrepo hgrepo2 - : TODO, avoid master bookmark - (cd hgrepo2 hg checkout gamma) + : Back to the common revision + (cd hgrepo hg checkout default) hg_log hgrepo expected hg_log hgrepo2 actual diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh index e116cb0..73ae18d 100755 --- a/contrib/remote-helpers/test-hg-hg-git.sh +++ b/contrib/remote-helpers/test-hg-hg-git.sh @@ -27,7 +27,6 @@ fi # clone to a git repo with git git_clone_git () { - hg -R $1 bookmark -f -r tip master git clone -q hg::$PWD/$1 $2 } @@ -48,7 +47,7 @@ git_clone_hg () { ( git init -q $2 cd $1 - hg bookmark -f -r tip master + hg bookmark -i -f -r tip master hg -q push -r master ../$2 || true ) } -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/13] remote-hg: redirect buggy mercurial output
We can't use stdout for that in remote helpers. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index b200e60..874ccd4 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -271,6 +271,7 @@ def get_repo(url, alias): myui = ui.ui() myui.setconfig('ui', 'interactive', 'off') +myui.fout = sys.stderr if hg.islocal(url): repo = hg.repository(myui, url) -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/13] remote-hg: split bookmark handling
Will be useful for remote bookmarks. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 38 +++- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 874ccd4..73d79cb 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -685,6 +685,8 @@ def parse_tag(parser): def do_export(parser): global parsed_refs, bmarks, peer +p_bmarks = [] + parser.next() for line in parser.each_block('done'): @@ -706,20 +708,9 @@ def do_export(parser): pass elif ref.startswith('refs/heads/'): bmark = ref[len('refs/heads/'):] -if bmark in bmarks: -old = bmarks[bmark].hex() -else: -old = '' - -if bmark == 'master' and 'master' not in parser.repo._bookmarks: -# fake bookmark -print ok %s % ref -continue - -if not bookmarks.pushbookmark(parser.repo, bmark, old, node): -print error %s % ref -continue - +p_bmarks.append((bmark, node)) +# handle below +continue elif ref.startswith('refs/tags/'): tag = ref[len('refs/tags/'):] parser.repo.tag([tag], node, None, True, None, {}) @@ -731,6 +722,25 @@ def do_export(parser): if peer: parser.repo.push(peer, force=False) +# handle bookmarks +for bmark, node in p_bmarks: + +if bmark in bmarks: +old = bmarks[bmark].hex() +else: +old = '' + +if bmark == 'master' and 'master' not in parser.repo._bookmarks: +# fake bookmark +print ok %s % ref +continue + +if not bookmarks.pushbookmark(parser.repo, bmark, old, node): +print error %s % ref +continue + +print ok %s % ref + print def fix_path(alias, repo, orig_url): -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/13] remote-hg: refactor export
No functional changes. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 73d79cb..11162a2 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -9,7 +9,7 @@ # Then you can clone with: # git clone hg::/path/to/mercurial/repo/ -from mercurial import hg, ui, bookmarks, context, util, encoding +from mercurial import hg, ui, bookmarks, context, util, encoding, node import re import sys @@ -60,6 +60,9 @@ def hgmode(mode): m = { '100755': 'x', '12': 'l' } return m.get(mode, '') +def hghex(node): +return hg.node.hex(node) + def get_config(config): cmd = ['git', 'config', '--get', config] process = subprocess.Popen(cmd, stdout=subprocess.PIPE) @@ -705,25 +708,25 @@ def do_export(parser): for ref, node in parsed_refs.iteritems(): if ref.startswith('refs/heads/branches'): -pass +print ok %s % ref elif ref.startswith('refs/heads/'): bmark = ref[len('refs/heads/'):] p_bmarks.append((bmark, node)) -# handle below continue elif ref.startswith('refs/tags/'): tag = ref[len('refs/tags/'):] parser.repo.tag([tag], node, None, True, None, {}) +print ok %s % ref else: # transport-helper/fast-export bugs continue -print ok %s % ref if peer: parser.repo.push(peer, force=False) # handle bookmarks for bmark, node in p_bmarks: +new = hghex(node) if bmark in bmarks: old = bmarks[bmark].hex() @@ -732,10 +735,11 @@ def do_export(parser): if bmark == 'master' and 'master' not in parser.repo._bookmarks: # fake bookmark -print ok %s % ref -continue - -if not bookmarks.pushbookmark(parser.repo, bmark, old, node): +pass +elif bookmarks.pushbookmark(parser.repo, bmark, old, new): +# updated locally +pass +else: print error %s % ref continue -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/13] remote-hg: update remote bookmarks
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 5 + 1 file changed, 5 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 11162a2..160f486 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -743,6 +743,11 @@ def do_export(parser): print error %s % ref continue +if peer: +if not peer.pushkey('bookmarks', bmark, old, new): +print error %s % ref +continue + print ok %s % ref print -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/13] remote-hg: force remote push
Ideally we shouldn't do this, as it's not recommended in mercurial documentation, but there's no other way to push multiple bookmarks (on the same branch), which would be the behavior most similar to git. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 160f486..a1b7e44 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -722,7 +722,7 @@ def do_export(parser): continue if peer: -parser.repo.push(peer, force=False) +parser.repo.push(peer, force=True) # handle bookmarks for bmark, node in p_bmarks: -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] branch: improve error message for missing --set-upstream-to ref
If we are trying to set the upstream config for a branch, the create_branch function will check both that the name resolves as a ref, and that it is either a local or remote-tracking branch. However, before we do so we run get_sha1 on it to find out whether it resolves at all (since the create_branch function is also used to create actual branches, it wants to know where to start the new branch). This means that if you feed a ref that does not exist to branch --set-upstream-to, rather than getting a helpful message about tracking, you only get not a valid object name. Signed-off-by: Jeff King p...@peff.net --- branch.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/branch.c b/branch.c index 1acbd4e..060e9e3 100644 --- a/branch.c +++ b/branch.c @@ -199,6 +199,8 @@ N_(Cannot setup tracking information; starting point is not a branch.); static const char upstream_not_branch[] = N_(Cannot setup tracking information; starting point is not a branch.); +static const char upstream_missing[] = +N_(Cannot setup tracking information; starting point does not exist); void create_branch(const char *head, const char *name, const char *start_name, @@ -227,8 +229,11 @@ void create_branch(const char *head, } real_ref = NULL; - if (get_sha1(start_name, sha1)) + if (get_sha1(start_name, sha1)) { + if (explicit_tracking) + die(_(upstream_missing)); die(Not a valid object name: '%s'., start_name); + } switch (dwim_ref(start_name, strlen(start_name), sha1, real_ref)) { case 0: -- 1.8.2.rc0.33.gd915649 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/13] remote-hg: don't update bookmarks unnecessarily
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index a1b7e44..3130b23 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -733,6 +733,9 @@ def do_export(parser): else: old = '' +if old == new: +continue + if bmark == 'master' and 'master' not in parser.repo._bookmarks: # fake bookmark pass -- 1.8.2 -- 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