[PATCH v5 3/4] status: give more information during rebase -i
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr git status gives more information during rebase -i, about the list of command that are done during the rebase. It displays the two last commands executed and the two next lines to be executed. It also gives hints to find the whole files in .git directory. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- Added sha1-shortening code. t/t7512-status-help.sh | 111 + wt-status.c| 109 2 files changed, 220 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 190656d..9be0235 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -134,9 +134,13 @@ test_expect_success 'prepare for rebase_i_conflicts' ' test_expect_success 'status during rebase -i when conflicts unresolved' ' test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse --short rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -159,10 +163,14 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' git reset --hard rebase_i_conflicts_second test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse --short rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -183,7 +191,9 @@ test_expect_success 'status when rebasing -i in edit mode' ' git checkout -b rebase_i_edit test_commit one_rebase_i main.txt one test_commit two_rebase_i main.txt two + COMMIT2=$(git rev-parse --short rebase_i_edit) test_commit three_rebase_i main.txt three + COMMIT3=$(git rev-parse --short rebase_i_edit) FAKE_LINES=1 edit 2 export FAKE_LINES test_when_finished git rebase --abort @@ -191,6 +201,10 @@ test_expect_success 'status when rebasing -i in edit mode' ' git rebase -i HEAD~2 cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_rebase_i + edit $COMMIT3 three_rebase_i +No commands remaining. You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -207,8 +221,11 @@ test_expect_success 'status when splitting a commit' ' git checkout -b split_commit test_commit one_split main.txt one test_commit two_split main.txt two + COMMIT2=$(git rev-parse --short split_commit) test_commit three_split main.txt three + COMMIT3=$(git rev-parse --short split_commit) test_commit four_split main.txt four + COMMIT4=$(git rev-parse --short split_commit) FAKE_LINES=1 edit 2 3 export FAKE_LINES test_when_finished git rebase --abort @@ -217,6 +234,12 @@ test_expect_success 'status when splitting a commit' ' git reset HEAD^ cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_split + edit $COMMIT3 three_split +Next command to do (1 remaining command): + pick $COMMIT4 four_split + (use git rebase --edit-todo to view and edit) You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -239,7 +262,9 @@ test_expect_success 'status after editing the last commit with --amend during a test_commit one_amend main.txt one test_commit two_amend main.txt two test_commit three_amend main.txt three + COMMIT3=$(git rev-parse --short amend_last) test_commit four_amend main.txt four + COMMIT4=$(git rev-parse --short amend_last) FAKE_LINES=1 2 edit 3 export FAKE_LINES test_when_finished git rebase --abort @@ -248,6 +273,11 @@ test_expect_success
[PATCH v5 4/4] status: add new tests for status during rebase -i
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Expand test coverage with one or more than two commands done and with zero, one or more than two commands remaining. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- Added --short where appropriate. t/t7512-status-help.sh | 87 ++ 1 file changed, 87 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 9be0235..49d19a3 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -856,4 +856,91 @@ EOF test_i18ncmp expected actual ' +test_expect_success 'prepare for different number of commits rebased' ' + git reset --hard master + git checkout -b several_commits + test_commit one_commit main.txt one + test_commit two_commit main.txt two + test_commit three_commit main.txt three + test_commit four_commit main.txt four +' + +test_expect_success 'status: one command done nothing remaining' ' + FAKE_LINES=exec_exit_15 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last command done (1 command done): + exec exit 15 +No commands remaining. +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two commands done with some white lines in done file' ' + FAKE_LINES=1 exec_exit_15 2 3 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + COMMIT4=$(git rev-parse --short HEAD) + COMMIT3=$(git rev-parse --short HEAD^) + COMMIT2=$(git rev-parse --short HEAD^^) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_commit + exec exit 15 +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two remaining commands with some white lines in todo file' ' + FAKE_LINES=1 2 exec_exit_15 3 4 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~4) + COMMIT4=$(git rev-parse --short HEAD) + COMMIT3=$(git rev-parse --short HEAD^) + COMMIT2=$(git rev-parse --short HEAD^^) + test_must_fail git rebase -i HEAD~4 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (3 commands done): + pick $COMMIT2 two_commit + exec exit 15 + (see more in file .git/rebase-merge/done) +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + test_done -- 2.5.0.rc0.7.ge1edd74 -- To unsubscribe from this list: send the line 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 v5 2/4] status: differentiate interactive from non-interactive rebases
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- No change since v4. t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 68ad2d7..190656d 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' ' ONTO=$(git rev-parse --short rebase_i_conflicts) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -190,7 +190,7 @@ test_expect_success 'status when rebasing -i in edit mode' ' ONTO=$(git rev-parse --short HEAD~2) git rebase -i HEAD~2 cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -216,7 +216,7 @@ test_expect_success 'status when splitting a commit' ' git rebase -i HEAD~3 git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -247,7 +247,7 @@ test_expect_success 'status after editing the last commit with --amend during a git rebase -i HEAD~3 git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -277,7 +277,7 @@ test_expect_success 'status: (continue first edit) second edit' ' git rebase -i HEAD~3 git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -299,7 +299,7 @@ test_expect_success 'status: (continue first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -326,7 +326,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' ' git rebase --continue git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -348,7 +348,7 @@ test_expect_success 'status: (amend first edit) second edit' ' git commit --amend -m a git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -371,7 +371,7 @@ test_expect_success 'status: (amend first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently
[PATCH v5 1/4] status: factor two rebase-related messages together
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- No change since v4. wt-status.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/wt-status.c b/wt-status.c index eaed4fe..8c4b806 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1027,6 +1027,20 @@ static int split_commit_in_progress(struct wt_status *s) return split_in_progress; } +static void print_rebase_state(struct wt_status *s, + struct wt_status_state *state, + const char *color) +{ + if (state-branch) + status_printf_ln(s, color, +_(You are currently rebasing branch '%s' on '%s'.), +state-branch, +state-onto); + else + status_printf_ln(s, color, +_(You are currently rebasing.)); +} + static void show_rebase_in_progress(struct wt_status *s, struct wt_status_state *state, const char *color) @@ -1034,14 +1048,7 @@ static void show_rebase_in_progress(struct wt_status *s, struct stat st; if (has_unmerged(s)) { - if (state-branch) - status_printf_ln(s, color, -_(You are currently rebasing branch '%s' on '%s'.), -state-branch, -state-onto); - else - status_printf_ln(s, color, -_(You are currently rebasing.)); + print_rebase_state(s, state, color); if (s-hints) { status_printf_ln(s, color, _( (fix conflicts and then run \git rebase --continue\))); @@ -1051,14 +1058,7 @@ static void show_rebase_in_progress(struct wt_status *s, _( (use \git rebase --abort\ to check out the original branch))); } } else if (state-rebase_in_progress || !stat(git_path(MERGE_MSG), st)) { - if (state-branch) - status_printf_ln(s, color, -_(You are currently rebasing branch '%s' on '%s'.), -state-branch, -state-onto); - else - status_printf_ln(s, color, -_(You are currently rebasing.)); + print_rebase_state(s, state, color); if (s-hints) status_printf_ln(s, color, _( (all conflicts fixed: run \git rebase --continue\))); -- 2.5.0.rc0.7.ge1edd74 -- To unsubscribe from this list: send the line 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 v7 07/10] send-email: reduce dependencies impact on parse_address_line
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: I'd vote for keeping it simple and not having the copyright notice. Most t/*.sh do not have one. The Git history + mailing-list archives are much better than in-code comments to keep track of who wrote what. Remi: any objection on removing it? Sorry for not having resent the patches myself, I currently have no Internet access, neither at work nor at home... Here's a try on my phone: I though the copyright line was necessary, but I did not know what to write after, and I forgot to ask, so I'm really happy with simply removing it. :) OK, so Junio, you can just remove it. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 40/44] builtin-am: support and auto-detect mercurial patches
On Tue, Jun 30, 2015 at 4:32 AM, Stefan Beller sbel...@google.com wrote: for calculating the minutes we would only need to take % 3600 (which you do), but then we still need to divide by 60 to convert seconds to minutes? Whoops, yes we do. It should be: tz = tz / 3600 * 100 + tz % 3600 / 60; tz = -tz; However... What happens if we have a negative input not matching a full hour, say -5400 ? (would equate to 0130 in git) Hmm, I assumed that in C, integer division would always truncate to zero, but turns out that is only so in C99. In C89, it is implementation defined whether it rounds towards zero or towards negative infinity. So, if the compiler rounds towards negative infinity, then the above will give the incorrect result. The best solution is probably to ensure that all the integers are positive: tz2 = labs(tz) / 3600 * 100 + labs(tz) % 3600 / 60; if (tz 0) tz2 = -tz2; Thanks for bringing this up. Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] More helpful 'git status' during 'rebase -i'
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: This series makes git status provide an output like interactive rebase in progress; onto $ONTO Last commands done (2 commands done): pick $COMMIT2 two_commit exec exit 15 Next commands to do (2 remaining commands): pick $COMMIT3 three_commit pick $COMMIT4 four_commit (use git rebase --edit-todo to view and edit) in addition to the existing output, when ran during an interactive rebase. I'd prefer to see these $COMMITn abbreviated, just like $ONTO. Indeed. It's not as easy as it would seem because we're in wt-status.c and can't call a shell function like collapse_todo_ids directly, but I've re-implemented it in C, it's not that bad. Patch follows. The first two patches are unchanged. wt-status.c now abbreviates the sha1, and the tests are adapted (rev-parse - rev-parse --short) where needed in patch 3 and 4. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 40/44] builtin-am: support and auto-detect mercurial patches
On Tue, Jun 30, 2015 at 4:32 AM, Stefan Beller sbel...@google.com wrote: On Mon, Jun 29, 2015 at 1:32 PM, Stefan Beller sbel...@google.com wrote: On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan pyoka...@gmail.com wrote: + tz = tz / (60 * 60) * 100 + tz % (60 * 60); What happens if we have a negative input not matching a full hour, say -5400 ? (would equate to 0130 in git) for calculating the minutes we would only need to take % 3600 (which you do), but then we still need to divide by 60 to convert seconds to minutes? That said, I wonder if we have some helper functions around somewhere as we need to convert the timezone data at many places. We convert timezone data at many places? Hmm, I thought all the time related stuff was confined to date.c. As far as I know from browsing around the source code, Git keeps the timezone in the 24-hour format at all times, so there isn't a helper function for converting from mercurial's seconds format to git's 24-hour format. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] pack-protocol.txt: Be more precise about pusher-key relationship
The use of must (albeit not in all caps) suggests that this is actually a requirement of the protocol. As no implementation exists that actually does this verification, this is misleading at best. Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index de3c72c..f37dcf1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -564,7 +564,8 @@ Currently, the following header fields are defined: The GPG signature lines are a detached signature for the contents recorded in the push certificate before the signature block begins. The detached signature is used to certify that the commands were -given by the pusher, who must be the signer. +given by the pusher, which verifier code SHOULD enforce is a valid User +ID associated with the signer. Report Status - -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark pushee field as optional
send-pack.c omits this field when args-url is null or empty. Fix the protocol specification to match reality. Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index f37dcf1..98e512d 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -495,7 +495,7 @@ references. push-cert = PKT-LINE(push-cert NUL capability-list LF) PKT-LINE(certificate version 0.1 LF) PKT-LINE(pusher SP push-cert-ident LF) - PKT-LINE(pushee SP url LF) + [PKT-LINE(pushee SP url LF)] PKT-LINE(nonce SP nonce LF) PKT-LINE(LF) *PKT-LINE(command LF) @@ -554,7 +554,8 @@ Currently, the following header fields are defined: `pushee` url:: The repository URL (anonymized, if the URL contains authentication material) the user who ran `git push` - intended to push into. + intended to push into. This field is optional, and included at + the client's discretion. `nonce` nonce:: The 'nonce' string the receiving repository asked the -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Add warning about protocol inaccuracies
We want to fix such inaccuracies, but there are a lot and there is no guarantee at any particular point in time that this document will be error-free. Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 4064fc7..66d2d95 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -14,6 +14,17 @@ data. The protocol functions to have a server tell a client what is currently on the server, then for the two to negotiate the smallest amount of data to send in order to fully update one or the other. +Notes to Implementors +- + +WARNING: This document is a work in progress. Some of the protocol +specifications below may be incomplete or inaccurate. When in doubt, +refer to the C code. + +One particular example is that many of the LFs referenced in the +specifications are optional, but may (yet) not be marked as such. If not +explicitly marked one way or the other, double-check with the C code. + Transports -- There are three transports over which the packfile protocol is -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark LF in command-list as optional
Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 66d2d95..1386840 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -481,7 +481,7 @@ references. shallow = PKT-LINE(shallow SP obj-id LF) command-list = PKT-LINE(command NUL capability-list LF) - *PKT-LINE(command LF) + *PKT-LINE(command LF?) flush-pkt command = create / delete / update -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line 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/7] Clarify signed push protocol documentation
The signed push protocol documentation did not really match the reality of what send-pack.c and receive-pack.c do, much to my chagrin as I attempted to implement this protocol in JGit. This series covers most of the issues I found on a first pass. Dave Borowitz (7): pack-protocol.txt: Add warning about protocol inaccuracies pack-protocol.txt: Mark LF in command-list as optional pack-protocol.txt: Mark all LFs in push-cert as required pack-protocol.txt: Elaborate on pusher identity pack-protocol.txt: Be more precise about pusher-key relationship pack-protocol.txt: Mark pushee field as optional send-pack.c: Die if the nonce is empty Documentation/technical/pack-protocol.txt | 38 +-- send-pack.c | 2 ++ 2 files changed, 33 insertions(+), 7 deletions(-) -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark LF in command-list as optional
On Wed, Jul 1, 2015 at 11:08 AM, Dave Borowitz dborow...@google.com wrote: Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 66d2d95..1386840 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -481,7 +481,7 @@ references. shallow = PKT-LINE(shallow SP obj-id LF) command-list = PKT-LINE(command NUL capability-list LF) We may also want to mark it in this line above as well as in the shallow line? I think the problem with this part of the documentation is its ambiguity on what exactly we want to document. The sender SHOULD put an LF, while the receiver MUST NOT assume the LF is there always, so I guess it's best to mark it optional from a receivers point of view. - *PKT-LINE(command LF) + *PKT-LINE(command LF?) flush-pkt command = create / delete / update -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line 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] rev-list: disable --use-bitmap-index when pruning commits
On Wed, Jul 01, 2015 at 08:19:40AM -0700, Junio C Hamano wrote: Sounds good. While at it, perhaps add a mention (perhaps by creating a BUGS section at the end of the file) that --count with --use-bitmap-index ignores pathspec silently? I think we can just fix it rather than documenting the problem. :) -- 8 -- Subject: rev-list: disable --use-bitmap-index when pruning commits The reachability bitmaps do not have enough information to tell us which commits might have changed path foo, so the current code produces wrong answers for: git rev-list --use-bitmap-index --count HEAD -- foo (it silently ignores the foo limiter). Instead, we should fall back to doing a normal traversal (it is OK to fall back rather than complain, because --use-bitmap-index is a pure optimization, and might not kick in for other reasons, such as there being no bitmaps in the repository). Signed-off-by: Jeff King p...@peff.net --- builtin/rev-list.c | 2 +- t/t5310-pack-bitmaps.sh | 6 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index ff84a82..88eddbd 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -355,7 +355,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) if (bisect_list) revs.limited = 1; - if (use_bitmap_index) { + if (use_bitmap_index !revs.prune) { if (revs.count !revs.left_right !revs.cherry_mark) { uint32_t commit_count; if (!prepare_bitmap_walk(revs)) { diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 6003490..d446706 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -53,6 +53,12 @@ rev_list_tests() { test_cmp expect actual ' + test_expect_success counting commits with limiting ($state) ' + git rev-list --count HEAD -- 1.t expect + git rev-list --use-bitmap-index --count HEAD -- 1.t actual + test_cmp expect actual + ' + test_expect_success enumerate --objects ($state) ' git rev-list --objects --use-bitmap-index HEAD tmp cut -d -f1 tmp tmp2 -- 2.5.0.rc0.336.g8460790 -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark LF in command-list as optional
On Wed, Jul 1, 2015 at 11:21 AM, Stefan Beller sbel...@google.com wrote: I think the problem with this part of the documentation is its ambiguity on what exactly we want to document. The sender SHOULD put an LF, while the receiver MUST NOT assume the LF is there always, so I guess it's best to mark it optional from a receivers point of view. To be clear, this patch is a partial fix to one particular spec in the file. See 1/7 for the general warning not to trust these. Auditing the file completely was not the goal of this series. -- To unsubscribe from this list: send the line 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] worktree: replace checkout --to with worktree new
Eric Sunshine sunsh...@sunshineco.com writes: On Wed, Jul 1, 2015 at 1:13 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jul 1, 2015 at 12:53 PM, Junio C Hamano gits...@pobox.com wrote: * Duy seems to think worktree add is coming, too, so here is a trivial tweak of your patch from the last month, with test adjustments to 7410 I sent earlier squashed in. Thanks. I was planning on re-rolling to use the new name (add rather than new) and to squash in the t7410 fix. Plus, I think the changes I had to make to prepare_linked_checkout() in order to move it to worktree.c should become separate preparatory patches so that the actual relocation can become pure code movement (as much as possible). Also, I was planning on including Duy's patch in the re-roll since it missed a s/prune --worktrees/worktree prune/ in Documentation/git-checkout.txt. Hmm, I see that Duy's patch is already in 'next'. Would it be better if I fixed the 's/prune --worktrees/worktree prune/' git-checkout.txt oversight via an incremental patch rather than including a corrected version of his patch with my re-roll? I may have mistaken what you said; I thought you were planning an incremental for the existing part, with a more complete reroll of worktree add than what I sent today, as separate patches. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] status: give more information during rebase -i
On Wed, Jul 1, 2015 at 12:36 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: I was about to mention the same shortcoming, but you beat me to it. Perhaps be more strict and do this instead (without leading strbuf_trim): if (!get_sha1_hex(split[1]-buf, sha1) !strcmp(split[1]-buf + 40, ) { replace split[1] with %s abbrev } Isn't it dangerous to assume that you can index 40 characters into split[1]? If (for some reason), the user botched the todo line such that the SHA1 is no longer a valid hex string, then split[1] will be that botched string, which might be shorter than 40 characters. For instance, if the user-edited todo line is: pick oops nothing then git-rebase--interactive.sh:transform_todo_ids() will leave oops as-is, since it can't unabbreviate it, and then this code will place oops into split[1]. Yeah, that is why get_sha1_hex() is checked before we try to make sure buf[40] has in the code snippet you quoted. Isn't that how short-cut works? Argh, I misread the code. Sorry for the noise. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] status: give more information during rebase -i
Matthieu Moy matthieu@imag.fr writes: +/* + * Turn + * pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message + * into + * pick d6a2f03 some message + */ +static void abbrev_sha1_in_line(struct strbuf *line) +{ + struct strbuf **split; + int i; + + if (starts_with(line-buf, exec ) || + starts_with(line-buf, x )) + return; + + split = strbuf_split_max(line, ' ', 3); + if (split[0] split[1]) { + unsigned char sha1[20]; + const char *abbrev; + + /* + * strbuf_split_max left a space. Trim it and re-add + * it after abbreviation. + */ + strbuf_trim(split[1]); + if (!get_sha1(split[1]-buf, sha1)) { + abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV); + strbuf_reset(split[1]); + strbuf_addf(split[1], %s , abbrev); + } ... else? That is, we thought there would be a full SHA-1, but it turns out that there wasn't, so we keep split[1] as-is would need to add the space back, no? Perhaps be more strict and do this instead (without leading strbuf_trim): if (!get_sha1_hex(split[1]-buf, sha1) !strcmp(split[1]-buf + 40, ) { replace split[1] with %s abbrev } + strbuf_reset(line); + for (i = 0; split[i]; i++) + strbuf_addstr(line, split[i]-buf); + } + for (i = 0; split[i]; i++) + strbuf_release(split[i]); + +} + +static void read_rebase_todolist(const char *fname, struct string_list *lines) +{ + struct strbuf line = STRBUF_INIT; + FILE *f = fopen(git_path(fname), r); + + if (!f) + die_errno(Could not open file %s for reading, git_path(fname)); + while (!strbuf_getline(line, f, '\n')) { + stripspace(line, 1); stripspace() is meant to be used for multi-line input (e.g. it collapses a multi-line paragraph break into one blank line) and it does not look a good fit in a loop that goes line-by-line. As you call (and you have to, because stripspace() fixes the incomplete line by adding LF at the end) rtrim() immediately afterward, this call is done only for removing comments (in other words, trailing whitespaces are removed without the call to stripspace() anyway). + /* Remove trailing \n */ + strbuf_rtrim(line); + abbrev_sha1_in_line(line); + string_list_append(lines, line.buf); + } + string_list_remove_empty_items(lines, 1); +} Perhaps while (!strbuf_getline(line, f, '\n')) { if (line.len line.len[0] == comment_line_char) continue; strbuf_rtrim(line); if (!line.len) continue; abbrev_sha1_in_line(line); string_list_append(lines, line.buf); } without the we may have added cruft, so discard them at the end? Other than these two minor nits, I didn't spot anything questionable in the diff between this and the previous round. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Add tests for wildcard path vs ref disambiguation
Thanks, will queue. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 31/44] builtin-am: implement -S/--gpg-sign, commit.gpgsign
On Wed, Jul 1, 2015 at 1:01 AM, Paul Tan pyoka...@gmail.com wrote: On Tue, Jun 30, 2015 at 7:51 AM, Stefan Beller sbel...@google.com wrote: I needed to read this patch a few times as this patch introduces `sign_commit` twice. This is mostly a review problem I'd guess as in the code it just affects this method and you'd see all the code of the method easily compared to hunks sent via email. But renaming this variable doesn't hurt. OK. What variable name do you want? Would gpgsign (to match commit.gpgsign) do? Fine with me. Thanks, Stefan -- To unsubscribe from this list: send the line 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] worktree: replace checkout --to with worktree new
From: Eric Sunshine sunsh...@sunshineco.com The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree add. As a side-effect, linked worktree creation becomes much more discoverable with its own dedicated command, whereas `--to` was easily overlooked amid the plethora of options recognized by git-checkout. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Junio C Hamano gits...@pobox.com --- * Duy seems to think worktree add is coming, too, so here is a trivial tweak of your patch from the last month, with test adjustments to 7410 I sent earlier squashed in. I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does not seem to be documented. Is this something we still need? The log message of 529fef20 (checkout: support checking out into a new working directory, 2014-11-30) does not tell us much. Documentation/git-checkout.txt| 72 - Documentation/git-worktree.txt| 79 ++- builtin/checkout.c| 152 +--- builtin/worktree.c| 157 ++ t/t2025-checkout-to.sh| 137 - t/t2025-worktree-add.sh | 137 + t/t2026-prune-linked-checkouts.sh | 2 +- t/t7410-submodule-checkout-to.sh | 4 +- 8 files changed, 377 insertions(+), 363 deletions(-) delete mode 100755 t/t2025-checkout-to.sh create mode 100755 t/t2025-worktree-add.sh diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 72def5b..efe6a02 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -225,13 +225,6 @@ This means that you can use `git checkout -p` to selectively discard edits from your current working tree. See the ``Interactive Mode'' section of linkgit:git-add[1] to learn how to operate the `--patch` mode. ---to=path:: - Check out a branch in a separate working directory at - `path`. A new working directory is linked to the current - repository, sharing everything except working directory - specific files such as HEAD, index... See MULTIPLE WORKING - TREES section for more information. - --ignore-other-worktrees:: `git checkout` refuses when the wanted ref is already checked out by another worktree. This option makes it check the ref @@ -401,71 +394,6 @@ $ git reflog -2 HEAD # or $ git log -g -2 HEAD -MULTIPLE WORKING TREES --- - -A git repository can support multiple working trees, allowing you to check -out more than one branch at a time. With `git checkout --to` a new working -tree is associated with the repository. This new working tree is called a -linked working tree as opposed to the main working tree prepared by git -init or git clone. A repository has one main working tree (if it's not a -bare repository) and zero or more linked working trees. - -Each linked working tree has a private sub-directory in the repository's -$GIT_DIR/worktrees directory. The private sub-directory's name is usually -the base name of the linked working tree's path, possibly appended with a -number to make it unique. For example, when `$GIT_DIR=/path/main/.git` the -command `git checkout --to /path/other/test-next next` creates the linked -working tree in `/path/other/test-next` and also creates a -`$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1` -if `test-next` is already taken). - -Within a linked working tree, $GIT_DIR is set to point to this private -directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and -$GIT_COMMON_DIR is set to point back to the main working tree's $GIT_DIR -(e.g. `/path/main/.git`). These settings are made in a `.git` file located at -the top directory of the linked working tree. - -Path resolution via `git rev-parse --git-path` uses either -$GIT_DIR or $GIT_COMMON_DIR depending on the path. For example, in the -linked working tree `git rev-parse --git-path HEAD` returns -`/path/main/.git/worktrees/test-next/HEAD` (not -`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git -rev-parse --git-path refs/heads/master` uses -$GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. - -See linkgit:gitrepository-layout[5] for more information. The rule of -thumb is do not make any assumption about whether a path belongs to -$GIT_DIR or $GIT_COMMON_DIR when you need to directly access something -inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. - -When you are done with a linked working tree you can simply delete it. -The working
Re: [PATCH 6/7] pack-protocol.txt: Mark pushee field as optional
Dave Borowitz dborow...@google.com writes: send-pack.c omits this field when args-url is null or empty. Fix the protocol specification to match reality. Do some clients omit this in the real world? As you say, send_pack() does omit it if args-url is null or empty, but args is prepared in transport.c as a copy of transport-url when the function is called, and that transport-url is how builtin/push.c reports where it is pushing with: if (verbosity 0) fprintf(stderr, _(Pushing to %s\n), transport-url); So I am somewhat puzzled... Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index f37dcf1..98e512d 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -495,7 +495,7 @@ references. push-cert = PKT-LINE(push-cert NUL capability-list LF) PKT-LINE(certificate version 0.1 LF) PKT-LINE(pusher SP push-cert-ident LF) - PKT-LINE(pushee SP url LF) + [PKT-LINE(pushee SP url LF)] PKT-LINE(nonce SP nonce LF) PKT-LINE(LF) *PKT-LINE(command LF) @@ -554,7 +554,8 @@ Currently, the following header fields are defined: `pushee` url:: The repository URL (anonymized, if the URL contains authentication material) the user who ran `git push` - intended to push into. + intended to push into. This field is optional, and included at + the client's discretion. `nonce` nonce:: The 'nonce' string the receiving repository asked the -- To unsubscribe from this list: send the line 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/7] send-pack.c: Die if the nonce is empty
pack-protocol.txt does not list the nonce as optional. Fortunately, it should be impossible to not have a nonce by this point in the code, as the caller should have died on line 380 prior to generating a push certificate with an empty nonce. Nonetheless, having this explicit error handling in the code reduces confusion for implementors trying to understand the signed push protocol by looking at the reference implementation. Signed-off-by: Dave Borowitz dborow...@google.com --- send-pack.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/send-pack.c b/send-pack.c index 2a64fec..77e2131 100644 --- a/send-pack.c +++ b/send-pack.c @@ -254,6 +254,8 @@ static int generate_push_cert(struct strbuf *req_buf, } if (push_cert_nonce[0]) strbuf_addf(cert, nonce %s\n, push_cert_nonce); + else + die(_(server did not provide a nonce)); strbuf_addstr(cert, \n); for (ref = remote_refs; ref; ref = ref-next) { -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line 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] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 1:13 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jul 1, 2015 at 12:53 PM, Junio C Hamano gits...@pobox.com wrote: * Duy seems to think worktree add is coming, too, so here is a trivial tweak of your patch from the last month, with test adjustments to 7410 I sent earlier squashed in. Thanks. I was planning on re-rolling to use the new name (add rather than new) and to squash in the t7410 fix. Plus, I think the changes I had to make to prepare_linked_checkout() in order to move it to worktree.c should become separate preparatory patches so that the actual relocation can become pure code movement (as much as possible). Also, I was planning on including Duy's patch in the re-roll since it missed a s/prune --worktrees/worktree prune/ in Documentation/git-checkout.txt. Hmm, I see that Duy's patch is already in 'next'. Would it be better if I fixed the 's/prune --worktrees/worktree prune/' git-checkout.txt oversight via an incremental patch rather than including a corrected version of his patch with my re-roll? -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Elaborate on pusher identity
Dave Borowitz dborow...@google.com writes: This is sort of like a standard identity, except that RFC 4880 section 4.11 allows any UTF-8 text in the User ID packet. It is trivial to get gpg to pass arbitrary text when generating a push cert by setting user.signingKey to that arbitrary value (assuming it is an actual user ID associated with that key). Signed-off-by: Dave Borowitz dborow...@google.com --- I think this is a good idea. I notice that nonce used near-by also lacks the definition, which we would want to document. Thanks. Documentation/technical/pack-protocol.txt | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 2d8b1a1..de3c72c 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -494,7 +494,7 @@ references. push-cert = PKT-LINE(push-cert NUL capability-list LF) PKT-LINE(certificate version 0.1 LF) - PKT-LINE(pusher SP ident LF) + PKT-LINE(pusher SP push-cert-ident LF) PKT-LINE(pushee SP url LF) PKT-LINE(nonce SP nonce LF) PKT-LINE(LF) @@ -502,6 +502,8 @@ references. *PKT-LINE(gpg-signature-lines LF) PKT-LINE(push-cert-end LF) + push-cert-ident = 1*(UTF8) SP [-] 1*(DIGIT) SP [-|+] 1*(DIGIT) + packfile = PACK 28*(OCTET) @@ -540,8 +542,14 @@ Note that (unlike other portions of the protocol), all LFs in the Currently, the following header fields are defined: `pusher` ident:: - Identify the GPG key in Human Readable Name email@address - format. + Identity of the GPG key. This is similar to the identify found + elsewhere, such as the author/committer field in commit headers, + in that it consists of a name portion, a timestamp, and a + timezone offset. However, unlike normal git identities, the name + field may be any valid OpenPGP User ID, which is any valid UTF-8 + string. (By convention this matches the form: + Human Readable Name (optional comment) email@address + but this is only a convention.) `pushee` url:: The repository URL (anonymized, if the URL contains -- To unsubscribe from this list: send the line 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 v4 28/44] builtin-am: pass git-apply's options to git-apply
On Tue, Jun 30, 2015 at 7:56 AM, Stefan Beller sbel...@google.com wrote: I realize this was in am.sh as well, but I find the help strings a bit unfortunate. (Yes, you actually need to look them up at another place as most people are not familiar with the apply options). Yeah I agree, it would be an improvement. I think the same can be said for git-mailinfo's and git-mailsplit's options. e.g. pass -k flag to git-mailinfo is not very descriptive either, so we should change their help strings as well. Since git-am combines most of the options from git-mailsplit, git-mailinfo and git-apply together, I wonder if we should split their options into different groups, e.g: usage: git am [options] [(mbox|Maildir)...] or: git am [options] (--continue | --skip | --abort) -i, --interactive run interactively -3, --3wayallow fall back on 3way merging if needed -q, --quiet be quiet -s, --signoff add a Signed-off-by line to the commit message --patch-format format format the patch(es) are in --resolvemsg ... override error message when patch failure occurs --continuecontinue applying patches after resolving a conflict -r, --resolvedsynonyms for --continue --skipskip the current patch --abort restore the original branch and abort the patching operation. --committer-date-is-author-date lie about committer date --ignore-date use current timestamp for author date --rerere-autoupdate update the index with reused conflict resolution if possible -S, --gpg-sign[=key-id] GPG-sign commits options for git-mailsplit --keep-cr pass --keep-cr flag to git-mailsplit for mbox format --no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr options for git-mailinfo -u, --utf8recode into utf8 (default) -m, --message-id pass -m flag to git-mailinfo -c, --scissorsstrip everything before a scissors line -k, --keeppass -k flag to git-mailinfo --keep-non-patch pass -b flag to git-mailinfo options for git-apply --whitespace action detect new or modified lines that have whitespace errors --ignore-space-change ignore changes in whitespace when finding context --ignore-whitespace ignore changes in whitespace when finding context --directory rootprepend root to all filenames --exclude path don't apply changes matching the given path --include path apply changes matching the given path -C nensure at least n lines of context match -p num remove num leading slashes from traditional diff paths --reject leave the rejected hunks in corresponding *.rej files We may wish to put these changes in their own preparatory patch series though. What do you think? Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rev-list: add --count to usage guide
--count should be mentioned in the usage guide, this updates code and documentation. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- Documentation/git-rev-list.txt | 1 + builtin/rev-list.c | 1 + 2 files changed, 2 insertions(+) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index b10ea60..7b49c85 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -56,6 +56,7 @@ SYNOPSIS [ --reverse ] [ --walk-reflogs ] [ --no-walk ] [ --do-walk ] +[ --count ] [ --use-bitmap-index ] commit... [ \-- paths... ] diff --git a/builtin/rev-list.c b/builtin/rev-list.c index ff84a82..ee9e872 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -42,6 +42,7 @@ static const char rev_list_usage[] = --abbrev=n | --no-abbrev\n --abbrev-commit\n --left-right\n +--count\n special purpose:\n --bisect\n --bisect-vars\n -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v3 01/31] wrapper: implement xopen()
On Thu, Jun 25, 2015 at 2:39 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: IMO the varargs make the code more cumbersome to read (and even fragile, because you can easily call `xopen(path, O_WRITE | O_CREATE)` and would not even get so much as a compiler warning!) I think that since xopen() is a wrapper around open(), it is best to follow its interface (as defined by the POSIX spec) as much as possible. It's important to note that the POSIX spec explicitly defines that open() takes a variable number of arguments, and that the `mode` argument is only used if O_CREAT is set. This means that if we cement xopen() to take 3 arguments, and the third is a mode_t (or an int), then we may not be able to keep up with changes in the POSIX spec which e.g. in the future may specify that open() takes 4 arguments if certain flags are set. and the error message does carry value: it helps you resolve the issue (it is completely unnecessary to check write permissions of the directory when a file could not be opened for reading, for example, yet if the error message does not say that and you suspect that the file could not be opened for *writing* that is exactly what you would waste your time checking). Good point, I agree with this. I'll look into putting the error messages back. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
pocket size selfie stick bt foldable monopod,only $3/pc now
Dear Good day Pocket size mini foldable monopod selfie stick with bt shutter button on stick, now only $3/pc ,only from us,the arcpeaks factory http://arcpeaks.en.alibaba.com/product/60228194355-801021588/2015_original_factory_foldable_bluetooth_monopod_mini_selfie_stick.html Please feel free to contact me for more details Thanks Best Regards Ray arcpeaks.en.alibaba.com Skype:sixiwenzhi MOBIL:+86 18924649532 We will attend Hong kong Electronics Fair(Autumn Edition) On Oct 13th to 16th,2015 Our booth located at ED-D26 at Expo Drive Hall Welcome
Re: [PATCH] --count feature for git shortlog
On Tue, Jun 30, 2015 at 08:00:53PM -0700, Lawrence Siebert wrote: The following doesn't currently run: `git rev-list --count --use-bitmap-index HEAD` This is an optional parameter for rev-list from commit aa32939fea9c8934b41efce56015732fa12b8247 which can't currently be used because of changes in parameter parsing, but which modifies `--count` and which may be faster. I've gotten it working again, both by changing the current repo code to make it work, and also by building from that commit, and when I tested it on the whole repo, it seems like it's less variable in speed then `git rev-list --count HEAD`. but not necessarily consistently faster like tests suggested it was when it was committed. Obviously I'm not testing on the same system as the original committer, or with the same compiler, or even using the same version of the linux kernel repo, so those may be a factor. It may also work better in a circumstance that I haven't accounted for, like an older repo, on a per file basis when getting per file commit counts for all files, or something like that. Can you give more details? In a copy of linux.git with bitmaps: $ git log -1 --oneline 64fb1d0 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc $ ls -l .git/objects/pack/ total 892792 -r--r--r-- 1 peff peff 24498374 May 21 13:39 pack-182149ca37c3f2d8fa190e4add772ae08af0e9d2.bitmap -r--r--r-- 1 peff peff 115283036 May 21 13:39 pack-182149ca37c3f2d8fa190e4add772ae08af0e9d2.idx -r--r--r-- 1 peff peff 774420808 May 21 13:39 pack-182149ca37c3f2d8fa190e4add772ae08af0e9d2.pack The packfiles were created with git repack -adb. It shows big speedups for this exact operation: $ git version git version 2.5.0.rc0 $ time git rev-list --count HEAD 515406 real0m9.500s user0m9.424s sys 0m0.092s $ time git rev-list --use-bitmap-index --count HEAD 515406 real0m0.392s user0m0.328s sys 0m0.064s Note that this would not work with, say: git rev-list --use-bitmap-index --count HEAD -- Makefile as the bitmap index does not have enough information to do path limiting (we should probably disallow this or fall back to the non-bitmap code path, but right now we just ignore the path limiter). I'm thinking I could submit a patch that makes it work again, and leave it to the user to decide whether to use it or not. There is also a --test-bitmap option which compares the regular count with the bitmap count. I'm not sure if the implication there was regression testing or that --use-bitmap-index might give the wrong results in certain circumstances. Vincent, could you clarify? Yes, `--test-bitmap` is just for internal testing; you should always get the same results. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] worktree: new place for git prune --worktrees
On Mon, Jun 29, 2015 at 11:13 PM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Commit 23af91d (prune: strategies for linked checkouts - 2014-11-30) adds --worktrees to git prune without realizing that git prune is for object database only. This patch moves the same functionality to a new command git worktree. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- In future I probably move the big block of text in git-checkout.txt to git-worktree.txt and add git worktree list. But let's start with something small and simple before git prune --worktrees is shipped out. Thanks. I notice that after applying this, builtin/prune.c does not revert to the original before 23af91d. It stops including dir.h and it adds an extra blank line. We've been including a header that is not necessary even in v2.4.0, it seems. We used to walk $GIT_DIR/objects ourselves to find loose object files and dir.h was needed for is_dot_or_dotdot(); for_each_loose_file_in_objdir() is what we use these days to hide the implementation details these days; 27e1e22 forgot to remove the inclusion when it did this. The C code part is mostly just \C-x \C-v and I found nothing questionable. I was about to prepare v2, but the only change in the end was removing this blank line, not worth another round. diff --git a/command-list.txt b/command-list.txt index b17c011..2a94137 100644 --- a/command-list.txt +++ b/command-list.txt @@ -148,4 +148,5 @@ git-verify-pack plumbinginterrogators git-verify-tag ancillaryinterrogators gitweb ancillaryinterrogators git-whatchanged ancillaryinterrogators +git-worktreemainporcelain I doubt that a helper that is primarily spawned from gc as its implementation detail is more mainporcelain than git config is: git-config ancillarymanipulators I also wonder if git worktree command should have a mode that works in a way similar to how new-workdir (in contrib/workdir) does, instead of an option checkout --to that looks just out of place as worktree prune was out of place in prune. The feature is doing a lot more than what checkout normally does (somewhere in between checkout and clone, I would say), and it may be cleaner to use an independent command git worktree to manage a separate worktree. And when that happens, the command should definitely be classified as a mainporcelain. And because git worktree add is coming, I guess it's ok to stick to mainporcelain here.. diff --git a/git.c b/git.c index 44374b1..fa77bc9 100644 --- a/git.c +++ b/git.c @@ -483,6 +483,7 @@ static struct cmd_struct commands[] = { { verify-tag, cmd_verify_tag, RUN_SETUP }, { version, cmd_version }, { whatchanged, cmd_whatchanged, RUN_SETUP }, + { worktree, cmd_worktree, RUN_SETUP }, We do not NEED_WORK_TREE because we can create a new worktree off of a bare repository? Yes. -- 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: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 11:33 PM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch and if I had a way to tell checkout to move away and delete previous branch, I would probably stop using git branch -d/-D too. --to is another -b in this sense. I didn't know checkout --to included create a worktree elsewhere and chdir there; if that and chdir there is not something you are doing, then I do not think checkout -b vs branch analogy applies. Heh.. I do want that chdir (even for git-init and git-clone). We can't issue cd command back to the parent shell, but we can spawn a new shell with new cwd. But because the target dir is usually at the end of the command line (except for --to) and cd !$ is not much to type, it never bothers me enough to do something more. I think this is another reason I prefer git worktree add to have the target dir at the end. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Add tests for wildcard path vs ref disambiguation
Commit 28fcc0b (pathspec: avoid the need of -- when wildcard is used - 2015-05-02) changes how the disambiguation rules work. This patch adds some tests to demonstrate, basically, if wildcard characters are in an argument: - if the argument is valid extended sha-1 syntax, -- must be used - otherwise the argument is considered a path, even without -- And wildcard can appear in extended sha-1 syntax, either as part of regex in :/regex or as the literal path in :path. The latter case is less likely to happen in real world. But if you do :/ a lot, you may need to type -- more. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- v2 just adds tests as the code is already in 'master' for some time. t/t2019-checkout-ambiguous-ref.sh | 26 ++ 1 file changed, 26 insertions(+) diff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh index b99d519..e5ceba3 100755 --- a/t/t2019-checkout-ambiguous-ref.sh +++ b/t/t2019-checkout-ambiguous-ref.sh @@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' ' test_i18ngrep ! ^HEAD is now at stderr ' +test_expect_success 'wildcard ambiguation, paths win' ' + git init ambi + ( + cd ambi + echo a a.c + git add a.c + echo b a.c + git checkout *.c + echo a expect + test_cmp expect a.c + ) +' + +test_expect_success 'wildcard ambiguation, refs lose' ' + git init ambi2 + ( + cd ambi2 + echo a *.c + git add . + test_must_fail git show :*.c + git show :*.c -- actual + echo a expect + test_cmp expect actual + ) +' + test_done -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line 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/WIP v3 01/31] wrapper: implement xopen()
On Wed, Jul 1, 2015 at 5:41 PM, Paul Tan pyoka...@gmail.com wrote: Good point, I agree with this. I'll look into putting the error messages back. This should work I think. It should take into account that O_RDONLY, O_WRONLY, O_RDWR is defines as 0, 1, 2 on glibc, while the POSIX spec also defines that O_RDONLY | O_WRONLY == O_RDWR. diff --git a/wrapper.c b/wrapper.c index c867ca9..e451463 100644 --- a/wrapper.c +++ b/wrapper.c @@ -214,7 +214,13 @@ int xopen(const char *path, int oflag, ...) return fd; if (errno == EINTR) continue; -die_errno(_(could not open '%s'), path); + +if ((oflag O_RDWR) == O_RDWR) +die_errno(_(could not open '%s' for reading and writing), path); +else if ((oflag O_WRONLY) == O_WRONLY) +die_errno(_(could not open '%s' for writing), path); +else +die_errno(_(could not open '%s' for reading), path); } } @@ -351,7 +357,13 @@ FILE *xfopen(const char *path, const char *mode) return fp; if (errno == EINTR) continue; -die_errno(_(could not open '%s'), path); + +if (*mode mode[1] == '+') +die_errno(_(could not open '%s' for reading and writing), path); +else if (*mode == 'w' || *mode == 'a') +die_errno(_(could not open '%s' for writing), path); +else +die_errno(_(could not open '%s' for reading), path); } } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] --count feature for git shortlog
Jeff King p...@peff.net writes: Note that this would not work with, say: git rev-list --use-bitmap-index --count HEAD -- Makefile as the bitmap index does not have enough information to do path limiting (we should probably disallow this or fall back to the non-bitmap code path, but right now we just ignore the path limiter). Yeah, at least the former but preferrably the latter. 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
[PATCH] fast-import: add a get-mark command
It is sometimes useful for importers to be able to read the SHA-1 corresponding to a mark that they have created via fast-import. For example, they might want to embed the SHA-1 into the commit message of a later commit. Or it might be useful for internal bookkeeping uses, or for logging. Add a get-mark command to git fast-import that allows the importer to ask for the value of a mark that has been created earlier. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This is something that we need for an internal GitHub project, but I think it will also be of general interest. Documentation/git-fast-import.txt | 39 +-- fast-import.c | 33 + t/t9300-fast-import.sh| 13 + 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index fd32895..66910aa 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -54,7 +54,7 @@ Options for Frontends ~ --cat-blob-fd=fd:: - Write responses to `cat-blob` and `ls` queries to the + Write responses to `get-mark`, `cat-blob`, and `ls` queries to the file descriptor fd instead of `stdout`. Allows `progress` output intended for the end-user to be separated from other output. @@ -350,6 +350,11 @@ and control the current import process. More detailed discussion unless the `done` feature was requested using the `--done` command-line option or `feature done` command. +`get-mark`:: + Causes fast-import to print the SHA-1 corresponding to a mark + to the file descriptor set with `--cat-blob-fd`, or `stdout` if + unspecified. + `cat-blob`:: Causes fast-import to print a blob in 'cat-file --batch' format to the file descriptor set with `--cat-blob-fd` or @@ -930,6 +935,25 @@ Placing a `progress` command immediately after a `checkpoint` will inform the reader when the `checkpoint` has been completed and it can safely access the refs that fast-import updated. +`get-mark` +~~ +Causes fast-import to print the SHA-1 corresponding to a mark to +stdout or to the file descriptor previously arranged with the +`--cat-blob-fd` argument. The command otherwise has no impact on the +current import; its purpose is to retrieve SHA-1s that later commits +might want to refer to in their commit messages. + + + 'get-mark' SP ':' idnum LF + + +This command can be used anywhere in the stream that comments are +accepted. In particular, the `get-mark` command can be used in the +middle of a commit but not in the middle of a `data` command. + +See ``Responses To Commands'' below for details about how to read +this output safely. + `cat-blob` ~~ Causes fast-import to print a blob to a file descriptor previously @@ -1000,7 +1024,8 @@ Output uses the same format as `git ls-tree tree -- path`: The dataref represents the blob, tree, or commit object at path -and can be used in later 'cat-blob', 'filemodify', or 'ls' commands. +and can be used in later 'get-mark', 'cat-blob', 'filemodify', or +'ls' commands. If there is no file or subtree at that path, 'git fast-import' will instead report @@ -1042,9 +1067,11 @@ import-marks-if-exists:: feature import-marks-if-exists like a corresponding command-line option silently skips a nonexistent file. +get-mark:: cat-blob:: ls:: - Require that the backend support the 'cat-blob' or 'ls' command. + Require that the backend support the 'get-mark', 'cat-blob', + or 'ls' command respectively. Versions of fast-import not supporting the specified command will exit with a message indicating so. This lets the import error out early with a clear message, @@ -1124,11 +1151,11 @@ bidirectional pipes: git fast-import fast-import-output -A frontend set up this way can use `progress`, `ls`, and `cat-blob` -commands to read information from the import in progress. +A frontend set up this way can use `progress`, `get-mark`, `ls`, and +`cat-blob` commands to read information from the import in progress. To avoid deadlock, such frontends must completely consume any -pending output from `progress`, `ls`, and `cat-blob` before +pending output from `progress`, `ls`, `get-mark`, and `cat-blob` before performing writes to fast-import that might block. Crash Reports diff --git a/fast-import.c b/fast-import.c index 6378726..1ba83d6 100644 --- a/fast-import.c +++ b/fast-import.c @@ -134,16 +134,17 @@ Format of STDIN stream: ts::= # time since the epoch in seconds, ascii base10 notation; tz::= # GIT style timezone; - # note: comments, ls and cat requests may appear anywhere - # in the input, except within a data command. Any form - # of the data command always escapes the related input -
Re: [PATCH] rev-list: add --count to usage guide
Lawrence Siebert lawrencesieb...@gmail.com writes: --count should be mentioned in the usage guide, this updates code and documentation. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- Sounds good. While at it, perhaps add a mention (perhaps by creating a BUGS section at the end of the file) that --count with --use-bitmap-index ignores pathspec silently? Documentation/git-rev-list.txt | 1 + builtin/rev-list.c | 1 + 2 files changed, 2 insertions(+) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index b10ea60..7b49c85 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -56,6 +56,7 @@ SYNOPSIS [ --reverse ] [ --walk-reflogs ] [ --no-walk ] [ --do-walk ] + [ --count ] [ --use-bitmap-index ] commit... [ \-- paths... ] diff --git a/builtin/rev-list.c b/builtin/rev-list.c index ff84a82..ee9e872 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -42,6 +42,7 @@ static const char rev_list_usage[] = --abbrev=n | --no-abbrev\n --abbrev-commit\n --left-right\n +--count\n special purpose:\n --bisect\n --bisect-vars\n -- To unsubscribe from this list: send the line 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 6/7] pack-protocol.txt: Mark pushee field as optional
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: Dave Borowitz dborow...@google.com writes: send-pack.c omits this field when args-url is null or empty. Fix the protocol specification to match reality. Do some clients omit this in the real world? As you say, send_pack() does omit it if args-url is null or empty, but args is prepared in transport.c as a copy of transport-url when the function is called, and that transport-url is how builtin/push.c reports where it is pushing with: if (verbosity 0) fprintf(stderr, _(Pushing to %s\n), transport-url); So I am somewhat puzzled... Answering myself, the most trivial example is git send-pack ;-) It passes args that has a NULL in the .url field. ... which may be something we want to fix, but that does not mean the field is mandatory, as we have implementations in the field that do not send it ;-) The patch looks good. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] pack-protocol.txt: Add warning about protocol inaccuracies
On Wed, Jul 1, 2015 at 12:39 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Dave Borowitz wrote: --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -14,6 +14,17 @@ data. The protocol functions to have a server tell a client what is currently on the server, then for the two to negotiate the smallest amount of data to send in order to fully update one or the other. +Notes to Implementors +- + +WARNING: This document is a work in progress. Some of the protocol +specifications below may be incomplete or inaccurate. When in doubt, +refer to the C code. If we include this warning, can it also say to contact git@vger.kernel.org for inaccuracies? Otherwise it is easy to misread as Some of this document may be inaccurate, and we're working on fixing that. Don't bug me --- I already know about the problems --- just be patient! I would rather that it would say something like Caveat -- This document contains some inaccuracies. Do not forget to also check against the C code, and please contact git@vger.kernel.org if you run into any unclear or inaccurate passages in this spec. Agreed with your rationale and suggested wording, thanks. + +One particular example is that many of the LFs referenced in the +specifications are optional, but may (yet) not be marked as such. If not +explicitly marked one way or the other, double-check with the C code. The 'Reference Discovery' section says: Server SHOULD terminate each non-flush line using LF (\n) terminator; client MUST NOT complain if there is no terminator. Unfortunately that's not explained in a section with broader scope. Isn't that actually the rule everywhere except for in push certs? It's certainly the rule in more places. I personally have partially audited send-pack.c, but there are many places that are not yet audited. I don't feel comfortable making a broader claim without having done so, and I do not have the time to do so at the moment. The documentation will be easier to use if it says so instead of asking implementers to check the source of all implementations (since interoperability with only one isn't enough). Unfortunately, the trust I have in this document at this point is less than zero. A handful of spot fixes, while useful, does not serve as any sort of assurance that we've gotten all or even most of the problems. Until such time as this document is actually reliable, implementors must check the source of all implementations if they want to be accurate. That sucks for implementors (believe me, I know), but it's the truth. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-list: disable --use-bitmap-index when pruning commits
Jeff King p...@peff.net writes: On Wed, Jul 01, 2015 at 08:19:40AM -0700, Junio C Hamano wrote: Sounds good. While at it, perhaps add a mention (perhaps by creating a BUGS section at the end of the file) that --count with --use-bitmap-index ignores pathspec silently? I think we can just fix it rather than documenting the problem. :) ;-) Good. Will queue; thanks. -- 8 -- Subject: rev-list: disable --use-bitmap-index when pruning commits The reachability bitmaps do not have enough information to tell us which commits might have changed path foo, so the current code produces wrong answers for: git rev-list --use-bitmap-index --count HEAD -- foo (it silently ignores the foo limiter). Instead, we should fall back to doing a normal traversal (it is OK to fall back rather than complain, because --use-bitmap-index is a pure optimization, and might not kick in for other reasons, such as there being no bitmaps in the repository). Signed-off-by: Jeff King p...@peff.net --- builtin/rev-list.c | 2 +- t/t5310-pack-bitmaps.sh | 6 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index ff84a82..88eddbd 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -355,7 +355,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) if (bisect_list) revs.limited = 1; - if (use_bitmap_index) { + if (use_bitmap_index !revs.prune) { if (revs.count !revs.left_right !revs.cherry_mark) { uint32_t commit_count; if (!prepare_bitmap_walk(revs)) { diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 6003490..d446706 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -53,6 +53,12 @@ rev_list_tests() { test_cmp expect actual ' + test_expect_success counting commits with limiting ($state) ' + git rev-list --count HEAD -- 1.t expect + git rev-list --use-bitmap-index --count HEAD -- 1.t actual + test_cmp expect actual + ' + test_expect_success enumerate --objects ($state) ' git rev-list --objects --use-bitmap-index HEAD tmp cut -d -f1 tmp tmp2 -- To unsubscribe from this list: send the line 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] log: add log.follow config option
Many users prefer to always use --follow with logs. Rather than aliasing the command, an option might be more convenient for some. Signed-off-by: David Turner dtur...@twopensource.com --- Why not just alias log=log --follow? At Twitter, we manage git config centrally, but we also allow users to add their own aliases. We would like to turn log.follow on globally, while not messing up any aliases users already have for log. Also, some users might have different log aliases for different repositories, but want to manage --follow globally. And in the future, we might want to make log --follow the default (it is what I usually want), so it would be nice to provide a way for users to turn that off globally from a config option. builtin/log.c | 7 +++ t/t4206-log-follow-harder-copies.sh | 23 +++ 2 files changed, 30 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index 8781049..11b8d82 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -31,6 +31,7 @@ static const char *default_date_mode = NULL; static int default_abbrev_commit; static int default_show_root = 1; +static int default_follow = 0; static int decoration_style; static int decoration_given; static int use_mailmap_config; @@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev) { if (fmt_pretty) get_commit_format(fmt_pretty, rev); + if (default_follow) + DIFF_OPT_SET(rev-diffopt, FOLLOW_RENAMES); rev-verbose_header = 1; DIFF_OPT_SET(rev-diffopt, RECURSIVE); rev-diffopt.stat_width = -1; /* use full terminal width */ @@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char *value, void *cb) default_show_root = git_config_bool(var, value); return 0; } + if (!strcmp(var, log.follow)) { + default_follow = git_config_bool(var, value); + return 0; + } if (skip_prefix(var, color.decorate., slot_name)) return parse_decorate_color_config(var, slot_name, value); if (!strcmp(var, log.mailmap)) { diff --git a/t/t4206-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh index ad29e65..6b2f3b9 100755 --- a/t/t4206-log-follow-harder-copies.sh +++ b/t/t4206-log-follow-harder-copies.sh @@ -53,4 +53,27 @@ test_expect_success \ 'validate the output.' \ 'compare_diff_patch current expected' +test_expect_success \ +'git config log.follow works like --follow' \ +'test_config log.follow true + git log --name-status --pretty=format:%s path1 current' + +test_expect_success \ +'validate the output.' \ +'compare_diff_patch current expected' + +test_expect_success \ +'git config log.follow is overridden by --no-follow' \ +'test_config log.follow true + git log --no-follow --name-status --pretty=format:%s path1 current' + +cat expected \EOF +Copy path1 from path0 +A path1 +EOF + +test_expect_success \ +'validate the output.' \ +'compare_diff_patch current expected' + test_done -- 2.0.5.499.g01f6352.dirty-twtrsrc -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark all LFs in push-cert as required
Dave Borowitz dborow...@google.com writes: Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 1386840..2d8b1a1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -534,6 +534,9 @@ A push certificate begins with a set of header lines. After the header and an empty line, the protocol commands follow, one per line. +Note that (unlike other portions of the protocol), all LFs in the +`push-cert` specification above MUST be present. + Currently, the following header fields are defined: `pusher` ident:: I am moderately negative about this; wouldn't it make the end result cleaner to fix the implementation? -- To unsubscribe from this list: send the line 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 6/7] pack-protocol.txt: Mark pushee field as optional
On Wed, Jul 1, 2015 at 12:07 PM, Junio C Hamano gits...@pobox.com wrote: Answering myself, the most trivial example is git send-pack ;-) It passes args that has a NULL in the .url field. Well, the example I have involves an actual git push command. The fact that .url is NULL in that case may be a separate bug. -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Add warning about protocol inaccuracies
Hi, Dave Borowitz wrote: --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -14,6 +14,17 @@ data. The protocol functions to have a server tell a client what is currently on the server, then for the two to negotiate the smallest amount of data to send in order to fully update one or the other. +Notes to Implementors +- + +WARNING: This document is a work in progress. Some of the protocol +specifications below may be incomplete or inaccurate. When in doubt, +refer to the C code. If we include this warning, can it also say to contact git@vger.kernel.org for inaccuracies? Otherwise it is easy to misread as Some of this document may be inaccurate, and we're working on fixing that. Don't bug me --- I already know about the problems --- just be patient! I would rather that it would say something like Caveat -- This document contains some inaccuracies. Do not forget to also check against the C code, and please contact git@vger.kernel.org if you run into any unclear or inaccurate passages in this spec. + +One particular example is that many of the LFs referenced in the +specifications are optional, but may (yet) not be marked as such. If not +explicitly marked one way or the other, double-check with the C code. The 'Reference Discovery' section says: Server SHOULD terminate each non-flush line using LF (\n) terminator; client MUST NOT complain if there is no terminator. Unfortunately that's not explained in a section with broader scope. Isn't that actually the rule everywhere except for in push certs? The documentation will be easier to use if it says so instead of asking implementers to check the source of all implementations (since interoperability with only one isn't enough). Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] pack-protocol.txt: Add warning about protocol inaccuracies
Jonathan Nieder jrnie...@gmail.com writes: The 'Reference Discovery' section says: Server SHOULD terminate each non-flush line using LF (\n) terminator; client MUST NOT complain if there is no terminator. I think these should be sender/receiver, not server/client. -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark all LFs in push-cert as required
Dave Borowitz dborow...@google.com writes: I am moderately negative about this; wouldn't it make the end result cleaner to fix the implementation? I'm not sure I understand your suggestion. Are you saying, you would prefer to make LFs optional in the push cert, for consistency with LFs being optional elsewhere? Absolutely. It is not make it optional, but even though it is optional, the receiver has not been following the spec, and it is not too late to fix it. The earliest these documentation updates can hit the public is 2.6; by that time I'd expect the deployed receivers would be fixed with 2.5.1 and 2.4.7 maintenance releases. If some third-party reimplemented their client not to terminate with LF, they wouldn't be working correctly with the deployed servers right now *anyway*. And with the more lenient receive-pack in 2.5.1 or 2.4.7, they will start working. And we will not change our client to drop LF termination. So overall I do not see that it is too much a price to pay for consistency across the protocol. If LF is optional, then with that approach you might end up with a section of that buffer like: I think I touched on this in my previous message. You cannot send an empty line anywhere, and this is not limited to push-cert section of the protocol. Strictly speaking, the wire level allows it, but I do not think the deployed client APIs easily lets you deal with it. So you must follow the SHOULD terminate with LF for an empty line, even when you choose to ignore the SHOULD in most other places. I do not think it is such a big loss, as long as it is properly documented. -- To unsubscribe from this list: send the line 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 v6 2/4] status: differentiate interactive from non-interactive rebases
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- No modification. t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 68ad2d7..190656d 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' ' ONTO=$(git rev-parse --short rebase_i_conflicts) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -190,7 +190,7 @@ test_expect_success 'status when rebasing -i in edit mode' ' ONTO=$(git rev-parse --short HEAD~2) git rebase -i HEAD~2 cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -216,7 +216,7 @@ test_expect_success 'status when splitting a commit' ' git rebase -i HEAD~3 git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -247,7 +247,7 @@ test_expect_success 'status after editing the last commit with --amend during a git rebase -i HEAD~3 git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -277,7 +277,7 @@ test_expect_success 'status: (continue first edit) second edit' ' git rebase -i HEAD~3 git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -299,7 +299,7 @@ test_expect_success 'status: (continue first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -326,7 +326,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' ' git rebase --continue git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -348,7 +348,7 @@ test_expect_success 'status: (amend first edit) second edit' ' git commit --amend -m a git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -371,7 +371,7 @@ test_expect_success 'status: (amend first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently
[PATCH v6 3/4] status: give more information during rebase -i
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr git status gives more information during rebase -i, about the list of command that are done during the rebase. It displays the two last commands executed and the two next lines to be executed. It also gives hints to find the whole files in .git directory. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- Applied Junio's remark on C code to abbreviate sha1s and filter comments. t/t7512-status-help.sh | 111 wt-status.c| 113 + 2 files changed, 224 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 190656d..9be0235 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -134,9 +134,13 @@ test_expect_success 'prepare for rebase_i_conflicts' ' test_expect_success 'status during rebase -i when conflicts unresolved' ' test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse --short rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -159,10 +163,14 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' git reset --hard rebase_i_conflicts_second test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse --short rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -183,7 +191,9 @@ test_expect_success 'status when rebasing -i in edit mode' ' git checkout -b rebase_i_edit test_commit one_rebase_i main.txt one test_commit two_rebase_i main.txt two + COMMIT2=$(git rev-parse --short rebase_i_edit) test_commit three_rebase_i main.txt three + COMMIT3=$(git rev-parse --short rebase_i_edit) FAKE_LINES=1 edit 2 export FAKE_LINES test_when_finished git rebase --abort @@ -191,6 +201,10 @@ test_expect_success 'status when rebasing -i in edit mode' ' git rebase -i HEAD~2 cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_rebase_i + edit $COMMIT3 three_rebase_i +No commands remaining. You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -207,8 +221,11 @@ test_expect_success 'status when splitting a commit' ' git checkout -b split_commit test_commit one_split main.txt one test_commit two_split main.txt two + COMMIT2=$(git rev-parse --short split_commit) test_commit three_split main.txt three + COMMIT3=$(git rev-parse --short split_commit) test_commit four_split main.txt four + COMMIT4=$(git rev-parse --short split_commit) FAKE_LINES=1 edit 2 3 export FAKE_LINES test_when_finished git rebase --abort @@ -217,6 +234,12 @@ test_expect_success 'status when splitting a commit' ' git reset HEAD^ cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_split + edit $COMMIT3 three_split +Next command to do (1 remaining command): + pick $COMMIT4 four_split + (use git rebase --edit-todo to view and edit) You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -239,7 +262,9 @@ test_expect_success 'status after editing the last commit with --amend during a test_commit one_amend main.txt one test_commit two_amend main.txt two test_commit three_amend main.txt three + COMMIT3=$(git rev-parse --short amend_last) test_commit four_amend main.txt four + COMMIT4=$(git rev-parse --short amend_last) FAKE_LINES=1 2 edit 3 export FAKE_LINES test_when_finished git rebase
[PATCH v6 4/4] status: add new tests for status during rebase -i
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Expand test coverage with one or more than two commands done and with zero, one or more than two commands remaining. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- No modification. t/t7512-status-help.sh | 87 ++ 1 file changed, 87 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 9be0235..49d19a3 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -856,4 +856,91 @@ EOF test_i18ncmp expected actual ' +test_expect_success 'prepare for different number of commits rebased' ' + git reset --hard master + git checkout -b several_commits + test_commit one_commit main.txt one + test_commit two_commit main.txt two + test_commit three_commit main.txt three + test_commit four_commit main.txt four +' + +test_expect_success 'status: one command done nothing remaining' ' + FAKE_LINES=exec_exit_15 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last command done (1 command done): + exec exit 15 +No commands remaining. +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two commands done with some white lines in done file' ' + FAKE_LINES=1 exec_exit_15 2 3 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + COMMIT4=$(git rev-parse --short HEAD) + COMMIT3=$(git rev-parse --short HEAD^) + COMMIT2=$(git rev-parse --short HEAD^^) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_commit + exec exit 15 +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two remaining commands with some white lines in todo file' ' + FAKE_LINES=1 2 exec_exit_15 3 4 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~4) + COMMIT4=$(git rev-parse --short HEAD) + COMMIT3=$(git rev-parse --short HEAD^) + COMMIT2=$(git rev-parse --short HEAD^^) + test_must_fail git rebase -i HEAD~4 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (3 commands done): + pick $COMMIT2 two_commit + exec exit 15 + (see more in file .git/rebase-merge/done) +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + test_done -- 2.5.0.rc0.7.ge1edd74.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/4] status: factor two rebase-related messages together
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- No modification. wt-status.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/wt-status.c b/wt-status.c index eaed4fe..8c4b806 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1027,6 +1027,20 @@ static int split_commit_in_progress(struct wt_status *s) return split_in_progress; } +static void print_rebase_state(struct wt_status *s, + struct wt_status_state *state, + const char *color) +{ + if (state-branch) + status_printf_ln(s, color, +_(You are currently rebasing branch '%s' on '%s'.), +state-branch, +state-onto); + else + status_printf_ln(s, color, +_(You are currently rebasing.)); +} + static void show_rebase_in_progress(struct wt_status *s, struct wt_status_state *state, const char *color) @@ -1034,14 +1048,7 @@ static void show_rebase_in_progress(struct wt_status *s, struct stat st; if (has_unmerged(s)) { - if (state-branch) - status_printf_ln(s, color, -_(You are currently rebasing branch '%s' on '%s'.), -state-branch, -state-onto); - else - status_printf_ln(s, color, -_(You are currently rebasing.)); + print_rebase_state(s, state, color); if (s-hints) { status_printf_ln(s, color, _( (fix conflicts and then run \git rebase --continue\))); @@ -1051,14 +1058,7 @@ static void show_rebase_in_progress(struct wt_status *s, _( (use \git rebase --abort\ to check out the original branch))); } } else if (state-rebase_in_progress || !stat(git_path(MERGE_MSG), st)) { - if (state-branch) - status_printf_ln(s, color, -_(You are currently rebasing branch '%s' on '%s'.), -state-branch, -state-onto); - else - status_printf_ln(s, color, -_(You are currently rebasing.)); + print_rebase_state(s, state, color); if (s-hints) status_printf_ln(s, color, _( (all conflicts fixed: run \git rebase --continue\))); -- 2.5.0.rc0.7.ge1edd74.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: [PATCH] log: add log.follow config option
David Turner dtur...@twopensource.com writes: diff --git a/builtin/log.c b/builtin/log.c index 8781049..11b8d82 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -31,6 +31,7 @@ static const char *default_date_mode = NULL; static int default_abbrev_commit; static int default_show_root = 1; +static int default_follow = 0; static int decoration_style; static int decoration_given; static int use_mailmap_config; @@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev) { if (fmt_pretty) get_commit_format(fmt_pretty, rev); + if (default_follow) + DIFF_OPT_SET(rev-diffopt, FOLLOW_RENAMES); Doesn't it activate --follow all the time, including when the user provides several paths? In this case, you get this: $ git log --follow *.c fatal: --follow requires exactly one pathspec So activating --follow for all git log calls would prevent log from being used with several pathspecs. Or, do you have a preparation patch that allows --follow with multiple pathspecs? ;-) In any case, you have to test git log -- path1 path2 with the option activated. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] log: add log.follow config option
Matthieu Moy matthieu@grenoble-inp.fr writes: So activating --follow for all git log calls would prevent log from being used with several pathspecs. Or, do you have a preparation patch that allows --follow with multiple pathspecs? ;-) In any case, you have to test git log -- path1 path2 with the option activated. Or more commonly, just git log with no pathspec. I also think that it is a bad idea to force log --follow to people before it is made into a true feature; as it stands, it is merely a checkbox item. It has too severe limitation to be used seriously in real projects, unless your history is completely linear. cf. http://thread.gmane.org/gmane.comp.version-control.git/269357/focus=269433 -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark all LFs in push-cert as required
Junio C Hamano gits...@pobox.com writes: I am moderately negative about this; wouldn't it make the end result cleaner to fix the implementation? I think that something like this should be sufficient. As the receiving end, we must not complain if there is no terminator. ... And the change we are *not* going to make, but I made temporarily only for testing, on the sending side to violate our sender SHOULD terminate with LF rule would look like this: There is a slight complication on sending an empty line without any termination, though ;-) The reader that calls packet_read() cannot tell such a payload from a flush packet, I think. *That* may be something we want to document. diff --git a/send-pack.c b/send-pack.c index 2a64fec..1a743db 100644 --- a/send-pack.c +++ b/send-pack.c @@ -273,9 +273,11 @@ static int generate_push_cert(struct strbuf *req_buf, packet_buf_write(req_buf, push-cert%c%s, 0, cap_string); for (cp = cert.buf; cp cert.buf + cert.len; cp = np) { + int len; np = next_line(cp, cert.buf + cert.len - cp); + len = (np = cp + 1) ? 1 : (np - cp - 1); packet_buf_write(req_buf, -%.*s, (int)(np - cp), cp); +%.*s, len, cp); } packet_buf_write(req_buf, push-cert-end\n); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] status: give more information during rebase -i
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: +strbuf_trim(split[1]); +if (!get_sha1(split[1]-buf, sha1)) { +abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV); +strbuf_reset(split[1]); +strbuf_addf(split[1], %s , abbrev); +} ... else? That is, we thought there would be a full SHA-1, but it turns out that there wasn't, so we keep split[1] as-is would need to add the space back, no? Right. Perhaps be more strict and do this instead (without leading strbuf_trim): if (!get_sha1_hex(split[1]-buf, sha1) !strcmp(split[1]-buf + 40, ) { replace split[1] with %s abbrev } Actually, we can do simpler: we still have the original line available, so if we don't find a sha1, we can just keep it. By just letting the few lines after the if enter the if, it just works: strbuf_trim(split[1]); if (!get_sha1(split[1]-buf, sha1)) { abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV); strbuf_reset(split[1]); strbuf_addf(split[1], %s , abbrev); strbuf_reset(line); for (i = 0; split[i]; i++) strbuf_addf(line, %s, split[i]-buf); } while (!strbuf_getline(line, f, '\n')) { if (line.len line.len[0] == comment_line_char) continue; strbuf_rtrim(line); if (!line.len) continue; abbrev_sha1_in_line(line); string_list_append(lines, line.buf); } I took this (modulo s/line.len[0]/line.buf[0]/, and s/rtrim/trim/ to be robust to leading whitespace (not really important, but doesn't harm). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] status: give more information during rebase -i
Matthieu Moy matthieu@grenoble-inp.fr writes: Actually, we can do simpler: we still have the original line available, ... I took this (modulo s/line.len[0]/line.buf[0]/, and s/rtrim/trim/ to be robust to leading whitespace (not really important, but doesn't harm). I'd prefer us to be more strict when we know we are reading our own output; rtrim is sensible, as the log line has end-user subject the end and the subject might have a trailing whitespace we want to trim, but there is no valid reason to expect leading whitespace. In any case, I wouldn't have much time during the remainder of the day to requeue and/or comment; please check what I push out on 'pu'. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
Junio C Hamano gits...@pobox.com writes: Dave Borowitz dborow...@google.com writes: Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 1386840..2d8b1a1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -534,6 +534,9 @@ A push certificate begins with a set of header lines. After the header and an empty line, the protocol commands follow, one per line. +Note that (unlike other portions of the protocol), all LFs in the +`push-cert` specification above MUST be present. + Currently, the following header fields are defined: `pusher` ident:: I am moderately negative about this; wouldn't it make the end result cleaner to fix the implementation? I think that something like this should be sufficient. As the receiving end, we must not complain if there is no terminator. diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 94d0571..303a1dd 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1415,9 +1415,12 @@ static struct command *read_head_info(struct sha1_array *shallow) true_flush = 1; break; } - if (!strcmp(certbuf, push-cert-end\n)) + if (!strcmp(certbuf, push-cert-end) || + !strcmp(certbuf, push-cert-end\n)) break; /* end of cert */ strbuf_addstr(push_cert, certbuf); + if (certbuf[len - 1] != '\n') + strbuf_addch(push_cert, '\n'); } if (true_flush) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] status: give more information during rebase -i
Eric Sunshine sunsh...@sunshineco.com writes: I was about to mention the same shortcoming, but you beat me to it. Perhaps be more strict and do this instead (without leading strbuf_trim): if (!get_sha1_hex(split[1]-buf, sha1) !strcmp(split[1]-buf + 40, ) { replace split[1] with %s abbrev } Isn't it dangerous to assume that you can index 40 characters into split[1]? If (for some reason), the user botched the todo line such that the SHA1 is no longer a valid hex string, then split[1] will be that botched string, which might be shorter than 40 characters. For instance, if the user-edited todo line is: pick oops nothing then git-rebase--interactive.sh:transform_todo_ids() will leave oops as-is, since it can't unabbreviate it, and then this code will place oops into split[1]. Yeah, that is why get_sha1_hex() is checked before we try to make sure buf[40] has in the code snippet you quoted. Isn't that how short-cut works? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] status: give more information during rebase -i
On Wed, Jul 1, 2015 at 12:18 PM, Junio C Hamano gits...@pobox.com wrote: Matthieu Moy matthieu@imag.fr writes: +/* + * Turn + * pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message + * into + * pick d6a2f03 some message + */ +static void abbrev_sha1_in_line(struct strbuf *line) +{ + struct strbuf **split; + int i; + + if (starts_with(line-buf, exec ) || + starts_with(line-buf, x )) + return; + + split = strbuf_split_max(line, ' ', 3); + if (split[0] split[1]) { + unsigned char sha1[20]; + const char *abbrev; + + /* + * strbuf_split_max left a space. Trim it and re-add + * it after abbreviation. + */ + strbuf_trim(split[1]); + if (!get_sha1(split[1]-buf, sha1)) { + abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV); + strbuf_reset(split[1]); + strbuf_addf(split[1], %s , abbrev); + } ... else? That is, we thought there would be a full SHA-1, but it turns out that there wasn't, so we keep split[1] as-is would need to add the space back, no? I was about to mention the same shortcoming, but you beat me to it. Perhaps be more strict and do this instead (without leading strbuf_trim): if (!get_sha1_hex(split[1]-buf, sha1) !strcmp(split[1]-buf + 40, ) { replace split[1] with %s abbrev } Isn't it dangerous to assume that you can index 40 characters into split[1]? If (for some reason), the user botched the todo line such that the SHA1 is no longer a valid hex string, then split[1] will be that botched string, which might be shorter than 40 characters. For instance, if the user-edited todo line is: pick oops nothing then git-rebase--interactive.sh:transform_todo_ids() will leave oops as-is, since it can't unabbreviate it, and then this code will place oops into split[1]. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 28/44] builtin-am: pass git-apply's options to git-apply
On Wed, Jul 1, 2015 at 3:22 AM, Paul Tan pyoka...@gmail.com wrote: On Tue, Jun 30, 2015 at 7:56 AM, Stefan Beller sbel...@google.com wrote: I realize this was in am.sh as well, but I find the help strings a bit unfortunate. (Yes, you actually need to look them up at another place as most people are not familiar with the apply options). Yeah I agree, it would be an improvement. I think the same can be said for git-mailinfo's and git-mailsplit's options. e.g. pass -k flag to git-mailinfo is not very descriptive either, so we should change their help strings as well. Since git-am combines most of the options from git-mailsplit, git-mailinfo and git-apply together, I wonder if we should split their options into different groups, e.g: usage: git am [options] [(mbox|Maildir)...] or: git am [options] (--continue | --skip | --abort) -i, --interactive run interactively -3, --3wayallow fall back on 3way merging if needed -q, --quiet be quiet -s, --signoff add a Signed-off-by line to the commit message --patch-format format format the patch(es) are in --resolvemsg ... override error message when patch failure occurs --continuecontinue applying patches after resolving a conflict -r, --resolvedsynonyms for --continue --skipskip the current patch --abort restore the original branch and abort the patching operation. --committer-date-is-author-date lie about committer date --ignore-date use current timestamp for author date --rerere-autoupdate update the index with reused conflict resolution if possible -S, --gpg-sign[=key-id] GPG-sign commits options for git-mailsplit --keep-cr pass --keep-cr flag to git-mailsplit for mbox format --no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr options for git-mailinfo -u, --utf8recode into utf8 (default) -m, --message-id pass -m flag to git-mailinfo -c, --scissorsstrip everything before a scissors line -k, --keeppass -k flag to git-mailinfo --keep-non-patch pass -b flag to git-mailinfo options for git-apply --whitespace action detect new or modified lines that have whitespace errors --ignore-space-change ignore changes in whitespace when finding context --ignore-whitespace ignore changes in whitespace when finding context --directory rootprepend root to all filenames --exclude path don't apply changes matching the given path --include path apply changes matching the given path -C nensure at least n lines of context match -p num remove num leading slashes from traditional diff paths --reject leave the rejected hunks in corresponding *.rej files We may wish to put these changes in their own preparatory patch series though. What do you think? I think this is an improvement! But as you said, we should do it as an additional patch on top of the series. Thanks, Stefan Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/13] rerere minor clean-up
Here is an collection of various minor clean-ups in the implementation of one of my most favourite feature, rerere. This still hasn't reached the step to make the right refactoring to allow me to fix a bug I wanted to fix, which prompted me to look at this code, but it should give me a good starting point. Junio C Hamano (13): rerere: fix an off-by-one non-bug rerere: plug conflict ID leaks rerere: lift PATH_MAX limitation rerere: write out each record of MERGE_RR in one go rerere: report autoupdated paths only after actually updating them rerere: drop want_sp parameter from is_cmarker() rerere: stop looping unnecessarily rerere: explain the rerere I/O abstraction rerere: explain MERGE_RR management helpers rerere: explain the primary codepath rerere: explain rerere forget codepath rerere: explain the remainder rerere: refactor replay part of do_plain_rerere() rerere.c | 389 +++ 1 file changed, 293 insertions(+), 96 deletions(-) -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line 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 v4 01/44] wrapper: implement xopen()
On Mon, Jun 29, 2015 at 12:48 PM, Torsten Bögershausen tbo...@web.de wrote: - Having xopen() with 2 or 3 parameter is good, but the current may need some tweaks for better portability: int xopen(const char *path, int oflag, ...) { mode_t mode = 0; if (oflag O_CREAT) { va_list ap; va_start(ap, oflag); mode = va_arg(ap, int); va_end(ap); } See e.g. http://blitiri.com.ar/git/r/libfiu/c/37f6a98110e3bb59bbb4971241baa3a385c3f724/ why va_arg(ap, int) should be used: Ah OK. I've learned about yet another dark corner of C :-). Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 38/44] builtin-am: support and auto-detect StGit patches
On Tue, Jun 30, 2015 at 5:39 AM, Junio C Hamano gits...@pobox.com wrote: Not using any increment inside isspace(), like you showed, is the most readable. Yup, I agree. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
I'd vote for keeping it simple and not having the copyright notice. Most t/*.sh do not have one. The Git history + mailing-list archives are much better than in-code comments to keep track of who wrote what. Remi: any objection on removing it? Sorry for not having resent the patches myself, I currently have no Internet access, neither at work nor at home... Here's a try on my phone: I though the copyright line was necessary, but I did not know what to write after, and I forgot to ask, so I'm really happy with simply removing it. :) Thanks! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 31/44] builtin-am: implement -S/--gpg-sign, commit.gpgsign
On Tue, Jun 30, 2015 at 7:51 AM, Stefan Beller sbel...@google.com wrote: I needed to read this patch a few times as this patch introduces `sign_commit` twice. This is mostly a review problem I'd guess as in the code it just affects this method and you'd see all the code of the method easily compared to hunks sent via email. But renaming this variable doesn't hurt. OK. What variable name do you want? Would gpgsign (to match commit.gpgsign) do? Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Documentation/i18n.txt: clarify character encoding support
On 07/01/2015 09:10 PM, Karsten Blees wrote: Of course, it would be nice to hear other opinions as well - this probably shouldn't be a discussion between the two of us :-) Karsten I like this paragraf from your previous mail, I think it can go into i18n.txt as is: ISO-8859-x file names may be fine if you won't ever need to: - use git-web, JGit, gitk, git-gui... - exchange repos with normal (UTF-8) Unices, Mac and Windows systems - publish your work on a git hosting service (and expect file and ref names to show up correctly in the web interface) - store the repo on Unicode-based file systems (JFS, Joliet, UDF, exFat, NTFS, HFS+, CIFS...) -- To unsubscribe from this list: send the line 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/4] list-object: add get_commit_count function
Moving commit counting from rev-list into list-object which is a step toward letting git log do counting as well. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- builtin/rev-list.c | 12 ++-- list-objects.c | 14 ++ list-objects.h | 1 + 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index ff84a82..7b091db 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -388,16 +388,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) traverse_commit_list(revs, show_commit, show_object, info); - if (revs.count) { - if (revs.left_right revs.cherry_mark) - printf(%d\t%d\t%d\n, revs.count_left, revs.count_right, revs.count_same); - else if (revs.left_right) - printf(%d\t%d\n, revs.count_left, revs.count_right); - else if (revs.cherry_mark) - printf(%d\t%d\n, revs.count_left + revs.count_right, revs.count_same); - else - printf(%d\n, revs.count_left + revs.count_right); - } + if (revs.count) + get_commit_count(revs); return 0; } diff --git a/list-objects.c b/list-objects.c index 41736d2..6f76301 100644 --- a/list-objects.c +++ b/list-objects.c @@ -234,3 +234,17 @@ void traverse_commit_list(struct rev_info *revs, object_array_clear(revs-pending); strbuf_release(base); } + +void get_commit_count(struct rev_info * revs) { + if (revs-count) { + if (revs-left_right revs-cherry_mark) + printf(%d\t%d\t%d\n, revs-count_left, revs-count_right, revs-count_same); + else if (revs-left_right) + printf(%d\t%d\n, revs-count_left, revs-count_right); + else if (revs-cherry_mark) + printf(%d\t%d\n, revs-count_left + revs-count_right, revs-count_same); + else + printf(%d\n, revs-count_left + revs-count_right); + } + return; +} diff --git a/list-objects.h b/list-objects.h index 136a1da..d28c1f3 100644 --- a/list-objects.h +++ b/list-objects.h @@ -7,5 +7,6 @@ void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, voi typedef void (*show_edge_fn)(struct commit *); void mark_edges_uninteresting(struct rev_info *, show_edge_fn); +void get_commit_count(struct rev_info * revs); #endif -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] log: add --count option to git log
adds --count from git rev-list to git log, without --use-bitmap-index for the moment. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- builtin/log.c | 29 + 1 file changed, 29 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index 8781049..ce6df1e 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -25,6 +25,7 @@ #include version.h #include mailmap.h #include gpg-interface.h +#include list-objects.h /* Set a default date-time format for git log (log.date config variable) */ static const char *default_date_mode = NULL; @@ -317,12 +318,40 @@ static void finish_early_output(struct rev_info *rev) show_early_header(rev, done, n); } +static void show_object(struct object *obj, + const struct name_path *path, const char *component, + void *cb_data) +{ + return; +} + +static void show_commit(struct commit *commit, void *data) +{ + struct rev_info *revs = (struct rev_info *)data; + if (commit-object.flags PATCHSAME) + revs-count_same++; + else if (commit-object.flags SYMMETRIC_LEFT) + revs-count_left++; + else + revs-count_right++; + if (commit-parents) { + free_commit_list(commit-parents); + commit-parents = NULL; + } + free_commit_buffer(commit); +} + static int cmd_log_walk(struct rev_info *rev) { struct commit *commit; int saved_nrl = 0; int saved_dcctc = 0; + if (rev-count) { + prepare_revision_walk(rev); + traverse_commit_list(rev, show_commit, show_object, rev); + get_commit_count(rev); + } if (rev-early_output) setup_early_output(rev); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] git-log: update man documentation for --count
I'm not altogether sure the best way to update the internal usage from git-log -h, but this at least updates the man page. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- Documentation/git-log.txt | 2 ++ Documentation/rev-list-options.txt | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 5692945..0e10e44 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -90,6 +90,8 @@ include::line-range-format.txt[] Paths may need to be prefixed with ``\-- '' to separate them from options or the revision range, when confusion arises. + +:git-log: 1 include::rev-list-options.txt[] include::pretty-formats.txt[] diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 77ac439..f4e15fb 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -797,7 +797,7 @@ This implies the `--topo-order` option by default, but the in between them in that case. If `barrier` is specified, it is the string that will be shown instead of the default one. -ifdef::git-rev-list[] +ifdef::git-log,git-rev-list[] --count:: Print a number stating how many commits would have been listed, and suppress all other output. When used together -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] log --count: added test
added test comparing output between git log --count HEAD and git rev-list --count HEAD Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- t/t4202-log.sh | 7 +++ 1 file changed, 7 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 1b2e981..077952b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' ' test_must_fail git log --graph --no-walk ' +test_expect_success 'log --count' ' + git log --count HEAD actual + git rev-list --count HEAD expect + test_cmp expect actual +' + + test_done -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] pack-protocol.txt: Mark pushee field as optional
On Wed, Jul 1, 2015 at 11:56 AM, Junio C Hamano gits...@pobox.com wrote: Do some clients omit this in the real world? I just sent you privately a trace where this happens using a recent git client. With that in the wild, I think our server is going to have to handle these even if there is a bug and it is fixed promptly. -- To unsubscribe from this list: send the line 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 6/7] pack-protocol.txt: Mark pushee field as optional
Junio C Hamano gits...@pobox.com writes: Dave Borowitz dborow...@google.com writes: send-pack.c omits this field when args-url is null or empty. Fix the protocol specification to match reality. Do some clients omit this in the real world? As you say, send_pack() does omit it if args-url is null or empty, but args is prepared in transport.c as a copy of transport-url when the function is called, and that transport-url is how builtin/push.c reports where it is pushing with: if (verbosity 0) fprintf(stderr, _(Pushing to %s\n), transport-url); So I am somewhat puzzled... Answering myself, the most trivial example is git send-pack ;-) It passes args that has a NULL in the .url field. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Documentation/i18n.txt: clarify character encoding support
As a distributed VCS, git should better define the encodings of its core textual data structures, in particular those that are part of the network protocol. That git is encoding agnostic is only really true for blob objects. E.g. the 'non-NUL bytes' requirement of tree and commit objects excludes UTF-16/32, and the special meaning of '/' in the index file as well as space and linefeed in commit objects eliminates EBCDIC and other non-ASCII encodings. Git expects bytes 0x80 to be pure ASCII, thus CJK encodings that partly overlap with the ASCII range are problematic as well. E.g. fmt_ident() removes trailing 0x5C from user names on the assumption that it is ASCII '\'. However, there are over 200 GBK double byte codes that end in 0x5C. UTF-8 as default encoding on Linux and respective path translations in the Mac and Windows versions have established UTF-8 NFC as de-facto standard for path names. Update the documentation in i18n.txt to reflect the current status-quo. Signed-off-by: Karsten Blees bl...@dcon.de --- Sorry for the delay, got swamped with other stuff... Am 17.06.2015 um 22:45 schrieb Junio C Hamano: ... I am OK to describe pathnames are mangled into UTF-8 NFC on certain filesystems as a warning. I am OK if we encourage the use of UTF-8, especially if a project wants to be forward looking (i.e. it may currently be a monoculture but may become cross platform in the future). I just do not want to see us saying you *must* encode your path in UTF-8 NFC. ... Yes, that is exatly what I said, isn't it? Use whatever works for your project, we do not dictate. IMO we *have* to clearly specify an encoding. This freedom of choice you're proclaiming just does not work in reality. E.g. Git for Windows prior to 1.7.10 recorded file names in Windows system encoding, which was perfectly legitimate according to the documentation. Yet we had numerous bug reports regarding file name encoding problems (you couldn't even share repos across different Windows versions, let alone with Linux / Mac / JGit...). You cannot simply tell users that this is because of Git's superior, flexible design and its their own fault...except of course if you want them to switch to VCSes that *do* properly define their file formats and network protocols - such as subversion or bazaar. (sorry for the sarcasm, couldn't resist) I think its important to realize that specifying an encoding is *not* a limitation - on the contrary: it *enables* us to do things that would be impossible if file names were just uninterpreted sequences of non-NUL bytes. This includes features that are so fundamental that we take them for granted, e.g. displaying file names using *real* characters rather than just octal escapes. I've rewritten the path name paragraph to better describe the problems to expect with legacy encodings. I hope you like this version better. Of course, it would be nice to hear other opinions as well - this probably shouldn't be a discussion between the two of us :-) Karsten Documentation/i18n.txt | 33 +++-- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/Documentation/i18n.txt b/Documentation/i18n.txt index e9a1d5d..2dd79db 100644 --- a/Documentation/i18n.txt +++ b/Documentation/i18n.txt @@ -1,18 +1,31 @@ -At the core level, Git is character encoding agnostic. - - - The pathnames recorded in the index and in the tree objects - are treated as uninterpreted sequences of non-NUL bytes. - What readdir(2) returns are what are recorded and compared - with the data Git keeps track of, which in turn are expected - to be what lstat(2) and creat(2) accepts. There is no such - thing as pathname encoding translation. +Git is to some extent character encoding agnostic. - The contents of the blob objects are uninterpreted sequences of bytes. There is no encoding translation at the core level. - - The commit log messages are uninterpreted sequences of non-NUL - bytes. + - Path names are encoded in UTF-8 normalization form C. This + applies to tree objects, the index file, ref names, as well as + path names in command line arguments, environment variables + and config files (`.git/config` (see linkgit:git-config[1]), + linkgit:gitignore[5], linkgit:gitattributes[5] and + linkgit:gitmodules[5]). ++ +Note that Git at the core level treats path names simply as +sequences of non-NUL bytes, there are no path name encoding +conversions (except on Mac and Windows). Therefore, using +non-ASCII path names will mostly work even on platforms and file +systems that use legacy extended ASCII encodings. However, +repositories created on such systems will not work properly on +UTF-8-based systems (e.g. Linux, Mac, Windows) and vice versa. +Additionally, many Git-based tools simply assume path names to +be UTF-8 and will fail to display other encodings correctly. + + - Commit log messages are typically encoded in UTF-8, but other + extended ASCII
[PATCH v2] Makefile / racy-git.txt: clarify USE_NSEC prerequisites
Signed-off-by: Karsten Blees bl...@dcon.de --- ...just changed wording as you suggested. Documentation/technical/racy-git.txt | 8 ++-- Makefile | 9 + 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Documentation/technical/racy-git.txt b/Documentation/technical/racy-git.txt index 242a044..4a8be4d 100644 --- a/Documentation/technical/racy-git.txt +++ b/Documentation/technical/racy-git.txt @@ -41,13 +41,17 @@ With a `USE_STDEV` compile-time option, `st_dev` is also compared, but this is not enabled by default because this member is not stable on network filesystems. With `USE_NSEC` compile-time option, `st_mtim.tv_nsec` and `st_ctim.tv_nsec` -members are also compared, but this is not enabled by default +members are also compared. On Linux, this is not enabled by default because in-core timestamps can have finer granularity than on-disk timestamps, resulting in meaningless changes when an inode is evicted from the inode cache. See commit 8ce13b0 of git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git ([PATCH] Sync in core time granularity with filesystems, -2005-01-04). +2005-01-04). This patch is included in kernel 2.6.11 and newer, but +only fixes the issue for file systems with exactly 1 ns or 1 s +resolution. Other file systems are still broken in current Linux +kernels (e.g. CEPH, CIFS, NTFS, UDF), see +https://lkml.org/lkml/2015/6/9/714 Racy Git diff --git a/Makefile b/Makefile index 54ec511..46d181a 100644 --- a/Makefile +++ b/Makefile @@ -217,10 +217,11 @@ all:: # as the compiler can crash (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49299) # # Define USE_NSEC below if you want git to care about sub-second file mtimes -# and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and -# it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely -# randomly break unless your underlying filesystem supports those sub-second -# times (my ext3 doesn't). +# and ctimes. Note that you need recent glibc (at least 2.2.4) for this. On +# Linux, kernel 2.6.11 or newer is required for reliable sub-second file times +# on file systems with exactly 1 ns or 1 s resolution. If you intend to use Git +# on other file systems (e.g. CEPH, CIFS, NTFS, UDF), don't enable USE_NSEC. See +# Documentation/technical/racy-git.txt for details. # # Define USE_ST_TIMESPEC if your struct stat uses st_ctimespec instead of # st_ctim -- 2.4.3.windows.1.1.g87477f9 -- To unsubscribe from this list: send the line 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/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Wed, Jul 1, 2015 at 1:00 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: Signed-off-by: Dave Borowitz dborow...@google.com --- Documentation/technical/pack-protocol.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 1386840..2d8b1a1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -534,6 +534,9 @@ A push certificate begins with a set of header lines. After the header and an empty line, the protocol commands follow, one per line. +Note that (unlike other portions of the protocol), all LFs in the +`push-cert` specification above MUST be present. + Currently, the following header fields are defined: `pusher` ident:: I am moderately negative about this; wouldn't it make the end result cleaner to fix the implementation? I'm not sure I understand your suggestion. Are you saying, you would prefer to make LFs optional in the push cert, for consistency with LFs being optional elsewhere? C git servers in the wild already require LFs when extracting the nonce value from the certificate (see find_header). If we make the LFs optional, then a conforming client may not send LFs, which will cause nonce verification to fail when pushing to an unfixed server. That is why I think we are stuck with this. (Also, this is probably not insurmountable, but the cert processing code in receive-pack.c would have to be substantially rewritten if we loosened this requirement. Currently it concatenates the cert contents without pkt-line framing into a buffer, and searches around for \n and \n\n. If LF is optional, then with that approach you might end up with a section of that buffer like: nonce 1234-abcd deadbeefdeadbeefdeadbeefdeadbeefdeadbeef refs/heads/master where it is impossible to distinguish between the end of the nonce and the start of the first command.) -- To unsubscribe from this list: send the line 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] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 9:07 PM, Duy Nguyen pclo...@gmail.com wrote: On Thu, Jul 2, 2015 at 12:13 AM, Eric Sunshine sunsh...@sunshineco.com wrote: I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does not seem to be documented. Is this something we still need? The log message of 529fef20 (checkout: support checking out into a new working directory, 2014-11-30) does not tell us much. Yes, it's still used for the same purpose as before the conversion: as a private signal to the sub git-checkout invocation that it's operating on a new worktree. When defined, it sets the 'new_worktree_mode' flag in checkout.c, and there are still a few bits of code which apparently need to know about it. It would be nice to eliminate this special knowledge from checkout.c, however, I'm not yet familiar enough with the checkout code to determine if doing so is viable. I think it can go away. When --to is used, I have to re-execute git checkout command again after creating the new worktree. I could process the command line arguments from the first execution, delete --to, then use the remaining options to run checkout the second time. But I chose to pass the entire command line to the second execution. The env is used to let the second run know it should ignore --to (or we get infinite recursion). With git worktree add this recursion disappears and this env var has no reason to exist. The recursion protection is indeed no longer needed and gets removed by the worktree add patch. However, there are still a few bits of code which want to know that the checkout is happening in a new worktree. I haven't examined them closely yet to diagnose if this specialized knowledge can be eliminated. Perhaps you can weight in. In particular: checkout_paths: if (opts-new_worktree) die(_('%s' cannot be used with updating paths), --to); merge_working_tree: tree = parse_tree_indirect(old-commit !opts-new_worktree_mode ? old-commit-object.sha1 : EMPTY_TREE_SHA1_BIN); switch_branches: if (!opts-quiet !old.path old.commit new-commit != old.commit !opts-new_worktree_mode) orphaned_commit_warning(old.commit, new-commit); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v2.5.0-rc1
The first release candidate for the upcoming Git v2.5.0 is now available for testing at the usual places. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The following public repositories all have a copy of the 'v2.5.0-rc1' tag and the 'master' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git 2.5 Release Notes (draft) = Updates since v2.4 -- UI, Workflows Features * The bash completion script (in contrib/) learned a few options that git revert takes. * Whitespace breakages in deleted and context lines can also be painted in the output of git diff and friends with the new --ws-error-highlight option. * List of commands shown by git help are grouped along the workflow elements to help early learners. * git p4 now detects the filetype (e.g. binary) correctly even when the files are opened exclusively. * git p4 attempts to better handle branches in Perforce. * git p4 learned --changes-block-size n to read the changes in chunks from Perforce, instead of making one call to p4 changes that may trigger too many rows scanned error from Perforce. * More workaround for Perforce's row number limit in git p4. * Unlike $EDITOR and $GIT_EDITOR that can hold the path to the command and initial options (e.g. /path/to/emacs -nw), 'git p4' did not let the shell interpolate the contents of the environment variable that name the editor $P4EDITOR (and $EDITOR, too). This release makes it in line with the rest of Git, as well as with Perforce. * A new short-hand branch@{push} denotes the remote-tracking branch that tracks the branch at the remote the branch would be pushed to. * git show-branch --topics HEAD (with no other arguments) did not do anything interesting. Instead, contrast the given revision against all the local branches by default. * A replacement for contrib/workdir/git-new-workdir that does not rely on symbolic links and make sharing of objects and refs safer by making the borrowee and borrowers aware of each other. Consider this as still an experimental feature; the UI will likely to change. * Tweak the sample store backend of the credential helper to honor XDG configuration file locations when specified. * A heuristic we use to catch mistyped paths on the command line git cmd revs pathspec is to make sure that all the non-rev parameters in the later part of the command line are names of the files in the working tree, but that means git grep $str -- \*.c must always be disambiguated with --, because nobody sane will create a file whose name literally is asterisk-dot-see. Loosen the heuristic to declare that with a wildcard string the user likely meant to give us a pathspec. * git merge FETCH_HEAD learned that the previous git fetch could be to create an Octopus merge, i.e. recording multiple branches that are not marked as not-for-merge; this allows us to lose an old style invocation git merge msg HEAD $commits... in the implementation of git pull script; the old style syntax can now be deprecated (but not removed yet). * Filter scripts were run with SIGPIPE disabled on the Git side, expecting that they may not read what Git feeds them to filter. We however treated a filter that does not read its input fully before exiting as an error. We no longer do and ignore EPIPE when writing to feed the filter scripts. This changes semantics, but arguably in a good way. If a filter can produce its output without fully consuming its input using whatever magic, we now let it do so, instead of diagnosing it as a programming error. * Instead of dying immediately upon failing to obtain a lock, the locking (of refs etc) retries after a short while with backoff. * Introduce http.url.SSLCipherList configuration variable to tweak the list of cipher suite to be used with libcURL when talking with https:// sites. * git subtree script (in contrib/) used echo -n to produce progress messages in a non-portable way. * git subtree script (in contrib/) does not have --squash option when pushing, but the documentation and help text pretended as if it did. * The Git subcommand completion (in contrib/) no longer lists credential helpers among candidates; they are not something the end user would invoke interactively. * The index file can be taught with update-index --untracked-cache to optionally remember already seen untracked files, in order to speed up git status in a working tree with tons of cruft. * git
What's cooking in git.git (Jul 2015, #01; Wed, 1)
What's cooking in git.git (Jul 2015, #01; Wed, 1) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * da/mergetool-winmerge (2015-06-19) 1 commit (merged to 'next' on 2015-06-24 at 2fb10c4) + mergetool-lib: fix default tool selection Hotfix for an earlier change already in 'master' that broke the default tool selection for mergetool. * jc/prompt-document-ps1-state-separator (2015-06-10) 1 commit (merged to 'next' on 2015-06-24 at e4d1bad) + git-prompt.sh: document GIT_PS1_STATESEPARATOR Docfix. * me/fetch-into-shallow-safety (2015-06-17) 1 commit (merged to 'next' on 2015-06-24 at 8ecc19a) + fetch-pack: check for shallow if depth given git fetch --depth=depth and git clone --depth=depth issued a shallow transfer request even to an upload-pack that does not support the capability. * mm/describe-doc (2015-06-16) 1 commit (merged to 'next' on 2015-06-24 at 75e34cc) + Documentation/describe: improve one-line summary Docfix. -- [New Topics] * jk/rev-list-no-bitmap-while-pruning (2015-07-01) 1 commit - rev-list: disable --use-bitmap-index when pruning commits A minor bugfix when pack bitmap was brought in. Will merge to 'next'. * kb/config-unmap-before-renaming (2015-06-30) 1 commit - config.c: fix writing config files on Windows network shares Will merge to 'next'. * ls/hint-rev-list-count (2015-07-01) 1 commit - rev-list: add --count to usage guide Will merge to 'next'. * mh/fast-import-get-mark (2015-07-01) 1 commit - fast-import: add a get-mark command Will merge to 'next'. * nd/dwim-wildcards-as-pathspecs (2015-07-01) 1 commit - Add tests for wildcard path vs ref disambiguation Will merge to 'next' and then to 'master'. -- [Stalled] * sg/config-name-only (2015-05-28) 3 commits - completion: use new 'git config' options to reliably list variable names - SQUASH - config: add options to list only variable names git config --list output was hard to parse when values consist of multiple lines. Introduce a way to show only the keys. Adding a single --name-only option may be a better way to go than adding two new options. Expecting a reroll. * kk/log-merges-config (2015-04-21) 5 commits - bash-completion: add support for git-log --merges= and log.merges - t4202-log: add tests for --merges= - Documentation: add git-log --merges= option and log.merges config. var - log: honor log.merges= option - revision: add --merges={show|only|hide} option git log (but not other commands in the log family) learned to pay attention to the log.merges configuration variable that can be set to show (the normal behaviour), only (hide non-merge commits), or hide (hide merge commits). --merges=(show|only|hide) can be used to override the setting from the command line. The documentation may need to be updated once more ($gmane/267250). Waiting for a reroll. * mg/httpd-tests-update-for-apache-2.4 (2015-04-08) 2 commits - t/lib-git-svn: check same httpd module dirs as lib-httpd - t/lib-httpd: load mod_unixd This is the first two commits in a three-patch series $gmane/266962 Will be rerolled. with updated log message ($gmane/268061). * mh/numparse (2015-03-19) 14 commits - diff_opt_parse(): use convert_i() when handling --abbrev=num - diff_opt_parse(): use convert_i() when handling -lnum - opt_arg(): simplify pointer handling - opt_arg(): report errors parsing option values - opt_arg(): use convert_i() in implementation - opt_arg(): val is always non-NULL - builtin_diff(): detect errors when parsing --unified argument - handle_revision_opt(): use convert_ui() when handling --abbrev= - strtoul_ui(), strtol_i(): remove functions - handle_revision_opt(): use convert_i() when handling -digit - handle_revision_opt(): use skip_prefix() in many places - write_subdirectory(): use convert_ui() for parsing mode - cacheinfo_callback(): use convert_ui() when handling --cacheinfo - numparse: new module for parsing integral numbers Many codepaths use unchecked use of strtol() and friends (or even worse, atoi()). Introduce a set of wrappers that try to be more careful. Expecting a reroll. ($gmane/268058). * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter - gitweb: fix typo in man page
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 6:02 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Jun 30, 2015 at 5:23 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Jun 30, 2015 at 11:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I could do git co -b foo --to bar for example. You can still do that with git worktree new bar -b foo, which is effectively the same as git checkout --to bar -b foo (with s/checkout/worktree/ and s/--to/new/ applied), though perhaps you don't find it as obvious or natural. I had never understood why you chose to plug the linked-worktree functionality into git-checkout via --to, but this usage pattern (creating a new branch and checking it out into a new worktree as one operation) goes a long way toward explaining why you consider git-checkout a proper home for linked-worktree creation. I don't think that justification was ever mentioned when the series was being presented (or, if it was, I must have missed it). Now it makes much more sense, and I can better appreciate your desire to keep git checkout --to as an alias for git worktree add. Thanks for explaining it. (Having said that, replacing git checkout --to with git worktree add still seems a preferable first step, while keeping open the door to re-add git checkout --to later if we become convinced that it's worthwhile.) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/13] rerere: refactor replay part of do_plain_rerere()
Extract the body of a loop that attempts to replay recorded resolution for each conflicted path into a helper function, not because I want to call it from multiple places later, but because the logic has become too deeply nested and hard to read. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 75 ++-- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/rerere.c b/rerere.c index 7ef951e..09b72ed 100644 --- a/rerere.c +++ b/rerere.c @@ -620,6 +620,44 @@ static void update_paths(struct string_list *update) rollback_lock_file(index_lock); } +/* + * The path indicated by rr_item may still have conflict for which we + * have a recorded resolution, in which case replay it and optionally + * update it. Or it may have been resolved by the user and we may + * only have the preimage for that conflict, in which case the result + * needs to be recorded as a resolution in a postimage file. + */ +static void do_rerere_one_path(struct string_list_item *rr_item, + struct string_list *update) +{ + const char *path = rr_item-string; + const char *name = (const char *)rr_item-util; + + /* Is there a recorded resolution we could attempt to apply? */ + if (has_rerere_resolution(name)) { + if (merge(name, path)) + return; /* failed to replay */ + + if (rerere_autoupdate) + string_list_insert(update, path); + else + fprintf(stderr, + Resolved '%s' using previous resolution.\n, + path); + goto mark_resolved; + } + + /* Let's see if the user has resolved it. */ + if (handle_file(path, NULL, NULL)) + return; /* not yet resolved */ + + copy_file(rerere_path(name, postimage), path, 0666); + fprintf(stderr, Recorded resolution for '%s'.\n, path); +mark_resolved: + free(rr_item-util); + rr_item-util = NULL; +} + static int do_plain_rerere(struct string_list *rr, int fd) { struct string_list conflict = STRING_LIST_INIT_DUP; @@ -673,41 +711,8 @@ static int do_plain_rerere(struct string_list *rr, int fd) } } - /* -* Some of the paths that had conflicts earlier might have -* been resolved by the user. Others may be similar to a -* conflict already that was resolved before. -*/ - for (i = 0; i rr-nr; i++) { - int ret; - const char *path = rr-items[i].string; - const char *name = (const char *)rr-items[i].util; - - /* Is there a recorded resolution we could attempt to apply? */ - if (has_rerere_resolution(name)) { - if (merge(name, path)) - continue; - - if (rerere_autoupdate) - string_list_insert(update, path); - else - fprintf(stderr, - Resolved '%s' using previous resolution.\n, - path); - goto mark_resolved; - } - - /* Let's see if the user has resolved it. */ - ret = handle_file(path, NULL, NULL); - if (ret) - continue; - - copy_file(rerere_path(name, postimage), path, 0666); - fprintf(stderr, Recorded resolution for '%s'.\n, path); - mark_resolved: - free(rr-items[i].util); - rr-items[i].util = NULL; - } + for (i = 0; i rr-nr; i++) + do_rerere_one_path(rr-items[i], update); if (update.nr) update_paths(update); -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/13] rerere: write out each record of MERGE_RR in one go
Instead of writing the hash for a conflict, a HT, and the path with three separate write_in_full() calls, format them into a single record into a strbuf and write it out in one go. As a more recent rerere remaining codepath abuses the .util field of the merge_rr data to store a sentinel token, make sure that codepath does not call into this function (of course, remaining is a read-only operation and currently does not call it). Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/rerere.c b/rerere.c index e854985..27b287d 100644 --- a/rerere.c +++ b/rerere.c @@ -65,16 +65,18 @@ static int write_rr(struct string_list *rr, int out_fd) { int i; for (i = 0; i rr-nr; i++) { - const char *path; - int length; + struct strbuf buf = STRBUF_INIT; + + assert(rr-items[i].util != RERERE_RESOLVED); if (!rr-items[i].util) continue; - path = rr-items[i].string; - length = strlen(path) + 1; - if (write_in_full(out_fd, rr-items[i].util, 40) != 40 || - write_str_in_full(out_fd, \t) != 1 || - write_in_full(out_fd, path, length) != length) + strbuf_addf(buf, %s\t%s%c, + (char *)rr-items[i].util, + rr-items[i].string, 0); + if (write_in_full(out_fd, buf.buf, buf.len) != buf.len) die(unable to write rerere record); + + strbuf_release(buf); } if (commit_lock_file(write_lock) != 0) die(unable to write rerere record); -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line 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 v4 01/44] wrapper: implement xopen()
On Tue, Jun 30, 2015 at 1:18 AM, Junio C Hamano gits...@pobox.com wrote: Torsten Bögershausen tbo...@web.de writes: 2 remarks: - I don't know if and why we need the assert() here (but don't know if we have a strategie in Git for assert()) There is no bright-line rules, but I think it is sensible to remove this. Nobody sane would throw a NULL at open(2) and xopen() is supposed to imitate that interface. We do protect ourselves from careless use of our own API, but no need to clutter the code with overly zealous check against insane code, I would say. Ah, okay. Will remove these assert()s. Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/13] rerere: stop looping unnecessarily
handle_cache() loops 3 times starting from an index entry that is unmerged, while ignoring an entry for a path that is different from what we are looking for. As the index is sorted, once we see a different path, we know we saw all stages for the path we are interested in. Just loop while we see the same path and then break, instead of continuing for 3 times. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/rerere.c b/rerere.c index 4c45f55..7b1419c 100644 --- a/rerere.c +++ b/rerere.c @@ -329,24 +329,21 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu return -1; pos = -pos - 1; - for (i = 0; i 3; i++) { + while (pos active_nr) { enum object_type type; unsigned long size; - int j; - if (active_nr = pos) - break; ce = active_cache[pos++]; if (ce_namelen(ce) != len || memcmp(ce-name, path, len)) - continue; - j = ce_stage(ce) - 1; - mmfile[j].ptr = read_sha1_file(ce-sha1, type, size); - mmfile[j].size = size; + break; + i = ce_stage(ce) - 1; + mmfile[i].ptr = read_sha1_file(ce-sha1, type, size); + mmfile[i].size = size; } - for (i = 0; i 3; i++) { + for (i = 0; i 3; i++) if (!mmfile[i].ptr !mmfile[i].size) mmfile[i].ptr = xstrdup(); - } + /* * NEEDSWORK: handle conflicts from merges with * merge.renormalize set, too -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/13] rerere: lift PATH_MAX limitation
The MERGE_RR file records a collection of NUL-terminated entries, each of which consists of - a hash that identifies the conflict - a HT - the pathname We used to read this piece-by-piece, and worse yet, read the pathname part a byte at a time into a fixed buffer of size PATH_MAX. Instead, read a whole entry using strbuf_getwholeline() and parse out the fields. This way, we issue fewer read(2) calls and more importantly we do not have to limit the pathname to PATH_MAX. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 35 +++ 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/rerere.c b/rerere.c index 3b9104d..e854985 100644 --- a/rerere.c +++ b/rerere.c @@ -35,32 +35,27 @@ static int has_rerere_resolution(const char *hex) static void read_rr(struct string_list *rr) { - unsigned char sha1[20]; - char buf[PATH_MAX]; + struct strbuf buf = STRBUF_INIT; FILE *in = fopen(merge_rr_path, r); + if (!in) return; - while (fread(buf, 40, 1, in) == 1) { - int i; - char *name; - if (get_sha1_hex(buf, sha1)) + while (!strbuf_getwholeline(buf, in, '\0')) { + char *path; + unsigned char sha1[20]; + + /* There has to be the hash, tab, path and then NUL */ + if (buf.len 42 || get_sha1_hex(buf.buf, sha1)) die(corrupt MERGE_RR); - buf[40] = '\0'; - name = xstrdup(buf); - if (fgetc(in) != '\t') + + if (buf.buf[40] != '\t') die(corrupt MERGE_RR); - for (i = 0; i sizeof(buf); i++) { - int c = fgetc(in); - if (c 0) - die(corrupt MERGE_RR); - buf[i] = c; - if (c == 0) -break; - } - if (i == sizeof(buf)) - die(filename too long); - string_list_insert(rr, buf)-util = name; + buf.buf[40] = '\0'; + path = buf.buf + 41; + + string_list_insert(rr, path)-util = xstrdup(buf.buf); } + strbuf_release(buf); fclose(in); } -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/13] rerere: explain the rerere I/O abstraction
Explain the internals of rerere as in-code comments. This one covers our thin I/O abstraction to read from either a file or a memory while optionally writing out to a file. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 38 +++--- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/rerere.c b/rerere.c index 7b1419c..7ed20f1 100644 --- a/rerere.c +++ b/rerere.c @@ -83,6 +83,21 @@ static int write_rr(struct string_list *rr, int out_fd) return 0; } +/* + * rerere interacts with conflicted file contents using this I/O + * abstraction. It reads a conflicted contents from one place via + * getline() method, and optionally can write it out after + * normalizing the conflicted hunks to the output. Subclasses of + * rerere_io embed this structure at the beginning of their own + * rerere_io object. + */ +struct rerere_io { + int (*getline)(struct strbuf *, struct rerere_io *); + FILE *output; + int wrerror; + /* some more stuff */ +}; + static void ferr_write(const void *p, size_t count, FILE *fp, int *err) { if (!count || *err) @@ -96,19 +111,15 @@ static inline void ferr_puts(const char *s, FILE *fp, int *err) ferr_write(s, strlen(s), fp, err); } -struct rerere_io { - int (*getline)(struct strbuf *, struct rerere_io *); - FILE *output; - int wrerror; - /* some more stuff */ -}; - static void rerere_io_putstr(const char *str, struct rerere_io *io) { if (io-output) ferr_puts(str, io-output, io-wrerror); } +/* + * Write a conflict marker to io-output (if defined). + */ static void rerere_io_putconflict(int ch, int size, struct rerere_io *io) { char buf[64]; @@ -137,11 +148,17 @@ static void rerere_io_putmem(const char *mem, size_t sz, struct rerere_io *io) ferr_write(mem, sz, io-output, io-wrerror); } +/* + * Subclass of rerere_io that reads from an on-disk file + */ struct rerere_io_file { struct rerere_io io; FILE *input; }; +/* + * ... and its getline() method implementation + */ static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_) { struct rerere_io_file *io = (struct rerere_io_file *)io_; @@ -286,11 +303,18 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output return hunk_no; } +/* + * Subclass of rerere_io that reads from an in-core buffer that is a + * strbuf + */ struct rerere_io_mem { struct rerere_io io; struct strbuf input; }; +/* + * ... and its getline() method implementation + */ static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_) { struct rerere_io_mem *io = (struct rerere_io_mem *)io_; -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/13] rerere: drop want_sp parameter from is_cmarker()
As the nature of the conflict marker line determies if there should a SP and label after it, the caller shouldn't have to pass the parameter redundantly. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 27 ++- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/rerere.c b/rerere.c index 304df02..4c45f55 100644 --- a/rerere.c +++ b/rerere.c @@ -148,8 +148,25 @@ static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_) return strbuf_getwholeline(sb, io-input, '\n'); } -static int is_cmarker(char *buf, int marker_char, int marker_size, int want_sp) +/* + * Require the exact number of conflict marker letters, no more, no + * less, followed by SP or any whitespace + * (including LF). + */ +static int is_cmarker(char *buf, int marker_char, int marker_size) { + int want_sp; + + /* +* The beginning of our version and the end of their version +* always are labeled like ours or theirs, +* hence we set want_sp for them. Note that the version from +* the common ancestor in diff3-style output is not always +* labelled (e.g. | common is often seen but | +* alone is also valid), so we do not set want_sp. +*/ + want_sp = (marker_char == '') || (marker_char == ''); + while (marker_size--) if (*buf++ != marker_char) return 0; @@ -172,19 +189,19 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz git_SHA1_Init(ctx); while (!io-getline(buf, io)) { - if (is_cmarker(buf.buf, '', marker_size, 1)) { + if (is_cmarker(buf.buf, '', marker_size)) { if (hunk != RR_CONTEXT) goto bad; hunk = RR_SIDE_1; - } else if (is_cmarker(buf.buf, '|', marker_size, 0)) { + } else if (is_cmarker(buf.buf, '|', marker_size)) { if (hunk != RR_SIDE_1) goto bad; hunk = RR_ORIGINAL; - } else if (is_cmarker(buf.buf, '=', marker_size, 0)) { + } else if (is_cmarker(buf.buf, '=', marker_size)) { if (hunk != RR_SIDE_1 hunk != RR_ORIGINAL) goto bad; hunk = RR_SIDE_2; - } else if (is_cmarker(buf.buf, '', marker_size, 1)) { + } else if (is_cmarker(buf.buf, '', marker_size)) { if (hunk != RR_SIDE_2) goto bad; if (strbuf_cmp(one, two) 0) -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/13] rerere: explain the primary codepath
Explain the internals of rerere as in-code comments, while sprinkling NEEDSWORK comment to highlight iffy bits and questionable assumptions. This one covers the codepath reached from rerere(), the primary interface to the subsystem. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 95 +++- 1 file changed, 82 insertions(+), 13 deletions(-) diff --git a/rerere.c b/rerere.c index d54bdb2..3d9c33b 100644 --- a/rerere.c +++ b/rerere.c @@ -199,6 +199,21 @@ static int is_cmarker(char *buf, int marker_char, int marker_size) return isspace(*buf); } +/* + * Read contents a file with conflicts, normalize the conflicts + * by (1) discarding the common ancestor version in diff3-style, + * (2) reordering our side and their side so that whichever sorts + * alphabetically earlier comes before the other one, while + * computing the conflict ID, which is just an SHA-1 hash of + * one side of the conflict, NUL, the other side of the conflict, + * and NUL concatenated together. + * + * Return the number of conflict hunks found. + * + * NEEDSWORK: the logic and theory of operation behind this conflict + * normalization may deserve to be documented somewhere, perhaps in + * Documentation/technical/rerere.txt. + */ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_size) { git_SHA_CTX ctx; @@ -269,6 +284,10 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz return hunk_no; } +/* + * Scan the path for conflicts, do the handle_path() thing above, and + * return the number of conflict hunks found. + */ static int handle_file(const char *path, unsigned char *sha1, const char *output) { int hunk_no = 0; @@ -506,29 +525,54 @@ int rerere_remaining(struct string_list *merge_rr) return 0; } +/* + * Find the conflict identified by name; the change between its + * preimage (i.e. a previous contents with conflict markers) and its + * postimage (i.e. the corresponding contents with conflicts + * resolved) may apply cleanly to the contents stored in path, i.e. + * the conflict this time around. + * + * Returns 0 for successful replay of recorded resolution, or non-zero + * for failure. + */ static int merge(const char *name, const char *path) { int ret; mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0}; mmbuffer_t result = {NULL, 0}; + /* +* Normalize the conflicts in path and write it out to +* thisimage temporary file. +*/ if (handle_file(path, NULL, rerere_path(name, thisimage)) 0) return 1; if (read_mmfile(cur, rerere_path(name, thisimage)) || - read_mmfile(base, rerere_path(name, preimage)) || - read_mmfile(other, rerere_path(name, postimage))) { + read_mmfile(base, rerere_path(name, preimage)) || + read_mmfile(other, rerere_path(name, postimage))) { ret = 1; goto out; } + + /* +* A three-way merge. Note that this honors user-customizable +* low-level merge driver settings. +*/ ret = ll_merge(result, path, base, NULL, cur, , other, , NULL); if (!ret) { FILE *f; + /* +* A successful replay of recorded resolution. +* Mark that postimage was used to help gc. +*/ if (utime(rerere_path(name, postimage), NULL) 0) warning(failed utime() on %s: %s, rerere_path(name, postimage), strerror(errno)); + + /* Update path with the resolution */ f = fopen(path, w); if (!f) return error(Could not open %s: %s, path, @@ -581,41 +625,61 @@ static int do_plain_rerere(struct string_list *rr, int fd) find_conflict(conflict); /* -* MERGE_RR records paths with conflicts immediately after merge -* failed. Some of the conflicted paths might have been hand resolved -* in the working tree since then, but the initial run would catch all -* and register their preimages. +* MERGE_RR records paths with conflicts immediately after +* merge failed. Some of the conflicted paths might have been +* hand resolved in the working tree since then, but the +* initial run would catch all and register their preimages. */ - for (i = 0; i conflict.nr; i++) { const char *path = conflict.items[i].string; if (!string_list_has_string(rr, path)) { unsigned char sha1[20]; char *hex; int ret; + + /* +* Ask handle_file() to scan and
[PATCH v2 01/13] rerere: fix an off-by-one non-bug
When ac49f5ca (rerere remaining, 2011-02-16) split out a new helper function check_one_conflict() out of find_conflict() function, so that the latter will use the returned value from the new helper to update the loop control variable that is an index into active_cache[], the new variable incremented the index by one too many when it found a path with only stage #1 entry at the very end of active_cache[]. This strange return value does not have any effect on the loop control of two callers of this function, as they all notice that active_nr+2 is larger than active_nr just like active_nr+1 is, but nevertheless it puzzles the readers when they are trying to figure out what the function is trying to do. In fact, there is no need to do an early return. The code that follows after skipping the stage #1 entry is fully prepared to handle a case where the entry is at the very end of active_cache[]. Help future readers from unnecessary confusion by dropping an early return. We skip the stage #1 entry, and if there are stage #2 and stage #3 entries for the same path, we diagnose the path as THREE_STAGED (otherwise we say PUNTED), and then we skip all entries for the same path. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rerere.c b/rerere.c index 31644de..e307711 100644 --- a/rerere.c +++ b/rerere.c @@ -369,10 +369,8 @@ static int check_one_conflict(int i, int *type) } *type = PUNTED; - if (ce_stage(e) == 1) { - if (active_nr = ++i) - return i + 1; - } + if (ce_stage(e) == 1) + i++; /* Only handle regular files with both stages #2 and #3 */ if (i + 1 active_nr) { -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/13] rerere: explain MERGE_RR management helpers
Explain the internals of rerere as in-code comments, while sprinkling NEEDSWORK comment to highlight iffy bits and questionable assumptions. This one covers the $GIT_DIR/MERGE_RR file and in-core merge_rr that are used to keep track of the status of rerere session in progress. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 41 + 1 file changed, 41 insertions(+) diff --git a/rerere.c b/rerere.c index 7ed20f1..d54bdb2 100644 --- a/rerere.c +++ b/rerere.c @@ -33,6 +33,13 @@ static int has_rerere_resolution(const char *hex) return !stat(rerere_path(hex, postimage), st); } +/* + * $GIT_DIR/MERGE_RR file is a collection of records, each of which is + * conflict ID, a HT and pathname, terminated with a NUL, and is + * used to keep track of the set of paths that rerere may need to + * work on (i.e. what is left by the previous invocation of git + * rerere during the current conflict resolution session). + */ static void read_rr(struct string_list *rr) { struct strbuf buf = STRBUF_INIT; @@ -394,6 +401,14 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu return hunk_no; } +/* + * Look at a cache entry at i and see if it is not conflicting, + * conflicting and we are willing to handle, or conflicting and + * we are unable to handle, and return the determination in *type. + * Return the cache index to be looked at next, by skipping the + * stages we have already looked at in this invocation of this + * function. + */ static int check_one_conflict(int i, int *type) { const struct cache_entry *e = active_cache[i]; @@ -425,6 +440,17 @@ static int check_one_conflict(int i, int *type) return i; } +/* + * Scan the index and find paths that have conflicts that rerere can + * handle, i.e. the ones that has both stages #2 and #3. + * + * NEEDSWORK: we do not record or replay a previous resolve by + * deletion for a delete-modify conflict, as that is inherently risky + * without knowing what modification is being discarded. The only + * safe case, i.e. both side doing the deletion and modification that + * are identical to the previous round, might want to be handled, + * though. + */ static int find_conflict(struct string_list *conflict) { int i; @@ -441,6 +467,21 @@ static int find_conflict(struct string_list *conflict) return 0; } +/* + * The merge_rr list is meant to hold outstanding conflicted paths + * that rerere could handle. Abuse the list by adding other types of + * entries to allow the caller to show rerere remaining. + * + * - Conflicted paths that rerere does not handle are added + * - Conflicted paths that have been resolved are marked as such + * by storing RERERE_RESOLVED to .util field (where conflict ID + * is expected to be stored). + * + * Do *not* write MERGE_RR file out after calling this function. + * + * NEEDSWORK: we may want to fix the caller that implements rerere + * remaining to do this without abusing merge_rr. + */ int rerere_remaining(struct string_list *merge_rr) { int i; -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/13] rerere: explain rerere forget codepath
Explain the internals of rerere as in-code comments, while sprinkling NEEDSWORK comment to highlight iffy bits and questionable assumptions. This covers the codepath that implements rerere forget. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 24 1 file changed, 24 insertions(+) diff --git a/rerere.c b/rerere.c index 3d9c33b..3782be6 100644 --- a/rerere.c +++ b/rerere.c @@ -413,6 +413,10 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu strbuf_init(io.input, 0); strbuf_attach(io.input, result.ptr, result.size, result.size); + /* +* Grab the conflict ID and optionally write the original +* contents with conflict markers out. +*/ hunk_no = handle_path(sha1, (struct rerere_io *)io, marker_size); strbuf_release(io.input); if (io.io.output) @@ -777,9 +781,15 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr) int ret; struct string_list_item *item; + /* +* Recreate the original conflict from the stages in the +* index and compute the conflict ID +*/ ret = handle_cache(path, sha1, NULL); if (ret 1) return error(Could not parse conflict hunks in '%s', path); + + /* Nuke the recorded resolution for the conflict */ hex = xstrdup(sha1_to_hex(sha1)); filename = rerere_path(hex, postimage); if (unlink(filename)) @@ -787,9 +797,18 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr) ? error(no remembered resolution for %s, path) : error(cannot unlink %s: %s, filename, strerror(errno))); + /* +* Update the preimage so that the user can resolve the +* conflict in the working tree, run us again to record +* the postimage. +*/ handle_cache(path, sha1, rerere_path(hex, preimage)); fprintf(stderr, Updated preimage for '%s'\n, path); + /* +* And remember that we can record resolution for this +* conflict when the user is done. +*/ item = string_list_insert(rr, path); free(item-util); item-util = hex; @@ -808,6 +827,11 @@ int rerere_forget(struct pathspec *pathspec) fd = setup_rerere(merge_rr, RERERE_NOAUTOUPDATE); + /* +* The paths may have been resolved (incorrectly); +* recover the original conflicted state and then +* find the conflicted paths. +*/ unmerge_cache(pathspec); find_conflict(conflict); for (i = 0; i conflict.nr; i++) { -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/13] rerere: plug conflict ID leaks
The merge_rr string list stores the conflict ID (a hexadecimal string that is used to index into $GIT_DIR/rr-cache) in the .util field of its elements, and when do_plain_rerere() resolves a conflict, the field is cleared. Also, when rerere_forget() recomputes the conflict ID to updates the preimage file, the conflict ID for the path is updated. We forgot to free the existing conflict ID when we did these two operations. Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rerere.c b/rerere.c index e307711..3b9104d 100644 --- a/rerere.c +++ b/rerere.c @@ -559,6 +559,7 @@ static int do_plain_rerere(struct string_list *rr, int fd) fprintf(stderr, Recorded resolution for '%s'.\n, path); copy_file(rerere_path(name, postimage), path, 0666); mark_resolved: + free(rr-items[i].util); rr-items[i].util = NULL; } @@ -627,6 +628,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr) char *hex; unsigned char sha1[20]; int ret; + struct string_list_item *item; ret = handle_cache(path, sha1, NULL); if (ret 1) @@ -641,8 +643,9 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr) handle_cache(path, sha1, rerere_path(hex, preimage)); fprintf(stderr, Updated preimage for '%s'\n, path); - - string_list_insert(rr, path)-util = hex; + item = string_list_insert(rr, path); + free(item-util); + item-util = hex; fprintf(stderr, Forgot resolution for %s\n, path); return 0; } -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/13] rerere: report autoupdated paths only after actually updating them
Signed-off-by: Junio C Hamano gits...@pobox.com --- rerere.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/rerere.c b/rerere.c index 27b287d..304df02 100644 --- a/rerere.c +++ b/rerere.c @@ -482,6 +482,8 @@ static void update_paths(struct string_list *update) struct string_list_item *item = update-items[i]; if (add_file_to_cache(item-string, 0)) exit(128); + fprintf(stderr, Staged '%s' using previous resolution.\n, + item-string); } if (active_cache_changed) { @@ -536,16 +538,16 @@ static int do_plain_rerere(struct string_list *rr, int fd) const char *name = (const char *)rr-items[i].util; if (has_rerere_resolution(name)) { - if (!merge(name, path)) { - const char *msg; - if (rerere_autoupdate) { - string_list_insert(update, path); - msg = Staged '%s' using previous resolution.\n; - } else - msg = Resolved '%s' using previous resolution.\n; - fprintf(stderr, msg, path); - goto mark_resolved; - } + if (merge(name, path)) + continue; + + if (rerere_autoupdate) + string_list_insert(update, path); + else + fprintf(stderr, + Resolved '%s' using previous resolution.\n, + path); + goto mark_resolved; } /* Let's see if we have resolved it. */ -- 2.5.0-rc0-209-g5e1f148 -- To unsubscribe from this list: send the line 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 v4 43/44] builtin-am: check for valid committer ident
On Tue, Jun 30, 2015 at 4:02 AM, Stefan Beller sbel...@google.com wrote: On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan pyoka...@gmail.com wrote: When commit_tree() is called, if the user does not have an explicit committer ident configured, it will attempt to construct a default committer ident based on the user's and system's info (e.g. gecos field, hostname etc.) However, if a default committer ident is unable to be constructed, commit_tree() will die(). However, at this point of git-am, s/. However,/, but/ ? Yeah, thanks. Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 12:13 AM, Eric Sunshine sunsh...@sunshineco.com wrote: I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does not seem to be documented. Is this something we still need? The log message of 529fef20 (checkout: support checking out into a new working directory, 2014-11-30) does not tell us much. Yes, it's still used for the same purpose as before the conversion: as a private signal to the sub git-checkout invocation that it's operating on a new worktree. When defined, it sets the 'new_worktree_mode' flag in checkout.c, and there are still a few bits of code which apparently need to know about it. It would be nice to eliminate this special knowledge from checkout.c, however, I'm not yet familiar enough with the checkout code to determine if doing so is viable. I think it can go away. When --to is used, I have to re-execute git checkout command again after creating the new worktree. I could process the command line arguments from the first execution, delete --to, then use the remaining options to run checkout the second time. But I chose to pass the entire command line to the second execution. The env is used to let the second run know it should ignore --to (or we get infinite recursion). With git worktree add this recursion disappears and this env var has no reason to exist. -- 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