[PATCH] gitk: Update Swedish translation (304t)
The patch is attached gzipped to avoid character encoding issues. -- \\// Peter - http://www.softwolves.pp.se/ 0001-gitk-Update-Swedish-translation-304t.patch.gz Description: [PATCH] gitk: Update Swedish translation (304t)
[PATCH 0/3] fetch: fix '.' fetching
Hi, It's quite annoying that 'git fetch' tries to fetch '.' when the upstream branch is a local one. This patch series fixes that, and while on it, it also fixes 'git push'. Felipe Contreras (3): fetch: add --allow-local option fetch: switch allow-local off by default remote: disable allow-local for pushes builtin/fetch.c | 6 +- git-pull.sh | 2 +- remote.c| 17 ++--- remote.h| 1 + t/t5513-fetch-track.sh | 14 ++ t/t5528-push-default.sh | 7 +++ 6 files changed, 42 insertions(+), 5 deletions(-) -- 1.8.3.rc1.579.g184e698 -- 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] fetch: add --allow-local option
So that it becomes possible to override the behavior when the remote tracked is '.'; if it is, we default back to 'origin'. To do this, we need to add a new helper fetchremote_get() that accepts the boolean to enable/disable this behavior. The default is 'true' which shouldn't cause any change in behavior. Signed-off-by: Felipe Contreras --- builtin/fetch.c | 6 +- remote.c| 17 ++--- remote.h| 1 + 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 4b6b1df..2efbd7b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -39,6 +39,7 @@ static struct strbuf default_rla = STRBUF_INIT; static struct transport *transport; static const char *submodule_prefix = ""; static const char *recurse_submodules_default; +static int allow_local = 1; static int option_parse_recurse_submodules(const struct option *opt, const char *arg, int unset) @@ -90,6 +91,9 @@ static struct option builtin_fetch_options[] = { { OPTION_STRING, 0, "recurse-submodules-default", &recurse_submodules_default, NULL, N_("default mode for recursion"), PARSE_OPT_HIDDEN }, + { OPTION_SET_INT, 0, "allow-local", &allow_local, NULL, + N_("allow fetching from local repository"), + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 }, OPT_END() }; @@ -1006,7 +1010,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) result = fetch_multiple(&list); } else if (argc == 0) { /* No arguments -- use default remote */ - remote = remote_get(NULL); + remote = fetchremote_get(NULL, allow_local); result = fetch_one(remote, argc, argv); } else if (multiple) { /* All arguments are assumed to be remotes or groups */ diff --git a/remote.c b/remote.c index 68eb99b..a7e59ab 100644 --- a/remote.c +++ b/remote.c @@ -682,7 +682,7 @@ static int valid_remote_nick(const char *name) return !strchr(name, '/'); /* no slash */ } -static struct remote *remote_get_1(const char *name, const char *pushremote_name) +static struct remote *remote_get_1(const char *name, const char *pushremote_name, int allow_local) { struct remote *ret; int name_given = 0; @@ -699,6 +699,11 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name } } + if (!allow_local && !strcmp(name, ".")) { + name = "origin"; + name_given = 0; + } + ret = make_remote(name, 0); if (valid_remote_nick(name)) { if (!valid_remote(ret)) @@ -718,13 +723,19 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name struct remote *remote_get(const char *name) { read_config(); - return remote_get_1(name, NULL); + return remote_get_1(name, NULL, 1); } struct remote *pushremote_get(const char *name) { read_config(); - return remote_get_1(name, pushremote_name); + return remote_get_1(name, pushremote_name, 1); +} + +struct remote *fetchremote_get(const char *name, int allow_local) +{ + read_config(); + return remote_get_1(name, NULL, allow_local); } int remote_is_configured(const char *name) diff --git a/remote.h b/remote.h index cf56724..f0d6cf3 100644 --- a/remote.h +++ b/remote.h @@ -52,6 +52,7 @@ struct remote { struct remote *remote_get(const char *name); struct remote *pushremote_get(const char *name); +struct remote *fetchremote_get(const char *name, int allow_local); int remote_is_configured(const char *name); typedef int each_remote_fn(struct remote *remote, void *priv); -- 1.8.3.rc1.579.g184e698 -- 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] fetch: switch allow-local off by default
And force it on in 'git push' to retain the old behavior. Signed-off-by: Felipe Contreras --- builtin/fetch.c| 2 +- git-pull.sh| 2 +- t/t5513-fetch-track.sh | 14 ++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 2efbd7b..c65c75b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -39,7 +39,7 @@ static struct strbuf default_rla = STRBUF_INIT; static struct transport *transport; static const char *submodule_prefix = ""; static const char *recurse_submodules_default; -static int allow_local = 1; +static int allow_local = 0; static int option_parse_recurse_submodules(const struct option *opt, const char *arg, int unset) diff --git a/git-pull.sh b/git-pull.sh index 638aabb..18c3793 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -220,7 +220,7 @@ test true = "$rebase" && { done } orig_head=$(git rev-parse -q --verify HEAD) -git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1 +git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok --allow-local "$@" || exit 1 test -z "$dry_run" || exit 0 curr_head=$(git rev-parse -q --verify HEAD) diff --git a/t/t5513-fetch-track.sh b/t/t5513-fetch-track.sh index 65d1e05..cb46747 100755 --- a/t/t5513-fetch-track.sh +++ b/t/t5513-fetch-track.sh @@ -27,4 +27,18 @@ test_expect_success fetch ' ) ' +test_expect_success 'fetch no-local' ' + ( + test_create_repo another && + cd another && + git remote add origin .. && + echo test > file && + git add . && + git commit -m test && + git checkout -t -b local-tracking master && + git fetch && + git rev-parse --verify refs/remotes/origin/b/one + ) +' + test_done -- 1.8.3.rc1.579.g184e698 -- 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] remote: disable allow-local for pushes
So that 'git push' uses 'origin', instead of '.' by default. Signed-off-by: Felipe Contreras --- remote.c| 2 +- t/t5528-push-default.sh | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/remote.c b/remote.c index a7e59ab..c1458dc 100644 --- a/remote.c +++ b/remote.c @@ -729,7 +729,7 @@ struct remote *remote_get(const char *name) struct remote *pushremote_get(const char *name) { read_config(); - return remote_get_1(name, pushremote_name, 1); + return remote_get_1(name, pushremote_name, 0); } struct remote *fetchremote_get(const char *name, int allow_local) diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index 4736da8..61df2a7 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -115,4 +115,11 @@ test_expect_success 'push to existing branch, upstream configured with different test_cmp expect-other-name actual-other-name ' +test_expect_success 'push to existing branch, upstream configured with same name' ' + git remote add origin repo1 && + git checkout -t -b local-tracking master && + test_commit ten && + test_push_success current local-tracking +' + test_done -- 1.8.3.rc1.579.g184e698 -- 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] branch: add show-tracking option
Showing the tracking information for all the branches takes significant amount of time. The user might not want that. The --no-show-tracking option allows that. Signed-off-by: Felipe Contreras --- Documentation/git-branch.txt | 5 - builtin/branch.c | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index b7cb625..1db04cd 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -9,7 +9,7 @@ SYNOPSIS [verse] 'git branch' [--color[=] | --no-color] [-r | -a] - [--list] [-v [--abbrev= | --no-abbrev]] + [--list] [-v [--abbrev= | --no-abbrev] --show-tracking] [--column[=] | --no-column] [(--merged | --no-merged | --contains) []] [...] 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] [] @@ -154,6 +154,9 @@ This option is only applicable in non-verbose mode. --no-abbrev:: Display the full sha1s in the output listing rather than abbreviating them. +--show-tracking:: + Display the tracking information when using --verbose, or not. + -t:: --track:: When creating a new branch, set up configuration to mark the diff --git a/builtin/branch.c b/builtin/branch.c index 0836890..2b586ea 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -33,6 +33,7 @@ static const char * const builtin_branch_usage[] = { static const char *head; static unsigned char head_sha1[20]; +static int show_tracking = 1; static int branch_use_color = -1; static char branch_colors[][COLOR_MAXLEN] = { @@ -424,7 +425,7 @@ static void fill_tracking_info(struct strbuf *stat, const char *branch_name, struct branch *branch = branch_get(branch_name); struct strbuf fancy = STRBUF_INIT; - if (!stat_tracking_info(branch, &ours, &theirs)) { + if (!(show_tracking && stat_tracking_info(branch, &ours, &theirs))) { if (branch && branch->merge && branch->merge[0]->dst && show_upstream_ref) { ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0); @@ -840,6 +841,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) opt_parse_merge_filter, (intptr_t) "HEAD", }, OPT_COLUMN(0, "column", &colopts, N_("list branches in columns")), + OPT_BOOL(0, "show-tracking", &show_tracking, N_("show tracking information")), OPT_END(), }; -- 1.8.3.rc1.579.g184e698 -- 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] branch: add show-tracking option
On Thu, May 16, 2013 at 2:48 AM, Felipe Contreras wrote: > Showing the tracking information for all the branches takes significant > amount of time. The user might not want that. The --no-show-tracking > option allows that. BTW. I ideally I would switch around so -v has upstream, and --vv has the tracking information. But that is changing the behavior. -- Felipe Contreras -- 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] branch: add show-tracking option
On Thu, May 16, 2013 at 2:48 PM, Felipe Contreras wrote: > Showing the tracking information for all the branches takes significant > amount of time. The user might not want that. The --no-show-tracking > option allows that. Or we could cache the information somewhere in .git. If a ref still points to as recorded in the cache, use the cached tracking information, otherwise go the slow way. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: add show-tracking option
On Thu, May 16, 2013 at 2:54 AM, Duy Nguyen wrote: > On Thu, May 16, 2013 at 2:48 PM, Felipe Contreras > wrote: >> Showing the tracking information for all the branches takes significant >> amount of time. The user might not want that. The --no-show-tracking >> option allows that. > > Or we could cache the information somewhere in .git. If a ref still > points to as recorded in the cache, use the cached tracking > information, otherwise go the slow way. That might be nice, but even if that was fast, I personally never use that information, so I still would want this option. -- Felipe Contreras -- 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] branch: add show-tracking option
Felipe Contreras venit, vidit, dixit 16.05.2013 09:48: > Showing the tracking information for all the branches takes significant > amount of time. The user might not want that. The --no-show-tracking > option allows that. I really like the idea of allowing that - not just because I've suggested so myself in the past ;) I feel, though, that we're really exploding our option and config realm. For "git branch" in list mode, we are already able to stack "-v", i.e. "-v" and "-vv" do different things. How about maybe adding "-vvv" and arranging things so that the verbosity and the run time increases with the number of v's? -v list with sha1 + subject of last commit -vv add upstream branch name -vvv add ahead/behind info (the only costly mode) - same with "--cherry" (ahead/behind/same) Or maybe combine the middle two cases into "-vv", which means it would be the same as "-vv", with only "-v" changing what it does now. Yes, this changes current behavior (at least fpr -v), which sucks anyways because of the costly lookup. And those scripting around "branch -v" should get what they deserve. (I may sound annoyed by our compatibility brakes, but here's a case where breaking it should be OK.) > Signed-off-by: Felipe Contreras > --- > Documentation/git-branch.txt | 5 - > builtin/branch.c | 4 +++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt > index b7cb625..1db04cd 100644 > --- a/Documentation/git-branch.txt > +++ b/Documentation/git-branch.txt > @@ -9,7 +9,7 @@ SYNOPSIS > > [verse] > 'git branch' [--color[=] | --no-color] [-r | -a] > - [--list] [-v [--abbrev= | --no-abbrev]] > + [--list] [-v [--abbrev= | --no-abbrev] --show-tracking] > [--column[=] | --no-column] > [(--merged | --no-merged | --contains) []] [...] > 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] > [] > @@ -154,6 +154,9 @@ This option is only applicable in non-verbose mode. > --no-abbrev:: > Display the full sha1s in the output listing rather than abbreviating > them. > > +--show-tracking:: > + Display the tracking information when using --verbose, or not. > + > -t:: > --track:: > When creating a new branch, set up configuration to mark the > diff --git a/builtin/branch.c b/builtin/branch.c > index 0836890..2b586ea 100644 > --- a/builtin/branch.c > +++ b/builtin/branch.c > @@ -33,6 +33,7 @@ static const char * const builtin_branch_usage[] = { > > static const char *head; > static unsigned char head_sha1[20]; > +static int show_tracking = 1; > > static int branch_use_color = -1; > static char branch_colors[][COLOR_MAXLEN] = { > @@ -424,7 +425,7 @@ static void fill_tracking_info(struct strbuf *stat, const > char *branch_name, > struct branch *branch = branch_get(branch_name); > struct strbuf fancy = STRBUF_INIT; > > - if (!stat_tracking_info(branch, &ours, &theirs)) { > + if (!(show_tracking && stat_tracking_info(branch, &ours, &theirs))) { > if (branch && branch->merge && branch->merge[0]->dst && > show_upstream_ref) { > ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0); > @@ -840,6 +841,7 @@ int cmd_branch(int argc, const char **argv, const char > *prefix) > opt_parse_merge_filter, (intptr_t) "HEAD", > }, > OPT_COLUMN(0, "column", &colopts, N_("list branches in > columns")), > + OPT_BOOL(0, "show-tracking", &show_tracking, N_("show tracking > information")), > OPT_END(), > }; > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: add show-tracking option
On Thu, May 16, 2013 at 3:00 AM, Michael J Gruber wrote: > Felipe Contreras venit, vidit, dixit 16.05.2013 09:48: >> Showing the tracking information for all the branches takes significant >> amount of time. The user might not want that. The --no-show-tracking >> option allows that. > > I really like the idea of allowing that - not just because I've > suggested so myself in the past ;) > > I feel, though, that we're really exploding our option and config realm. > For "git branch" in list mode, we are already able to stack "-v", i.e. > "-v" and "-vv" do different things. How about maybe adding "-vvv" and > arranging things so that the verbosity and the run time increases with > the number of v's? > > -v list with sha1 + subject of last commit > -vv add upstream branch name > -vvv add ahead/behind info (the only costly mode) > - same with "--cherry" (ahead/behind/same) Yeah, I thought about that too. > Or maybe combine the middle two cases into "-vv", which means it would > be the same as "-vv", with only "-v" changing what it does now. Please no, I would like to see 'upstream', but not ahead/behind info. In fact, that's my whole motivation behind this patch. > Yes, this changes current behavior (at least fpr -v), which sucks > anyways because of the costly lookup. And those scripting around "branch > -v" should get what they deserve. (I may sound annoyed by our > compatibility brakes, but here's a case where breaking it should be OK.) I also agree that it would be OK to break this. Alternatively, I've been thinking that we should have a v2.0 mode, or a v2.0 branch, where all the compatibility breakage things go. We have been so careful at not breaking things, that we have been too good at stacking hacks and configurations all over the place, and in my experience it's not unusual that right after an incompatibility release, someone realizes that we forgot some compat breakage things, and oh, well, maybe for v3.0. The ones I have in mind are: color.ui=true merge.defaulttoupstream=true format.coverletter=auto branch.autosetupmerge=always mergetool.prompt=false Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
Felipe Contreras wrote: > [branch "master"] > remote = origin > merge = refs/heads/master > pushremote = github > push = refs/heads/master Hm. Some thoughts: fetch and push aren't symmetric. By default fetches are batched: when you say 'git fetch', it fetches all the refs and uses the remote..fetch refspec to update the refs on your side. Now, I would argue that this is the correct design, because I rarely want to fetch just one ref (and that is broken, by the way: refs on my side aren't updated for some weird reason). The other reason this is correct is because fetching has nothing to do with local branches: how you _merge_ your remote branches to your local branches is entirely orthogonal to the issue (and that is controlled by branch..merge). Now, push is a different ball game altogether. There are people who do batched pushes (push.default = matching has been the default for 8 years now). However, the support for a batched push in a triangular workflow is very limited: I can't say git push master hot-branch pickaxe-doc, and expect them to go to the right remotes (this idea has already been discussed and rejected). Back to your patch: if you want to support batched pushes to map refs correctly, you should write a patch for remote..push. It has a very valid usecase too: there are people who use Gerrit and they shouldn't have to do git push :refs/for/ every single time. Neither should they have to configure each branch..push. The ref-mapping is an inherent property of the remote, not of the local branch. And branch..merge is entirely orthogonal to ref-mapping, as I already explained. That said, I think the concept of a downstream can be useful. I have branch.hot-branch.remote set to origin, and branch.hot-branch.pushremote set to ram. Now, I push some changes: my prompt still says > (indicating unpushed changes), and this is very annoying. I would definitely like git to be able to recognize that I'm ahead of upstream, but in-sync with my downstream. So, your branch..push should probably be named branch..downstreamref and be used only for informational purposes (@{d} and git status)? Wait, why do we need it at all? Is there something that we cannot infer from a proposed remote..push? Why will we ever need to override that refspec mapping with a local branch configuration? -- 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] branch: add show-tracking option
Felipe Contreras venit, vidit, dixit 16.05.2013 10:09: > On Thu, May 16, 2013 at 3:00 AM, Michael J Gruber > wrote: >> Felipe Contreras venit, vidit, dixit 16.05.2013 09:48: >>> Showing the tracking information for all the branches takes significant >>> amount of time. The user might not want that. The --no-show-tracking >>> option allows that. >> >> I really like the idea of allowing that - not just because I've >> suggested so myself in the past ;) >> >> I feel, though, that we're really exploding our option and config realm. >> For "git branch" in list mode, we are already able to stack "-v", i.e. >> "-v" and "-vv" do different things. How about maybe adding "-vvv" and >> arranging things so that the verbosity and the run time increases with >> the number of v's? >> >> -v list with sha1 + subject of last commit >> -vv add upstream branch name >> -vvv add ahead/behind info (the only costly mode) >> - same with "--cherry" (ahead/behind/same) > > Yeah, I thought about that too. > >> Or maybe combine the middle two cases into "-vv", which means it would >> be the same as "-vv", with only "-v" changing what it does now. > > Please no, I would like to see 'upstream', but not ahead/behind info. > In fact, that's my whole motivation behind this patch. I'd be fine with combining the first two also. >> Yes, this changes current behavior (at least fpr -v), which sucks >> anyways because of the costly lookup. And those scripting around "branch >> -v" should get what they deserve. (I may sound annoyed by our >> compatibility brakes, but here's a case where breaking it should be OK.) > > I also agree that it would be OK to break this. > > Alternatively, I've been thinking that we should have a v2.0 mode, or > a v2.0 branch, where all the compatibility breakage things go. We have > been so careful at not breaking things, that we have been too good at > stacking hacks and configurations all over the place, and in my > experience it's not unusual that right after an incompatibility > release, someone realizes that we forgot some compat breakage things, > and oh, well, maybe for v3.0. > > The ones I have in mind are: > > color.ui=true > merge.defaulttoupstream=true > format.coverletter=auto > branch.autosetupmerge=always > mergetool.prompt=false > > Cheers. Yes. Additionally, there are things which we can break during normal releases, but somehow the compatibility discussions have kept us from doing so. Maybe we need a clearer separation of porcellain and plumbing again? Or a clearer definition of x, y, z in x.y.z releases? We haven't even used y increases for incompatible ui changes that much. Michael -- 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] branch: add show-tracking option
On Thu, May 16, 2013 at 3:00 PM, Michael J Gruber wrote: > I feel, though, that we're really exploding our option and config realm. > For "git branch" in list mode, we are already able to stack "-v", i.e. > "-v" and "-vv" do different things. How about maybe adding "-vvv" and > arranging things so that the verbosity and the run time increases with > the number of v's? > > -v list with sha1 + subject of last commit > -vv add upstream branch name > -vvv add ahead/behind info (the only costly mode) > - same with "--cherry" (ahead/behind/same) > > Or maybe combine the middle two cases into "-vv", which means it would > be the same as "-vv", with only "-v" changing what it does now. What if I want something in - except some in -vv? I think to avoid option explosion, maybe we can adopt --pretty=format:xxx from "git log" and let the user decideswhat (and how) to display. "pretty" code learns about alignment already, which may be useful here. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] fetch: add --allow-local option
Felipe Contreras wrote: > So that it becomes possible to override the behavior when the remote > tracked is '.'; if it is, we default back to 'origin'. What is the problem you're trying to solve? Why do you have branch..remote set to '.' in the first place, if you meant origin? 'git fetch .' currently just updates FETCH_HEAD; while I'm not sure how that is useful, I still don't understand _why_ you want to change that behavior. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
Felipe Contreras wrote: > git-pull.sh | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) This one obviously looks good. I'm not sure why you sent it along with the other two patches though. On a related note, I'm interested in fixing pull. What I have on paper so far: - pull.condition = clean-worktree | ff-update | no-local-changes | always | never - pull.action = merge | rebase* | reset - pull.resetType = soft | hard | merge | keep - pull.autostash = true | false (ff-update is satisfied when FETCH_HEAD is directly ahead of refs/remotes/, while no-local-changes is satisfied when FETCH_HEAD is directly ahead of refs/heads/) Any clue how to design branch-specific pull configuration? -- 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/8] remote-bzr: patches for next
Hi, These patches have been cooking in my github repository, and improve the situation when bzr servers don't support repositories properly. Felipe Contreras (8): remote-bzr: recover from failed clones remote-bzr: fix for files with spaces remote-bzr: simplify get_remote_branch() remote-bzr: delay cloning/pulling remote-bzr: change global repo remote-bzr: trivial cleanups remote-bzr: reorganize the way 'wanted' works remote-bzr: add fallback check for a partial clone contrib/remote-helpers/git-remote-bzr | 102 +- 1 file changed, 50 insertions(+), 52 deletions(-) -- 1.8.3.rc2.542.g24820ba -- 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/8] remote-bzr: recover from failed clones
Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index b295dd4..7cd9ed8 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -768,22 +768,24 @@ def get_remote_branch(origin, remote_branch, name): global dirname, peers branch_path = os.path.join(dirname, 'clone', name) -if os.path.exists(branch_path): -# pull + +try: d = bzrlib.bzrdir.BzrDir.open(branch_path) branch = d.open_branch() -try: -branch.pull(remote_branch, [], None, False) -except bzrlib.errors.DivergedBranches: -# use remote branch for now -return remote_branch -else: +except bzrlib.errors.NotBranchError: # clone d = origin.sprout(branch_path, None, hardlink=True, create_tree_if_local=False, force_new_repo=False, source_branch=remote_branch) branch = d.open_branch() +else: +# pull +try: +branch.pull(remote_branch, [], None, False) +except bzrlib.errors.DivergedBranches: +# use remote branch for now +return remote_branch return branch -- 1.8.3.rc2.542.g24820ba -- 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/8] remote-bzr: fix for files with spaces
Set the maximum number of splits to make when dividing the diff stat lines based on space characters. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 7cd9ed8..b849336 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -620,7 +620,7 @@ def parse_commit(parser): mark = int(mark_ref[1:]) f = { 'mode' : m, 'mark' : mark } elif parser.check('D'): -t, path = line.split(' ') +t, path = line.split(' ', 1) f = { 'deleted' : True } else: die('Unknown file command: %s' % line) -- 1.8.3.rc2.542.g24820ba -- 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/8] remote-bzr: simplify get_remote_branch()
No need for 'origin', it's only needed for the bzrdir 'sprout' method, which can be greatly simplified. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index b849336..b7656df 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -764,25 +764,26 @@ def do_list(parser): print "@refs/heads/%s HEAD" % master_branch print -def get_remote_branch(origin, remote_branch, name): +def clone(path, remote_branch): +bdir = bzrlib.bzrdir.BzrDir.create(path) +repo = bdir.find_repository() +repo.fetch(remote_branch.repository) +return remote_branch.sprout(bdir, repository=repo) + +def get_remote_branch(remote_branch, name): global dirname, peers branch_path = os.path.join(dirname, 'clone', name) try: -d = bzrlib.bzrdir.BzrDir.open(branch_path) -branch = d.open_branch() +branch = bzrlib.branch.Branch.open(branch_path) except bzrlib.errors.NotBranchError: # clone -d = origin.sprout(branch_path, None, -hardlink=True, create_tree_if_local=False, -force_new_repo=False, -source_branch=remote_branch) -branch = d.open_branch() +branch = clone(branch_path, remote_branch) else: # pull try: -branch.pull(remote_branch, [], None, False) +branch.pull(remote_branch, overwrite=True) except bzrlib.errors.DivergedBranches: # use remote branch for now return remote_branch @@ -850,7 +851,7 @@ def get_repo(url, alias): if not is_local: peers[name] = remote_branch.base -branch = get_remote_branch(origin, remote_branch, name) +branch = get_remote_branch(remote_branch, name) else: branch = remote_branch @@ -868,7 +869,7 @@ def get_repo(url, alias): if not is_local: peers[name] = remote_branch.base -branch = get_remote_branch(origin, remote_branch, name) +branch = get_remote_branch(remote_branch, name) else: branch = remote_branch -- 1.8.3.rc2.542.g24820ba -- 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 4/8] remote-bzr: delay cloning/pulling
Until the branch is actually going to be used. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index b7656df..2ba49ff 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -277,7 +277,7 @@ def export_branch(repo, name): ref = '%s/heads/%s' % (prefix, name) tip = marks.get_tip(name) -branch = bzrlib.branch.Branch.open(branches[name]) +branch = get_remote_branch(name) repo = branch.repository branch.lock_read() @@ -589,7 +589,7 @@ def parse_commit(parser): if ref.startswith('refs/heads/'): name = ref[len('refs/heads/'):] -branch = bzrlib.branch.Branch.open(branches[name]) +branch = get_remote_branch(name) else: die('unknown ref') @@ -691,7 +691,7 @@ def do_export(parser): for ref, revid in parsed_refs.iteritems(): if ref.startswith('refs/heads/'): name = ref[len('refs/heads/'):] -branch = bzrlib.branch.Branch.open(branches[name]) +branch = get_remote_branch(name) branch.generate_revision_history(revid, marks.get_tip(name)) if name in peers: @@ -748,7 +748,7 @@ def do_list(parser): master_branch = name print "? refs/heads/%s" % name -branch = bzrlib.branch.Branch.open(branches[master_branch]) +branch = get_remote_branch(master_branch) branch.lock_read() for tag, revid in branch.tags.get_tag_dict().items(): try: @@ -770,8 +770,12 @@ def clone(path, remote_branch): repo.fetch(remote_branch.repository) return remote_branch.sprout(bdir, repository=repo) -def get_remote_branch(remote_branch, name): -global dirname, peers +def get_remote_branch(name): +global dirname, branches + +remote_branch = bzrlib.branch.Branch.open(branches[name]) +if isinstance(remote_branch.user_transport, bzrlib.transport.local.LocalTransport): +return remote_branch branch_path = os.path.join(dirname, 'clone', name) @@ -851,13 +855,10 @@ def get_repo(url, alias): if not is_local: peers[name] = remote_branch.base -branch = get_remote_branch(remote_branch, name) -else: -branch = remote_branch -branches[name] = branch.base +branches[name] = remote_branch.base -return branch.repository +return remote_branch.repository else: # repository @@ -869,11 +870,8 @@ def get_repo(url, alias): if not is_local: peers[name] = remote_branch.base -branch = get_remote_branch(remote_branch, name) -else: -branch = remote_branch -branches[name] = branch.base +branches[name] = remote_branch.base return repo -- 1.8.3.rc2.542.g24820ba -- 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/8] remote-bzr: change global repo
It's not used anyway. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 2ba49ff..fdc2e69 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -858,7 +858,7 @@ def get_repo(url, alias): branches[name] = remote_branch.base -return remote_branch.repository +return origin else: # repository @@ -873,7 +873,7 @@ def get_repo(url, alias): branches[name] = remote_branch.base -return repo +return origin def fix_path(alias, orig_url): url = urlparse.urlparse(orig_url, 'file') -- 1.8.3.rc2.542.g24820ba -- 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/8] remote-bzr: trivial cleanups
Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index fdc2e69..dd3d71c 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -814,7 +814,7 @@ def find_branches(repo, wanted): except bzrlib.errors.NotBranchError: continue else: -yield name, branch +yield name, branch.base def get_repo(url, alias): global dirname, peer, branches @@ -851,12 +851,12 @@ def get_repo(url, alias): # branch name = 'master' -remote_branch = origin.open_branch() +branch = origin.open_branch().base if not is_local: -peers[name] = remote_branch.base +peers[name] = branch -branches[name] = remote_branch.base +branches[name] = branch return origin else: @@ -866,12 +866,12 @@ def get_repo(url, alias): # stupid python wanted = [e for e in wanted if e] -for name, remote_branch in find_branches(repo, wanted): +for name, branch in find_branches(repo, wanted): if not is_local: -peers[name] = remote_branch.base +peers[name] = branch -branches[name] = remote_branch.base +branches[name] = branch return origin -- 1.8.3.rc2.542.g24820ba -- 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 8/8] remote-bzr: add fallback check for a partial clone
Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 434e613..acc0dc9 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -765,7 +765,10 @@ def do_list(parser): print def clone(path, remote_branch): -bdir = bzrlib.bzrdir.BzrDir.create(path) +try: +bdir = bzrlib.bzrdir.BzrDir.create(path) +except bzrlib.errors.AlreadyControlDirError: +bdir = bzrlib.bzrdir.BzrDir.open(path) repo = bdir.find_repository() repo.fetch(remote_branch.repository) return remote_branch.sprout(bdir, repository=repo) -- 1.8.3.rc2.542.g24820ba -- 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 7/8] remote-bzr: reorganize the way 'wanted' works
If the user specified a list of branches, we ignore what the remote repository lists, and simply use the branches directly. Since some remotes don't report the branches correctly, this is useful. Otherwise either fetch the repo, or the branch. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-bzr | 48 +++ 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index dd3d71c..434e613 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -794,7 +794,7 @@ def get_remote_branch(name): return branch -def find_branches(repo, wanted): +def find_branches(repo): transport = repo.user_transport for fn in transport.iter_files_recursive(): @@ -805,9 +805,6 @@ def find_branches(repo, wanted): name = name if name != '' else 'master' name = name.replace('/', '+') -if wanted and not name in wanted: -continue - try: cur = transport.clone(subdir) branch = bzrlib.branch.Branch.open_from_transport(cur) @@ -845,35 +842,32 @@ def get_repo(url, alias): except bzrlib.errors.NotBranchError: pass -try: -repo = origin.open_repository() -except bzrlib.errors.NoRepositoryPresent: -# branch - -name = 'master' -branch = origin.open_branch().base +wanted = get_config('remote-bzr.branches').rstrip().split(', ') +# stupid python +wanted = [e for e in wanted if e] -if not is_local: -peers[name] = branch +if not wanted: +try: +repo = origin.open_repository() +except bzrlib.errors.NoRepositoryPresent: +wanted = ['master'] -branches[name] = branch +if wanted: +def list_wanted(url, wanted): +for name in wanted: +subdir = name if name != 'master' else '' +yield name, bzrlib.urlutils.join(url, subdir) -return origin +branch_list = list_wanted(url, wanted) else: -# repository - -wanted = get_config('remote-bzr.branches').rstrip().split(', ') -# stupid python -wanted = [e for e in wanted if e] - -for name, branch in find_branches(repo, wanted): +branch_list = find_branches(repo) -if not is_local: -peers[name] = branch - -branches[name] = branch +for name, url in branch_list: +if not is_local: +peers[name] = url +branches[name] = url -return origin +return origin def fix_path(alias, orig_url): url = urlparse.urlparse(orig_url, 'file') -- 1.8.3.rc2.542.g24820ba -- 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] branch: add show-tracking option
Duy Nguyen venit, vidit, dixit 16.05.2013 10:23: > On Thu, May 16, 2013 at 3:00 PM, Michael J Gruber > wrote: >> I feel, though, that we're really exploding our option and config realm. >> For "git branch" in list mode, we are already able to stack "-v", i.e. >> "-v" and "-vv" do different things. How about maybe adding "-vvv" and >> arranging things so that the verbosity and the run time increases with >> the number of v's? >> >> -v list with sha1 + subject of last commit >> -vv add upstream branch name >> -vvv add ahead/behind info (the only costly mode) >> - same with "--cherry" (ahead/behind/same) >> >> Or maybe combine the middle two cases into "-vv", which means it would >> be the same as "-vv", with only "-v" changing what it does now. > > What if I want something in - except some in -vv? I think to avoid > option explosion, maybe we can adopt --pretty=format:xxx from "git > log" and let the user decideswhat (and how) to display. "pretty" code > learns about alignment already, which may be useful here. > -- > Duy Sure, that is the big solution we've been talking about. Unify for-each-ref formats and log formats and use that. After all, "git branch" in list mode really is for-each-ref, and should be transparently so; same goes for "git tag". Think "git rev-list" and "git ref-list"! But I guess we'll be compabeaten ;) Michael -- 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: English/German terminology, git.git's de.po, and pro-git
+bare repository= bloßes Repository Since "bloßes Rep." does not convey any sensible meaning to a german reader (at least it doesn't to me) it might as well be "bare". Also bare is used as parameter to commands +remote tracking branch = externer Übernahmezweig Anyone used to the english client will switch as soon as he has to read this. No idea how to improve that though except to just use the english terms like the pro git translation does. +upstream branch= -||- Use upstream as it is used as parameter to commands +fetch = anfordern fetch = fetch +pull = zusammenführen pull = pull +push = versenden push = push established vocabulary used in stack programming as well as in vcs. Should not be translated. +clean(verb)= clean(verb) = säubern/aufräumen +clean(noun)= clean(noun) = Säuberung "aufräumen" is the better verb but there is no noun for it. +whitespace = Leerzeichen (FIXME?) (maybe "Leerraum") whitespace = whitespace There is no german word for whitespace +Still being worked out: + +prune = veraltete(n) Zweig(e) entfernen +checkout(verb) = auschecken + +git add = hinzufügen "mittels "git add" hinzufügen" if you want to emphasize that you add something with the command + +merge conflict = Merge-Konflikt +3-way merge= 3-Wege-Merge +paths = Pfade + +symbolic link = symbolische Verknüfung +path = Pfad +link = Verknüpfung + +reflog = Referenzprotokoll +partial commit = teilweise committen, partiell committen As a noun, "Teil-Commit" + +reset = neu setzen (maybe "umsetzen"?) "zurücksetzen" -- 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] fetch: add --allow-local option
On Thu, May 16, 2013 at 3:25 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> So that it becomes possible to override the behavior when the remote >> tracked is '.'; if it is, we default back to 'origin'. > > What is the problem you're trying to solve? Why do you have > branch..remote set to '.' in the first place, if you meant > origin? 'git fetch .' currently just updates FETCH_HEAD; while I'm > not sure how that is useful, I still don't understand _why_ you want > to change that behavior. % git checkout -t -b devel master -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 3:29 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> git-pull.sh | 6 ++ >> 1 file changed, 2 insertions(+), 4 deletions(-) > > This one obviously looks good. I'm not sure why you sent it along > with the other two patches though. Because this patch depends on the previous one. > On a related note, I'm interested in fixing pull. What I have on paper so > far: > > - pull.condition = clean-worktree | ff-update | no-local-changes | > always | never > - pull.action = merge | rebase* | reset > - pull.resetType = soft | hard | merge | keep > - pull.autostash = true | false I don't understand that. But my only concern is that there's no way to do something like: % git fetch backup 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' -- Felipe Contreras -- 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] branch: add show-tracking option
On Thu, May 16, 2013 at 3:40 PM, Michael J Gruber wrote: >> What if I want something in - except some in -vv? I think to avoid >> option explosion, maybe we can adopt --pretty=format:xxx from "git >> log" and let the user decideswhat (and how) to display. "pretty" code >> learns about alignment already, which may be useful here. >> -- >> Duy > > Sure, that is the big solution we've been talking about. Unify > for-each-ref formats and log formats and use that. After all, "git > branch" in list mode really is for-each-ref, and should be transparently > so; same goes for "git tag". Think "git rev-list" and "git ref-list"! Again I forgot about for-each-ref. Sounds like sharing code between for-each-ref and branch is a good thing to do. Then just add more candy placeholders from git-log like %C(xx). Sounds like a fun topic. > But I guess we'll be compabeaten ;) No idea what that last word means :( -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: English/German terminology, git.git's de.po, and pro-git
Ralf Thielow writes: > Hi, > > I think the discussion might work better via ML than GitHub. > This is the full glossary of git's de.po as it would look > like with (hopefully) all the changes included that have been > discussed here. Still without any reasoning for decisions > (except HEAD). [...] > +remote branch = externer Zweig > +remote tracking branch = externer Übernahmezweig Hrm, before we (erm, you) do any extensive work on redoing the glossary, I think we should step back and agree on a direction. Remember what this thread started with: } However, an unfortunate and unsatisfactory situation has developed: } Christian Stimming's git-gui de.po uses a Ger translation, and Ralf } Thielow built core git's de.po on top of it, so it's also Ger. } } Meanwhile, and independently, Sven Fuchs and Ralph Haussmann wrote a } translation of pro-git (which is also quite mature at this point, having } apparently begun in 2009), and as you probably guessed by now, it's G+E. } } So that leaves us at a point where "the" libre Git book (and also the } one that happens to be hosted on git-scm.com, the official site) does } not match the terminology used by German git. } } Like, at all. They're not even remotely near each other. My thinly veiled opinion in the thread starter was that we should redo git's de.po from scratch using a translation similar to pro-git. I can accept that discussion takes a different turn, and thus the translation does something else. But my impression in the thread so far was that: * Everyone voted for G+E. * The thread went of on a tangent, bikeshedding on some Ger translations. Please tell me I'm wrong... Otherwise, assuming any agreement can be reached, IMHO the first step must be to write/complete a glossary that matches *current usage* in pro-git. We can perhaps bikeshed about some glaring issues in the result, but remember that -- again assuming G+E is the conclusion -- any such change again either means a divergence between book and git (bad!) or a lot of work for the book translators. -- Thomas Rast trast@{inf,student}.ethz.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
On Thu, May 16, 2013 at 3:21 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> [branch "master"] >> remote = origin >> merge = refs/heads/master >> pushremote = github >> push = refs/heads/master > > Hm. Some thoughts: > > fetch and push aren't symmetric. By default fetches are batched: when > you say 'git fetch', it fetches all the refs and uses the > remote..fetch refspec to update the refs on your side. Now, I > would argue that this is the correct design, because I rarely want to > fetch just one ref (and that is broken, by the way: refs on my side > aren't updated for some weird reason). The other reason this is > correct is because fetching has nothing to do with local branches: how > you _merge_ your remote branches to your local branches is entirely > orthogonal to the issue (and that is controlled by > branch..merge). > > Now, push is a different ball game altogether. There are people who > do batched pushes (push.default = matching has been the default for 8 > years now). And is going to change soon. > However, the support for a batched push in a triangular > workflow is very limited: I can't say git push master hot-branch > pickaxe-doc, and expect them to go to the right remotes (this idea has > already been discussed and rejected). Back to your patch: if you want > to support batched pushes to map refs correctly, you should write a > patch for remote..push. It has a very valid usecase too: there > are people who use Gerrit and they shouldn't have to do git push > :refs/for/ every single time. Neither should they have to > configure each branch..push. The ref-mapping is an inherent > property of the remote, not of the local branch. And > branch..merge is entirely orthogonal to ref-mapping, as I > already explained. > > That said, I think the concept of a downstream can be useful. I have > branch.hot-branch.remote set to origin, and > branch.hot-branch.pushremote set to ram. Now, I push some changes: my > prompt still says > (indicating unpushed changes), and this is very > annoying. I would definitely like git to be able to recognize that > I'm ahead of upstream, but in-sync with my downstream. So, your > branch..push should probably be named > branch..downstreamref and be used only for informational > purposes (@{d} and git status)? That makes absolutely no sense. [branch "master"] remote = origin merge = refs/heads/master pushremote = github downstreamref = refs/heads/whaaa:refs/heads/master What is the point of 'refs/heads/whaaa'? > Wait, why do we need it at all? Is > there something that we cannot infer from a proposed > remote..push? Why will we ever need to override that refspec > mapping with a local branch configuration? [branch "master"] remote = origin merge = refs/heads/master pushremote = github push = refs/heads/fc/master [branch "fc/old-remote/hg"] remote = . merge = refs/heads/master pushremote = github push = refs/heads/fc/remote/hg Tell me how you express that without 'remote.branch.push'. Cheers. -- Felipe Contreras -- 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] fast-export: add new --refspec option
On Thu, May 9, 2013 at 8:13 PM, Junio C Hamano wrote: >It is a very powerful concept that we can generate data once, >cast it in stone, and delay the decision on _how_ it is used >until the last minute, much better than mapping at export time. >So bundle does not need a similar refspec mechanism to map what >it exports. I haven't thought things through to see if the same >logic applies to fast-export output (if so, it would mean it is >better to allow the consumer of the stream take the refspec >parameter and map the tips it finds in its input), though. It's possible to delay the renaming of refspecs to leave that duty to the remote-helper, however, each and every remote-helper would need advertise a new capability, and then implement the renaming. I think it makes much more sense to implement something that just works for free in all remote-helpers. But I wasn't even that interested in this patch, I implemented it because while I was helping the emacs folks I had to explain to them that old:new didn't work, and I thought it would be trivial to make it work. I'm dropping this series. Somebody should at least make it so a proper error is displayed so that the users are not left in the dark wondering what the hell is going on. -- Felipe Contreras -- 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] Support for old:new remote-helper push
On Wed, May 8, 2013 at 8:31 PM, Felipe Contreras wrote: > In order to support pushing old:new refspecs in remote-helpers, the best way > to > do what is to add an option to fast-export to rename the refs in the stream, > so > here it is: I'm not going to work on this series any more. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] fast-export: trivial cleanup
On Wed, May 8, 2013 at 8:16 PM, Felipe Contreras wrote: > Cast the object to a commit, only to get the object back? I'm dropping this one. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
Felipe Contreras wrote: > And is going to change soon. Your point being? How will this patch interact with push.default = matching? >> branch..push should probably be named >> branch..downstreamref and be used only for informational >> purposes (@{d} and git status)? > > That makes absolutely no sense. I said downstreamref, not downstreamrefspec. > [branch "master"] > remote = origin > merge = refs/heads/master > pushremote = github > push = refs/heads/fc/master > > [branch "fc/old-remote/hg"] > remote = . > merge = refs/heads/master > pushremote = github > push = refs/heads/fc/remote/hg > > Tell me how you express that without 'remote.branch.push'. [remote "origin"] push = refs/heads/master:refs/heads/fc/master [remote "."] push = refs/heads/fc/old-remote/hg:refs/heads/fc/remote/hg Advantage being you can do: [remote "origin"] push = refs/heads/*:refs/for/* While you can't with branch..push. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] {fast-export,transport-helper}: style cleanups
On Thu, May 9, 2013 at 6:12 PM, Felipe Contreras wrote: > On Thu, May 9, 2013 at 6:09 PM, Junio C Hamano wrote: >> John Szakmeister writes: >> >>> On Wed, May 8, 2013 at 9:16 PM, Felipe Contreras >>> wrote: Signed-off-by: Felipe Contreras --- builtin/fast-export.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index d60d675..8091354 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -135,7 +135,7 @@ static void export_blob(const unsigned char *sha1) >>> [snip] @@ -289,13 +289,13 @@ static void handle_commit(struct commit *commit, struct rev_info *rev) parse_commit(commit); author = strstr(commit->buffer, "\nauthor "); if (!author) - die ("Could not find author in commit %s", + die("Could not find author in commit %s", sha1_to_hex(commit->object.sha1)); >>> >>> It looks like your simple replace didn't account for calls with >>> multiple lines. Now the remaining lines don't line up. >>> :-) There's several more places like this in the patch. >> >> Good eyes. >> >> Matching the coding-style to have no SP between function name and >> its argument list is just as important as matching the indentation >> style used in the project; trading one breakage with another does >> not make much sense. > > Where exactly in Documentation/CodingGuidelines is the "indentation > style" used in the project specified that is being violated? I find it extremely annoying that an obviously correct patch is not merged because it's not conforming to a non-existing coding-style guideline that not even the Linux project follows. I've sent many patches where I change the alignment from cino=(0, to cino=(2s, and the get applied because if it's not mentioned in Documentation/CodingStyle, it cannot be used as a reason for rejection. I fixed the style so it conforms to Documentation/CodingGuidelines, and nowhere in there is the open parenthesis alignment mentioned, so using that as a reason to reject this patch is a mistake in my opinion. If you prefer the code to not follow Documentation/CodingGuidelines, so be it. I'm not going to work on this patch any more. -- Felipe Contreras -- 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] fetch: add --allow-local option
Felipe Contreras wrote: > % git checkout -t -b devel master Interesting. Have you considered changing -t to inherit the parent branch's remote? (Would everyone like that?) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
On Thu, May 16, 2013 at 4:20 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> And is going to change soon. > > Your point being? How will this patch interact with push.default = matching? > >>> branch..push should probably be named >>> branch..downstreamref and be used only for informational >>> purposes (@{d} and git status)? >> >> That makes absolutely no sense. > > I said downstreamref, not downstreamrefspec. That's not consistent with branch.A.merge, which is not named branch.A.upstreamref. >> [branch "master"] >> remote = origin >> merge = refs/heads/master >> pushremote = github >> push = refs/heads/fc/master >> >> [branch "fc/old-remote/hg"] >> remote = . >> merge = refs/heads/master >> pushremote = github >> push = refs/heads/fc/remote/hg >> >> Tell me how you express that without 'remote.branch.push'. > > [remote "origin"] > push = refs/heads/master:refs/heads/fc/master > > [remote "."] > push = refs/heads/fc/old-remote/hg:refs/heads/fc/remote/hg Let's see: % git checkout master % git push It will try to push to 'origin/fc/master' not 'github/fc/master', which is what I intended. % git checkout fc/old-remote/hg % git push It will try to push to 'fc/remote/hg' not 'github/fc/remote/hg', which is what I intended. > Advantage being you can do: > > [remote "origin"] > push = refs/heads/*:refs/for/* > > While you can't with branch..push. But I can do 'git push origin "refs/head/*:refs/heads/for/*"', not that I've ever had the need to do something like that, so I don't care. -- Felipe Contreras -- 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] fetch: add --allow-local option
On Thu, May 16, 2013 at 4:27 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> % git checkout -t -b devel master > > Interesting. Have you considered changing -t to inherit the parent > branch's remote? (Would everyone like that?) Why would I do that? When I do 'git rebase' I want to rebase on top of 'master', not 'origin/master' (or whatever the upstream of 'master' is). -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
Felipe Contreras wrote: > But my only concern is that there's no way to > do something like: > > % git fetch backup 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' [remote "backup"] fetch = refs/tags/*:refs/tags/* fetch = refs/heads/*:refs/heads/* What am I missing? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 4:34 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> But my only concern is that there's no way to >> do something like: >> >> % git fetch backup 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' > > [remote "backup"] > fetch = refs/tags/*:refs/tags/* > fetch = refs/heads/*:refs/heads/* > > What am I missing? Actually trying that command: % git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' fatal: Refusing to fetch into current branch refs/heads/fc/fast-export/cleanup of non-bare repository fatal: The remote end hung up unexpectedly -- Felipe Contreras -- 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: [PATCHv2 04/10] remote: Reject remote names containing '/'
Johan Herland wrote: > The disambiguation can probably be resolved, although the resulting > code will obviously be somewhat more cumbersome and ugly (and IMHO the > current code is plenty of that already...). Combine this with the > problems of clobbering of the same remote-tracking ref (describe > above), and the fact that AFAIK a multi-level remote name has never > been observed "in the wild" (none of the people I asked at the Git > Merge conference had ever observed multi-level remote names, nor did > they directly oppose banning them), I'm not at all sure it's worth > bothering about this at all. Simply disallowing multi-level remote > names seems like the simpler and naturally ambiguity-resistant > approach. The problem with multi-level remote names is that we use the same delimiter as in the ref namespace: '/'. So, obviously, there's a lot of room for confusion. I wonder if we should banish multi-level remotes altogether though: is it possible that they will be useful to someone in the future? -- 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 v10 00/14] interactive git-clean
Updates since v9: * Patch 01-04: refactor relative_path() in path.c, and reused it in quote.c and clean.c. * Patch 14: add testcases as t7301, and catch two bugs by running testcases. * Bugfix: when select by number, range like "5-" does not select last entry. @@ -485,7 +443,7 @@ static int parse_choice(struct menu_stuff *menu_stuff, bottom = atoi((*ptr)->buf); /* a range can be specified like 5-7 or 5- */ if (!*(strchr((*ptr)->buf, '-') + 1)) { - top = menu_stuff->nr - 1; + top = menu_stuff->nr; } else { top = atoi(strchr((*ptr)->buf, '-') + 1); } * For the "ask each" action, confirmation other than "yes" should not delete files. @@ -772,7 +730,7 @@ static int ask_each_cmd(void) eof = 1; } } - if (!confirm.len || !strncasecmp(confirm.buf, "no", confirm.len)) { + if (!confirm.len || strncasecmp(confirm.buf, "yes", confirm.len)) { *item->string = '\0'; changed++; } Jiang Xin (14): path.c: refactor relative_path(), not only strip prefix quote.c: remove path_relative, use relative_path instead Refactor quote_path_relative, remove unused params Refactor write_name_quoted_relative, remove unused params git-clean: refactor git-clean into two phases git-clean: add support for -i/--interactive git-clean: show items of del_list in columns git-clean: add colors to interactive git-clean git-clean: use a git-add-interactive compatible UI git-clean: add filter by pattern interactive action git-clean: add select by numbers interactive action git-clean: add ask each interactive action git-clean: add documentation for interactive git-clean test: add t7301 for git-clean--interactive Documentation/config.txt | 21 +- Documentation/git-clean.txt | 71 +++- builtin/clean.c | 771 +-- builtin/grep.c | 5 +- builtin/ls-files.c | 16 +- cache.h | 2 +- path.c | 112 +-- quote.c | 65 +--- quote.h | 7 +- setup.c | 5 +- t/t7301-clean-interactive.sh | 439 wt-status.c | 17 +- 12 files changed, 1380 insertions(+), 151 deletions(-) create mode 100755 t/t7301-clean-interactive.sh -- 1.8.3.rc1.407.g762149a -- 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 v10 01/14] path.c: refactor relative_path(), not only strip prefix
Original design of relative_path() is simple, just strip the prefix (*base) from the abosolute path (*abs). In most cases, we need a real relative path, such as: ../foo, ../../bar. That's why there is another reimplementation (path_relative()) in quote.c. Refactor relative_path() in path.c to return real relative path, so that user can reuse this function without reimplement his/her own. I will use this method for interactive git-clean later. Some of the implementations are borrowed from path_relative() in quote.c. Different results for relative_path() before and after this refactor: base path abs path relative (orignal) relative (refactor) = == === /a/b /a/b/c/ c/ c/ //a///b/ /a/b//c/ c/ c/ /a/b /a/b . ./ /a/b /a/b/ . ./ /a/b/ /a/a ../ /a/b/ / / ../../ /a/b/ /a/c /a/c../c (empty)/a/b /a/b/a/b NULL /a/b /a/b/a/b /a/b (empty) (empty) ./ Signed-off-by: Jiang Xin --- cache.h | 2 +- path.c | 112 +--- setup.c | 5 ++- 3 files changed, 92 insertions(+), 27 deletions(-) diff --git a/cache.h b/cache.h index 7ce9..f0c69 100644 --- a/cache.h +++ b/cache.h @@ -737,7 +737,7 @@ int is_directory(const char *); const char *real_path(const char *path); const char *real_path_if_valid(const char *path); const char *absolute_path(const char *path); -const char *relative_path(const char *abs, const char *base); +const char *relative_path(const char *abs, const char *base, struct strbuf *sb); int normalize_path_copy(char *dst, const char *src); int longest_ancestor_length(const char *path, struct string_list *prefixes); char *strip_path_suffix(const char *path, const char *suffix); diff --git a/path.c b/path.c index 04ff..e24ea 100644 --- a/path.c +++ b/path.c @@ -441,42 +441,104 @@ int adjust_shared_perm(const char *path) return 0; } -const char *relative_path(const char *abs, const char *base) +/* + * Give path as relative to prefix. + * + * The strbuf may or may not be used, so do not assume it contains the + * returned path. + */ +const char *relative_path(const char *abs, const char *base, + struct strbuf *sb) { - static char buf[PATH_MAX + 1]; - int i = 0, j = 0; - - if (!base || !base[0]) + int abs_off, base_off, i, j; + int abs_len, base_len; + + abs_len = abs? strlen(abs): 0; + base_len = base ? strlen(base) : 0; + abs_off = 0; + base_off = 0; + i = 0; + j = 0; + + if (!abs_len) + return "./"; + else if (!base_len) return abs; - while (base[i]) { + + while (i < base_len && j < abs_len && base[i] == abs[j]) { if (is_dir_sep(base[i])) { - if (!is_dir_sep(abs[j])) - return abs; while (is_dir_sep(base[i])) i++; while (is_dir_sep(abs[j])) j++; - continue; - } else if (abs[j] != base[i]) { + base_off = i; + abs_off = j; + } else { + i++; + j++; + } + } + + if ( + /* base seems like prefix of abs */ + i >= base_len && + /* +* but "/foo" is not a prefix of "/foobar" +* (i.e. base not end with '/') +*/ + base_off < base_len) { + if (j >= abs_len) { + /* abs="/a/b", base="/a/b" */ + abs_off = abs_len; + } else if (is_dir_sep(abs[j])) { + /* abs="/a/b/c", base="/a/b" */ + while (is_dir_sep(abs[j])) + j++; + abs_off = j; + } else { + /* abs="/a/bbb/c", base="/a/b" */ + i = base_off; + } + } else if ( + /* abs is short than base (prefix of base) */ + j >= abs_len && + /* abs not end with '/' */ + abs_off < abs_len) { + if (is_dir_sep(base[i])) { + /* abs="/a/b", base="/a/b/c/" */ + while (is_dir_sep(base[i])) + i++; + abs_off = abs_len; + } + } + abs += abs_off; + abs_len -= abs_off; + + if (i >= base_len) { + if (!abs_len) + return "./"; +
[PATCH v10 02/14] quote.c: remove path_relative, use relative_path instead
Since there is an enhanced version of relative_path() in path.c, remove duplicate counterpart path_relative() in quote.c. Signed-off-by: Jiang Xin --- quote.c | 55 ++- 1 file changed, 2 insertions(+), 53 deletions(-) diff --git a/quote.c b/quote.c index 91122..64ff3 100644 --- a/quote.c +++ b/quote.c @@ -312,75 +312,24 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen, fputc(terminator, fp); } -static const char *path_relative(const char *in, int len, -struct strbuf *sb, const char *prefix, -int prefix_len); - void write_name_quoted_relative(const char *name, size_t len, const char *prefix, size_t prefix_len, FILE *fp, int terminator) { struct strbuf sb = STRBUF_INIT; - name = path_relative(name, len, &sb, prefix, prefix_len); + name = relative_path(name, prefix, &sb); write_name_quoted(name, fp, terminator); strbuf_release(&sb); } -/* - * Give path as relative to prefix. - * - * The strbuf may or may not be used, so do not assume it contains the - * returned path. - */ -static const char *path_relative(const char *in, int len, -struct strbuf *sb, const char *prefix, -int prefix_len) -{ - int off, i; - - if (len < 0) - len = strlen(in); - if (prefix_len < 0) { - if (prefix) - prefix_len = strlen(prefix); - else - prefix_len = 0; - } - - off = 0; - i = 0; - while (i < prefix_len && i < len && prefix[i] == in[i]) { - if (prefix[i] == '/') - off = i + 1; - i++; - } - in += off; - len -= off; - - if (i >= prefix_len) - return in; - - strbuf_reset(sb); - strbuf_grow(sb, len); - - while (i < prefix_len) { - if (prefix[i] == '/') - strbuf_addstr(sb, "../"); - i++; - } - strbuf_add(sb, in, len); - - return sb->buf; -} - /* quote path as relative to the given prefix */ char *quote_path_relative(const char *in, int len, struct strbuf *out, const char *prefix) { struct strbuf sb = STRBUF_INIT; - const char *rel = path_relative(in, len, &sb, prefix, -1); + const char *rel = relative_path(in, prefix, &sb); strbuf_reset(out); quote_c_style_counted(rel, strlen(rel), out, NULL, 0); strbuf_release(&sb); -- 1.8.3.rc1.407.g762149a -- 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 v10 03/14] Refactor quote_path_relative, remove unused params
After substitude path_relative() in quote.c with relative_path() from path.c, parameters (such as len and prefix_len) are obsolte in function quote_path_relative(). Remove unused parameters and the order of parameters for quote_path_relative() function. Signed-off-by: Jiang Xin --- builtin/clean.c| 18 +- builtin/grep.c | 5 ++--- builtin/ls-files.c | 2 +- quote.c| 7 ++- quote.h| 4 ++-- wt-status.c| 17 - 6 files changed, 24 insertions(+), 29 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 04e39..f77f95 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -56,7 +56,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) && !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) { if (!quiet) { - quote_path_relative(path->buf, strlen(path->buf), "ed, prefix); + quote_path_relative(path->buf, prefix, "ed); printf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir), quoted.buf); } @@ -70,7 +70,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, /* an empty dir could be removed even if it is unreadble */ res = dry_run ? 0 : rmdir(path->buf); if (res) { - quote_path_relative(path->buf, strlen(path->buf), "ed, prefix); + quote_path_relative(path->buf, prefix, "ed); warning(_(msg_warn_remove_failed), quoted.buf); *dir_gone = 0; } @@ -94,7 +94,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone)) ret = 1; if (gone) { - quote_path_relative(path->buf, strlen(path->buf), "ed, prefix); + quote_path_relative(path->buf, prefix, "ed); string_list_append(&dels, quoted.buf); } else *dir_gone = 0; @@ -102,10 +102,10 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, } else { res = dry_run ? 0 : unlink(path->buf); if (!res) { - quote_path_relative(path->buf, strlen(path->buf), "ed, prefix); + quote_path_relative(path->buf, prefix, "ed); string_list_append(&dels, quoted.buf); } else { - quote_path_relative(path->buf, strlen(path->buf), "ed, prefix); + quote_path_relative(path->buf, prefix, "ed); warning(_(msg_warn_remove_failed), quoted.buf); *dir_gone = 0; ret = 1; @@ -127,7 +127,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, if (!res) *dir_gone = 1; else { - quote_path_relative(path->buf, strlen(path->buf), "ed, prefix); + quote_path_relative(path->buf, prefix, "ed); warning(_(msg_warn_remove_failed), quoted.buf); *dir_gone = 0; ret = 1; @@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone)) errors++; if (gone && !quiet) { - qname = quote_path_relative(directory.buf, directory.len, &buf, prefix); + qname = quote_path_relative(directory.buf, prefix, &buf); printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname); } } @@ -272,11 +272,11 @@ int cmd_clean(int argc, const char **argv, const char *prefix) continue; res = dry_run ? 0 : unlink(ent->name); if (res) { - qname = quote_path_relative(ent->name, -1, &buf, prefix); + qname = quote_path_relative(ent->name, prefix, &buf); warning(_(msg_warn_remove_failed), qname); errors++; } else if (!quiet) { - qname =
[PATCH v10 04/14] Refactor write_name_quoted_relative, remove unused params
After substitude path_relative() in quote.c with relative_path() from path.c, parameters (such as len and prefix_len) are obsolte in function write_name_quoted_relative(). Remove unused parameters from write_name_quoted_relative() and related functions. Signed-off-by: Jiang Xin --- builtin/ls-files.c | 14 -- quote.c| 3 +-- quote.h| 3 +-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 00f07..d3b33 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -46,10 +46,12 @@ static const char *tag_modified = ""; static const char *tag_skip_worktree = ""; static const char *tag_resolve_undo = ""; -static void write_name(const char* name, size_t len) +static void write_name(const char* name) { - write_name_quoted_relative(name, len, prefix, prefix_len, stdout, - line_terminator); + + /* turn off prefix, if run with "--full-name" */ + write_name_quoted_relative(name, prefix_len > 0 ? prefix : NULL, + stdout, line_terminator); } static void show_dir_entry(const char *tag, struct dir_entry *ent) @@ -63,7 +65,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent) return; fputs(tag, stdout); - write_name(ent->name, ent->len); + write_name(ent->name); } static void show_other_files(struct dir_struct *dir) @@ -163,7 +165,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce) find_unique_abbrev(ce->sha1,abbrev), ce_stage(ce)); } - write_name(ce->name, ce_namelen(ce)); + write_name(ce->name); if (debug_mode) { printf(" ctime: %d:%d\n", ce->ce_ctime.sec, ce->ce_ctime.nsec); printf(" mtime: %d:%d\n", ce->ce_mtime.sec, ce->ce_mtime.nsec); @@ -196,7 +198,7 @@ static void show_ru_info(void) printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i], find_unique_abbrev(ui->sha1[i], abbrev), i + 1); - write_name(path, len); + write_name(path); } } } diff --git a/quote.c b/quote.c index ebb8..5c880 100644 --- a/quote.c +++ b/quote.c @@ -312,8 +312,7 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen, fputc(terminator, fp); } -void write_name_quoted_relative(const char *name, size_t len, - const char *prefix, size_t prefix_len, +void write_name_quoted_relative(const char *name, const char *prefix, FILE *fp, int terminator) { struct strbuf sb = STRBUF_INIT; diff --git a/quote.h b/quote.h index 5610159..ed110 100644 --- a/quote.h +++ b/quote.h @@ -60,8 +60,7 @@ extern void quote_two_c_style(struct strbuf *, const char *, const char *, int); extern void write_name_quoted(const char *name, FILE *, int terminator); extern void write_name_quotedpfx(const char *pfx, size_t pfxlen, const char *name, FILE *, int terminator); -extern void write_name_quoted_relative(const char *name, size_t len, - const char *prefix, size_t prefix_len, +extern void write_name_quoted_relative(const char *name, const char *prefix, FILE *fp, int terminator); /* quote path as relative to the given prefix */ -- 1.8.3.rc1.407.g762149a -- 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 v10 05/14] git-clean: refactor git-clean into two phases
Before introducing interactive git-clean, refactor git-clean operations into two phases: * hold cleaning items in del_list, * and remove them in a separate loop at the end. We will introduce interactive git-clean between the two phases. The interactive git-clean will show what would be done and must confirm before do real cleaning. Signed-off-by: Jiang Xin --- builtin/clean.c | 64 - 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index f77f95..23e1f 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -15,6 +15,7 @@ #include "quote.h" static int force = -1; /* unset */ +static struct string_list del_list = STRING_LIST_INIT_DUP; static const char *const builtin_clean_usage[] = { N_("git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ..."), @@ -148,12 +149,13 @@ int cmd_clean(int argc, const char **argv, const char *prefix) int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0; int ignored_only = 0, config_set = 0, errors = 0, gone = 1; int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT; - struct strbuf directory = STRBUF_INIT; + struct strbuf abs_path = STRBUF_INIT; struct dir_struct dir; static const char **pathspec; struct strbuf buf = STRBUF_INIT; struct string_list exclude_list = STRING_LIST_INIT_NODUP; struct exclude_list *el; + struct string_list_item *item; const char *qname; char *seen = NULL; struct option options[] = { @@ -223,6 +225,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) int matches = 0; struct cache_entry *ce; struct stat st; + const char *rel; /* * Remove the '/' at the end that directory @@ -242,13 +245,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix) continue; /* Yup, this one exists unmerged */ } - /* -* we might have removed this as part of earlier -* recursive directory removal, so lstat() here could -* fail with ENOENT. -*/ if (lstat(ent->name, &st)) - continue; + die_errno("Cannot lstat '%s'", ent->name); if (pathspec) { memset(seen, 0, argc > 0 ? argc : 1); @@ -257,33 +255,61 @@ int cmd_clean(int argc, const char **argv, const char *prefix) } if (S_ISDIR(st.st_mode)) { - strbuf_addstr(&directory, ent->name); if (remove_directories || (matches == MATCHED_EXACTLY)) { - if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone)) - errors++; - if (gone && !quiet) { - qname = quote_path_relative(directory.buf, prefix, &buf); - printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname); - } + rel = relative_path(ent->name, prefix, &buf); + string_list_append(&del_list, rel); } - strbuf_reset(&directory); } else { if (pathspec && !matches) continue; - res = dry_run ? 0 : unlink(ent->name); + rel = relative_path(ent->name, prefix, &buf); + string_list_append(&del_list, rel); + } + } + + /* TODO: do interactive git-clean here, which will modify del_list */ + + for_each_string_list_item(item, &del_list) { + struct stat st; + + if (prefix) { + strbuf_addstr(&abs_path, prefix); + } + strbuf_addstr(&abs_path, item->string); + + /* +* we might have removed this as part of earlier +* recursive directory removal, so lstat() here could +* fail with ENOENT. +*/ + if (lstat(abs_path.buf, &st)) + continue; + + if (S_ISDIR(st.st_mode)) { + if (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone)) + errors++; + if (gone && !quiet) { + qname = quote_path_relative(item->string, NULL, &buf); + printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname); + } + } else { + res = dry_run ? 0 : unlink(abs_path.buf); i
[PATCH v10 06/14] git-clean: add support for -i/--interactive
Show what would be done and the user must confirm before actually cleaning. Would remove ... Would remove ... Would remove ... Remove [y/n]? Press "y" to start cleaning, and press "n" if you want to abort. Signed-off-by: Jiang Xin --- Documentation/git-clean.txt | 10 ++-- builtin/clean.c | 57 + 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index bdc3a..186e34 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree SYNOPSIS [verse] -'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ... +'git clean' [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ... DESCRIPTION --- @@ -34,7 +34,13 @@ OPTIONS -f:: --force:: If the Git configuration variable clean.requireForce is not set - to false, 'git clean' will refuse to run unless given -f or -n. + to false, 'git clean' will refuse to run unless given -f, -n or + -i. + +-i:: +--interactive:: + Show what would be done and the user must confirm before actually + cleaning. -n:: --dry-run:: diff --git a/builtin/clean.c b/builtin/clean.c index 23e1f..f28d6 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -15,10 +15,11 @@ #include "quote.h" static int force = -1; /* unset */ +static int interactive; static struct string_list del_list = STRING_LIST_INIT_DUP; static const char *const builtin_clean_usage[] = { - N_("git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ..."), + N_("git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ..."), NULL }; @@ -143,6 +144,50 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, return ret; } +static void interactive_main_loop(void) +{ + struct strbuf confirm = STRBUF_INIT; + struct strbuf buf = STRBUF_INIT; + struct string_list_item *item; + const char *qname; + + while (del_list.nr) { + putchar('\n'); + for_each_string_list_item(item, &del_list) { + qname = quote_path_relative(item->string, NULL, &buf); + printf(_(msg_would_remove), qname); + } + putchar('\n'); + + printf(_("Remove [y/n]? ")); + if (strbuf_getline(&confirm, stdin, '\n') != EOF) { + strbuf_trim(&confirm); + } else { + /* Ctrl-D is the same as "quit" */ + string_list_clear(&del_list, 0); + putchar('\n'); + printf_ln("Bye."); + break; + } + + if (confirm.len) { + if (!strncasecmp(confirm.buf, "yes", confirm.len)) { + break; + } else if (!strncasecmp(confirm.buf, "no", confirm.len) || + !strncasecmp(confirm.buf, "quit", confirm.len)) { + string_list_clear(&del_list, 0); + printf_ln("Bye."); + break; + } else { + continue; + } + } + } + + strbuf_release(&buf); + strbuf_release(&confirm); +} + int cmd_clean(int argc, const char **argv, const char *prefix) { int i, res; @@ -162,6 +207,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) OPT__QUIET(&quiet, N_("do not print names of files removed")), OPT__DRY_RUN(&dry_run, N_("dry run")), OPT__FORCE(&force, N_("force")), + OPT_BOOL('i', "interactive", &interactive, N_("interactive cleaning")), OPT_BOOLEAN('d', NULL, &remove_directories, N_("remove whole directories")), { OPTION_CALLBACK, 'e', "exclude", &exclude_list, N_("pattern"), @@ -188,12 +234,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (ignored && ignored_only) die(_("-x and -X cannot be used together")); - if (!dry_run && !force) { + if (!interactive && !dry_run && !force) { if (config_set) - die(_("clean.requireForce set to true and neither -n nor -f given; " + die(_("clean.requireForce set to true and neither -i, -n nor -f given; " "refusing to clean")); else - die(_("clean.requireForce defaults to true and neither -n nor -f given; " + die(_("clean.requireForce defaults to true and neither -i, -n nor -f given; " "refusing to clean")); } @@
[PATCH v10 07/14] git-clean: show items of del_list in columns
When there are lots of items to be cleaned, it is hard to see them all in one screen. Show them in columns will solve this problem. Signed-off-by: Jiang Xin Comments-by: Matthieu Moy --- Documentation/config.txt | 4 builtin/clean.c | 49 +++- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6e53f..e031b 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -955,6 +955,10 @@ column.branch:: Specify whether to output branch listing in `git branch` in columns. See `column.ui` for details. +column.clean:: + Specify the layout when list items in `git clean -i`, which always + shows files and directories in columns. See `column.ui` for details. + column.status:: Specify whether to output untracked files in `git status` in columns. See `column.ui` for details. diff --git a/builtin/clean.c b/builtin/clean.c index f28d6..f25ba 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -13,10 +13,12 @@ #include "refs.h" #include "string-list.h" #include "quote.h" +#include "column.h" static int force = -1; /* unset */ static int interactive; static struct string_list del_list = STRING_LIST_INIT_DUP; +static unsigned int colopts; static const char *const builtin_clean_usage[] = { N_("git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ..."), @@ -31,8 +33,13 @@ static const char *msg_warn_remove_failed = N_("failed to remove %s"); static int git_clean_config(const char *var, const char *value, void *cb) { - if (!strcmp(var, "clean.requireforce")) + if (!prefixcmp(var, "column.")) + return git_column_config(var, value, "clean", &colopts); + + if (!strcmp(var, "clean.requireforce")) { force = !git_config_bool(var, value); + return 0; + } return git_default_config(var, value, cb); } @@ -144,21 +151,46 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, return ret; } -static void interactive_main_loop(void) +static void pretty_print_dels(void) { - struct strbuf confirm = STRBUF_INIT; - struct strbuf buf = STRBUF_INIT; + struct string_list list = STRING_LIST_INIT_DUP; struct string_list_item *item; + struct strbuf buf = STRBUF_INIT; const char *qname; + struct column_options copts; + + for_each_string_list_item(item, &del_list) { + qname = quote_path_relative(item->string, NULL, &buf); + string_list_append(&list, qname); + } + + /* +* always enable column display, we only consult column.* +* about layout strategy and stuff +*/ + colopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED; + memset(&copts, 0, sizeof(copts)); + copts.indent = " "; + copts.padding = 2; + print_columns(&list, colopts, &copts); + putchar('\n'); + strbuf_release(&buf); + string_list_clear(&list, 0); +} + +static void interactive_main_loop(void) +{ + struct strbuf confirm = STRBUF_INIT; while (del_list.nr) { putchar('\n'); - for_each_string_list_item(item, &del_list) { - qname = quote_path_relative(item->string, NULL, &buf); - printf(_(msg_would_remove), qname); - } + printf_ln(Q_("Would remove the following item:", +"Would remove the following items:", +del_list.nr)); putchar('\n'); + pretty_print_dels(); + printf(_("Remove [y/n]? ")); if (strbuf_getline(&confirm, stdin, '\n') != EOF) { strbuf_trim(&confirm); @@ -184,7 +216,6 @@ static void interactive_main_loop(void) } } - strbuf_release(&buf); strbuf_release(&confirm); } -- 1.8.3.rc1.407.g762149a -- 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 v10 08/14] git-clean: add colors to interactive git-clean
Show header, help, error messages, and prompt in colors for interactive git-clean. Re-use config variables, such as "color.interactive" and "color.interactive." for command `git-add--interactive`. Signed-off-by: Jiang Xin Comments-by: Matthieu Moy --- Documentation/config.txt | 17 +-- builtin/clean.c | 73 +++- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index e031b..83613 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -876,16 +876,17 @@ The values of these variables may be specified as in color.branch.. color.interactive:: When set to `always`, always use colors for interactive prompts - and displays (such as those used by "git-add --interactive"). - When false (or `never`), never. When set to `true` or `auto`, use - colors only when the output is to the terminal. Defaults to false. + and displays (such as those used by "git-add --interactive" and + "git-clean --interactive"). When false (or `never`), never. + When set to `true` or `auto`, use colors only when the output is + to the terminal. Defaults to false. color.interactive.:: - Use customized color for 'git add --interactive' - output. `` may be `prompt`, `header`, `help` or `error`, for - four distinct types of normal output from interactive - commands. The values of these variables may be specified as - in color.branch.. + Use customized color for 'git add --interactive' and 'git clean + --interactive' output. `` may be `prompt`, `header`, `help` + or `error`, for four distinct types of normal output from + interactive commands. The values of these variables may be + specified as in color.branch.. color.pager:: A boolean to enable/disable colored output when the pager is in diff --git a/builtin/clean.c b/builtin/clean.c index f25ba..0778a 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -14,6 +14,7 @@ #include "string-list.h" #include "quote.h" #include "column.h" +#include "color.h" static int force = -1; /* unset */ static int interactive; @@ -31,16 +32,82 @@ static const char *msg_skip_git_dir = N_("Skipping repository %s\n"); static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n"); static const char *msg_warn_remove_failed = N_("failed to remove %s"); +static int clean_use_color = -1; +static char clean_colors[][COLOR_MAXLEN] = { + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_BOLD_BLUE,/* PROMPT */ + GIT_COLOR_BOLD, /* HEADER */ + GIT_COLOR_BOLD_RED, /* HELP */ + GIT_COLOR_BOLD_RED, /* ERROR */ +}; +enum color_clean { + CLEAN_COLOR_RESET = 0, + CLEAN_COLOR_PLAIN = 1, + CLEAN_COLOR_PROMPT = 2, + CLEAN_COLOR_HEADER = 3, + CLEAN_COLOR_HELP = 4, + CLEAN_COLOR_ERROR = 5, +}; + +static int parse_clean_color_slot(const char *var) +{ + if (!strcasecmp(var, "reset")) + return CLEAN_COLOR_RESET; + if (!strcasecmp(var, "plain")) + return CLEAN_COLOR_PLAIN; + if (!strcasecmp(var, "prompt")) + return CLEAN_COLOR_PROMPT; + if (!strcasecmp(var, "header")) + return CLEAN_COLOR_HEADER; + if (!strcasecmp(var, "help")) + return CLEAN_COLOR_HELP; + if (!strcasecmp(var, "error")) + return CLEAN_COLOR_ERROR; + return -1; +} + static int git_clean_config(const char *var, const char *value, void *cb) { if (!prefixcmp(var, "column.")) return git_column_config(var, value, "clean", &colopts); + /* honors the color.interactive* config variables which also + applied in git-add--interactive and git-stash */ + if (!strcmp(var, "color.interactive")) { + clean_use_color = git_config_colorbool(var, value); + return 0; + } + if (!prefixcmp(var, "color.interactive.")) { + int slot = parse_clean_color_slot(var + + strlen("color.interactive.")); + if (slot < 0) + return 0; + if (!value) + return config_error_nonbool(var); + color_parse(value, var, clean_colors[slot]); + return 0; + } + if (!strcmp(var, "clean.requireforce")) { force = !git_config_bool(var, value); return 0; } - return git_default_config(var, value, cb); + + /* inspect the color.ui config variable and others */ + return git_color_default_config(var, value, cb); +} + +static const char *clean_get_color(enum color_clean ix) +{ + if (want_color(clean_use_color)) + return clean_colors[ix]; + return ""; +} + +static
[PATCH v10 09/14] git-clean: use a git-add-interactive compatible UI
Rewrite menu using a new method `list_and_choose`, which is borrowed from `git-add--interactive.perl`. We will use this framework to add new actions for interactive git-clean later. Please NOTE: * Method `list_and_choose` return an array of integers, and * it is up to you to free the allocated memory of the array. * The array ends with EOF. * If user pressed CTRL-D (i.e. EOF), no selection returned. Signed-off-by: Jiang Xin --- builtin/clean.c | 449 1 file changed, 420 insertions(+), 29 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 0778a..f59ed 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -50,6 +50,36 @@ enum color_clean { CLEAN_COLOR_ERROR = 5, }; +#define MENU_OPTS_SINGLETON01 +#define MENU_OPTS_IMMEDIATE02 +#define MENU_OPTS_LIST_ONLY04 + +struct menu_opts { + const char *header; + const char *prompt; + int flags; +}; + +#define MENU_RETURN_NO_LOOP10 + +struct menu_item { + char hotkey; + char *title; + int selected; + int (*fn)(); +}; + +enum menu_stuff_type { + MENU_STUFF_TYPE_STRING_LIST = 1, + MENU_STUFF_TYPE_MENU_ITEM +}; + +struct menu_stuff { + enum menu_stuff_type type; + int nr; + void *stuff; +}; + static int parse_clean_color_slot(const char *var) { if (!strcasecmp(var, "reset")) @@ -240,54 +270,415 @@ static void pretty_print_dels(void) copts.indent = " "; copts.padding = 2; print_columns(&list, colopts, &copts); - putchar('\n'); strbuf_release(&buf); string_list_clear(&list, 0); } -static void interactive_main_loop(void) +static void pretty_print_menus(struct string_list *menu_list) +{ + unsigned int local_colopts = 0; + struct column_options copts; + + local_colopts = COL_ENABLED | COL_ROW; + memset(&copts, 0, sizeof(copts)); + copts.indent = " "; + copts.padding = 2; + print_columns(menu_list, local_colopts, &copts); +} + +static void prompt_help_cmd(int singleton) +{ + clean_print_color(CLEAN_COLOR_HELP); + printf_ln(singleton ? + _("Prompt help:\n" + "1 - select a numbered item\n" + "foo- select item based on unique prefix\n" + " - (empty) select nothing") : + _("Prompt help:\n" + "1 - select a single item\n" + "3-5- select a range of items\n" + "2-3,6-9- select multiple ranges\n" + "foo- select item based on unique prefix\n" + "-... - unselect specified items\n" + "* - choose all items\n" + " - (empty) finish selecting")); + clean_print_color(CLEAN_COLOR_RESET); +} + +/* + * display menu stuff with number prefix and hotkey highlight + */ +static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen) +{ + struct string_list menu_list = STRING_LIST_INIT_DUP; + struct strbuf menu = STRBUF_INIT; + int i; + + if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) { + struct menu_item *item; + + item = (struct menu_item *)stuff->stuff; + for (i = 0; i < stuff->nr; i++, item++) { + char *p; + int highlighted = 0; + + p = item->title; + if ((*chosen)[i] < 0) + (*chosen)[i] = item->selected ? 1 : 0; + strbuf_addf(&menu, "%s%2d: ", (*chosen)[i] ? "*" : " ", i+1); + for (; *p; p++) { + if (!highlighted && *p == item->hotkey) { + strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_PROMPT)); + strbuf_addch(&menu, *p); + strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_RESET)); + highlighted = 1; + } else { + strbuf_addch(&menu, *p); + } + } + string_list_append(&menu_list, menu.buf); + strbuf_reset(&menu); + } + } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) { + struct string_list_item *item; + struct strbuf buf = STRBUF_INIT; + i = 0; + + for_each_string_list_item(item, (struct string_list *)stuff->stuff) { + if ((*chosen)[i] < 0) + (*chosen)[i] = 0; + strbuf_addf(&menu, "%s%2d: %s", (*chosen)[i] ? "*" : " ", ++i, item->string); +
[PATCH v10 10/14] git-clean: add filter by pattern interactive action
Add a new action for interactive git-clean: filter by pattern. When the user chooses this action, user can input space-separated patterns (the same syntax as gitignore), and each clean candidate that matches with one of the patterns will be excluded from cleaning. When the user feels it's OK, presses ENTER and back to the confirmation dialog. Signed-off-by: Jiang Xin Suggested-by: Junio C Hamano --- builtin/clean.c | 68 + 1 file changed, 68 insertions(+) diff --git a/builtin/clean.c b/builtin/clean.c index f59ed..35b9f3 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -607,6 +607,72 @@ static int clean_cmd(void) return MENU_RETURN_NO_LOOP; } +static int filter_by_patterns_cmd(void) +{ + struct dir_struct dir; + struct strbuf confirm = STRBUF_INIT; + struct strbuf **ignore_list; + struct string_list_item *item; + struct exclude_list *el; + int changed = -1, i; + + for (;;) { + if (!del_list.nr) + break; + + if (changed) + pretty_print_dels(); + + clean_print_color(CLEAN_COLOR_PROMPT); + printf(_("Input ignore patterns>> ")); + clean_print_color(CLEAN_COLOR_RESET); + if (strbuf_getline(&confirm, stdin, '\n') != EOF) + strbuf_trim(&confirm); + else + putchar('\n'); + + /* quit filter_by_pattern mode if press ENTER or Ctrl-D */ + if (!confirm.len) + break; + + memset(&dir, 0, sizeof(dir)); + el = add_exclude_list(&dir, EXC_CMDL, "manual exclude"); + ignore_list = strbuf_split_max(&confirm, ' ', 0); + + for (i = 0; ignore_list[i]; i++) { + strbuf_trim(ignore_list[i]); + if (!ignore_list[i]->len) + continue; + + add_exclude(ignore_list[i]->buf, "", 0, el, -(i+1)); + } + + changed = 0; + for_each_string_list_item(item, &del_list) { + int dtype = DT_UNKNOWN; + + if (is_excluded(&dir, item->string, &dtype)) { + *item->string = '\0'; + changed++; + } + } + + if (changed) { + string_list_remove_empty_items(&del_list, 0); + } else { + clean_print_color(CLEAN_COLOR_ERROR); + printf_ln(_("WARNING: Cannot find items matched by: %s"), confirm.buf); + clean_print_color(CLEAN_COLOR_RESET); + } + + strbuf_list_free(ignore_list); + clear_directory(&dir); + } + + strbuf_release(&confirm); + return 0; +} + static int quit_cmd(void) { string_list_clear(&del_list, 0); @@ -619,6 +685,7 @@ static int help_cmd(void) clean_print_color(CLEAN_COLOR_HELP); printf_ln(_( "clean - start cleaning\n" + "filter by pattern - exclude items from deletion\n" "quit- stop cleaning\n" "help- this screen\n" "? - help for prompt selection" @@ -634,6 +701,7 @@ static void interactive_main_loop(void) struct menu_stuff menu_stuff; struct menu_item menus[] = { {'c', "clean", 0, clean_cmd}, + {'f', "filter by pattern", 0, filter_by_patterns_cmd}, {'q', "quit", 0, quit_cmd}, {'h', "help", 0, help_cmd}, }; -- 1.8.3.rc1.407.g762149a -- 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 v10 11/14] git-clean: add select by numbers interactive action
Draw a multiple choice menu using `list_and_choose` to select items to be deleted by numbers. User can input: * 1,5-7 : select 1,5,6,7 items to be deleted * * : select all items to be deleted * -*: unselect all, nothing will be deleted *: (empty) finish selecting, and return back to main menu Signed-off-by: Jiang Xin --- builtin/clean.c | 39 +++ 1 file changed, 39 insertions(+) diff --git a/builtin/clean.c b/builtin/clean.c index 35b9f3..96a5bb 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -673,6 +673,43 @@ static int filter_by_patterns_cmd(void) return 0; } +static int select_by_numbers_cmd(void) +{ + struct menu_opts menu_opts; + struct menu_stuff menu_stuff; + struct string_list_item *items; + int *chosen; + int i, j; + + menu_opts.header = NULL; + menu_opts.prompt = N_("Select items to delete"); + menu_opts.flags = 0; + + menu_stuff.type = MENU_STUFF_TYPE_STRING_LIST; + menu_stuff.stuff = &del_list; + menu_stuff.nr = del_list.nr; + + chosen = list_and_choose(&menu_opts, &menu_stuff); + items = del_list.items; + for(i = 0, j = 0; i < del_list.nr; i++) { + if (i < chosen[j]) { + *(items[i].string) = '\0'; + } else if (i == chosen[j]) { + /* delete selected item */ + j++; + continue; + } else { + /* end of chosen (chosen[j] == EOF), won't delete */ + *(items[i].string) = '\0'; + } + } + + string_list_remove_empty_items(&del_list, 0); + + free(chosen); + return 0; +} + static int quit_cmd(void) { string_list_clear(&del_list, 0); @@ -686,6 +723,7 @@ static int help_cmd(void) printf_ln(_( "clean - start cleaning\n" "filter by pattern - exclude items from deletion\n" + "select by numbers - select items to be deleted by numbers\n" "quit- stop cleaning\n" "help- this screen\n" "? - help for prompt selection" @@ -702,6 +740,7 @@ static void interactive_main_loop(void) struct menu_item menus[] = { {'c', "clean", 0, clean_cmd}, {'f', "filter by pattern", 0, filter_by_patterns_cmd}, + {'s', "select by numbers", 0, select_by_numbers_cmd}, {'q', "quit", 0, quit_cmd}, {'h', "help", 0, help_cmd}, }; -- 1.8.3.rc1.407.g762149a -- 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 v10 12/14] git-clean: add ask each interactive action
Add a new action for interactive git-clean: ask each. It's just like the "rm -i" command, that the user must confirm one by one for each file or directory to be cleaned. Signed-off-by: Jiang Xin --- builtin/clean.c | 36 1 file changed, 36 insertions(+) diff --git a/builtin/clean.c b/builtin/clean.c index 96a5bb..1c6315 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -710,6 +710,40 @@ static int select_by_numbers_cmd(void) return 0; } +static int ask_each_cmd(void) +{ + struct strbuf confirm = STRBUF_INIT; + struct strbuf buf = STRBUF_INIT; + struct string_list_item *item; + const char *qname; + int changed = 0, eof = 0; + + for_each_string_list_item(item, &del_list) { + /* Ctrl-D should stop removing files */ + if (!eof) { + qname = quote_path_relative(item->string, NULL, &buf); + printf(_("remove %s? "), qname); + if (strbuf_getline(&confirm, stdin, '\n') != EOF) { + strbuf_trim(&confirm); + } else { + putchar('\n'); + eof = 1; + } + } + if (!confirm.len || strncasecmp(confirm.buf, "yes", confirm.len)) { + *item->string = '\0'; + changed++; + } + } + + if (changed) + string_list_remove_empty_items(&del_list, 0); + + strbuf_release(&buf); + strbuf_release(&confirm); + return MENU_RETURN_NO_LOOP; +} + static int quit_cmd(void) { string_list_clear(&del_list, 0); @@ -724,6 +758,7 @@ static int help_cmd(void) "clean - start cleaning\n" "filter by pattern - exclude items from deletion\n" "select by numbers - select items to be deleted by numbers\n" + "ask each- confirm each deletion (like \"rm -i\")\n" "quit- stop cleaning\n" "help- this screen\n" "? - help for prompt selection" @@ -741,6 +776,7 @@ static void interactive_main_loop(void) {'c', "clean", 0, clean_cmd}, {'f', "filter by pattern", 0, filter_by_patterns_cmd}, {'s', "select by numbers", 0, select_by_numbers_cmd}, + {'a', "ask each", 0, ask_each_cmd}, {'q', "quit", 0, quit_cmd}, {'h', "help", 0, help_cmd}, }; -- 1.8.3.rc1.407.g762149a -- 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 v10 13/14] git-clean: add documentation for interactive git-clean
Add new section "Interactive mode" for documentation of interactive git-clean. Signed-off-by: Jiang Xin Helped-by: Eric Sunshine --- Documentation/git-clean.txt | 65 +++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 186e34..5bf76 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -39,8 +39,8 @@ OPTIONS -i:: --interactive:: - Show what would be done and the user must confirm before actually - cleaning. + Show what would be done and clean files interactively. See + ``Interactive mode'' for details. -n:: --dry-run:: @@ -69,6 +69,67 @@ OPTIONS Remove only files ignored by Git. This may be useful to rebuild everything from scratch, but keep manually created files. +Interactive mode + +When the command enters the interactive mode, it shows the +files and directories to be cleaned, and goes into its +interactive command loop. + +The command loop shows the list of subcommands available, and +gives a prompt "What now> ". In general, when the prompt ends +with a single '>', you can pick only one of the choices given +and type return, like this: + + +*** Commands *** +1: clean2: filter by pattern3: select by numbers +4: ask each 5: quit 6: help +What now> 1 + + +You also could say `c` or `clean` above as long as the choice is unique. + +The main command loop has 6 subcommands. + +clean:: + + Start cleaning files and directories, and then quit. + +filter by pattern:: + + This shows the files and directories to be deleted and issues an + "Input ignore patterns>>" prompt. You can input space-seperated + patterns to exclude files and directories from deletion. + E.g. "*.c *.h" will excludes files end with ".c" and ".h" from + deletion. When you are satisfied with the filtered result, press + ENTER (empty) back to the main menu. + +select by numbers:: + + This shows the files and directories to be deleted and issues an + "Select items to delete>>" prompt. When the prompt ends with double + '>>' like this, you can make more than one selection, concatenated + with whitespace or comma. Also you can say ranges. E.g. "2-5 7,9" + to choose 2,3,4,5,7,9 from the list. If the second number in a + range is omitted, all remaining patches are taken. E.g. "7-" to + choose 7,8,9 from the list. You can say '*' to choose everything. + Also when you are satisfied with the filtered result, press ENTER + (empty) back to the main menu. + +ask each:: + + This will start to clean, and you must confirm one by one in order + to delete items. Please note that this action is not as efficient + as the above two actions. + +quit:: + + This lets you quit without do cleaning. + +help:: + + Show brief usage of interactive git-clean. + SEE ALSO linkgit:gitignore[5] -- 1.8.3.rc1.407.g762149a -- 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 v10 14/14] test: add t7301 for git-clean--interactive
Add testcases for git-clean--interactive. Signed-off-by: Jiang Xin --- t/t7301-clean-interactive.sh | 439 +++ 1 file changed, 439 insertions(+) create mode 100755 t/t7301-clean-interactive.sh diff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh new file mode 100755 index 0..27465 --- /dev/null +++ b/t/t7301-clean-interactive.sh @@ -0,0 +1,439 @@ +#!/bin/sh + +test_description='git clean -i basic tests' + +. ./test-lib.sh + +test_expect_success 'setup' ' + + mkdir -p src && + touch src/part1.c Makefile && + echo build >.gitignore && + echo \*.o >>.gitignore && + git add . && + git commit -m setup && + touch src/part2.c README && + git add . + +' + +test_expect_success 'git clean -i (clean)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + echo c | git clean -i && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test ! -f a.out && + test -f docs/manual.txt && + test ! -f src/part3.c && + test ! -f src/part3.h && + test ! -f src/part4.c && + test ! -f src/part4.h && + test -f obj.o && + test -f build/lib.so + +' + +test_expect_success 'git clean -i (quit)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + echo q | git clean -i && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test -f a.out && + test -f docs/manual.txt && + test -f src/part3.c && + test -f src/part3.h && + test -f src/part4.c && + test -f src/part4.h && + test -f obj.o && + test -f build/lib.so + +' + +test_expect_success 'git clean -i (Ctrl+D)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + echo "\04" | git clean -i && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test -f a.out && + test -f docs/manual.txt && + test -f src/part3.c && + test -f src/part3.h && + test -f src/part4.c && + test -f src/part4.h && + test -f obj.o && + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter all)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + (echo f; echo "*"; echo; echo c) | \ + git clean -id && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test -f a.out && + test -f docs/manual.txt && + test -f src/part3.c && + test -f src/part3.h && + test -f src/part4.c && + test -f src/part4.h && + test -f obj.o && + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter patterns)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + (echo f; echo "part3.* *.out"; echo; echo c) | \ + git clean -id && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test -f a.out && + test ! -f docs/manual.txt && + test -f src/part3.c && + test -f src/part3.h && + test ! -f src/part4.c && + test ! -f src/part4.h && + test -f obj.o && + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter patterns 2)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + (echo f; echo "* !*.out"; echo; echo c) | \ + git clean -id && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test ! -f a.out && + test -f docs/manual.txt && + test -f src/part3.c && + test -f src/part3.h && + test -f src/part4.c && + test -f src/part4.h && + test -f obj.o && + test -f build/lib.so + +' + +test_expect_success 'git clean -id (select - all)' ' + + mkdir -p build docs && + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ +docs/manual.txt obj.o build/lib.so && + (echo s; echo "*"; echo; echo c) | \ + git clean -id && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test ! -f a.out && + test ! -f docs/manual.txt && + test ! -f src/part3.c && + test ! -f
Re: [RFC/PATCH 2/3] pull: trivial cleanups
Felipe Contreras wrote: > % git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' > fatal: Refusing to fetch into current branch > refs/heads/fc/fast-export/cleanup of non-bare repository > fatal: The remote end hung up unexpectedly That's because your HEAD is pointing to something under refs/heads/*: it would work otherwise. When I'm developing on my personal branch, I sometimes do 'git fetch origin master:master +pu:pu +next:next', and it works as expected. When on master branch, I can't git fetch origin master:master and this is a cute safety feature. Either way, I think it's a bad practice to fetch directly into refs/heads/*: you should really be fetching to refs/remotes and then merging your changes in. I do want shortcuts though, which is why I'm interested in fixing pull: there is nothing to fix in fetch as far as I'm concerned. -- 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] fetch: add --allow-local option
Felipe Contreras wrote: > Why would I do that? When I do 'git rebase' I want to rebase on top of > 'master', not 'origin/master' (or whatever the upstream of 'master' > is). Ah, so you want @{u} to point to refs/heads/master, but want to modify fetch to act on the hard-coded "origin", not @{u} (wouldn't you like to be able to configure this?). Seems a bit yuck overall; I wonder if there's some other way to achieve what you want. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] remote-bzr: fix cloning of non-listable repos
Commit 95b0c60 (remote-bzr: add support for bzr repos) introduced a regression by assuming all bzr remote repos are listable, but they are not. If they are not listable they are basically useless, so let's assume there is no bzr repo. Reported-by: Thorsten Kranzkowski Signed-off-by: Felipe Contreras --- We want this on master for v1.8.3. contrib/remote-helpers/git-remote-bzr | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index b295dd4..ad42317 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -840,6 +840,9 @@ def get_repo(url, alias): try: repo = origin.open_repository() +if not repo.user_transport.listable(): +# this repository is not usable for us +raise bzrlib.errors.NoRepositoryPresent(repo.bzrdir) except bzrlib.errors.NoRepositoryPresent: # branch -- 1.8.3.rc2.542.g24820ba -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
Felipe Contreras wrote: >> [remote "origin"] >> push = refs/heads/master:refs/heads/fc/master >> >> [remote "."] >> push = refs/heads/fc/old-remote/hg:refs/heads/fc/remote/hg Major thinko. It should be: [remote "github"] push = refs/heads/master:refs/heads/fc/master push = refs/heads/fc/old-remote/hg:refs/heads/fc/remote/hg >> Advantage being you can do: >> >> [remote "origin"] >> push = refs/heads/*:refs/for/* >> >> While you can't with branch..push. > > But I can do 'git push origin "refs/head/*:refs/heads/for/*"', not > that I've ever had the need to do something like that, so I don't > care. Isn't the entire point of this exercise getting git to dwim without being explicit? I don't care about it personally either, which is why I haven't written a patch yet. However, there are users of Gerrit who would appreciate this feature: in the remote.pushdefault thread, some people requested this feature. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 4:54 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> % git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' >> fatal: Refusing to fetch into current branch >> refs/heads/fc/fast-export/cleanup of non-bare repository >> fatal: The remote end hung up unexpectedly > > That's because your HEAD is pointing to something under refs/heads/*: > it would work otherwise. When I'm developing on my personal branch, I > sometimes do 'git fetch origin master:master +pu:pu +next:next', and > it works as expected. When on master branch, I can't git fetch origin > master:master and this is a cute safety feature. Now you know what's the problem. > Either way, I think it's a bad practice to fetch directly into > refs/heads/*: you should really be fetching to refs/remotes and then > merging your changes in. I do want shortcuts though, which is why I'm > interested in fixing pull: there is nothing to fix in fetch as far as > I'm concerned. It doesn't matter if you think it's a bad practice or not, 'git push --mirror' works, and it's possible to update a branch even if HEAD is point to it. There should be a possibility to do the same with 'git fetch'. Right now the user is forced to do something like: git checkout -q -b tmp && git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' && git checkout -q @{-1} && git branch -q -D tmp 2> /dev/null Which doesn't seem to be quite right. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
Felipe Contreras wrote: > It doesn't matter if you think it's a bad practice or not, 'git push > --mirror' works, and it's possible to update a branch even if HEAD is > point to it. There should be a possibility to do the same with 'git > fetch'. So, introduce a configuration variable to turn off this safety feature? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
On Thu, May 16, 2013 at 5:06 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >>> [remote "origin"] >>> push = refs/heads/master:refs/heads/fc/master >>> >>> [remote "."] >>> push = refs/heads/fc/old-remote/hg:refs/heads/fc/remote/hg > > Major thinko. It should be: > > [remote "github"] > push = refs/heads/master:refs/heads/fc/master > push = refs/heads/fc/old-remote/hg:refs/heads/fc/remote/hg Would I be able to do: % git branch --set-upstream-to origin/master --set-downstream-to github/fc/master ? Would I see these branches when I do 'git branch -vv'? Would I be able to do 'git push next@{downstream}'? >>> Advantage being you can do: >>> >>> [remote "origin"] >>> push = refs/heads/*:refs/for/* >>> >>> While you can't with branch..push. >> >> But I can do 'git push origin "refs/head/*:refs/heads/for/*"', not >> that I've ever had the need to do something like that, so I don't >> care. > > Isn't the entire point of this exercise getting git to dwim without > being explicit? > > I don't care about it personally either, which is why I haven't > written a patch yet. However, there are users of Gerrit who would > appreciate this feature: in the remote.pushdefault thread, some people > requested this feature. That is orthogonal to 'branch.A.push' the same way 'remote.B.fetch' is orthogonal to 'branch.A.merge'. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 5:18 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> It doesn't matter if you think it's a bad practice or not, 'git push >> --mirror' works, and it's possible to update a branch even if HEAD is >> point to it. There should be a possibility to do the same with 'git >> fetch'. > > So, introduce a configuration variable to turn off this safety feature? I thought you were interested in fixing 'git pull'. My bad. -- Felipe Contreras -- 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] fetch: add --allow-local option
On Thu, May 16, 2013 at 4:58 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> Why would I do that? When I do 'git rebase' I want to rebase on top of >> 'master', not 'origin/master' (or whatever the upstream of 'master' >> is). > > Ah, so you want @{u} to point to refs/heads/master, but want to modify > fetch to act on the hard-coded "origin", not @{u} (wouldn't you like > to be able to configure this?). No. What's the point of 'git fetch .'? What does 'git fetch' does when there's no configured upstream branch? Why doesn't 'git fetch' default to 'git fetch .' in those cases? Answer: because 'git fetch .' doesn't make any sense. So if 'branch.HEAD.remote' is '.' it doesn't make sense to do 'git fetch .'. > Seems a bit yuck overall; I wonder if > there's some other way to achieve what you want. Yeah, add 'branch.A.base' that would be used only by 'git rebase', which I already suggested before, but I changed my mind. Fixing 'git fetch' makes much more sense. -- Felipe Contreras -- 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] fetch: add --allow-local option
Felipe Contreras wrote: > Answer: because 'git fetch .' doesn't make any sense. So if > 'branch.HEAD.remote' is '.' it doesn't make sense to do 'git fetch .'. I agree that 'git fetch .' is currently not useful (and I am not against changing its behavior), but my question pertains to why you are replacing it with the hard-coded "origin". What happens when I git branch -t -b devel hot-branch where branch.hot-branch.remote = ram and not origin? -- 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: English/German terminology, git.git's de.po, and pro-git
Dear translators, Here's the main point in this discussion: The translation is not for us! The translation is for those who don't speak much English and who don't know the English git terminology very well. By definition, this target audience is not present here on this mailing list and in this discussion. Hence, arguments such as "I like word x better" are rather weak. Instead, stating "Word x gives the intended target audience a better picture of what is going on" is probably a better argument. Am Montag, 13. Mai 2013, 14:54:51 schrieb Thomas Rast: > However, an unfortunate and unsatisfactory situation has developed: > Christian Stimming's git-gui de.po uses a Ger translation, and Ralf > Thielow built core git's de.po on top of it, so it's also Ger. > > Meanwhile, and independently, Sven Fuchs and Ralph Haussmann wrote a > translation of pro-git (which is also quite mature at this point, having > apparently begun in 2009), and as you probably guessed by now, it's G+E. Thanks, Thomas, for spotting the conflicting translations in those excellent book projects vs. the git core and git gui. I think it's rather obvious why the pro-git translators chose the G+E approach for their work: Their goal is to explain the command line usage of git, which means they inevitably have to use the git command names, which happen to be in English (and will surely stay so). Hence, any translation approach will have to deal with the English command names as useful words in the normal translated text. That's probably a constraint that is true for any translation of a command-line tool to stay useful. I noticed with some amusement, though, that even in the pro-git book with the described constraint there are places where a "pure Ger" translation is almost shining through... Such as in [1]: "Jedes Mal, wenn Du committest (d.h. den gegenwärtigen Status deines Projektes als eine Version in Git speicherst)..." Can you notice how the translators identified "Version" as translation for "commit (noun)" and "speichern" as translation for "commit (verb)" :-) ? Of course this is just the explanation and not the actual translation later during the text. However, I take this spot as an example that there exist meaningful pure-Ger translations even for the most important git terminology. In fact, to find useful Ger translations, I wonder how I would talk to someone from the target audience a sentence such as "Finde mal den richtigen Commit, also die Version, ..." When I find myself saying such an " - also das xy -" appendix often enough, I take this as an indication that the latter word can just as well be used as the main translation. Back to the original question: I think the book shows quite nicely that for working with the git command line, a G+E translation is more useful as long as the command names also appear unchangedly in the translation. However, everything else that does not appear as a command name can be translated either in G+E or in Ger. The argument can go on to state that someone who is geek enough to use the command line is probably more proficient in English language anyway. Hence, using more English terms in the translation is probably fine as well and a full G+E translation is probably a good approach. The pro-git book has some places where the translated word is not always used consistently (e.g. in [2] "Externes Repository" vs. "Remote Repository"), and some G+E suggestions from this mailing list have been translated Ger in the book (they use "zusammenführen" in [2] and [3] instead of "merge" with only a few exceptions). It is also a good point to make the pro-git and git core translation consistent, once the approach is decided on. *However*: This argument is completely different when we talk about the GUI tools. The target audience of the git gui etc. are those developers who write great code, but #1 do not know the English language well enough, and #2 are so far away from the geek corner that they use a development workflow purely in GUI tools. The question is: What GUI button labels helps those people the most to get a good picture of what is going on? And in this case I still believe a pure Ger translation is the better choice! I wonder how feedback on this claim can be collected from developers of the target audience. When I started on the git-gui translation, I asked some coworkers that fall into this category for feedback on the wordings, and their response indicated agreement to my approach. What feedback have others here heard from people who fall into described category? At the end of the day that sort of feedback has to be the ground for a decision on the approach in the GUI translation. In the meantime I think a different translation approach between git core and git gui is not a problem at all. For git gui I propose to stick to a Ger translation. For git core and the books that describe the command line interface, a G+E translation is pro
Re: [PATCHv2 04/10] remote: Reject remote names containing '/'
On Thu, May 16, 2013 at 11:48 AM, Ramkumar Ramachandra wrote: > Johan Herland wrote: >> The disambiguation can probably be resolved, although the resulting >> code will obviously be somewhat more cumbersome and ugly (and IMHO the >> current code is plenty of that already...). Combine this with the >> problems of clobbering of the same remote-tracking ref (describe >> above), and the fact that AFAIK a multi-level remote name has never >> been observed "in the wild" (none of the people I asked at the Git >> Merge conference had ever observed multi-level remote names, nor did >> they directly oppose banning them), I'm not at all sure it's worth >> bothering about this at all. Simply disallowing multi-level remote >> names seems like the simpler and naturally ambiguity-resistant >> approach. > > The problem with multi-level remote names is that we use the same > delimiter as in the ref namespace: '/'. So, obviously, there's a lot > of room for confusion. I wonder if we should banish multi-level > remotes altogether though: is it possible that they will be useful to > someone in the future? Technically, the problem is that we don't use a different delimiter between the $remote and $ref parts. If we had used "multi/level/remote:nested/ref" we would have been OK (on this issue at least, probably not OK on other issues). FWIW, I've abandoned this patch, and don't care much about multi-level remote names anymore. They work in current git, and they will sort-of work with remote ref namespaces as well, although you will have to use full refnames when referring to their remote-tracking refs. If multi-level remote names suddenly become popular, we can change the code to try to resolve them unambiguously. ...Johan -- Johan Herland, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
Felipe Contreras wrote: > Would I be able to do: > > % git branch --set-upstream-to origin/master --set-downstream-to > github/fc/master > > ? > > Would I see these branches when I do 'git branch -vv'? > Would I be able to do 'git push next@{downstream}'? Hm, losing this functionality in the name of generality would certainly be very undesirable. > That is orthogonal to 'branch.A.push' the same way 'remote.B.fetch' is > orthogonal to 'branch.A.merge'. Not at all (which is what I've been trying to say). remote..fetch is operated on by fetch, while branch..merge is operated on by merge; they are really orthogonal. What happens if both branch..push and remote..push are set? What will push do? Perhaps we should get both, and get branch..push to override remote..push. The issue being @{d} will not work if remote..push is set. Then again, since we're targeting Gerrit users here, I don't really think it's an issue: refs/for/master is not really a "downstream branch"; it's a pseudo-ref that Gerrit handles internally. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 11:36 AM, Felipe Contreras wrote: > Actually trying that command: > > % git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' > fatal: Refusing to fetch into current branch > refs/heads/fc/fast-export/cleanup of non-bare repository > fatal: The remote end hung up unexpectedly Can you try something like this instead ? git fetch --update-head-ok # or -u -- 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] prompt: introduce GIT_PS1_STATESEPARATOR
Junio C Hamano wrote: > It is simpler to use 'default values', no? > > local z=${GIT_PS1_STATESEPARATOR-" "} There don't seem to be any more comments. If there are no issues, could you fix this up locally before queueing? 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: [PATCHv2 04/10] remote: Reject remote names containing '/'
Johan Herland wrote: > FWIW, I've abandoned this patch, and don't care much about multi-level > remote names anymore. They work in current git, and they will sort-of > work with remote ref namespaces as well, although you will have to use > full refnames when referring to their remote-tracking refs. If > multi-level remote names suddenly become popular, we can change the > code to try to resolve them unambiguously. Makes sense. We can deal with the issue if and when multi-level remotes become popular. -- 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] Revert "remote-hg: update bookmarks when pulling"
This reverts commit 24317ef32ac3111ed00792f9b2921dc19dd28fe2. Different versions of Mercurial have different arguments for bookmarks.updatefromremote(), while it should be possible to call the right function with the right arguments depending on the version, it's safer to restore the old behavior for now. Reported by Rodney Lorrimar. Signed-off-by: Felipe Contreras --- Intended for master (v1.8.3). contrib/remote-helpers/git-remote-hg | 3 --- 1 file changed, 3 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index dc276af..beb864b 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -363,9 +363,6 @@ def get_repo(url, alias): die('Repository error') repo.pull(peer, heads=None, force=True) -rb = peer.listkeys('bookmarks') -bookmarks.updatefromremote(myui, repo, rb, url) - return repo def rev_to_mark(rev): -- 1.8.3.rc2.542.g24820ba -- 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] fetch: add --allow-local option
On Thu, May 16, 2013 at 5:32 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> Answer: because 'git fetch .' doesn't make any sense. So if >> 'branch.HEAD.remote' is '.' it doesn't make sense to do 'git fetch .'. > > I agree that 'git fetch .' is currently not useful (and I am not > against changing its behavior), but my question pertains to why you > are replacing it with the hard-coded "origin". 'origin' is already hard-coded. % git clone git://git.kernel.org/pub/scm/git/git.git What would be the name of the remote? 'origin'. % git checkout --no-track -b test % git fetch What is the remote that is used? 'origin'. > What happens when I > git branch -t -b devel hot-branch where branch.hot-branch.remote = ram > and not origin? The same thing that happens when you git branch --no-track -b devel hot-branch. -- Felipe Contreras -- 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] color: set color.ui to auto by default
This is a good default. Users who don't want color can look for "color" in the git-config(1), and turn it off. Signed-off-by: Ramkumar Ramachandra --- color.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/color.c b/color.c index e8e2681..f672885 100644 --- a/color.c +++ b/color.c @@ -1,7 +1,7 @@ #include "cache.h" #include "color.h" -static int git_use_color_default = 0; +static int git_use_color_default = GIT_COLOR_AUTO; int color_stdout_is_tty = -1; /* -- 1.8.3.rc2.10.g0c2b1cf.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Segmentation fault with latest git (070c57df)
sam wrote: > gdb /usr/bin/git core What exactly are you doing? Is core aliased to something? > [...] > Core was generated by `git apply --verbose --check --ignore-whitespace > --directory=/home/sam/P'. What is this? Can you give us clear instructions on how to reproduce the segfault? -- 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: Segmentation fault with latest git (070c57df)
Ramkumar Ramachandra wrote: > sam wrote: >> gdb /usr/bin/git core > > What exactly are you doing? Is core aliased to something? Sorry about that. I just realized you're loading a core dump. Please tell us how to reproduce this, or give us the backtrace with debugging symbols. -- 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] color: set color.ui to auto by default
Ramkumar Ramachandra writes: > This is a good default. Users who don't want color can look for "color" > in the git-config(1), and turn it off. Err, you're comming a bit late ;-). See my patch, posted yesterday, for the same topic. -- 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: [RFC/PATCH 3/3] push: add separate 'downstream' branch
On Thu, May 16, 2013 at 6:31 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> Would I be able to do: >> >> % git branch --set-upstream-to origin/master --set-downstream-to >> github/fc/master >> >> ? >> >> Would I see these branches when I do 'git branch -vv'? >> Would I be able to do 'git push next@{downstream}'? > > Hm, losing this functionality in the name of generality would > certainly be very undesirable. I don't even know what that means. >> That is orthogonal to 'branch.A.push' the same way 'remote.B.fetch' is >> orthogonal to 'branch.A.merge'. > > Not at all (which is what I've been trying to say). > remote..fetch is operated on by fetch, while branch..merge > is operated on by merge; they are really orthogonal. What happens if > both branch..push and remote..push are set? What will > push do? The same that 'git pull' does when both branch..merge and remote..fetch are set. > Perhaps we should get both, and get branch..push to override > remote..push. Does branch..merge overrides remote..fetch? No. They complement each other. > The issue being @{d} will not work if > remote..push is set. Of course it would work. Does @{u} stop working when remote..fetch is set? > Then again, since we're targeting Gerrit > users here, I don't really think it's an issue: refs/for/master is not > really a "downstream branch"; it's a pseudo-ref that Gerrit handles > internally. It is a downstream branch. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 6:54 AM, Antoine Pelisse wrote: > On Thu, May 16, 2013 at 11:36 AM, Felipe Contreras > wrote: >> Actually trying that command: >> >> % git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' >> fatal: Refusing to fetch into current branch >> refs/heads/fc/fast-export/cleanup of non-bare repository >> fatal: The remote end hung up unexpectedly > > Can you try something like this instead ? > > git fetch --update-head-ok # or -u Nice! That does the trick. -- Felipe Contreras -- 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] color: set color.ui to auto by default
Matthieu Moy wrote: > Err, you're comming a bit late ;-). See my patch, posted yesterday, for > the same topic. Ah, too much email :| Your patch looks good. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
Felipe Contreras wrote: > Does branch..merge overrides remote..fetch? No. They > complement each other. I often wonder if you are reading what you're responding to: remote..fetch is operated on by fetch, while branch..merge is operated on by merge; they are really orthogonal. > The same that 'git pull' does when both branch..merge and > remote..fetch are set. Are you reading this? git pull _fetches_ from remote..fetch and _merges_ from branch..merge. What is "the same" in git push terms? It's a simple question; which ref does push update: the one specified by remote..push or branch..push? > Of course it would work. Does @{u} stop working when remote..fetch is > set? It doesn't work when _only_ remote..fetch is set: you need branch..merge to determine @{u}, just like you would need branch..push to determine @{d}. > It is a downstream branch. Which commit does git show @{d} show you? There is no ref called refs/for/master. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] branch: show me the hot branches
Duy Nguyen wrote: > I tried a more generic approach a while ago. > > http://thread.gmane.org/gmane.comp.version-control.git/188705 Looks good. Why didn't you polish it for inclusion? It's a very useful feature in my opinion: the default git branch output is quite horrible. I want to make sort, count, and verbosity (poorly chosen name?) configuration variables. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
On Thu, May 16, 2013 at 8:52 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> Does branch..merge overrides remote..fetch? No. They >> complement each other. > > I often wonder if you are reading what you're responding to: And I wonder if you care if what you say is actually true. > remote..fetch is operated on by fetch, while branch..merge > is operated on by merge; they are really orthogonal. You keep saying they are orthogonal, but they are not. Read remote.c::branch_get(). And try this: [remote "origin"] url = g...@github.com:felipec/git.git # fetch = +refs/heads/*:refs/remotes/origin/* [branch "master"] remote = origin merge = refs/heads/master Can you merge? No. Can you do master@{u}? No. Several other things don't work. >> The same that 'git pull' does when both branch..merge and >> remote..fetch are set. > > Are you reading this? > > git pull _fetches_ from remote..fetch and _merges_ from > branch..merge. What is "the same" in git push terms? It's a > simple question; which ref does push update: the one specified by > remote..push or branch..push? [remote] pushdefault = origin [remote "origin"] push = refs/head/*:refs/heads/for/* [branch "master"] pushremote = github push = refs/heads/new-master [branch "next"] pushremote = origin push = refs/heads/new-next % git checkout master && git push Where should it go? github/new-master. Obviously. % git checkout next && git push Where should it go? origin/new-next. Obviously. % git checkout main && git push Where should it go? origin/for/maint. Obviously. I don't see what the big deal is. >> Of course it would work. Does @{u} stop working when remote..fetch is >> set? > > It doesn't work when _only_ remote..fetch is set: you need > branch..merge to determine @{u}, just like you would need > branch..push to determine @{d}. So? The answer is no. >> It is a downstream branch. > > Which commit does git show @{d} show you? There is no ref called > refs/for/master. The same commit 'git show @{-1}' shows you. The fact it doesn't resolve to a commit doesn't mean @{-100} is not the nth-hundredth previous checkout. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] push: add separate 'downstream' branch
Junio C Hamano writes: > Felipe Contreras writes: > >> It doesn't make sense to push to the upstream branch, so create new >> configurations for the notion of 'downstream' branch, which is basically >> the branch to push to by default. > > It doesn't? That depends. > > To people coming from (and people who are still using) central > shared repository workflow, pushing to anywhere other than the > upstream makes no sense. > > If qualified with something like "When using a triangular workflow > to pull from one place and push to another place" in front, I can > see why having a separate upstream and downstream makes sense, and... > >> The upstream branch is remote+merge, the downstream branch is >> pushremote+push. > > ... this is a perfect explanation of what a downsream is. After thinking about it, I do not think downstream is a good word to describe where you push to at all. I'll have more on this topic but the above is the short of it for now. -- 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 v4 01/15] decorate.c: compact table when growing
When growing the table, take the opportunity to "compact" it by removing entries with NULL decoration. Users may have "removed" decorations by passing NULL to insert_decoration. An object's table entry can't actually be removed during normal operation, as it would break the linear hash collision search. But we can remove NULL decoration entries when rebuilding the table. Signed-off-by: Kevin Bracey --- decorate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decorate.c b/decorate.c index 2f8a63e..7cb5d29 100644 --- a/decorate.c +++ b/decorate.c @@ -49,7 +49,7 @@ static void grow_decoration(struct decoration *n) const struct object *base = old_hash[i].base; void *decoration = old_hash[i].decoration; - if (!base) + if (!decoration) continue; insert_decoration(n, base, decoration); } -- 1.8.3.rc0.28.g4b02ef5 -- 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 v4 09/15] t6012: update test for tweaked full-history traversal
From: Junio C Hamano Signed-off-by: Junio C Hamano --- t/t6012-rev-list-simplify.sh | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh index dd6dc84..4e55872 100755 --- a/t/t6012-rev-list-simplify.sh +++ b/t/t6012-rev-list-simplify.sh @@ -14,21 +14,24 @@ unnote () { test_expect_success setup ' echo "Hi there" >file && - git add file && - test_tick && git commit -m "Initial file" && + echo "initial" >lost && + git add file lost && + test_tick && git commit -m "Initial file and lost" && note A && git branch other-branch && echo "Hello" >file && - git add file && - test_tick && git commit -m "Modified file" && + echo "second" >lost && + git add file lost && + test_tick && git commit -m "Modified file and lost" && note B && git checkout other-branch && echo "Hello" >file && - git add file && + >lost && + git add file lost && test_tick && git commit -m "Modified the file identically" && note C && @@ -37,7 +40,9 @@ test_expect_success setup ' test_tick && git commit -m "Add another file" && note D && - test_tick && git merge -m "merge" master && + test_tick && + test_must_fail git merge -m "merge" master && + >lost && git commit -a -m "merge" && note E && echo "Yet another" >elif && @@ -110,4 +115,16 @@ check_result 'I B A' -- file check_result 'I B A' --topo-order -- file check_result 'H' --first-parent -- another-file +check_result 'E C B A' --full-history E -- lost +test_expect_success 'full history simplification without parent' ' + printf "%s\n" E C B A >expect && + git log --pretty="$FMT" --full-history E -- lost | + unnote >actual && + sed -e "s/^.* \([^ ]*\) .*/\1/" >check http://vger.kernel.org/majordomo-info.html
[PATCH v4 11/15] simplify-merges: drop merge from irrelevant side branch
Reimplement commit 4b7f53da on top of the new simplify-merges infrastructure, tightening the condition to only consider root parents; the original version incorrectly dropped parents that were TREESAME to anything. Original log message follows. The merge simplification rule stated in 6546b59 (revision traversal: show full history with merge simplification, 2008-07-31) still treated merge commits too specially. Namely, in a history with this shape: ---o---o---M / x---x---x where three 'x' were on a history completely unrelated to the main history 'o' and do not touch any of the paths we are following, we still said that after simplifying all of the parents of M, 'x' (which is the leftmost 'x' that rightmost 'x simplifies down to) and 'o' (which would be the last commit on the main history that touches the paths we are following) are independent from each other, and both need to be kept. That is incorrect; when the side branch 'x' never touches the paths, it should be removed to allow M to simplify down to the last commit on the main history that touches the paths. Suggested-by: Junio C Hamano Signed-off-by: Kevin Bracey --- Documentation/rev-list-options.txt | 34 +- revision.c | 26 +- t/t6012-rev-list-simplify.sh | 2 +- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index f41e865..b462f17 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -342,13 +342,13 @@ In the following, we will always refer to the same example history to illustrate the differences between simplification settings. We assume that you are filtering for a file `foo` in this commit graph: --- - .-A---M---N---O---P -/ / / / / - I B C D E -\ / / / / - `-' + .-A---M---N---O---P---Q +/ / / / / / + I B C D E Y +\ / / / / / + `-' X --- -The horizontal line of history A---P is taken to be the first parent of +The horizontal line of history A---Q is taken to be the first parent of each merge. The commits are: * `I` is the initial commit, in which `foo` exists with contents @@ -369,6 +369,10 @@ each merge. The commits are: * `E` changes `quux` to "xyzzy", and its merge `P` combines the strings to "quux xyzzy". `P` is TREESAME to `O`, but not to `E`. +* `X` is an indpendent root commit that added a new file `side`, and `Y` + modified it. `Y` is TREESAME to `X`. Its merge `Q` added `side` to `P`, and + `Q` is TREESAME to `P`, but not to `Y`. + 'rev-list' walks backwards through history, including or excluding commits based on whether '\--full-history' and/or parent rewriting (via '\--parents' or '\--children') are used. The following settings @@ -409,7 +413,7 @@ parent lines. the example, we get + --- - I A B N D O P + I A B N D O P Q --- + `M` was excluded because it is TREESAME to both parents. `E`, @@ -430,7 +434,7 @@ Along each parent, prune away commits that are not included themselves. This results in + --- - .-A---M---N---O---P + .-A---M---N---O---P---Q / / / / / I B / D / \ / / / / @@ -440,7 +444,7 @@ themselves. This results in Compare to '\--full-history' without rewriting above. Note that `E` was pruned away because it is TREESAME, but the parent list of P was rewritten to contain `E`'s parent `I`. The same happened for `C` and -`N`. +`N`, and `X`, `Y` and `Q`. In addition to the above settings, you can change whether TREESAME affects inclusion: @@ -470,9 +474,9 @@ history according to the following rules: * Set `C'` to `C`. + * Replace each parent `P` of `C'` with its simplification `P'`. In - the process, drop parents that are ancestors of other parents, and - remove duplicates, but take care to never drop all parents that - we are TREESAME to. + the process, drop parents that are ancestors of other parents or that are + root commits TREESAME to an empty tree, and remove duplicates, but take care + to never drop all parents that we are TREESAME to. + * If after this parent rewriting, `C'` is a root or merge commit (has zero or >1 parents), a boundary commit, or !TREESAME, it remains. @@ -490,7 +494,7 @@ The effect of this is best shown by way of comparing to `-' --
[PATCH v4 08/15] revision.c: Make --full-history consider more merges
History simplification previously always treated merges as TREESAME if they were TREESAME to any parent. While this was consistent with the default behaviour, this could be extremely unhelpful when searching detailed history, and could not be overridden. For example, if a merge had ignored a change, as if by "-s ours", then: git log -m -p --full-history -Schange file would successfully locate "change"'s addition but would not locate the merge that resolved against it. Futher, simplify_merges could drop the actual parent that a commit was TREESAME to, leaving it as a normal commit marked TREESAME that isn't actually TREESAME to its remaining parent. Now redefine a commit's TREESAME flag to be true only if a commit is TREESAME to _all_ of its parents. This doesn't affect either the default simplify_history behaviour (because partially TREESAME merges are turned into normal commits), or full-history with parent rewriting (because all merges are output). But it does affect other modes. The clearest difference is that --full-history will show more merges - sufficient to ensure that -m -p --full-history log searches can really explain every change to the file, including those changes' ultimate fate in merges. Also modify simplify_merges to recalculate TREESAME after removing a parent. This is achieved by storing per-parent TREESAME flags on the initial scan, so the combined flag can be easily recomputed. This fixes some t6111 failures, but creates a couple of new ones - we are now showing some merges that don't need to be shown. Signed-off-by: Kevin Bracey --- Documentation/rev-list-options.txt | 6 +- revision.c | 241 - revision.h | 1 + t/t6019-rev-list-ancestry-path.sh | 2 +- t/t6111-rev-list-treesame.sh | 26 ++-- 5 files changed, 229 insertions(+), 47 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 55ddf33..d166384 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -409,10 +409,10 @@ parent lines. the example, we get + --- - I A B N D O + I A B N D O P --- + -`P` and `M` were excluded because they are TREESAME to a parent. `E`, +`M` was excluded because it is TREESAME to both parents. `E`, `C` and `B` were all walked, but only `B` was !TREESAME, so the others do not appear. + @@ -440,7 +440,7 @@ themselves. This results in Compare to '\--full-history' without rewriting above. Note that `E` was pruned away because it is TREESAME, but the parent list of P was rewritten to contain `E`'s parent `I`. The same happened for `C` and -`N`. Note also that `P` was included despite being TREESAME. +`N`. In addition to the above settings, you can change whether TREESAME affects inclusion: diff --git a/revision.c b/revision.c index 7f7a8ab..64b86ae 100644 --- a/revision.c +++ b/revision.c @@ -429,10 +429,100 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit) return retval >= 0 && (tree_difference == REV_TREE_SAME); } +struct treesame_state { + unsigned int nparents; + unsigned char treesame[FLEX_ARRAY]; +}; + +static struct treesame_state *initialise_treesame(struct rev_info *revs, struct commit *commit) +{ + unsigned n = commit_list_count(commit->parents); + struct treesame_state *st = xcalloc(1, sizeof(*st) + n); + st->nparents = n; + add_decoration(&revs->treesame, &commit->object, st); + return st; +} + +/* + * Must be called immediately after removing the nth_parent from a commit's + * parent list, if we are maintaining the per-parent treesame[] decoration. + * This does not recalculate the master TREESAME flag - update_treesame() + * should be called to update it after a sequence of treesame[] modifications + * that may have affected it. + */ +static int compact_treesame(struct rev_info *revs, struct commit *commit, unsigned nth_parent) +{ + struct treesame_state *st; + int old_same; + + if (!commit->parents) { + /* +* Have just removed the only parent from a non-merge. +* Different handling, as we lack decoration. +*/ + if (nth_parent != 0) + die("compact_treesame %u", nth_parent); + old_same = !!(commit->object.flags & TREESAME); + if (rev_same_tree_as_empty(revs, commit)) + commit->object.flags |= TREESAME; + else + commit->object.flags &= ~TREESAME; + return old_same; + } + + st = lookup_decoration(&revs->treesame, &commit->object); + if (!st || nth_parent >= st->nparents) + die("compact_tre
[PATCH v4 14/15] revision.c: don't show all merges for --parents
When using --parents or --children, get_commit_action() previously showed all merges, even if TREESAME to both parents. This was intended to tie together the topology of the rewritten parents, but it was excessive - in fact we only need to show merges that have two or more relevant parents. Merges at the boundary do not necessarily need to be shown. Signed-off-by: Kevin Bracey --- revision.c | 22 +++--- t/t6111-rev-list-treesame.sh | 4 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/revision.c b/revision.c index 1c75070..edb7e1c 100644 --- a/revision.c +++ b/revision.c @@ -2760,10 +2760,7 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi if (revs->min_age != -1 && (commit->date > revs->min_age)) return commit_ignore; if (revs->min_parents || (revs->max_parents >= 0)) { - int n = 0; - struct commit_list *p; - for (p = commit->parents; p; p = p->next) - n++; + int n = commit_list_count(commit->parents); if ((n < revs->min_parents) || ((revs->max_parents >= 0) && (n > revs->max_parents))) return commit_ignore; @@ -2773,12 +2770,23 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi if (revs->prune && revs->dense) { /* Commit without changes? */ if (commit->object.flags & TREESAME) { + int n; + struct commit_list *p; /* drop merges unless we want parenthood */ if (!want_ancestry(revs)) return commit_ignore; - /* non-merge - always ignore it */ - if (!commit->parents || !commit->parents->next) - return commit_ignore; + /* +* If we want ancestry, then need to keep any merges +* between relevant commits to tie together topology. +* For consistency with TREESAME and simplification +* use "relevant" here rather than just INTERESTING, +* to treat bottom commit(s) as part of the topology. +*/ + for (n = 0, p = commit->parents; p; p = p->next) + if (relevant_commit(p->item)) + if (++n >= 2) + return commit_show; + return commit_ignore; } } return commit_show; diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh index e32b373..25cc8ad 100755 --- a/t/t6111-rev-list-treesame.sh +++ b/t/t6111-rev-list-treesame.sh @@ -139,7 +139,7 @@ check_result 'M L G' F..M --first-parent -- file # If we want history since E, then we're quite happy to ignore G that took E. check_result 'M L K J I H G' E..M --ancestry-path check_result 'M L J I H' E..M --ancestry-path -- file -check_outcome failure '(LH)M (K)L (EJ)K (I)J (E)I (E)H' E..M --ancestry-path --parents -- file # includes G +check_result '(LH)M (K)L (EJ)K (I)J (E)I (E)H' E..M --ancestry-path --parents -- file check_result '(LH)M (E)H (J)L (I)J (E)I' E..M --ancestry-path --simplify-merges -- file # Should still be able to ignore I-J branch in simple log, despite limiting @@ -168,7 +168,7 @@ check_result '(D)F (BA)D' B..F --full-history --parents -- file check_result '(B)F' B..F --simplify-merges -- file check_result 'F D' B..F --ancestry-path check_result 'F' B..F --ancestry-path -- file -check_outcome failure 'F' B..F --ancestry-path --parents -- file # includes D +check_result 'F' B..F --ancestry-path --parents -- file check_result 'F' B..F --ancestry-path --simplify-merges -- file check_result 'F D' B..F --first-parent check_result 'F' B..F --first-parent -- file -- 1.8.3.rc0.28.g4b02ef5 -- 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 v4 12/15] revision.c: add BOTTOM flag for commits
When performing edge-based operations on the revision graph, it can be useful to be able to identify the INTERESTING graph's connection(s) to the bottom commit(s) specified by the user. Conceptually when the user specifies "A..B" (== B ^A), they are asking for the history from A to B. The first connection from A onto the INTERESTING graph is part of that history, and should be considered. If we consider only INTERESTING nodes and their connections, then we're really only considering the history from A's immediate descendants to B. This patch does not change behaviour, but adds a new BOTTOM flag to indicate the bottom commits specified by the user, ready to be used by following patches. We immediately use the BOTTOM flag to return collect_bottom_commits() to its original approach of examining the pending commit list rather than the command line. This will ensure alignment of the definition of "bottom" with future patches. Signed-off-by: Kevin Bracey --- revision.c | 34 -- revision.h | 3 ++- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/revision.c b/revision.c index 4f7446c..6607dab 100644 --- a/revision.c +++ b/revision.c @@ -909,16 +909,12 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li * to filter the result of "A..B" further to the ones that can actually * reach A. */ -static struct commit_list *collect_bottom_commits(struct rev_info *revs) +static struct commit_list *collect_bottom_commits(struct commit_list *list) { - struct commit_list *bottom = NULL; - int i; - for (i = 0; i < revs->cmdline.nr; i++) { - struct rev_cmdline_entry *elem = &revs->cmdline.rev[i]; - if ((elem->flags & UNINTERESTING) && - elem->item->type == OBJ_COMMIT) - commit_list_insert((struct commit *)elem->item, &bottom); - } + struct commit_list *elem, *bottom = NULL; + for (elem = list; elem; elem = elem->next) + if (elem->item->object.flags & BOTTOM) + commit_list_insert(elem->item, &bottom); return bottom; } @@ -949,7 +945,7 @@ static int limit_list(struct rev_info *revs) struct commit_list *bottom = NULL; if (revs->ancestry_path) { - bottom = collect_bottom_commits(revs); + bottom = collect_bottom_commits(list); if (!bottom) die("--ancestry-path given but there are no bottom commits"); } @@ -1121,7 +1117,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags) const char *arg = arg_; if (*arg == '^') { - flags ^= UNINTERESTING; + flags ^= UNINTERESTING | BOTTOM; arg++; } if (get_sha1_committish(arg, sha1)) @@ -1213,8 +1209,8 @@ static void prepare_show_merge(struct rev_info *revs) add_pending_object(revs, &head->object, "HEAD"); add_pending_object(revs, &other->object, "MERGE_HEAD"); bases = get_merge_bases(head, other, 1); - add_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING); - add_pending_commit_list(revs, bases, UNINTERESTING); + add_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM); + add_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM); free_commit_list(bases); head->object.flags |= SYMMETRIC_LEFT; @@ -1250,13 +1246,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME; unsigned get_sha1_flags = 0; + flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM; + dotdot = strstr(arg, ".."); if (dotdot) { unsigned char from_sha1[20]; const char *next = dotdot + 2; const char *this = arg; int symmetric = *next == '.'; - unsigned int flags_exclude = flags ^ UNINTERESTING; + unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM); static const char head_by_default[] = "HEAD"; unsigned int a_flags; @@ -1332,13 +1330,13 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi dotdot = strstr(arg, "^!"); if (dotdot && !dotdot[2]) { *dotdot = 0; - if (!add_parents_only(revs, arg, flags ^ UNINTERESTING)) + if (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM))) *dotdot = '^'; } local_flags = 0; if (*arg == '^') { - local_flags = UNINTERESTING; + local_flags = UNINTERESTING | BOTTOM; arg++; } @@ -1815,7 +1813,7 @@ static int handle_revision_pseudo_opt(const char *submodule, handle_refs
[PATCH v4 15/15] revision.c: make default history consider bottom commits
Previously, the default history treated bottom commits the same as any other UNINTERESTING commit, which could force it down side branches. Consider the following history: *A--*B---D--*F * marks !TREESAME parent paths \ /* `-C-' When requesting "B..F", B is UNINTERESTING but TREESAME to D. C is !UNINTERESTING. So default following would go from D into the irrelevant side branch C to A, rather than to B. Note also that if there had been an extra !UNINTERESTING commit B1 between B and D, it wouldn't have gone down C. Change the default following to test relevant_commit() instead of !UNINTERESTING, so it can proceed straight from D to B, thus finishing the traversal of that path. Signed-off-by: Kevin Bracey --- revision.c | 2 +- t/t6111-rev-list-treesame.sh | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/revision.c b/revision.c index edb7e1c..914ac78 100644 --- a/revision.c +++ b/revision.c @@ -684,7 +684,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit) sha1_to_hex(p->object.sha1)); switch (rev_compare_tree(revs, p, commit)) { case REV_TREE_SAME: - if (!revs->simplify_history || (p->object.flags & UNINTERESTING)) { + if (!revs->simplify_history || !relevant_commit(p)) { /* Even if a merge with an uninteresting * side branch brought the entire change * we are interested in, we do not want diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh index 25cc8ad..88b84df 100755 --- a/t/t6111-rev-list-treesame.sh +++ b/t/t6111-rev-list-treesame.sh @@ -146,8 +146,8 @@ check_result '(LH)M (E)H (J)L (I)J (E)I' E..M --ancestry-path --simplify-merges # to G. check_result 'M L K J I H' G..M check_result 'M H L K J I' G..M --topo-order -check_outcome failure 'M L H' G..M -- file # includes J I -check_outcome failure '(LH)M (G)L (G)H' G..M --parents -- file # includes J I +check_result 'M L H' G..M -- file +check_result '(LH)M (G)L (G)H' G..M --parents -- file check_result 'M L J I H' G..M --full-history -- file check_result 'M L K J I H' G..M --full-history --parents -- file check_result 'M H L J I' G..M --simplify-merges -- file @@ -161,8 +161,8 @@ check_result 'M H L J I' G..M --ancestry-path --simplify-merges -- file # But --full-history shouldn't drop D on its own - without simplification, # we can't decide if the merge from INTERESTING commit C was sensible. check_result 'F D C' B..F -check_outcome failure 'F' B..F -- file # includes D -check_outcome failure '(B)F' B..F --parents -- file # includes D +check_result 'F' B..F -- file +check_result '(B)F' B..F --parents -- file check_result 'F D' B..F --full-history -- file check_result '(D)F (BA)D' B..F --full-history --parents -- file check_result '(B)F' B..F --simplify-merges -- file @@ -174,8 +174,8 @@ check_result 'F D' B..F --first-parent check_result 'F' B..F --first-parent -- file # E...F should be equivalent to E F ^B, and be able to drop D as above. -check_outcome failure 'F' E F ^B -- file # includes D -check_outcome failure 'F' E...F -- file # includes D +check_result 'F' E F ^B -- file # includes D +check_result 'F' E...F -- file # includes D # Any sort of full history of C..F should show D, as it's the connection to C, # and it differs from it. -- 1.8.3.rc0.28.g4b02ef5 -- 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 v4 03/15] t6111: new TREESAME test set
Some side branching and odd merging to illustrate various flaws in revision list scans, particularly when limiting the list. Many expected failures, which will be gone by the end of the "history traversal refinements" series. Signed-off-by: Kevin Bracey --- t/t6111-rev-list-treesame.sh | 184 +++ 1 file changed, 184 insertions(+) create mode 100755 t/t6111-rev-list-treesame.sh diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh new file mode 100755 index 000..b2bca77 --- /dev/null +++ b/t/t6111-rev-list-treesame.sh @@ -0,0 +1,184 @@ +#!/bin/sh +# +#,---E--. *H--. * marks !TREESAME parent paths +# /\ / \* +# *A--*B---D--*F-*G-K-*L-*M +# \ /* \ / +#`-C-' `-*I-*J +# +# A creates "file", B and F change it. +# Odd merge G takes the old version from B. +# I changes it, but J reverts it, so K is TREESAME to both parents. +# H and L both change "file", and M merges those changes. + +test_description='TREESAME and limiting' + +. ./test-lib.sh + +note () { + git tag "$1" +} + +unnote () { + git name-rev --tags --stdin | sed -e "s|$_x40 (tags/\([^)]*\)) |\1 |g" +} + +test_expect_success setup ' + test_commit "Initial file" file "Hi there" A && + git branch other-branch && + + test_commit "file=Hello" file "Hello" B && + git branch third-branch && + + git checkout other-branch && + test_commit "Added other" other "Hello" C && + + git checkout master && + test_merge D other-branch && + + git checkout third-branch && + test_commit "Third file" third "Nothing" E && + + git checkout master && + test_commit "file=Blah" file "Blah" F && + + test_tick && git merge --no-commit third-branch && + git checkout third-branch file && + git commit && + note G && + git branch fiddler-branch && + + git checkout -b part2-branch && + test_commit "file=Part 2" file "Part 2" H && + + git checkout fiddler-branch && + test_commit "Bad commit" file "Silly" I && + + test_tick && git revert I && note J && + + git checkout master && + test_tick && git merge --no-ff fiddler-branch && + note K + + test_commit "file=Part 1" file "Part 1" L && + + test_tick && test_must_fail git merge part2-branch && + test_commit M file "Parts 1+2" +' + +FMT='tformat:%P%H | %s' + +# could we soup this up to optionally check parents? So "(BA)C" would check +# that C is shown and has parents B A. +check_outcome () { + outcome=$1 + shift + for c in $1 + do + echo "$c" + done >expect && + shift && + param="$*" && + test_expect_$outcome "log $param" ' + git log --format="$FMT" $param | + unnote >actual && + sed -e "s/^.* \([^ ]*\) .*/\1/" >check http://vger.kernel.org/majordomo-info.html
[PATCH v4 13/15] revision.c: discount side branches when computing TREESAME
Use the BOTTOM flag to define relevance for pruning. Relevant commits are those that are !UNINTERESTING or BOTTOM, and this allows us to identify irrelevant side branches (UNINTERESTING && !BOTTOM). If a merge has relevant parents, and it is TREESAME to them, then do not let irrelevant parents cause the merge to be treated as !TREESAME. When considering simplification, don't always include all merges - merges with exactly one relevant parent can be simplified, if TREESAME according to the above rule. These two changes greatly increase simplification in limited, pruned revision lists. Signed-off-by: Kevin Bracey --- revision.c| 171 +- t/t6019-rev-list-ancestry-path.sh | 12 ++- t/t6111-rev-list-treesame.sh | 8 +- 3 files changed, 164 insertions(+), 27 deletions(-) diff --git a/revision.c b/revision.c index 6607dab..1c75070 100644 --- a/revision.c +++ b/revision.c @@ -333,6 +333,80 @@ static int everybody_uninteresting(struct commit_list *orig) } /* + * A definition of "relevant" commit that we can use to simplify limited graphs + * by eliminating side branches. + * + * A "relevant" commit is one that is !UNINTERESTING (ie we are including it + * in our list), or that is a specified BOTTOM commit. Then after computing + * a limited list, during processing we can generally ignore boundary merges + * coming from outside the graph, (ie from irrelevant parents), and treat + * those merges as if they were single-parent. TREESAME is defined to consider + * only relevant parents, if any. If we are TREESAME to our on-graph parents, + * we don't care if we were !TREESAME to non-graph parents. + * + * Treating bottom commits as relevant ensures that a limited graph's + * connection to the actual bottom commit is not viewed as a side branch, but + * treated as part of the graph. For example: + * + * Z...A---X---o---o---B + *. / + * W---Y + * + * When computing "A..B", the A-X connection is at least as important as + * Y-X, despite A being flagged UNINTERESTING. + * + * And when computing --ancestry-path "A..B", the A-X connection is more + * important than Y-X, despite both A and Y being flagged UNINTERESTING. + */ +static inline int relevant_commit(struct commit *commit) +{ + return (commit->object.flags & (UNINTERESTING | BOTTOM)) != UNINTERESTING; +} + +/* + * Return a single relevant commit from a parent list. If we are a TREESAME + * commit, and this selects one of our parents, then we can safely simplify to + * that parent. + */ +static struct commit *one_relevant_parent(const struct rev_info *revs, + struct commit_list *orig) +{ + struct commit_list *list = orig; + struct commit *relevant = NULL; + + if (!orig) + return NULL; + + /* +* For 1-parent commits, or if first-parent-only, then return that +* first parent (even if not "relevant" by the above definition). +* TREESAME will have been set purely on that parent. +*/ + if (revs->first_parent_only || !orig->next) + return orig->item; + + /* +* For multi-parent commits, identify a sole relevant parent, if any. +* If we have only one relevant parent, then TREESAME will be set purely +* with regard to that parent, and we can simplify accordingly. +* +* If we have more than one relevant parent, or no relevant parents +* (and multiple irrelevant ones), then we can't select a parent here +* and return NULL. +*/ + while (list) { + struct commit *commit = list->item; + list = list->next; + if (relevant_commit(commit)) { + if (relevant) + return NULL; + relevant = commit; + } + } + return relevant; +} + +/* * The goal is to get REV_TREE_NEW as the result only if the * diff consists of all '+' (and no other changes), REV_TREE_OLD * if the whole diff is removal of old data, and otherwise @@ -502,27 +576,52 @@ static unsigned update_treesame(struct rev_info *revs, struct commit *commit) if (commit->parents && commit->parents->next) { unsigned n; struct treesame_state *st; + struct commit_list *p; + unsigned relevant_parents; + unsigned relevant_change, irrelevant_change; st = lookup_decoration(&revs->treesame, &commit->object); if (!st) die("update_treesame %s", sha1_to_hex(commit->object.sha1)); - commit->object.flags |= TREESAME; - for (n = 0; n < st->nparents; n++) { - if (!st->treesame[n]) { - commit->object.flags &= ~TREESAME; - break; -
[PATCH v4 10/15] simplify-merges: never remove all TREESAME parents
When simplifying an odd merge, such as one that used "-s ours", we may find ourselves TREESAME to apparently redundant parents. Prevent simplify_merges() from removing every TREESAME parent; if this would happen reinstate the first TREESAME parent - the one that the default log would have followed. This avoids producing a totally disjoint history from the default log when the default log is a better explanation of the end result, and aids visualisation of odd merges. Signed-off-by: Kevin Bracey --- Documentation/rev-list-options.txt | 3 +- revision.c | 69 ++ t/t6111-rev-list-treesame.sh | 4 +-- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index d166384..f41e865 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -471,7 +471,8 @@ history according to the following rules: + * Replace each parent `P` of `C'` with its simplification `P'`. In the process, drop parents that are ancestors of other parents, and - remove duplicates. + remove duplicates, but take care to never drop all parents that + we are TREESAME to. + * If after this parent rewriting, `C'` is a root or merge commit (has zero or >1 parents), a boundary commit, or !TREESAME, it remains. diff --git a/revision.c b/revision.c index 64b86ae..62f399c 100644 --- a/revision.c +++ b/revision.c @@ -2136,6 +2136,73 @@ static int mark_redundant_parents(struct rev_info *revs, struct commit *commit) return marked; } +/* + * Awkward naming - this means one parent we are TREESAME to. + * cf mark_treesame_root_parents: root parents that are TREESAME (to an + * empty tree). Better name suggestions? + */ +static int leave_one_treesame_to_parent(struct rev_info *revs, struct commit *commit) +{ + struct treesame_state *ts = lookup_decoration(&revs->treesame, &commit->object); + struct commit *unmarked = NULL, *marked = NULL; + struct commit_list *p; + unsigned n; + + for (p = commit->parents, n = 0; p; p = p->next, n++) { + if (ts->treesame[n]) { + if (p->item->object.flags & TMP_MARK) { + if (!marked) + marked = p->item; + } else { + if (!unmarked) { + unmarked = p->item; + break; + } + } + } + } + + /* +* If we are TREESAME to a marked-for-deletion parent, but not to any +* unmarked parents, unmark the first TREESAME parent. This is the +* parent that the default simplify_history==1 scan would have followed, +* and it doesn't make sense to omit that path when asking for a +* simplified full history. Retaining it improves the chances of +* understanding odd missed merges that took an old version of a file. +* +* Example: +* +* I*X A modified the file, but mainline merge X used +*\ /"-s ours", so took the version from I. X is +* `-*A--' TREESAME to I and !TREESAME to A. +* +* Default log from X would produce "I". Without this check, +* --full-history --simplify-merges would produce "I-A-X", showing +* the merge commit X and that it changed A, but not making clear that +* it had just taken the I version. With this check, the topology above +* is retained. +* +* Note that it is possible that the simplification chooses a different +* TREESAME parent from the default, in which case this test doesn't +* activate, and we _do_ drop the default parent. Example: +* +* I--X A modified the file, but it was reverted in B, +*\/ meaning mainline merge X is TREESAME to both +**A-*B parents. +* +* Default log would produce "I" by following the first parent; +* --full-history --simplify-merges will produce "I-A-B". But this is a +* reasonable result - it presents a logical full history leading from +* I to X, and X is not an important merge. +*/ + if (!unmarked && marked) { + marked->object.flags &= ~TMP_MARK; + return 1; + } + + return 0; +} + static int remove_marked_parents(struct rev_info *revs, struct commit *commit) { struct commit_list **pp, *p; @@ -2239,6 +2306,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c if (1 < cnt) { int marked = mark_redundant_parents(revs, commit); if (marked) + marked -= leave_one_treesame_to
[PATCH v4 06/15] rev-list-options.txt: correct TREESAME for P
In the example given, P is not TREESAME to E. This doesn't affect the current result, but it will matter when we change behaviour. Signed-off-by: Kevin Bracey --- Documentation/rev-list-options.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 3bdbf5e..50bbff7 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -367,8 +367,7 @@ each merge. The commits are: `N` and `D` to "foobarbaz"; i.e., it is not TREESAME to any parent. * `E` changes `quux` to "xyzzy", and its merge `P` combines the - strings to "quux xyzzy". Despite appearing interesting, `P` is - TREESAME to all parents. + strings to "quux xyzzy". `P` is TREESAME to `O`, but not to `E`. 'rev-list' walks backwards through history, including or excluding commits based on whether '\--full-history' and/or parent rewriting -- 1.8.3.rc0.28.g4b02ef5 -- 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 v4 07/15] Documentation: avoid "uninteresting"
The documentation of --boundary uses the term "uninteresting", which is not used or defined anywhere else in the documentation. This is unhelpful and confusing to anyone who hasn't seen the UNINTERESTING flag in the source code. Change to use "excluded", as per revisions.txt. Signed-off-by: Kevin Bracey --- Documentation/rev-list-options.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 50bbff7..55ddf33 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -271,8 +271,8 @@ See also linkgit:git-reflog[1]. --boundary:: - Output uninteresting commits at the boundary, which are usually - not shown. + Output excluded boundary commits. Boundary commits are + prefixed with `-`. -- -- 1.8.3.rc0.28.g4b02ef5 -- 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 v4 00/15] History traversal refinements
No new functionality or bug fixes since v3, just tidying: * Tests now use Junio's parent-checking functionality * BOTTOM flags now set in a neater fashion (I think), separating it out from the cmdline stuff. * Creation and use of BOTTOM flag now split into 4 separate commits - last version was too much for one commit, I feel. * Finally decided that "relevant" is the word I was looking for. Obvious, really. * On the subject of words, remove the only technical use of "uninteresting" that I found in the Documentation - I know that it always confused me until I read the source. This sequence is based on my 2-commit ancestry-path "..." series, but no longer depends on it due to the new way the BOTTOM flag is initialised. But they both touch the t6019 test, so applying this on top will avoid conflicts. Junio C Hamano (2): t6111: allow checking the parents as well t6012: update test for tweaked full-history traversal Kevin Bracey (13): decorate.c: compact table when growing t6019: test file dropped in -s ours merge t6111: new TREESAME test set t6111: add parents to tests rev-list-options.txt: correct TREESAME for P Documentation: avoid "uninteresting" revision.c: Make --full-history consider more merges simplify-merges: never remove all TREESAME parents simplify-merges: drop merge from irrelevant side branch revision.c: add BOTTOM flag for commits revision.c: discount side branches when computing TREESAME revision.c: don't show all merges for --parents revision.c: make default history consider bottom commits Documentation/rev-list-options.txt | 42 +-- decorate.c | 2 +- revision.c | 539 - revision.h | 4 +- t/t6012-rev-list-simplify.sh | 31 ++- t/t6019-rev-list-ancestry-path.sh | 27 ++ t/t6111-rev-list-treesame.sh | 196 ++ 7 files changed, 750 insertions(+), 91 deletions(-) create mode 100755 t/t6111-rev-list-treesame.sh -- 1.8.3.rc0.28.g4b02ef5 -- 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