Re: [PATCH v2 13/23] worktree: introduce add command
On Sat, Jul 4, 2015 at 3:54 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Fri, Jul 3, 2015 at 10:53 PM, Duy Nguyen pclo...@gmail.com wrote: On Sat, Jul 4, 2015 at 7:17 AM, Eric Sunshine sunsh...@sunshineco.com wrote: COMMANDS +add path branch:: + +Check out `branch` into a separate working directory, `path`, creating +`path` if necessary. The new working directory is linked to the current +repository, sharing everything except working directory specific files +such as HEAD, index, etc. If `path` already exists, it must be empty. Side note, must be empty is an implementation limitation. I think the two-way merge employed by git-checkout can deal with dirty path and only perform the checkout if there is no data loss. But we can leave this for later. Perhaps we should omit the bit about an existing but empty directory for now? Like this: Create `path` and checkout `branch` into it. The new working directory is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. I think either way is ok. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase: return non-zero error code if format-patch fails
On Fri, Jul 03, 2015 at 10:52:32AM -0700, Junio C Hamano wrote: Cc: Andrew Wong andrew.k...@gmail.com Signed-off-by: Clemens Buchacher clemens.buchac...@intel.com Reviewed-by: Jorge Nunes jorge.nu...@intel.com Where was this review made? I may have missed a recent discussion, and that is why I am asking, because Reviewed-by: lines that cannot be validated by going back to the list archive does not add much value. Jorge helped me by reviewing the patch before I submitted it to the list. My intention is to give credit for his contribution, and to involve him in any discussion regarding the patch. Maybe it makes more sense to say Helped-by:? Please feel free to change as you see fit. I will follow your recommendation in the future. 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] l10n: de.po: translate 65 new messages
Hi, This message is the title of a listing of git commands, that are available from $PATH but not located in git's exec path. Vorhandene Git-Kommandos in '/usr/libexec/git-core' add merge-octopus ... Vorhandene Git-Kommandos irgendwo in Ihrem $PATH imerge ... What about changing it to Vorhandene Git-Kommandos anderswo in Ihrem $PATH? Yeah that's good. Phillip -- To unsubscribe from this list: send the line 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] grep: use regcomp() for icase search with non-ascii patterns
Noticed-by: Plamen Totev plamen.to...@abv.bg Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- grep.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/grep.c b/grep.c index b58c7c6..48db15a 100644 --- a/grep.c +++ b/grep.c @@ -378,7 +378,7 @@ static void free_pcre_regexp(struct grep_pat *p) } #endif /* !USE_LIBPCRE */ -static int is_fixed(const char *s, size_t len) +static int is_fixed(const char *s, size_t len, int ignore_icase) { size_t i; @@ -391,6 +391,13 @@ static int is_fixed(const char *s, size_t len) for (i = 0; i len; i++) { if (is_regex_special(s[i])) return 0; + /* +* The builtin substring search can only deal with case +* insensitivity in ascii range. If there is something outside +* of that range, fall back to regcomp. +*/ + if (ignore_icase (unsigned char)s[i] = 128) + return 0; } return 1; @@ -398,18 +405,19 @@ static int is_fixed(const char *s, size_t len) static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) { + int ignore_icase = opt-regflags REG_ICASE || p-ignore_case; int err; p-word_regexp = opt-word_regexp; p-ignore_case = opt-ignore_case; - if (opt-fixed || is_fixed(p-pattern, p-patternlen)) + if (opt-fixed || is_fixed(p-pattern, p-patternlen, ignore_icase)) p-fixed = 1; else p-fixed = 0; if (p-fixed) { - if (opt-regflags REG_ICASE || p-ignore_case) + if (ignore_case) p-kws = kwsalloc(tolower_trans_tbl); else p-kws = kwsalloc(NULL); -- 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
Git grep does not support multi-byte characters (like UTF-8)
Hello, It looks like the git grep command does not support multi-byte character sets like UTF-8. As a result some of the grep functionality is not working. For example if you search for non Latin words the ignore case flag does not have effect(the search is case sensitive). I suspect there are some regular expressions that will not work as expected too. When I'm using the git from the shell I could use the GNU grep utility instead. But some tools like gitweb use git grep so they are not working properly with UTF-8 files with non Latin characters. Much of the grep code seems to be copied from the GNU grep utility but the multi-byte support is left out. I just wondered what could be the reason. Regards, Plamen Totev -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git grep does not support multi-byte characters (like UTF-8)
On Mon, Jul 6, 2015 at 6:28 PM, Plamen Totev plamen.to...@abv.bg wrote: Hello, It looks like the git grep command does not support multi-byte character sets like UTF-8. As a result some of the grep functionality is not working. For example if you search for non Latin words the ignore case flag does not have effect(the search is case sensitive). I suspect there are some regular expressions that will not work as expected too. I think we over-optimized a bit. If you your system provides regex with locale support (e.g. Linux) and you don't explicitly use fallback regex implementation, it should work. I suppose your test patterns look fixed (i.e. no regex special characters)? Can you try just add . and see if case insensitive matching works? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep: use regcomp() for icase search with non-ascii patterns
Am 06.07.2015 um 14:42 schrieb Nguyễn Thái Ngọc Duy: Noticed-by: Plamen Totev plamen.to...@abv.bg Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- grep.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/grep.c b/grep.c index b58c7c6..48db15a 100644 --- a/grep.c +++ b/grep.c @@ -378,7 +378,7 @@ static void free_pcre_regexp(struct grep_pat *p) } #endif /* !USE_LIBPCRE */ -static int is_fixed(const char *s, size_t len) +static int is_fixed(const char *s, size_t len, int ignore_icase) { size_t i; @@ -391,6 +391,13 @@ static int is_fixed(const char *s, size_t len) for (i = 0; i len; i++) { if (is_regex_special(s[i])) return 0; + /* +* The builtin substring search can only deal with case +* insensitivity in ascii range. If there is something outside +* of that range, fall back to regcomp. +*/ + if (ignore_icase (unsigned char)s[i] = 128) + return 0; How about isascii(s[i])? } return 1; @@ -398,18 +405,19 @@ static int is_fixed(const char *s, size_t len) static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) { + int ignore_icase = opt-regflags REG_ICASE || p-ignore_case; int err; p-word_regexp = opt-word_regexp; p-ignore_case = opt-ignore_case; Using p-ignore_case before this line, as in initialization of the new variable ignore_icase above, changes the meaning. - if (opt-fixed || is_fixed(p-pattern, p-patternlen)) + if (opt-fixed || is_fixed(p-pattern, p-patternlen, ignore_icase)) p-fixed = 1; else p-fixed = 0; if (p-fixed) { - if (opt-regflags REG_ICASE || p-ignore_case) + if (ignore_case) ignore_icase instead? ignore_case is for the config variable core.ignorecase. Tricky. p-kws = kwsalloc(tolower_trans_tbl); else p-kws = kwsalloc(NULL); So the optimization before this patch was that if a string was searched for without -F then it would be treated as a fixed string anyway unless it contained regex special characters. Searching for fixed strings using the kwset functions is faster than using regcomp and regexec, which makes the exercise worthwhile. Your patch disables the optimization if non-ASCII characters are searched for because kwset handles case transformations only for ASCII chars. Another consequence of this limitation is that -Fi (explicit case-insensitive fixed-string search) doesn't work properly with non-ASCII chars neither. How can we handle this one? Fall back to regcomp by escaping all special characters? Or at least warn? Tests would be nice. :) René -- To unsubscribe from this list: send the line 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 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 --- t/t7512-status-help.sh | 111 +++ wt-status.c| 114 + 2 files changed, 225 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 'status after editing the last
[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 --- 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 splitting a commit
[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 --- 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
[PATCH v6 0/4] status: give more information during rebase
This is almost identical to v5. I turned git_path(var) into git_path(%s, var) as Junio noticed, but I still prefer my version on the other points discussed. Guillaume Pagès (4): status: factor two rebase-related messages together status: differentiate interactive from non-interactive rebases status: give more information during rebase -i status: add new tests for status during rebase -i t/t7512-status-help.sh | 226 ++--- wt-status.c| 151 + 2 files changed, 346 insertions(+), 31 deletions(-) -- 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
undocumented core.sharedRepository=2 set by git init --shared=world
joey@darkstar:~/tmpgit init --shared=world testrepo Initialized empty shared Git repository in /home/joey/tmp/testrepo/.git/ joey@darkstar:~/tmpgrep shared testrepo/.git/config sharedrepository = 2 This magic value of 2 seems to be undocumented, as is the magic value of 1 that's equvilant to group. I think it would be better to have git init put in world or group and not these magic values. Anyway, I suppose they ought to be documented too. -- see shy jo signature.asc Description: Digital signature
[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 --- 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
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Mon, Jul 6, 2015 at 11:22 AM, Dave Borowitz dborow...@google.com wrote: The alternatives for someone writing a parser are: a. Store the original pkt-line framing. Or obviously, a2. Frame in some other way, e.g. JSON array of strings (complete straw man, not seriously proposing this). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git merge commit message issue
It was a bit sad since we use scripts to do the merge, then we can check the conflict list in the log message, and assign to separate owners to resolve them. Would it be possible to make it as an config option? On Tue, Jul 7, 2015 at 12:08 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Andrey Hsiao andreyhs...@gmail.com writes: Dear list: In the past, when we do the merge in git, if conflicts occurred, when we commit, the conflict list will be appended to the default commit message automatically. eg: Conflicts: a.java b.java Today, after using Git 2.3.0 for a merge, it seems now the conflict list was commented out by default. Yes, this changed here: https://github.com/git/git/commit/261f315bebfa9af2d09f20211960100ff06f87cb -- 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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
Shawn Pearce spea...@spearce.org writes: push cert format is like commit or tag format. You need those LFs. We can't just go declare them optional because of the way pkt-line read function is implemented in git-core. As I said, I view each of the packets between push-cert and push-cert-end packets representing the meat of each line in the cert. The sending end takes a cert as a long multi-line string, splits them into an array, each of whose element represents a line in it (iow certlines = certstring.split('\n')), and sends them packetised. The receiver receives a sequence of packets, notices push-cert packet, collects packets until it sees push-cert-end packet and treats them as elements of this array. pkt-line deframing process would have to strip optional LFs to reconstruct the original array the sender had (i.e. the above certlines array). The receiver needs to join the array with LF to recover the long multi-line string once it received the array. But this LF does not have anything to do with the optional trailing LF in pkt-line. If you sent the original certlines array via different RPC mechanism, you need to join them together with your own LF to reconstruct the multi-line srring. -- To unsubscribe from this list: send the line unsubscribe 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
Shawn Pearce spea...@spearce.org writes: push cert format is like commit or tag format. You need those LFs. We can't just go declare them optional because of the way pkt-line read function is implemented in git-core. As I said, I view each of the packets between push-cert and push-cert-end packets representing the meat of each line in the cert. The sending end takes a cert as a long multi-line string, splits them into an array, each of whose element represents a line in it (iow certlines = certstring.split('\n')), and sends them packetised. The receiver receives a sequence of packets, notices push-cert packet, collects packets until it sees push-cert-end packet and treats them as elements of this array. pkt-line deframing process would have to strip optional LFs to reconstruct the original array the sender had (i.e. the above certlines array). The receiver needs to join the array with LF to recover the long multi-line string once it received the array. But this LF does not have anything to do with the optional trailing LF in pkt-line. If you sent the original certlines array via different RPC mechanism, you need to join them together with your own LF to reconstruct the multi-line string. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Draft of Git Rev News edition 5
Eric Sunshine sunsh...@sunshineco.com writes: I'm not familiar with the criteria for deciding what merits mention in the newsletter. Is the recent introduction of git-worktree and the attendant relocation of add and prune functionality worthy? If so, perhaps the following write-up would be suitable? One issue I had with this text was that it was not immediately clear what the end-game UI of the feature was. Is checkout --to they way the user is expected to trigger this? It appears in the very early part of the multi-paragraph description and I suspect that the majority of the users would think that way, not with worktree add that appears a lot later. -- To unsubscribe from this list: send the line unsubscribe 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] rebase: return non-zero error code if format-patch fails
Clemens Buchacher clemens.buchac...@intel.com writes: On Fri, Jul 03, 2015 at 10:52:32AM -0700, Junio C Hamano wrote: Cc: Andrew Wong andrew.k...@gmail.com Signed-off-by: Clemens Buchacher clemens.buchac...@intel.com Reviewed-by: Jorge Nunes jorge.nu...@intel.com Where was this review made? I may have missed a recent discussion, and that is why I am asking, because Reviewed-by: lines that cannot be validated by going back to the list archive does not add much value. Jorge helped me by reviewing the patch before I submitted it to the list. My intention is to give credit for his contribution, and to involve him in any discussion regarding the patch. Maybe it makes more sense to say Helped-by:? Thanks; I think that clarifies it, and I think that is how people seem to use Helped-by around here. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 02/23] Documentation/git-worktree: associate options with commands
git-worktree options affect some worktree commands but not others, but this is not necessarily obvious from the option descriptions. Make this clear by indicating explicitly which commands are affected by which options. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 41103e5..1ac1217 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -28,15 +28,15 @@ OPTIONS -n:: --dry-run:: - Do not remove anything; just report what it would + With `prune`, do not remove anything; just report what it would remove. -v:: --verbose:: - Report all removals. + With `prune`, report all removals. --expire time:: - Only expire unused worktrees older than time. + With `prune`, only expire unused worktrees older than time. SEE ALSO -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe 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: Another way of looking at the problem with my assumptions is, I was assuming pkt-line framing was the same thing as pkt-line header. You seem to be saying the definition of pkt-line framing is header, and optional trailing newline. Yes. I thought that was what Server SHOULD terminate with LF; client MUST NOT require it in the existing text meant. Ah, that reminds me of one thing I already said elsewhere. We need to correct the above with s/Server/Sender/; s/Client/Receiver/; I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/23] Documentation/git-checkout: fix incorrect worktree prune command
This was missed when git prune --worktrees became git worktree prune. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-checkout.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 72def5b..ce223e6 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -444,7 +444,7 @@ When you are done with a linked working tree you can simply delete it. The working tree's entry in the repository's $GIT_DIR/worktrees directory will eventually be removed automatically (see `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run -`git prune --worktrees` in the main or any linked working tree to +`git worktree prune` in the main or any linked working tree to clean up any stale entries in $GIT_DIR/worktrees. If you move a linked working directory to another file system, or -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 04/23] Documentation/git-worktree: add BUGS section
Relocate submodule warning to BUGS and enumerate missing commands. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 19 --- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 3d28896..bf6a1e2 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -76,9 +76,6 @@ to `/path/main/.git/worktrees/test-next` then a file named `test-next` entry from being pruned. See linkgit:gitrepository-layout[5] for details. -Multiple checkout support for submodules is incomplete. It is NOT -recommended to make multiple checkouts of a superproject. - COMMANDS prune:: @@ -100,6 +97,22 @@ OPTIONS --expire time:: With `prune`, only expire unused worktrees older than time. +BUGS + +Multiple checkout support for submodules is incomplete. It is NOT +recommended to make multiple checkouts of a superproject. + +git-worktree could provide more automation for tasks currently +performed manually or via other commands, such as: + +- `add` to create a new linked worktree +- `remove` to remove a linked worktree and its administrative files (and + warn if the worktree is dirty) +- `mv` to move or rename a worktree and update its administrative files +- `list` to list linked worktrees +- `lock` to prevent automatic pruning of administrative files (for instance, + for a worktree on a portable device) + SEE ALSO -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/23] Documentation/git-worktree: add high-level 'lock' overview
Due to the (current) absence of a git worktree lock command, locking a worktree's administrative files to prevent automatic pruning is a manual task, necessarily requiring low-level understanding of linked worktree functionality. However, this level of detail does not belong in the high-level DESCRIPTION section, so add a generalized discussion of locking to DESCRIPTION and move the technical information to DETAILS. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 2954dc6..0385e9a 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -37,15 +37,11 @@ at least one git command inside the linked working directory (e.g. `git status`) in order to update its administrative files in the repository so that they do not get automatically pruned. -To prevent a $GIT_DIR/worktrees entry from from being pruned (which -can be useful in some situations, such as when the -entry's working tree is stored on a portable device), add a file named -'locked' to the entry's directory. The file contains the reason in -plain text. For example, if a linked working tree's `.git` file points -to `/path/main/.git/worktrees/test-next` then a file named -`/path/main/.git/worktrees/test-next/locked` will prevent the -`test-next` entry from being pruned. See -linkgit:gitrepository-layout[5] for details. +If a linked working tree is stored on a portable device or network share +which is not always mounted, you can prevent its administrative files from +being pruned by creating a file named 'lock' alongside the other +administrative files, optionally containing a plain text reason that +pruning should be suppressed. See section DETAILS for more information. COMMANDS @@ -99,6 +95,16 @@ 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. +To prevent a $GIT_DIR/worktrees entry from from being pruned (which +can be useful in some situations, such as when the +entry's working tree is stored on a portable device), add a file named +'locked' to the entry's directory. The file contains the reason in +plain text. For example, if a linked working tree's `.git` file points +to `/path/main/.git/worktrees/test-next` then a file named +`/path/main/.git/worktrees/test-next/locked` will prevent the +`test-next` entry from being pruned. See +linkgit:gitrepository-layout[5] for details. + BUGS Multiple checkout support for submodules is incomplete. It is NOT -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/23] Documentation/git-worktree: add EXAMPLES section
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 22 ++ 1 file changed, 22 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 0385e9a..6afff1e 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -105,6 +105,28 @@ to `/path/main/.git/worktrees/test-next` then a file named `test-next` entry from being pruned. See linkgit:gitrepository-layout[5] for details. +EXAMPLES + +You are in the middle of a refactoring session and your boss comes in and +demands that you fix something immediately. You might typically use +linkgit:git-stash[1] to store your changes away temporarily, however, your +worktree is in such a state of disarray (with new, moved, and removed files, +and other bits and pieces strewn around) that you don't want to risk +disturbing any of it. Instead, you create a temporary linked worktree to +make the emergency fix, remove it when done, and then resume your earlier +refactoring session. + + +$ git branch emergency-fix master +$ git checkout --to ../temp emergency-fix +$ pushd ../temp +# ... hack hack hack ... +$ git commit -a -m 'emergency fix for boss' +$ popd +$ rm -rf ../temp +$ git worktree prune + + BUGS Multiple checkout support for submodules is incomplete. It is NOT -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/23] Documentation/git-worktree: split technical info from general description
The DESCRIPTION section should provide a high-level overview of linked worktree functionality to bring users up to speed quickly, without overloading them with low-level details, so relocate the technical information to a new DETAILS section. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 70 ++ 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index bf6a1e2..2954dc6 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -24,47 +24,18 @@ tree is associated with the repository. This new working tree is called a 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 tree's entry in the repository's $GIT_DIR/worktrees -directory will eventually be removed automatically (see +The working tree's administrative files in the repository (see +DETAILS below) will eventually be removed automatically (see `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run `git worktree prune` in the main or any linked working tree to -clean up any stale entries in $GIT_DIR/worktrees. +clean up any stale administrative files. If you move a linked working directory to another file system, or within a file system that does not support hard links, you need to run at least one git command inside the linked working directory -(e.g. `git status`) in order to update its entry in $GIT_DIR/worktrees -so that it does not get automatically removed. +(e.g. `git status`) in order to update its administrative files in the +repository so that they do not get automatically pruned. To prevent a $GIT_DIR/worktrees entry from from being pruned (which can be useful in some situations, such as when the @@ -97,6 +68,37 @@ OPTIONS --expire time:: With `prune`, only expire unused worktrees older than time. +DETAILS +--- +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
Re: Draft of Git Rev News edition 5
Eric Sunshine sunsh...@sunshineco.com writes: How about this instead: prefixing with As originally implemented, with a couple s/is/was/ thrown in... As originally implemented, creation of linked-worktrees was accomplished via `git checkout --to path branch`, and cleanup of leftover administrative files, after `path` is deleted, was done with `git prune --worktrees`. However, a recent unrelated change to `git prune` led to a discussion that concluded that worktree-related maintenance functionality didn't belong in `git prune`. Is that sufficient to clue in the reader that checkout --to is not final form,... Yeah, I think that is a good way to address my concern. The current draft release notes to 2.5 mentions this feature as experimental and warns that its UI is bound to change. We will ship the upcoming release with checkout --to and the more places we advise the users that this UI is not final, the better. -- To unsubscribe from this list: send the line unsubscribe 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: That existing implementations of the receivers treat an empty packet (i.e. 0004) or 0005\n ;) Is that true? I think len = pkt_line(); if (!len) break; /* flush */ would give you len == 1 and would not confuse it with a 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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Mon, Jul 6, 2015 at 2:06 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: On Mon, Jul 6, 2015 at 1:34 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: Another way of looking at the problem with my assumptions is, I was assuming pkt-line framing was the same thing as pkt-line header. You seem to be saying the definition of pkt-line framing is header, and optional trailing newline. Yes. I thought that was what Server SHOULD terminate with LF; client MUST NOT require it in the existing text meant. Unfortunately, the existing text is littered with examples of PKT-LINE(foo SP bar LF). If we assume PKT-LINE(...) means apply pkt-line framing to the [...], then this strongly implies that pkt-line framing does _not_ include the trailing LF. (Or the logical but bizarre alternative reading that such an example might have _two_ trailing LFs :) Yes, But I never viewed PKT-LINE() as an element that strictly defines the grammar of the packet protocol ;-) By clarifying that sender SHOULD terminate with LF, receiver MUST NOT require it is the rule (and fixing the existing implementations at places where they violate the MUST NOT part, which I think are very small number of places), I think we can drop these LF (or LF? for that matter) from all of the PKT-LINE() in the construction in the pack-protocol.txt, which would be a very good thing to do. Completely agree, and that is what I meant when I said The additional upside [to explicitly defining pkt-line framing in this way] is that we could then potentially remove all or almost all LFs from this document. The example in your sentence will become PKT-LINE(foo SP bar) and the there may be an LF at the end would only be at one place, as a part of the definition of PKT-LINE(). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] pull: allow dirty tree when rebase.autostash enabled
Kevin Daudt m...@ikke.info writes: rebase learned to stash changes when it encounters a dirty work tree, but git pull --rebase does not. Only verify if the working tree is dirty when rebase.autostash is not enabled. Signed-off-by: Kevin Daudt m...@ikke.info Helped-by: Paul Tan pyoka...@gmail.com --- I applied it, tried to run today's integration cycle, and then ended up ejecting it from my tree for now, as this seemed to break 5520 when merged to 'pu' X-. Well, that is partly expected, as Paul's builtin/pull.c does not know about it (yet). git-pull.sh | 3 ++- t/t5520-pull.sh | 11 +++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/git-pull.sh b/git-pull.sh index a814bf6..ff28d3f 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -284,7 +284,8 @@ test true = $rebase { then die $(gettext updating an unborn branch with changes added to the index) fi - else + elif test $(git config --bool --get rebase.autostash || echo false) = false + then require_clean_work_tree pull with rebase Please commit or stash them. fi oldremoteref= diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index f4a7193..a0013ee 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -245,6 +245,17 @@ test_expect_success '--rebase fails with multiple branches' ' test modified = $(git show HEAD:file) ' +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' + test_config rebase.autostash true + git reset --hard before-rebase + echo dirty new_file + git add new_file + git pull --rebase . copy + test_cmp_rev HEAD^ copy + test $(cat new_file) = dirty + test $(cat file) = modified again +' + test_expect_success 'pull.rebase' ' git reset --hard before-rebase test_config pull.rebase true -- To unsubscribe from this list: send the line unsubscribe 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: Unfortunately, optional LFs still make the stored certs for later auditing and parsing a bit illegible. This is one way in which push certs are fundamentally different from the rest of the wire protocol, which is not intended to be persisted. Hmm, I am not sure I follow. The corner case I pointed out before where nonce runs into commands is not the only one. Consider the following cert fragment: 001fpushee git://localhost/repo 0029nonce 1433954361-bde756572d665bba81d8 A naive cert storage/auditing implementation would store the raw payload that needs to be verified, without the pkt-line framing. In this case: pushee git://localhost/repononce 1433954361-bde756572d665bba81d8 Without the pkt-line framing is fine, but my understanding (or, the intention of the original implementor) of this part of the protocol is that packets between the push-cert packet and the push-cert-end packet carry the meat of the each line of the certificate, one packet per line. If pkt-line is allowed to omit the terminating LFs, then it follows that the receiving ends can simply do something like what I illustrated in $gmane/273196 (in java or whatever other implementation platform they use) to collect packets between push-cert and push-cert-end, knowing that the packets may or may not have terminating LF and supplying the omitted LFs themselves when they receive the cert before verifying and storing. So in order to reconstitute the raw payload without pkt-line framing, the omitted LF obviously needs to be added. Why is that a problem? Side note: think of it in a different way. The key word of the first paragraph above is the meat of; if your cert has two lines pushee $URLLFnonce 1234-5670LF the lines in it are pushee $URLLF and nonce 1234-5670LF but the meat of them are pushee $URL and nonce 1234-5670. The protocol wants to carry an array with two elements, (pushee $URL, nonce 1234-5670), as the hypothetical cert has two lines. And then \n.join(the cert array) . \n would be how you reconstruct the original payload. The illustration in $gmane/273196 is slightly cheating in that sense. Instead of first creating an array of plain strings without LF termination and joining them together later, it knows that we will LF-join in the end, and abuses the LF in the original payload that came from the sender and supplies its own if the sender omitted it. It is very similar to and in the opposite of how each ref advertisement is handled. Until the first flush, each packet is expected to carry the object name and the ref name. A pkt-line framing may add terminating LF but that obviously is not part of the ref name. A naive parser that wants to find the pushee would look for pushee urlish, which would be wrong in this case. (To say nothing of the fact that pushee might actually be -0700pushee.) The alternatives for someone writing a parser are: a. Store the original pkt-line framing. b. Write a parser in some other clever way, e.g. parsing the entire cert in reverse might work. Neither of these is very satisfying, and both reduce human legibility of the stored payload. 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
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Mon, Jul 6, 2015 at 12:57 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: The server can munge pkt-lines and reinsert LFs, but it _must_ have some way of reconstructing the payload that the client signed in order to verify the signature. If we just naively insert LFs where missing, we lose the ability to verify the signature. I still do not understand this part. There is no way to naively insert, is there? You have an array of lines (each of which you have already stripped its terminating LF at its end). How else other than adding one LF at the end of each element do you reconstruct the original multi-line string the client signed? Are there other ways that makes the result ambiguous?? I think I understand the confusion now. I think you and I are working from different assumptions about the client behavior. My assumption was: the intended behavior for the client is to sign the exact payload that was sent over the wire, minus pkt-line headers. For example, under my assumption, if the client sent: 0008foo\n 0007bar 0008baz\n then this indicates the client signed: foo\nbarbaz\n Under this assumption, naively inserting LF means inserting an LF after bar. Thus the server would record the following in a persistent store: foo\nbar\nbaz\n If we only store this string, and do not remember the fact that the client originally omitted one of those LFs, then when we go back to verify that signature later, it will fail. That was my assumption. Your assumption, IIUC, is that the payload the client signed MUST have contained LFs in between each line. When framing the content for the wire, the client MUST send one logical line, which has no trailing LF, per pkt-line, and furthermore the pkt-line content MAY contain an additional trailing LF. Under your assumption, the server always has enough information to reconstruct the original signed payload. The problem with the documentation, then, is that the documentation does not say anything to indicate that the signed payload is anything other than what is on the wire. So maybe this series should include an explicit description of the singed payload outside of the context of a push. Then, in the push section, we can describe the set of transformations that the client MUST perform (splitting on LF; adding pkt-line headers) and MAY perform (adding LFs). If we say the payload the client signs MUST have LFs only in certain places, then that gives the server enough information to reconstruct the payload and verify the signature. But if we say the signed payload MUST have LFs and the wire payload MAY have LFs, then now we have two completely different formats, only one of which is documented. I thought that was what I was saying. The wire protocol sends the contents of each line (both what is signed and the signature) on a separate packet. When I say contents of a line, I do not include the terminating LF as part of the line (iow, LF is not even optional; the terminating LF is not considered as part of the contents of a line). It becomes irrelevant that a pkt-line may or may not have a trailing optional LF. If there is LF at the end of a packet between push-cert and push-cert-end packets, that LF by definition cannot be part of the contents of a line in a certificate. It is just a pkt-line framing artifact you can and should remove if you are doing a split to array, join with LF implementation to recover the original string. And that is very much consistent with the way we send other things with pkt-line protocol. Each packet up to the first flush is expected to have object name and refname as ref advertisement. The pkt-line framing may or may not add a trailing LF, but LF is not part of refname. It is not even part of the payload; merely an artifact of pkt-line framing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Git doesn't allow to apply a patch with empty filenames.
git-apply fails if '-p' option truncate at least one filename in patch to zero size. This solution tries to bring closer git-apply to standard Linux patch tool. Standard patch tool skip the truncated filenames and successfully applies patch. Evgen Druzhynin (1): Added ability to scip patch hunk with an empty file. builtin/apply.c | 16 +--- po/bg.po| 15 --- po/ca.po| 15 --- po/de.po| 15 --- po/fr.po| 15 --- po/git.pot | 11 --- po/it.po| 11 --- po/pt_PT.po | 7 --- po/ru.po| 12 po/sv.po| 16 po/vi.po| 12 po/zh_CN.po | 11 --- 12 files changed, 5 insertions(+), 151 deletions(-) -- 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] Added ability to scip patch hunk with an empty filename.
--- builtin/apply.c | 16 +--- po/bg.po| 15 --- po/ca.po| 15 --- po/de.po| 15 --- po/fr.po| 15 --- po/git.pot | 11 --- po/it.po| 11 --- po/pt_PT.po | 7 --- po/ru.po| 12 po/sv.po| 16 po/vi.po| 12 po/zh_CN.po | 11 --- 12 files changed, 5 insertions(+), 151 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 54aba4e..e4481b4 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1488,17 +1488,11 @@ static int find_header(const char *line, unsigned long size, int *hdrsize, struc int git_hdr_len = parse_git_header(line, len, size, patch); if (git_hdr_len = len) continue; - if (!patch-old_name !patch-new_name) { - if (!patch-def_name) - die(Q_(git diff header lacks filename information when removing - %d leading pathname component (line %d), - git diff header lacks filename information when removing - %d leading pathname components (line %d), - p_value), - p_value, linenr); - patch-old_name = xstrdup(patch-def_name); - patch-new_name = xstrdup(patch-def_name); - } + + /* pass this hunk if the filename length is zero */ + if (!patch-old_name !patch-new_name) + return -1; + if (!patch-is_delete !patch-new_name) die(git diff header lacks filename information (line %d), linenr); diff --git a/po/bg.po b/po/bg.po index 171f813..f2cc286 100644 --- a/po/bg.po +++ b/po/bg.po @@ -2373,21 +2373,6 @@ msgstr при повторното преброяване бе получен msgid patch fragment without header at line %d: %.*s msgstr част от кръпка без заглавна част на ред %d: %.*s -#: builtin/apply.c:1493 -#, c-format -msgid -git diff header lacks filename information when removing %d leading pathname -component (line %d) -msgid_plural -git diff header lacks filename information when removing %d leading pathname -components (line %d) -msgstr[0] -След съкращаването на %d-та част от компонентите на пътя, в заглавната част -на „git diff“ липсва информация за име на файл (ред: %d) -msgstr[1] -След съкращаването на първите %d части от компонентите на пътя, в заглавната -част на „git diff“ липсва информация за име на файл (ред: %d) - #: builtin/apply.c:1656 msgid new file depends on old contents msgstr новият файл зависи от старото съдържание на файла diff --git a/po/ca.po b/po/ca.po index c733483..c893285 100644 --- a/po/ca.po +++ b/po/ca.po @@ -2216,21 +2216,6 @@ msgstr recompte: línia inesperada: %.*s msgid patch fragment without header at line %d: %.*s msgstr fragment de pedaç sense capçalera a la línia %d: %.*s -#: builtin/apply.c:1493 -#, c-format -msgid -git diff header lacks filename information when removing %d leading pathname -component (line %d) -msgid_plural -git diff header lacks filename information when removing %d leading pathname -components (line %d) -msgstr[0] -a la capçalera de git diff li manca informació de nom de fitxer en eliminar -%d component de nom de camí inicial (línia %d) -msgstr[1] -a la capçalera de git diff li manca informació de nom de fitxer en eliminar -%d components de nom de camí inicial (línia %d) - #: builtin/apply.c:1656 msgid new file depends on old contents msgstr el fitxer nou depèn dels continguts antics diff --git a/po/de.po b/po/de.po index 7d603c2..ea6b555 100644 --- a/po/de.po +++ b/po/de.po @@ -2257,21 +2257,6 @@ msgstr recount: unerwartete Zeile: %.*s msgid patch fragment without header at line %d: %.*s msgstr Patch-Fragment ohne Kopfbereich bei Zeile %d: %.*s -#: builtin/apply.c:1493 -#, c-format -msgid -git diff header lacks filename information when removing %d leading pathname -component (line %d) -msgid_plural -git diff header lacks filename information when removing %d leading pathname -components (line %d) -msgstr[0] -Dem Kopfbereich von \git diff\ fehlen Informationen zum Dateinamen, wenn -%d vorangestellter Teil des Pfades entfernt wird (Zeile %d) -msgstr[1] -Dem Kopfbereich von \git diff\ fehlen Informationen zum Dateinamen, wenn -%d vorangestellte Teile des Pfades entfernt werden (Zeile %d) - #: builtin/apply.c:1656 msgid new file depends on old contents msgstr neue Datei hängt von alten Inhalten ab diff --git a/po/fr.po
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Mon, Jul 6, 2015 at 1:30 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: I think I understand the confusion now. I think you and I are working from different assumptions about the client behavior. I agree that we now both understand where we come from ;-) And sorry for not being clear when I did the push-cert originally in the documentation. As I already said, packets between push-cert and push-cert-end are contents of individual lines of the GPG signed push certificate This sentence makes sense to me now, but only because we now agree that contents does not include the LF. Different people may have different initial assumptions about whether the contents of a line includes the trailing newline or not. was the design meant from day one, and a85b377d (push: the beginning of git push --signed, 2014-09-12) could have made it clearer. The problem with the documentation, then, is that the documentation does not say anything to indicate that the signed payload is anything other than what is on the wire. Yeah, that was untold assumption, as I considered what is on the wire to include pkt-line framing when I wrote a85b377d (push: the beginning of git push --signed, 2014-09-12). So maybe this series should include an explicit description of the singed payload outside of the context of a push. Then, in the push section, we can describe the set of transformations that the client MUST perform (splitting on LF; adding pkt-line headers) and MAY perform (adding LFs). Yes, and the latter is not limited to push-cert but anything sent on pkt-line. That incidentally is the only point I deeply care about. I just want to minimize the protocol is this way in general, but only for this one you must do it differently. Understood, and I'm glad we have finally come to an arrangement that is both consistent and easy to implement on the server side. One example of only for this one you must do it differently is another caveat for protocol implementors for the sending side (again not limited to push cert). That existing implementations of the receivers treat an empty packet (i.e. 0004) or 0005\n ;) as if it is the same as a flush packet (i.e. ), so even if the sending side chooses to ignore the SHOULD terminate each non-flush line using LF, it is strongly advised not to do so when it wants to send an empty payload. This needs to be documented. The receiving end SHOULD NOT treat 0004 the same way as . I think that must be documented and implementations (including our own) should be fixed. Agreed. 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
On Mon, Jul 6, 2015 at 1:11 PM, Dave Borowitz dborow...@google.com wrote: On Mon, Jul 6, 2015 at 12:57 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: The server can munge pkt-lines and reinsert LFs, but it _must_ have some way of reconstructing the payload that the client signed in order to verify the signature. If we just naively insert LFs where missing, we lose the ability to verify the signature. I still do not understand this part. There is no way to naively insert, is there? You have an array of lines (each of which you have already stripped its terminating LF at its end). How else other than adding one LF at the end of each element do you reconstruct the original multi-line string the client signed? Are there other ways that makes the result ambiguous?? I think I understand the confusion now. I think you and I are working from different assumptions about the client behavior. My assumption was: the intended behavior for the client is to sign the exact payload that was sent over the wire, minus pkt-line headers. For example, under my assumption, if the client sent: 0008foo\n 0007bar 0008baz\n then this indicates the client signed: foo\nbarbaz\n Under this assumption, naively inserting LF means inserting an LF after bar. Thus the server would record the following in a persistent store: foo\nbar\nbaz\n If we only store this string, and do not remember the fact that the client originally omitted one of those LFs, then when we go back to verify that signature later, it will fail. That was my assumption. Your assumption, IIUC, is that the payload the client signed MUST have contained LFs in between each line. When framing the content for the wire, the client MUST send one logical line, which has no trailing LF, per pkt-line, and furthermore the pkt-line content MAY contain an additional trailing LF. Under your assumption, the server always has enough information to reconstruct the original signed payload. The problem with the documentation, then, is that the documentation does not say anything to indicate that the signed payload is anything other than what is on the wire. Another way of looking at the problem with my assumptions is, I was assuming pkt-line framing was the same thing as pkt-line header. You seem to be saying the definition of pkt-line framing is header, and optional trailing newline. A quick scan of pack-protocol.txt did not turn up anything one way or the other on this issue, so perhaps we could make it more explicit. The additional upside here is that we could then potentially remove all or almost all LFs from this document. So maybe this series should include an explicit description of the singed payload outside of the context of a push. Then, in the push section, we can describe the set of transformations that the client MUST perform (splitting on LF; adding pkt-line headers) and MAY perform (adding LFs). If we say the payload the client signs MUST have LFs only in certain places, then that gives the server enough information to reconstruct the payload and verify the signature. But if we say the signed payload MUST have LFs and the wire payload MAY have LFs, then now we have two completely different formats, only one of which is documented. I thought that was what I was saying. The wire protocol sends the contents of each line (both what is signed and the signature) on a separate packet. When I say contents of a line, I do not include the terminating LF as part of the line (iow, LF is not even optional; the terminating LF is not considered as part of the contents of a line). It becomes irrelevant that a pkt-line may or may not have a trailing optional LF. If there is LF at the end of a packet between push-cert and push-cert-end packets, that LF by definition cannot be part of the contents of a line in a certificate. It is just a pkt-line framing artifact you can and should remove if you are doing a split to array, join with LF implementation to recover the original string. And that is very much consistent with the way we send other things with pkt-line protocol. Each packet up to the first flush is expected to have object name and refname as ref advertisement. The pkt-line framing may or may not add a trailing LF, but LF is not part of refname. It is not even part of the payload; merely an artifact of pkt-line framing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 12/23] checkout: drop 'checkout_opts' dependency from prepare_linked_checkout
The plan is to relocate git checkout --to functionality to git worktree add, however, worktree.c won't have access to the 'struct checkout_opts' passed to prepare_linked_worktree(), which it consults for the pathname of the new worktree and the argv[] of the command it should run to populate the new worktree. Facilitate relocation of prepare_linked_worktree() by instead having it accept the pathname and argv[] directly, thus eliminating the final references to 'struct checkout_opts'. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 90bb3cd..e4064a8 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -854,11 +854,11 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static int prepare_linked_checkout(const struct checkout_opts *opts) +static int prepare_linked_checkout(const char *path, const char **child_argv) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; - const char *path = opts-new_worktree, *name; + const char *name; struct stat st; struct child_process cp; int counter = 0, len, ret; @@ -943,7 +943,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts) setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1); memset(cp, 0, sizeof(cp)); cp.git_cmd = 1; - cp.argv = opts-saved_argv; + cp.argv = child_argv; ret = run_command(cp); if (!ret) { is_junk = 0; @@ -1302,7 +1302,8 @@ static int checkout_branch(struct checkout_opts *opts, if (opts-new_worktree) { if (!new-commit) die(_(no branch specified)); - return prepare_linked_checkout(opts); + return prepare_linked_checkout(opts-new_worktree, + opts-saved_argv); } if (!new-commit opts-new_branch) { -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 20/23] worktree: extract basename computation to new function
A subsequent patch will also need to compute the basename of the new worktree, so factor out this logic into a new function. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/worktree.c | 29 - 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 04e6d0f..25fe25b 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -152,6 +152,25 @@ static void remove_junk_on_signal(int signo) raise(signo); } +static const char *worktree_basename(const char *path, int *olen) +{ + const char *name; + int len; + + len = strlen(path); + while (len is_dir_sep(path[len - 1])) + len--; + + for (name = path + len - 1; name path; name--) + if (is_dir_sep(*name)) { + name++; + break; + } + + *olen = len; + return name; +} + static int add_worktree(const char *path, const char **child_argv) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; @@ -165,15 +184,7 @@ static int add_worktree(const char *path, const char **child_argv) if (file_exists(path) !is_empty_dir(path)) die(_('%s' already exists), path); - len = strlen(path); - while (len is_dir_sep(path[len - 1])) - len--; - - for (name = path + len - 1; name path; name--) - if (is_dir_sep(*name)) { - name++; - break; - } + name = worktree_basename(path, len); strbuf_addstr(sb_repo, git_path(worktrees/%.*s, (int)(path + len - name), name)); len = sb_repo.len; -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 18/23] checkout: retire --to option
Now that git worktree add has achieved user-facing feature-parity with git checkout --to, retire the latter. Move the actual linked worktree creation functionality, prepare_linked_checkout() and its helpers, verbatim from checkout.c to worktree.c. This effectively reverts changes to checkout.c by 529fef2 (checkout: support checking out into a new working directory, 2014-11-30) with the exception of merge_working_tree() and switch_branches() which still require specialized knowledge that a the checkout is occurring in a newly-created linked worktree (signaled to them by the private GIT_CHECKOUT_NEW_WORKTREE environment variable). Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-checkout.txt | 7 -- builtin/checkout.c | 161 + builtin/worktree.c | 144 ++-- 3 files changed, 139 insertions(+), 173 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 77b7141..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, etc. See - linkgit:git-worktree[1] for a description of linked worktrees. - --ignore-other-worktrees:: `git checkout` refuses when the wanted ref is already checked out by another worktree. This option makes it check the ref diff --git a/builtin/checkout.c b/builtin/checkout.c index e4064a8..b1e68b3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -19,8 +19,6 @@ #include ll-merge.h #include resolve-undo.h #include submodule.h -#include argv-array.h -#include sigchain.h static const char * const checkout_usage[] = { N_(git checkout [options] branch), @@ -51,8 +49,6 @@ struct checkout_opts { struct pathspec pathspec; struct tree *source_tree; - const char *new_worktree; - const char **saved_argv; int new_worktree_mode; }; @@ -255,9 +251,6 @@ static int checkout_paths(const struct checkout_opts *opts, die(_(Cannot update paths and switch to branch '%s' at the same time.), opts-new_branch); - if (opts-new_worktree) - die(_('%s' cannot be used with updating paths), --to); - if (opts-patch_mode) return run_add_interactive(revision, --patch=checkout, opts-pathspec); @@ -825,142 +818,6 @@ static int switch_branches(const struct checkout_opts *opts, return ret || writeout_error; } -static char *junk_work_tree; -static char *junk_git_dir; -static int is_junk; -static pid_t junk_pid; - -static void remove_junk(void) -{ - struct strbuf sb = STRBUF_INIT; - if (!is_junk || getpid() != junk_pid) - return; - if (junk_git_dir) { - strbuf_addstr(sb, junk_git_dir); - remove_dir_recursively(sb, 0); - strbuf_reset(sb); - } - if (junk_work_tree) { - strbuf_addstr(sb, junk_work_tree); - remove_dir_recursively(sb, 0); - } - strbuf_release(sb); -} - -static void remove_junk_on_signal(int signo) -{ - remove_junk(); - sigchain_pop(signo); - raise(signo); -} - -static int prepare_linked_checkout(const char *path, const char **child_argv) -{ - struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; - struct strbuf sb = STRBUF_INIT; - const char *name; - struct stat st; - struct child_process cp; - int counter = 0, len, ret; - unsigned char rev[20]; - - if (file_exists(path) !is_empty_dir(path)) - die(_('%s' already exists), path); - - len = strlen(path); - while (len is_dir_sep(path[len - 1])) - len--; - - for (name = path + len - 1; name path; name--) - if (is_dir_sep(*name)) { - name++; - break; - } - strbuf_addstr(sb_repo, - git_path(worktrees/%.*s, (int)(path + len - name), name)); - len = sb_repo.len; - if (safe_create_leading_directories_const(sb_repo.buf)) - die_errno(_(could not create leading directories of '%s'), - sb_repo.buf); - while (!stat(sb_repo.buf, st)) { - counter++; - strbuf_setlen(sb_repo, len); - strbuf_addf(sb_repo, %d, counter); - } - name = strrchr(sb_repo.buf,
[PATCH v3 17/23] tests: worktree: retrofit checkout --to tests for worktree add
With the introduction of git worktree add, git checkout --to is slated for removal. Therefore, retrofit linked worktree creation tests to use git worktree add instead. (The test to check exclusivity of checkout --to and checkout paths is dropped altogether since it becomes meaningless with retirement of checkout --to.) Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- t/{t2025-checkout-to.sh = t2025-worktree-add.sh} | 48 +++ t/t2026-prune-linked-checkouts.sh | 2 +- t/t7410-submodule-checkout-to.sh | 4 +- 3 files changed, 25 insertions(+), 29 deletions(-) rename t/{t2025-checkout-to.sh = t2025-worktree-add.sh} (59%) diff --git a/t/t2025-checkout-to.sh b/t/t2025-worktree-add.sh similarity index 59% rename from t/t2025-checkout-to.sh rename to t/t2025-worktree-add.sh index 0fd731b..192c936 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-worktree-add.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='test git checkout --to' +test_description='test git worktree add' . ./test-lib.sh @@ -8,22 +8,18 @@ test_expect_success 'setup' ' test_commit init ' -test_expect_success 'checkout --to not updating paths' ' - test_must_fail git checkout --to -- init.t -' - -test_expect_success 'checkout --to an existing worktree' ' +test_expect_success 'add an existing worktree' ' mkdir -p existing/subtree - test_must_fail git checkout --detach --to existing master + test_must_fail git worktree add --detach existing master ' -test_expect_success 'checkout --to an existing empty worktree' ' +test_expect_success 'add an existing empty worktree' ' mkdir existing_empty - git checkout --detach --to existing_empty master + git worktree add --detach existing_empty master ' -test_expect_success 'checkout --to refuses to checkout locked branch' ' - test_must_fail git checkout --to zere master +test_expect_success 'add refuses to checkout locked branch' ' + test_must_fail git worktree add zere master ! test -d zere ! test -d .git/worktrees/zere ' @@ -36,9 +32,9 @@ test_expect_success 'checking out paths not complaining about linked checkouts' ) ' -test_expect_success 'checkout --to a new worktree' ' +test_expect_success 'add worktree' ' git rev-parse HEAD expect - git checkout --detach --to here master + git worktree add --detach here master ( cd here test_cmp ../init.t init.t @@ -49,27 +45,27 @@ test_expect_success 'checkout --to a new worktree' ' ) ' -test_expect_success 'checkout --to a new worktree from a subdir' ' +test_expect_success 'add worktree from a subdir' ' ( mkdir sub cd sub - git checkout --detach --to here master + git worktree add --detach here master cd here test_cmp ../../init.t init.t ) ' -test_expect_success 'checkout --to from a linked checkout' ' +test_expect_success 'add from a linked checkout' ' ( cd here - git checkout --detach --to nested-here master + git worktree add --detach nested-here master cd nested-here git fsck ) ' -test_expect_success 'checkout --to a new worktree creating new branch' ' - git checkout --to there -b newmaster master +test_expect_success 'add worktree creating new branch' ' + git worktree add -b newmaster there master ( cd there test_cmp ../init.t init.t @@ -90,7 +86,7 @@ test_expect_success 'die the same branch is already checked out' ' test_expect_success 'not die the same branch is already checked out' ' ( cd here - git checkout --ignore-other-worktrees --to anothernewmaster newmaster + git worktree add --force anothernewmaster newmaster ) ' @@ -101,15 +97,15 @@ test_expect_success 'not die on re-checking out current branch' ' ) ' -test_expect_success 'checkout --to from a bare repo' ' +test_expect_success 'add from a bare repo' ' ( git clone --bare . bare cd bare - git checkout --to ../there2 -b bare-master master + git worktree add -b bare-master ../there2 master ) ' -test_expect_success 'checkout from a bare repo without --to' ' +test_expect_success 'checkout from a bare repo without add' ' ( cd bare test_must_fail git checkout master @@ -129,17 +125,17 @@ test_expect_success 'checkout with grafts' ' EOF git log --format=%s -2 actual test_cmp expected actual - git checkout --detach --to grafted master + git worktree add --detach grafted master git --git-dir=grafted/.git log --format=%s -2 actual test_cmp
[PATCH v3 10/23] checkout: prepare_linked_checkout: drop now-unused 'new' argument
The only references to 'new' were folded out by the last two patches. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 162c822..134b6d6 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -854,8 +854,7 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static int prepare_linked_checkout(const struct checkout_opts *opts, - struct branch_info *new) +static int prepare_linked_checkout(const struct checkout_opts *opts) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; @@ -1304,7 +1303,7 @@ static int checkout_branch(struct checkout_opts *opts, if (opts-new_worktree) { if (!new-commit) die(_(no branch specified)); - return prepare_linked_checkout(opts, new); + return prepare_linked_checkout(opts); } if (!new-commit opts-new_branch) { -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/23] checkout: make --to unconditionally verbose
prepare_linked_checkout() respects git-checkout's --quiet flag, however, the plan is to relocate git checkout --to functionality to git worktree add, and git-worktree does not (yet) have a --quiet flag. Consequently, make prepare_linked_checkout() unconditionally verbose to ease eventual code movement to worktree.c. (A --quiet flag can be added to git-worktree later if there is demand for it.) Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 134b6d6..90bb3cd 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -936,8 +936,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts) strbuf_addf(sb, %s/commondir, sb_repo.buf); write_file(sb.buf, 1, ../..\n); - if (!opts-quiet) - fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name); + fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name); setenv(GIT_CHECKOUT_NEW_WORKTREE, 1, 1); setenv(GIT_DIR_ENVIRONMENT, sb_git.buf, 1); -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 19/23] checkout: require worktree unconditionally
In order to allow linked worktree creation via git checkout --to from a bare repository, 3473ad0 (checkout: don't require a work tree when checking out into a new one, 2014-11-30) dropped git-checkout's unconditional NEED_WORK_TREE requirement and instead performed worktree setup conditionally based upon presence or absence of the --to option. Now that --to has been retired and git-checkout is no longer responsible for linked worktree creation, the NEED_WORK_TREE requirement can be re-instated. This effectively reverts 3473ad0, except for the tests it added which now check bare repository behavior of git worktree add instead. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 2 -- git.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index b1e68b3..5754554 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1218,8 +1218,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.new_worktree_mode = getenv(GIT_CHECKOUT_NEW_WORKTREE) != NULL; - setup_work_tree(); - if (conflict_style) { opts.merge = 1; /* implied */ git_xmerge_config(merge.conflictstyle, conflict_style, NULL); diff --git a/git.c b/git.c index f227838..21a6398 100644 --- a/git.c +++ b/git.c @@ -383,7 +383,7 @@ static struct cmd_struct commands[] = { { check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE }, { check-mailmap, cmd_check_mailmap, RUN_SETUP }, { check-ref-format, cmd_check_ref_format }, - { checkout, cmd_checkout, RUN_SETUP }, + { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE }, { checkout-index, cmd_checkout_index, RUN_SETUP | NEED_WORK_TREE}, { cherry, cmd_cherry, RUN_SETUP }, -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git merge commit message issue
[ Please, don't top-post on this list ] Andrey Hsiao andreyhs...@gmail.com writes: It was a bit sad since we use scripts to do the merge, then we can check the conflict list in the log message, and assign to separate owners to resolve them. Would it be possible to make it as an config option? If a script is doing the merge, then I guess you can let your script add the entry in .git/MERGE_MSG (based on something like 'git ls-files -u' or 'git status --porcelain | grep ^U'). This way, your script would work with any version of Git. Having a config option to restore the old behavior would make sense to me at least. I guess it falls in the category patches welcome. -- 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 v6 6/7] git-reflog: add create and exists functions
On 06/29/2015 10:17 PM, David Turner wrote: These are necessary because alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. In a moment, we will use these functions to make git stash work with alternate ref backends. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/git-reflog.txt | 10 ++ builtin/reflog.c | 75 +++- t/t1411-reflog-show.sh | 12 +++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 5e7908e..2bf8aa6 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,6 +23,8 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | refs...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... +'git reflog create' refs... +'git reflog exists' ref Reference logs, or reflogs, record when the tips of branches and other references were updated in the local repository. Reflogs are @@ -52,6 +54,14 @@ argument must be an _exact_ entry (e.g. `git reflog delete master@{2}`). This subcommand is also typically not used directly by end users. +The create subcommand creates a reflog for one or more refs. Most +refs (those under refs/heads, refs/remotes, and refs/tags) will +automatically have reflogs created. Other refs will not. This command +allows manual ref creation. + +The exists subcommand checks whether a ref has a reflog. It exists +with zero status if the reflog exists, and non-zero status if it does +not. OPTIONS --- diff --git a/builtin/reflog.c b/builtin/reflog.c index c2eb8ff..3080865 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -13,6 +13,10 @@ static const char reflog_expire_usage[] = git reflog expire [--expire=time] [--expire-unreachable=time] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] refs...; static const char reflog_delete_usage[] = git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] refs...; +static const char reflog_create_usage[] = +git reflog create refs...; +static const char reflog_exists_usage[] = +git reflog exists ref; static unsigned long default_reflog_expire; static unsigned long default_reflog_expire_unreachable; @@ -699,12 +703,75 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) return status; } +static int cmd_reflog_create(int argc, const char **argv, const char *prefix) +{ + int i, status = 0, start = 0; It looks like start is initialized unconditionally after the first loop, so the initialization here is a red herring. + struct strbuf err = STRBUF_INIT; + + for (i = 1; i argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, --)) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_create_usage); + else + break; + } + + start = i; + + if (argc - start 1) if (start == argc) seems clearer, but that might just be me. + return error(Nothing to create?); + + for (i = start; i argc; i++) { + if (check_refname_format(argv[i], REFNAME_ALLOW_ONELEVEL)) + die(invalid ref format: %s, argv[i]); + } + for (i = start; i argc; i++) { + if (safe_create_reflog(argv[i], err, 1)) { + error(could not create reflog %s: %s, argv[i], + err.buf); + status = 1; + strbuf_release(err); + } + } + return status; +} + So, I have a philosophical question here with a practical side... It appears that this command allows you to create a reflog for a reference that doesn't yet exist. At first blush, this seems to make sense. Suppose you want the creation of a reference to be logged. Then you can first turn on its reflog, then create it. But I think this is going to create problems. Reflogs are rather intimately connected to their references. For example, writes to a reflog are guarded by locking the corresponding reference. And currently a reflog file is deleted when the corresponding reference is deleted. Also, there are constraints about which references can coexist; for example, references refs/heads/foo and refs/heads/foo/bar cannot exist at the same time, because (at least when using the filesystem backend), refs/heads/foo would have to be both a file and a directory at the same time. So if one of these references already exists, it would be an error to create a reflog for the other one. Similarly, if there is a reflog file for one of these references
[PATCH v3 15/23] worktree: add --detach option
One of git-worktree's roles is to populate the new worktree, much like git-checkout, and thus, for convenience, ought to support several of the same shortcuts. Toward this goal, add a --detach option to detach HEAD in the new worktree. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 6 +- builtin/worktree.c | 5 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 8891de0..231271b 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] path branch +'git worktree add' [-f] [--detach] path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -64,6 +64,10 @@ OPTIONS is already checked out by another worktree. This option overrides that safeguard. +--detach:: + With `add`, detach HEAD in the new worktree. See DETACHED HEAD in + linkgit:git-checkout[1]. + -n:: --dry-run:: With `prune`, do not remove anything; just report what it would diff --git a/builtin/worktree.c b/builtin/worktree.c index 8c35023..9dc92b0 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -125,11 +125,12 @@ static int prune(int ac, const char **av, const char *prefix) static int add(int ac, const char **av, const char *prefix) { struct child_process c; - int force = 0; + int force = 0, detach = 0; const char *path, *branch; struct argv_array cmd = ARGV_ARRAY_INIT; struct option options[] = { OPT__FORCE(force, N_(checkout branch even if already checked out in other worktree)), + OPT_BOOL(0, detach, detach, N_(detach HEAD at named commit)), OPT_END() }; @@ -144,6 +145,8 @@ static int add(int ac, const char **av, const char *prefix) argv_array_pushl(cmd, --to, path, NULL); if (force) argv_array_push(cmd, --ignore-other-worktrees); + if (detach) + argv_array_push(cmd, --detach); argv_array_push(cmd, branch); memset(c, 0, sizeof(c)); -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/23] replace checkout --to with worktree add
This is v3 of the series to replace git checkout --to with git worktree add. It's built atop Duy's df0b6cf (worktree: new place for git prune --worktrees, 2015-06-29). Thanks to Duy for his review of v2[*1*]. A v2 to v3 interdiff is included below for ease of review. Changes since v2: * retire --to from git-checkout documentation (v1 did this, but v2 forgot) * add git worktree list to the enumeration of not-yet-implemented commands in the BUGS section of the documentation * state only that git worktree add will create path rather than mentioning that a pre-existing path is valid as long as it is an empty directory[*2*] * fix comment stating that any valid value is acceptable as temporary HEAD in the newly created worktree, as patch 8/23 proves this to be false * use test_cmp_rev to simplify a couple new tests * minor grammatical fixes to documentation and a few commit messages [*1*]: http://thread.gmane.org/gmane.comp.version-control.git/273316 [*2*]: http://article.gmane.org/gmane.comp.version-control.git/273358 Eric Sunshine (23): Documentation/git-checkout: fix incorrect worktree prune command Documentation/git-worktree: associate options with commands Documentation: move linked worktree description from checkout to worktree Documentation/git-worktree: add BUGS section Documentation/git-worktree: split technical info from general description Documentation/git-worktree: add high-level 'lock' overview Documentation/git-worktree: add EXAMPLES section checkout: fix bug with --to and relative HEAD checkout: relocate --to's no branch specified check checkout: prepare_linked_checkout: drop now-unused 'new' argument checkout: make --to unconditionally verbose checkout: drop 'checkout_opts' dependency from prepare_linked_checkout worktree: introduce add command worktree: add --force option worktree: add --detach option worktree: add -b/-B options tests: worktree: retrofit checkout --to tests for worktree add checkout: retire --to option checkout: require worktree unconditionally worktree: extract basename computation to new function worktree: add: make -b/-B default to HEAD when branch is omitted worktree: add: auto-vivify new branch when branch is omitted checkout: retire --ignore-other-worktrees in favor of --force Documentation/git-checkout.txt| 81 + Documentation/git-worktree.txt| 141 ++- builtin/checkout.c| 161 +- builtin/worktree.c| 198 ++ git.c | 2 +- t/{t2025-checkout-to.sh = t2025-worktree-add.sh} | 64 --- t/t2026-prune-linked-checkouts.sh | 2 +- t/t7410-submodule-checkout-to.sh | 4 +- 8 files changed, 382 insertions(+), 271 deletions(-) rename t/{t2025-checkout-to.sh = t2025-worktree-add.sh} (54%) -- 2.5.0.rc1.197.g417e668 --- 8 --- diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 41148ce..6c3085d 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -228,13 +228,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, etc. See - linkgit:git-worktree[1] for a description of linked worktrees. - branch:: Branch to checkout; if it refers to a branch (i.e., a name that, when prepended with refs/heads/, is a valid ref), then that diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 938bdab..da71f50 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -47,13 +47,12 @@ COMMANDS add path [branch]:: -Check out `branch` into a separate working directory, `path`, creating -`path` if necessary. The new working directory is linked to the current -repository, sharing everything except working directory specific files -such as HEAD, index, etc. If `path` already exists, it must be empty. +Create `path` and checkout `branch` into it. The new working directory +is linked to the current repository, sharing everything except working +directory specific files such as HEAD, index, etc. + -If `branch` is omitted and neither `-b` nor `-B` used, then, as a -convenience, a new branch rooted at HEAD is created automatically, as if +If `branch` is omitted and neither `-b` nor `-B` is used, then, as a +convenience, a new branch based at HEAD is created automatically, as if `-b $(basename path)` was specified. prune:: @@ -137,10
[PATCH v3 22/23] worktree: add: auto-vivify new branch when branch is omitted
As a convenience, when branch is omitted from git worktree path branch and neither -b nor -B is used, automatically create a new branch named after path, as if -b $(basename path) was specified. Thus, git worktree add ../hotfix creates a new branch named hotfix and associates it with new worktree ../hotfix. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 8 ++-- builtin/worktree.c | 8 ++-- t/t2025-worktree-add.sh| 5 + 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 377ae0f..da71f50 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] [-b new-branch] path branch +'git worktree add' [-f] [--detach] [-b new-branch] path [branch] 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -45,11 +45,15 @@ pruning should be suppressed. See section DETAILS for more information. COMMANDS -add path branch:: +add path [branch]:: Create `path` and checkout `branch` into it. The new working directory is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. ++ +If `branch` is omitted and neither `-b` nor `-B` is used, then, as a +convenience, a new branch based at HEAD is created automatically, as if +`-b $(basename path)` was specified. prune:: diff --git a/builtin/worktree.c b/builtin/worktree.c index 323d444..69248ba 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -291,12 +291,16 @@ static int add(int ac, const char **av, const char *prefix) die(_(-b and -B are mutually exclusive)); if (ac 1 || ac 2) usage_with_options(worktree_usage, options); - if (ac 2 !new_branch !new_branch_force) - usage_with_options(worktree_usage, options); path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0]; branch = ac 2 ? HEAD : av[1]; + if (ac 2 !new_branch !new_branch_force) { + int n; + const char *s = worktree_basename(path, n); + new_branch = xstrndup(s, n); + } + argv_array_push(cmd, checkout); if (force) argv_array_push(cmd, --ignore-other-worktrees); diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 8964dec..8fe242f 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -145,4 +145,9 @@ test_expect_success 'add -b with branch omitted' ' test_cmp_rev HEAD burble ' +test_expect_success 'add with branch omitted' ' + git worktree add wiffle/bat + test_cmp_rev HEAD bat +' + test_done -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 16/23] worktree: add -b/-B options
One of git-worktree's roles is to populate the new worktree, much like git-checkout, and thus, for convenience, ought to support several of the same shortcuts. Toward this goal, add -b/-B options to create a new branch and check it out in the new worktree. (For brevity, only -b is mentioned in the synopsis; -B is omitted.) Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 13 ++--- builtin/worktree.c | 11 +++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 231271b..f44cd78 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] path branch +'git worktree add' [-f] [--detach] [-b new-branch] path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -64,6 +64,14 @@ OPTIONS is already checked out by another worktree. This option overrides that safeguard. +-b new-branch:: +-B new-branch:: + With `add`, create a new branch named `new-branch` starting at + `branch`, and check out `new-branch` into the new worktree. + By default, `-b` refuses to create a new branch if it already + exists. `-B` overrides this safeguard, resetting `new-branch` to + `branch`. + --detach:: With `add`, detach HEAD in the new worktree. See DETACHED HEAD in linkgit:git-checkout[1]. @@ -133,8 +141,7 @@ make the emergency fix, remove it when done, and then resume your earlier refactoring session. -$ git branch emergency-fix master -$ git worktree add ../temp emergency-fix +$ git worktree add -b emergency-fix ../temp master $ pushd ../temp # ... hack hack hack ... $ git commit -a -m 'emergency fix for boss' diff --git a/builtin/worktree.c b/builtin/worktree.c index 9dc92b0..d6d0eee 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -126,15 +126,22 @@ static int add(int ac, const char **av, const char *prefix) { struct child_process c; int force = 0, detach = 0; + const char *new_branch = NULL, *new_branch_force = NULL; const char *path, *branch; struct argv_array cmd = ARGV_ARRAY_INIT; struct option options[] = { OPT__FORCE(force, N_(checkout branch even if already checked out in other worktree)), + OPT_STRING('b', NULL, new_branch, N_(branch), + N_(create a new branch)), + OPT_STRING('B', NULL, new_branch_force, N_(branch), + N_(create or reset a branch)), OPT_BOOL(0, detach, detach, N_(detach HEAD at named commit)), OPT_END() }; ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (new_branch new_branch_force) + die(_(-b and -B are mutually exclusive)); if (ac != 2) usage_with_options(worktree_usage, options); @@ -145,6 +152,10 @@ static int add(int ac, const char **av, const char *prefix) argv_array_pushl(cmd, --to, path, NULL); if (force) argv_array_push(cmd, --ignore-other-worktrees); + if (new_branch) + argv_array_pushl(cmd, -b, new_branch, NULL); + if (new_branch_force) + argv_array_pushl(cmd, -B, new_branch_force, NULL); if (detach) argv_array_push(cmd, --detach); argv_array_push(cmd, branch); -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 03/23] Documentation: move linked worktree description from checkout to worktree
Now that the git-worktree command exists, its documentation page is the natural place for the linked worktree description to reside. Relocate the MULTIPLE WORKING TREES description verbatim from git-checkout.txt to git-worktree.txt. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-checkout.txt | 69 ++ Documentation/git-worktree.txt | 62 + 2 files changed, 64 insertions(+), 67 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ce223e6..77b7141 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -229,8 +229,8 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode. 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. + specific files such as HEAD, index, etc. See + linkgit:git-worktree[1] for a description of linked worktrees. --ignore-other-worktrees:: `git checkout` refuses when the wanted ref is already checked @@ -401,71 +401,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 tree's entry in the repository's $GIT_DIR/worktrees -directory will eventually be removed automatically (see -`gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run -`git worktree prune` in the main or any linked working tree to -clean up any stale entries in $GIT_DIR/worktrees. - -If you move a linked working directory to another file system, or -within a file system that does not support hard links, you need to run -at least one git command inside the linked working directory -(e.g. `git status`) in order to update its entry in $GIT_DIR/worktrees -so that it does not get automatically removed. - -To prevent a $GIT_DIR/worktrees entry from from being pruned (which -can be useful in some situations, such as when the -entry's working tree is stored on a portable device), add a file named -'locked' to the entry's directory. The file contains the reason in -plain text. For example, if a linked working tree's `.git` file points -to `/path/main/.git/worktrees/test-next` then a file named -`/path/main/.git/worktrees/test-next/locked` will prevent the -`test-next` entry from being pruned. See -linkgit:gitrepository-layout[5] for details. - -Multiple checkout support for submodules is incomplete. It is NOT -recommended to make multiple checkouts of a superproject. - EXAMPLES diff --git a/Documentation/git-worktree.txt
[PATCH v3 09/23] checkout: relocate --to's no branch specified check
The plan is to relocate git checkout --to functionality to git worktree add, however, this check expects a 'struct branch_info' which git-worktree won't have at hand. It will, however, have access to its own command-line from which it can pick up the branch name. Therefore, as a preparatory step, rather than having prepare_linked_checkout() perform this check, make it the caller's responsibility. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 5ada22a..162c822 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -865,8 +865,6 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, int counter = 0, len, ret; unsigned char rev[20]; - if (!new-commit) - die(_(no branch specified)); if (file_exists(path) !is_empty_dir(path)) die(_('%s' already exists), path); @@ -1303,8 +1301,11 @@ static int checkout_branch(struct checkout_opts *opts, free(head_ref); } - if (opts-new_worktree) + if (opts-new_worktree) { + if (!new-commit) + die(_(no branch specified)); return prepare_linked_checkout(opts, new); + } if (!new-commit opts-new_branch) { unsigned char rev[20]; -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 23/23] checkout: retire --ignore-other-worktrees in favor of --force
As a safeguard, checking out a branch already checked out by a different worktree is disallowed. This behavior can be overridden with --ignore-other-worktrees, however, this option is neither obvious nor particularly discoverable. As a common safeguard override, --force is more likely to come to mind. Therefore, overload it to also suppress the check for a branch already checked out elsewhere. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- I plopped this patch at the end of the series so that it can be dropped easily if desired since Junio indicated[1] that he would moderately object to this change. [1]: http://thread.gmane.org/gmane.comp.version-control.git/273032/focus=273108 Documentation/git-checkout.txt | 9 +++-- builtin/checkout.c | 8 +++- builtin/worktree.c | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index efe6a02..6c3085d 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -111,6 +111,9 @@ OPTIONS + When checking out paths from the index, do not fail upon unmerged entries; instead, unmerged entries are ignored. ++ +By default, checking out a branch already checked out in another worktree +is disallowed. This overrides that safeguard. --ours:: --theirs:: @@ -225,12 +228,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. ---ignore-other-worktrees:: - `git checkout` refuses when the wanted ref is already checked - out by another worktree. This option makes it check the ref - out anyway. In other words, the ref can be held by more than one - worktree. - branch:: Branch to checkout; if it refers to a branch (i.e., a name that, when prepended with refs/heads/, is a valid ref), then that diff --git a/builtin/checkout.c b/builtin/checkout.c index 5754554..01c7c30 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -35,7 +35,6 @@ struct checkout_opts { int writeout_stage; int overwrite_ignore; int ignore_skipworktree; - int ignore_other_worktrees; const char *new_branch; const char *new_branch_force; @@ -903,7 +902,8 @@ static void check_linked_checkout(struct branch_info *new, const char *id) strbuf_rtrim(gitdir); } else strbuf_addstr(gitdir, get_git_common_dir()); - die(_('%s' is already checked out at '%s'), new-name, gitdir.buf); + die(_('%s' is already checked out at '%s'; use --force to override), + new-name, gitdir.buf); done: strbuf_release(path); strbuf_release(sb); @@ -1151,7 +1151,7 @@ static int checkout_branch(struct checkout_opts *opts, char *head_ref = resolve_refdup(HEAD, 0, sha1, flag); if (head_ref (!(flag REF_ISSYMREF) || strcmp(head_ref, new-path)) - !opts-ignore_other_worktrees) + !opts-force) check_linked_checkouts(new); free(head_ref); } @@ -1198,8 +1198,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) N_(do not limit pathspecs to sparse entries only)), OPT_HIDDEN_BOOL(0, guess, dwim_new_local_branch, N_(second guess 'git checkout no-such-branch')), - OPT_BOOL(0, ignore-other-worktrees, opts.ignore_other_worktrees, -N_(do not check if another worktree is holding the given ref)), OPT_END(), }; diff --git a/builtin/worktree.c b/builtin/worktree.c index 69248ba..050b443 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -303,7 +303,7 @@ static int add(int ac, const char **av, const char *prefix) argv_array_push(cmd, checkout); if (force) - argv_array_push(cmd, --ignore-other-worktrees); + argv_array_push(cmd, --force); if (new_branch) argv_array_pushl(cmd, -b, new_branch, NULL); if (new_branch_force) -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 08/23] checkout: fix bug with --to and relative HEAD
Given git checkout --to path HEAD~1, the new worktree's HEAD should begin life at the current branch's HEAD~1, however, it actually ends up at HEAD~2. This happens because: 1. git-checkout resolves HEAD~1 2. to satisfy is_git_directory(), prepare_linked_worktree() creates a HEAD for the new worktree with the value of the resolved HEAD~1 3. git-checkout re-invokes itself with the same arguments within the new worktree to populate the worktree 4. the sub git-checkout resolves HEAD~1 relative to its own HEAD, which is the resolved HEAD~1 from the original invocation, resulting unexpectedly and incorrectly in HEAD~2 (relative to the original) Fix this by unconditionally assigning the current worktree's HEAD as the value of the new worktree's HEAD. As a side-effect, this change also eliminates a dependence within prepare_linked_checkout() upon 'struct branch_info'. The plan is to eventually relocate git checkout --to functionality to git worktree add, and worktree.c won't have knowledge of 'struct branch_info', so removal of this dependency is a step toward that goal. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 16 t/t2025-checkout-to.sh | 10 ++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 2079aa4..5ada22a 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -863,6 +863,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, struct stat st; struct child_process cp; int counter = 0, len, ret; + unsigned char rev[20]; if (!new-commit) die(_(no branch specified)); @@ -920,13 +921,20 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, real_path(get_git_common_dir()), name); /* * This is to keep resolve_ref() happy. We need a valid HEAD -* or is_git_directory() will reject the directory. Any valid -* value would do because this value will be ignored and -* replaced at the next (real) checkout. +* or is_git_directory() will reject the directory. Moreover, HEAD +* in the new worktree must resolve to the same value as HEAD in +* the current tree since the command invoked to populate the new +* worktree will be handed the branch/ref specified by the user. +* For instance, if the user asks for the new worktree to be based +* at HEAD~5, then the resolved HEAD~5 in the new worktree must +* match the resolved HEAD~5 in the current tree in order to match +* the user's expectation. */ + if (!resolve_ref_unsafe(HEAD, 0, rev, NULL)) + die(_(unable to resolve HEAD)); strbuf_reset(sb); strbuf_addf(sb, %s/HEAD, sb_repo.buf); - write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1)); + write_file(sb.buf, 1, %s\n, sha1_to_hex(rev)); strbuf_reset(sb); strbuf_addf(sb, %s/commondir, sb_repo.buf); write_file(sb.buf, 1, ../..\n); diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index a8d9336..0fd731b 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -134,4 +134,14 @@ test_expect_success 'checkout with grafts' ' test_cmp expected actual ' +test_expect_success 'checkout --to from relative HEAD' ' + test_commit a + test_commit b + test_commit c + git rev-parse HEAD~1 expected + git checkout --to relhead HEAD~1 + git -C relhead rev-parse HEAD actual + test_cmp expected actual +' + test_done -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 21/23] worktree: add: make -b/-B default to HEAD when branch is omitted
As a convenience, like git branch and git checkout -b, make git worktree add -b newbranch path branch default to HEAD when branch is omitted. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 1 + builtin/worktree.c | 6 -- t/t2025-worktree-add.sh| 5 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index f44cd78..377ae0f 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -68,6 +68,7 @@ OPTIONS -B new-branch:: With `add`, create a new branch named `new-branch` starting at `branch`, and check out `new-branch` into the new worktree. + If `branch` is omitted, it defaults to HEAD. By default, `-b` refuses to create a new branch if it already exists. `-B` overrides this safeguard, resetting `new-branch` to `branch`. diff --git a/builtin/worktree.c b/builtin/worktree.c index 25fe25b..323d444 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -289,11 +289,13 @@ static int add(int ac, const char **av, const char *prefix) ac = parse_options(ac, av, prefix, options, worktree_usage, 0); if (new_branch new_branch_force) die(_(-b and -B are mutually exclusive)); - if (ac != 2) + if (ac 1 || ac 2) + usage_with_options(worktree_usage, options); + if (ac 2 !new_branch !new_branch_force) usage_with_options(worktree_usage, options); path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0]; - branch = av[1]; + branch = ac 2 ? HEAD : av[1]; argv_array_push(cmd, checkout); if (force) diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 192c936..8964dec 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -140,4 +140,9 @@ test_expect_success 'add from relative HEAD' ' test_cmp expected actual ' +test_expect_success 'add -b with branch omitted' ' + git worktree add -b burble flornk + test_cmp_rev HEAD burble +' + test_done -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 13/23] worktree: introduce add command
The plan is to relocate git checkout --to functionality to git worktree add. As a first step, introduce a bare-bones git-worktree add command along with documentation. At this stage, git worktree add merely invokes git checkout --to behind the scenes, but an upcoming patch will move the actual functionality (checkout.c:prepare_linked_checkout() and its helpers) to worktree.c. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 20 ++-- builtin/worktree.c | 31 +++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 6afff1e..def3027 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,13 +9,13 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] +'git worktree add' path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION --- -Manage multiple worktrees attached to the same repository. These are -created by the command `git checkout --to`. +Manage multiple worktrees attached to the same repository. 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 @@ -45,6 +45,12 @@ pruning should be suppressed. See section DETAILS for more information. COMMANDS +add path branch:: + +Create `path` and checkout `branch` into it. The new working directory +is linked to the current repository, sharing everything except working +directory specific files such as HEAD, index, etc. + prune:: Prune working tree information in $GIT_DIR/worktrees. @@ -118,7 +124,7 @@ refactoring session. $ git branch emergency-fix master -$ git checkout --to ../temp emergency-fix +$ git worktree add ../temp emergency-fix $ pushd ../temp # ... hack hack hack ... $ git commit -a -m 'emergency fix for boss' @@ -133,9 +139,8 @@ Multiple checkout support for submodules is incomplete. It is NOT recommended to make multiple checkouts of a superproject. git-worktree could provide more automation for tasks currently -performed manually or via other commands, such as: +performed manually, such as: -- `add` to create a new linked worktree - `remove` to remove a linked worktree and its administrative files (and warn if the worktree is dirty) - `mv` to move or rename a worktree and update its administrative files @@ -143,11 +148,6 @@ performed manually or via other commands, such as: - `lock` to prevent automatic pruning of administrative files (for instance, for a worktree on a portable device) -SEE ALSO - - -linkgit:git-checkout[1] - GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/worktree.c b/builtin/worktree.c index 2a729c6..e0749c0 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -2,8 +2,11 @@ #include builtin.h #include dir.h #include parse-options.h +#include argv-array.h +#include run-command.h static const char * const worktree_usage[] = { + N_(git worktree add path branch), N_(git worktree prune [options]), NULL }; @@ -119,6 +122,32 @@ static int prune(int ac, const char **av, const char *prefix) return 0; } +static int add(int ac, const char **av, const char *prefix) +{ + struct child_process c; + const char *path, *branch; + struct argv_array cmd = ARGV_ARRAY_INIT; + struct option options[] = { + OPT_END() + }; + + ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (ac != 2) + usage_with_options(worktree_usage, options); + + path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0]; + branch = av[1]; + + argv_array_push(cmd, checkout); + argv_array_pushl(cmd, --to, path, NULL); + argv_array_push(cmd, branch); + + memset(c, 0, sizeof(c)); + c.git_cmd = 1; + c.argv = cmd.argv; + return run_command(c); +} + int cmd_worktree(int ac, const char **av, const char *prefix) { struct option options[] = { @@ -127,6 +156,8 @@ int cmd_worktree(int ac, const char **av, const char *prefix) if (ac 2) usage_with_options(worktree_usage, options); + if (!strcmp(av[1], add)) + return add(ac - 1, av + 1, prefix); if (!strcmp(av[1], prune)) return prune(ac - 1, av + 1, prefix); usage_with_options(worktree_usage, options); -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 14/23] worktree: add --force option
By default, git worktree add refuses to create a new worktree when the requested branch is already checked out elsewhere. Add a --force option to override this safeguard. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 8 +++- builtin/worktree.c | 6 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index def3027..8891de0 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' path branch +'git worktree add' [-f] path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -58,6 +58,12 @@ Prune working tree information in $GIT_DIR/worktrees. OPTIONS --- +-f:: +--force:: + By default, `add` refuses to create a new worktree when `branch` + is already checked out by another worktree. This option overrides + that safeguard. + -n:: --dry-run:: With `prune`, do not remove anything; just report what it would diff --git a/builtin/worktree.c b/builtin/worktree.c index e0749c0..8c35023 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -6,7 +6,7 @@ #include run-command.h static const char * const worktree_usage[] = { - N_(git worktree add path branch), + N_(git worktree add [options] path branch), N_(git worktree prune [options]), NULL }; @@ -125,9 +125,11 @@ static int prune(int ac, const char **av, const char *prefix) static int add(int ac, const char **av, const char *prefix) { struct child_process c; + int force = 0; const char *path, *branch; struct argv_array cmd = ARGV_ARRAY_INIT; struct option options[] = { + OPT__FORCE(force, N_(checkout branch even if already checked out in other worktree)), OPT_END() }; @@ -140,6 +142,8 @@ static int add(int ac, const char **av, const char *prefix) argv_array_push(cmd, checkout); argv_array_pushl(cmd, --to, path, NULL); + if (force) + argv_array_push(cmd, --ignore-other-worktrees); argv_array_push(cmd, branch); memset(c, 0, sizeof(c)); -- 2.5.0.rc1.197.g417e668 -- To unsubscribe from this list: send the line unsubscribe 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: The server can munge pkt-lines and reinsert LFs, but it _must_ have some way of reconstructing the payload that the client signed in order to verify the signature. If we just naively insert LFs where missing, we lose the ability to verify the signature. I still do not understand this part. There is no way to naively insert, is there? You have an array of lines (each of which you have already stripped its terminating LF at its end). How else other than adding one LF at the end of each element do you reconstruct the original multi-line string the client signed? Are there other ways that makes the result ambiguous?? If we say the payload the client signs MUST have LFs only in certain places, then that gives the server enough information to reconstruct the payload and verify the signature. But if we say the signed payload MUST have LFs and the wire payload MAY have LFs, then now we have two completely different formats, only one of which is documented. I thought that was what I was saying. The wire protocol sends the contents of each line (both what is signed and the signature) on a separate packet. When I say contents of a line, I do not include the terminating LF as part of the line (iow, LF is not even optional; the terminating LF is not considered as part of the contents of a line). It becomes irrelevant that a pkt-line may or may not have a trailing optional LF. If there is LF at the end of a packet between push-cert and push-cert-end packets, that LF by definition cannot be part of the contents of a line in a certificate. It is just a pkt-line framing artifact you can and should remove if you are doing a split to array, join with LF implementation to recover the original string. And that is very much consistent with the way we send other things with pkt-line protocol. Each packet up to the first flush is expected to have object name and refname as ref advertisement. The pkt-line framing may or may not add a trailing LF, but LF is not part of refname. It is not even part of the payload; merely an artifact of pkt-line framing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Draft of Git Rev News edition 5
On Mon, Jul 6, 2015 at 12:38 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: I'm not familiar with the criteria for deciding what merits mention in the newsletter. Is the recent introduction of git-worktree and the attendant relocation of add and prune functionality worthy? If so, perhaps the following write-up would be suitable? One issue I had with this text was that it was not immediately clear what the end-game UI of the feature was. Is checkout --to they way the user is expected to trigger this? It appears in the very early part of the multi-paragraph description and I suspect that the majority of the users would think that way, not with worktree add that appears a lot later. I had the same concern when proof-reading, but wasn't sure if the concern was warranted. Since you reacted to the text in the same way, I'd say the concern was justified. How about this instead: prefixing with As originally implemented, with a couple s/is/was/ thrown in... As originally implemented, creation of linked-worktrees was accomplished via `git checkout --to path branch`, and cleanup of leftover administrative files, after `path` is deleted, was done with `git prune --worktrees`. However, a recent unrelated change to `git prune` led to a discussion that concluded that worktree-related maintenance functionality didn't belong in `git prune`. Is that sufficient to clue in the reader that checkout --to is not final form, or should we mention worktree add and worktree prune upfront? -- To unsubscribe from this list: send the line unsubscribe 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 Mon, Jul 6, 2015 at 1:34 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: Another way of looking at the problem with my assumptions is, I was assuming pkt-line framing was the same thing as pkt-line header. You seem to be saying the definition of pkt-line framing is header, and optional trailing newline. Yes. I thought that was what Server SHOULD terminate with LF; client MUST NOT require it in the existing text meant. Unfortunately, the existing text is littered with examples of PKT-LINE(foo SP bar LF). If we assume PKT-LINE(...) means apply pkt-line framing to the [...], then this strongly implies that pkt-line framing does _not_ include the trailing LF. (Or the logical but bizarre alternative reading that such an example might have _two_ trailing LFs :) Ah, that reminds me of one thing I already said elsewhere. We need to correct the above with s/Server/Sender/; s/Client/Receiver/; I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
Dave Borowitz dborow...@google.com writes: On Mon, Jul 6, 2015 at 1:34 PM, Junio C Hamano gits...@pobox.com wrote: Dave Borowitz dborow...@google.com writes: Another way of looking at the problem with my assumptions is, I was assuming pkt-line framing was the same thing as pkt-line header. You seem to be saying the definition of pkt-line framing is header, and optional trailing newline. Yes. I thought that was what Server SHOULD terminate with LF; client MUST NOT require it in the existing text meant. Unfortunately, the existing text is littered with examples of PKT-LINE(foo SP bar LF). If we assume PKT-LINE(...) means apply pkt-line framing to the [...], then this strongly implies that pkt-line framing does _not_ include the trailing LF. (Or the logical but bizarre alternative reading that such an example might have _two_ trailing LFs :) Yes, But I never viewed PKT-LINE() as an element that strictly defines the grammar of the packet protocol ;-) By clarifying that sender SHOULD terminate with LF, receiver MUST NOT require it is the rule (and fixing the existing implementations at places where they violate the MUST NOT part, which I think are very small number of places), I think we can drop these LF (or LF? for that matter) from all of the PKT-LINE() in the construction in the pack-protocol.txt, which would be a very good thing to do. The example in your sentence will become PKT-LINE(foo SP bar) and the there may be an LF at the end would only be at one place, as a part of the definition of PKT-LINE(). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 5/7] refs: new public ref function: safe_create_reflog
On 06/29/2015 10:17 PM, David Turner wrote: The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 10 +- refs.c | 25 + refs.h | 2 +- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 93f63d3..9f68399 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -620,19 +620,11 @@ static void update_refs_for_switch(const struct checkout_opts *opts, if (opts-new_branch) { if (opts-new_orphan_branch) { if (opts-new_branch_log !log_all_ref_updates) { - int temp; - struct strbuf log_file = STRBUF_INIT; - int ret; const char *ref_name; struct strbuf err = STRBUF_INIT; ref_name = mkpath(refs/heads/%s, opts-new_orphan_branch); - temp = log_all_ref_updates; - log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, log_file, err); - log_all_ref_updates = temp; - strbuf_release(log_file); - if (ret) { + if (safe_create_reflog(ref_name, err, 1)) { fprintf(stderr, _(Can not do reflog for '%s'. %s\n), opts-new_orphan_branch, err.buf); strbuf_release(err); This was probably already dangerous before your change, but... mkpath() returns a pointer to a static buffer. It is subject to being overwritten if any of a number of path-related functions is called. So passing it into a function is dangerous. Instead, you should store it into memory that you control, for example by using a strbuf and strbuf_addf(). Also, we usually call variables holding reference names refname, not ref_name. Maybe rename the variable while you are in the area. diff --git a/refs.c b/refs.c index 30e81ba..1e53ef0 100644 --- a/refs.c +++ b/refs.c @@ -3128,8 +3128,14 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, HEAD); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * This function creates a reflog for a ref. If force_create = 0, the + * reflog will only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, it will be + * created regardless of the ref name. This function will fill in *err + * and return -1 on failure + */ It is preferable to write function docstrings in the imperative voice: Create a reflog for a ref. If force_create == 0, only create the reflog for certain refs... +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3138,7 +3144,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile-buf; /* make sure the rest of the function can't change logfile */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) 0) { strbuf_addf(err, unable to create directory for %s. %s, logfile, strerror(errno)); @@ -3173,6 +3179,17 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf return 0; } + +int safe_create_reflog(const char *refname, struct strbuf *err, int force_create) +{ + int ret; + struct strbuf sb = STRBUF_INIT; + + ret = log_ref_setup(refname, sb, err, force_create); + strbuf_release(sb); + return ret; +} + Is it really necessary to have two functions, safe_create_reflog() and log_ref_setup()? I don't see any of the callers doing anything special with the sb_logfile argument from the latter, so maybe it could be inlined into safe_create_reflog()? Maybe I'm overlooking something. [...] Michael -- Michael Haggerty
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Mon, Jul 6, 2015 at 12:28 PM, Junio C Hamano gits...@pobox.com wrote: Shawn Pearce spea...@spearce.org writes: push cert format is like commit or tag format. You need those LFs. We can't just go declare them optional because of the way pkt-line read function is implemented in git-core. As I said, I view each of the packets between push-cert and push-cert-end packets representing the meat of each line in the cert. The sending end takes a cert as a long multi-line string, splits them into an array, each of whose element represents a line in it (iow certlines = certstring.split('\n')), and sends them packetised. Right now the sending end sends newlines. The receiver receives a sequence of packets, notices push-cert packet, collects packets until it sees push-cert-end packet and treats them as elements of this array. pkt-line deframing process would have to strip optional LFs to reconstruct the original array the sender had (i.e. the above certlines array). The problem is the signature. Today, the client computes the signature over the payload it actually sends (minus pkt-line headers) The server can munge pkt-lines and reinsert LFs, but it _must_ have some way of reconstructing the payload that the client signed in order to verify the signature. If we just naively insert LFs where missing, we lose the ability to verify the signature. If we say the payload the client signs MUST have LFs only in certain places, then that gives the server enough information to reconstruct the payload and verify the signature. But if we say the signed payload MUST have LFs and the wire payload MAY have LFs, then now we have two completely different formats, only one of which is documented. The receiver needs to join the array with LF to recover the long multi-line string once it received the array. But this LF does not have anything to do with the optional trailing LF in pkt-line. If you sent the original certlines array via different RPC mechanism, you need to join them together with your own LF to reconstruct the multi-line string. -- To unsubscribe from this list: send the line unsubscribe 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 think I understand the confusion now. I think you and I are working from different assumptions about the client behavior. I agree that we now both understand where we come from ;-) And sorry for not being clear when I did the push-cert originally in the documentation. As I already said, packets between push-cert and push-cert-end are contents of individual lines of the GPG signed push certificate was the design meant from day one, and a85b377d (push: the beginning of git push --signed, 2014-09-12) could have made it clearer. The problem with the documentation, then, is that the documentation does not say anything to indicate that the signed payload is anything other than what is on the wire. Yeah, that was untold assumption, as I considered what is on the wire to include pkt-line framing when I wrote a85b377d (push: the beginning of git push --signed, 2014-09-12). So maybe this series should include an explicit description of the singed payload outside of the context of a push. Then, in the push section, we can describe the set of transformations that the client MUST perform (splitting on LF; adding pkt-line headers) and MAY perform (adding LFs). Yes, and the latter is not limited to push-cert but anything sent on pkt-line. That incidentally is the only point I deeply care about. I just want to minimize the protocol is this way in general, but only for this one you must do it differently. One example of only for this one you must do it differently is another caveat for protocol implementors for the sending side (again not limited to push cert). That existing implementations of the receivers treat an empty packet (i.e. 0004) as if it is the same as a flush packet (i.e. ), so even if the sending side chooses to ignore the SHOULD terminate each non-flush line using LF, it is strongly advised not to do so when it wants to send an empty payload. This needs to be documented. The receiving end SHOULD NOT treat 0004 the same way as . I think that must be documented and implementations (including our own) should be fixed. 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 01/12] t4150: am.messageid really adds the message id
Thanks, both. -- To unsubscribe from this list: send the line unsubscribe 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 07/12] t4150: am with applypatch-msg hook
Paul Tan pyoka...@gmail.com writes: +test_expect_success 'am with applypatch-msg hook' ' + test_when_finished rm -f .git/hooks/applypatch-msg + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/applypatch-msg -\EOF + #!/bin/sh + cat $1 actual-msg + echo hook-message $1 + EOF + chmod +x .git/hooks/applypatch-msg This (and the other one below) looks like a good candidate for the write_script helper. + git am patch1 + test_path_is_missing .git/rebase-apply + git diff --exit-code second + echo hook-message expected + git log -1 --format=format:%B actual + test_cmp expected actual + git log -1 --format=format:%B second expected + test_cmp expected actual-msg +' + +test_expect_success 'am with failing applypatch-msg hook' ' + test_when_finished rm -f .git/hooks/applypatch-msg + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/applypatch-msg -\EOF + #!/bin/sh + exit 1 + EOF + chmod +x .git/hooks/applypatch-msg + test_must_fail git am patch1 + test_path_is_dir .git/rebase-apply + git diff --exit-code first + test_cmp_rev first HEAD +' + test_expect_success 'setup: new author and committer' ' GIT_AUTHOR_NAME=Another Thor GIT_AUTHOR_EMAIL=a.t...@example.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added ability to scip patch hunk with an empty filename.
Evgen Druzhynin evgen.druzhy...@gmail.com writes: This space is for you to explain why it is a good idea to do this change. What problem does it solve, how often does that problem appear, what other workarounds exist if any and why they are not satisfactory, etc, etc? --- Above this line, we would want you to sign-off your patch, certifying that you read DCO (see Documentation/SubmittingPatches) and you wrote it or otherwise have the right to pass it on as an open-source patch. builtin/apply.c | 16 +--- po/bg.po| 15 --- po/ca.po| 15 --- po/de.po| 15 --- po/fr.po| 15 --- po/git.pot | 11 --- po/it.po| 11 --- po/pt_PT.po | 7 --- po/ru.po| 12 po/sv.po| 16 po/vi.po| 12 po/zh_CN.po | 11 --- We do not want anybody who is not doing the i18n/l10n to touch any po/* files. The i18n/l10n team will adjust their translations accordingly when the code changes. 12 files changed, 5 insertions(+), 151 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 54aba4e..e4481b4 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1488,17 +1488,11 @@ static int find_header(const char *line, unsigned long size, int *hdrsize, struc int git_hdr_len = parse_git_header(line, len, size, patch); if (git_hdr_len = len) continue; - if (!patch-old_name !patch-new_name) { - if (!patch-def_name) - die(Q_(git diff header lacks filename information when removing -%d leading pathname component (line %d), -git diff header lacks filename information when removing -%d leading pathname components (line %d), -p_value), - p_value, linenr); - patch-old_name = xstrdup(patch-def_name); - patch-new_name = xstrdup(patch-def_name); - } + + /* pass this hunk if the filename length is zero */ + if (!patch-old_name !patch-new_name) + return -1; Please explain why this is a good idea. This can happen only when (1) the user gave too large a number to the -p option or (2) the original patch was corrupt. In either case, we would want to stop and give the user a chance to correct it, either by specifying the correct -p number, or editing the corrupt patch to state the right path the patch applies to. Silently dropping the patch is never a good thing to do, is 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 v3 18/23] checkout: retire --to option
Eric Sunshine sunsh...@sunshineco.com writes: Now that git worktree add has achieved user-facing feature-parity with git checkout --to, retire the latter. Move the actual linked worktree creation functionality, prepare_linked_checkout() and its helpers, verbatim from checkout.c to worktree.c. This effectively reverts changes to checkout.c by 529fef2 (checkout: support checking out into a new working directory, 2014-11-30) with the exception of merge_working_tree() and switch_branches() which still require specialized knowledge that a the checkout is occurring in a newly-created linked worktree (signaled to them by the private GIT_CHECKOUT_NEW_WORKTREE environment variable). I do not quite understand why we still need the hidden environment variable. Is this a sign that the implementation is shared too much between unrelated codepaths (or to put it another way, perhaps API functions that are not best fit are being used)? Stepping back a bit, with or without the new linked worktree feature, when you came across a repository whose working tree does not have any file (i.e. somebody ran git ls-files | xargs rm), you do not know and care what is in .git/index right now, you do not know and care what branch its .git/HEAD points at, but you *do* know what branch you want to be on (or where you want its HEAD detached at), what would be the command you would use? The state immediately after a new worktree is constructed by populating /path/main/.git/worktrees/test-next/ and pointing it from /path/other/test-next/.git but before the index or the files under /path/other/test-next/ are populated is exactly that situation, no? Wouldn't symbolic-ref HEAD the-branch-i-want (or update-ref HEAD the-commit-i-want in the detached case) followed by reset --hard the more natural thing to use, instead of merge-working-tree and switch-branches that are implementation details of checkout? -- To unsubscribe from this list: send the line unsubscribe 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: By clarifying that sender SHOULD terminate with LF, receiver MUST NOT require it is the rule (and fixing the existing implementations at places where they violate the MUST NOT part, which I think are very small number of places), I think we can drop these LF (or LF? for that matter) from all of the PKT-LINE() in the construction in the pack-protocol.txt, which would be a very good thing to do. The example in your sentence will become PKT-LINE(foo SP bar) and the there may be an LF at the end would only be at one place, as a part of the definition of PKT-LINE(). I quickly scanned both the sources where we use packet_write() in the code and say PKT-LINE in the doc; aside from the actual packfile transfer that happens on the sideband, which technically _is_ a user of PKT-LINE, we do not send anything that does not end with a text in PKT-LINE. I just wanted to make sure that there may or may not be an LF at the end; if there is, it is not part of the payload but is part of the framing does not invite new implementors to break their binary transfer by reading the definition of PKT-LINE too literally to mean ok, so I stuffed this 998 byte binary gunk to the packet and insert an optional LF before sending the remainder in separate packets. 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
refspecs with '*' as part of pattern
Hi, I've been looking at the refspecs for git fetch, and noticed that globs are partially supported. I wanted to use something like: refs/tags/some-prefix-*:refs/tags/some-prefix-* as a refspec, so that I can fetch only tags which have a specific prefix. I know that I could use namespaces to separate tags, but unfortunately, I am unable to fix the tag format. The specific repository in question is also generating several tags which are not relevant to me, in formats that are not really useful for human consumption. I am also not able to fix this less than useful practice. However, I noticed that refspecs only support * as a single component. The match algorithm works perfectly fine, as documented in abd2bde78bd9 (Support '*' in the middle of a refspec) What is the reason for not allowing slightly more arbitrary expressions? Obviously no more than one *... It seems like the strict requirements, as in the above commit, but i an unable to find what these requirements are, and why they can't allow more expressions. It's possible that users might not expect it to work, but maybe it could be configured behind an extra option to prevent accidental use? I think it's quite intuitive though. Maybe because it allows some shenanagins to rename tags, but... that's really the user's fault... Another reason for putting this behind an option possibly? Personally I would like to be able to use this as it allows much more fine grained control over what I fetch, and lets me stick with the un-namespaced tags which are something I can't change. If this isn't something that can be done I would appreciate a good explanation of what it might break and why it's a bad idea... Regards, Jake -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Visualizing merge conflicts after the fact (using kdiff3)
On 16.06.2015 03:17, Eric Raible wrote: So naturally I want to check each of them for correctness. Sorry for joining this thread so late, I only come to know about it from the draft of the upcoming Git Rev News 5 [1]. A while ago Robin Green was asking a very similar question on StackOverflow [2], and I came up with a script called git-show-merge-resolution.sh [3]. Maybe that's something you're interested in, too. [1] https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-5.md#support [2] stackoverflow.com/questions/24958182/kdiff3-to-code-review-merge-commit/24958228 [3] https://github.com/sschuberth/dev-scripts/blob/master/git/git-show-merge-resolution.sh -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/23] worktree: add: auto-vivify new branch when branch is omitted
Eric Sunshine sunsh...@sunshineco.com writes: diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 377ae0f..da71f50 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] [-b new-branch] path branch +'git worktree add' [-f] [--detach] [-b new-branch] path [branch] Ahh, OK, this answers my previous question. + if (ac 2 !new_branch !new_branch_force) { + int n; + const char *s = worktree_basename(path, n); + new_branch = xstrndup(s, n); + } + and because this is new_branch, not new_branch_force, we will not accidentally clobber an existing branch. The hotfix time is when the end-user tends to be less careful, and it is a good thing to make sure git worktree add ../hotfix will not clobber an unrelated hotfix branch. Good. Which may be something we would want to have a test for, though. diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 8964dec..8fe242f 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -145,4 +145,9 @@ test_expect_success 'add -b with branch omitted' ' test_cmp_rev HEAD burble ' +test_expect_success 'add with branch omitted' ' + git worktree add wiffle/bat + test_cmp_rev HEAD bat +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 23/23] checkout: retire --ignore-other-worktrees in favor of --force
Eric Sunshine sunsh...@sunshineco.com writes: As a safeguard, checking out a branch already checked out by a different worktree is disallowed. This behavior can be overridden with --ignore-other-worktrees, however, this option is neither obvious nor particularly discoverable. As a common safeguard override, --force is more likely to come to mind. Therefore, overload it to also suppress the check for a branch already checked out elsewhere. I hate to be asking this again but why is it a good idea to allow 'ignore-other-worktrees' in the first place (let alone making it more discoverable)? You'll have multiple working trees, either using the new git worktree or using the old contrib/workdir, for one of the two reasons: * You need a separate work area to build a new history. * You need a separate work area to expand the contents of a specific commit. Here create binary by running make falls into the latter category; as far as Git is concerned, you are only looking at, not extending the history of any specific branch. If you are extending the history of some branch, then you would want to be on that branch. Why would you want to have another worktree that will get into a confusing state once you create that commit on the checked out branch in this newly created worktree? Wasn't the whole point of making the primary repository aware of the secondary worktrees via the linked checkout mechanism because that confusion was the biggest sore point of the old contrib/workdir 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] grep: use regcomp() for icase search with non-ascii patterns
On Tue, Jul 7, 2015 at 3:10 AM, René Scharfe l@web.de wrote: Am 06.07.2015 um 14:42 schrieb Nguyễn Thái Ngọc Duy: Noticed-by: Plamen Totev plamen.to...@abv.bg Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- grep.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/grep.c b/grep.c index b58c7c6..48db15a 100644 --- a/grep.c +++ b/grep.c @@ -378,7 +378,7 @@ static void free_pcre_regexp(struct grep_pat *p) } #endif /* !USE_LIBPCRE */ -static int is_fixed(const char *s, size_t len) +static int is_fixed(const char *s, size_t len, int ignore_icase) { size_t i; @@ -391,6 +391,13 @@ static int is_fixed(const char *s, size_t len) for (i = 0; i len; i++) { if (is_regex_special(s[i])) return 0; + /* +* The builtin substring search can only deal with case +* insensitivity in ascii range. If there is something outside +* of that range, fall back to regcomp. +*/ + if (ignore_icase (unsigned char)s[i] = 128) + return 0; How about isascii(s[i])? Yes, better. } return 1; @@ -398,18 +405,19 @@ static int is_fixed(const char *s, size_t len) static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) { + int ignore_icase = opt-regflags REG_ICASE || p-ignore_case; int err; p-word_regexp = opt-word_regexp; p-ignore_case = opt-ignore_case; Using p-ignore_case before this line, as in initialization of the new variable ignore_icase above, changes the meaning. Oops. - if (opt-fixed || is_fixed(p-pattern, p-patternlen)) + if (opt-fixed || is_fixed(p-pattern, p-patternlen, ignore_icase)) p-fixed = 1; else p-fixed = 0; if (p-fixed) { - if (opt-regflags REG_ICASE || p-ignore_case) + if (ignore_case) ignore_icase instead? ignore_case is for the config variable core.ignorecase. Tricky. Maybe we can test isascii separately and save the result in has_non_ascii, then we can avoid ignore_(i)case p-kws = kwsalloc(tolower_trans_tbl); else p-kws = kwsalloc(NULL); So the optimization before this patch was that if a string was searched for without -F then it would be treated as a fixed string anyway unless it contained regex special characters. Searching for fixed strings using the kwset functions is faster than using regcomp and regexec, which makes the exercise worthwhile. Your patch disables the optimization if non-ASCII characters are searched for because kwset handles case transformations only for ASCII chars. Another consequence of this limitation is that -Fi (explicit case-insensitive fixed-string search) doesn't work properly with non-ASCII chars neither. How can we handle this one? Fall back to regcomp by escaping all special characters? Or at least warn? Hehe.. I noticed it too shortly after sending the patch. I was torn between simply documenting the limitation and waiting for the next person to come and fix it, or quoting the regex then passing to regcomp. GNU grep does the quoting in this case, but that code is GPLv3 so we can't simply copy over. It could be a problem if we need to quote a regex in a multibyte charset where ascii is not a subset. But i guess we can just go with utf-8.. Tests would be nice. :) Yeah.. but we now rely on system regcomp which may behave differently across platforms. Then we need some locale to be always there. Some platforms (like Gentoo) even allow building glibc without i18n.. So I'm not sure how we know when to test or skip. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] index-pack: fix allocation of sorted_by_pos array
Junio C Hamano gits...@pobox.com writes: When c6458e60 (index-pack: kill union delta_base to save memory, 2015-04-18) attempted to reduce the memory footprint of index-pack, one of the key thing it did was to keep track of ref-deltas and ofs-deltas separately. In fix_unresolved_deltas(), however it forgot that it now wants to look only at ref deltas in one place. The code allocated an array for nr_unresolved, which is sum of number of ref- and ofs-deltas minus nr_resolved, which may be larger or smaller than the number ref-deltas. Depending on nr_resolved, this was either under or over allocating. Also, the old code before this change had to use 'i' and 'n' because some of the things we see in the (old) deltas[] array we scanned with 'i' would not make it into the sorted_by_pos[] array in the old world order, but now because you have only ref delta in a separate ref_deltas[] array, they increment lockstep. We no longer need separate variables. And most importantly, we shouldn't pass the nr_unresolved parameter, as this number does not play a role in the working of this helper function. Helped-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- * This time, correcting the analysis; the previous one claimed that this was not a bug but just an overallocation. It indeed is a bug that uses an unrelated value that may or may not be sufficiently large to hold the whole thing, I think. I think this one as a real crash hotfix must be in the upcoming release (the obvious alternative is to revert the series with c6458e60 which I really want to avoid). As Eric's worktree add would need some more time to solidify, let's delay the -rc2 and later by a few weeks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: refspecs with '*' as part of pattern
On Mon, Jul 6, 2015 at 4:01 PM, Junio C Hamano gits...@pobox.com wrote: Jacob Keller jacob.kel...@gmail.com writes: What is the reason for not allowing slightly more arbitrary expressions? Obviously no more than one *... I cannot seem to be able to find related discussions around that patch, so this is only my guess, but I suspect that this is to discourage people from doing something like: refs/tags/*:refs/tags/foo-* which would open can of worms (e.g. imagine you fetch with that pathspec and then push with refs/tags/*:refs/tags/* back there; would you now get foo-v1.0.0 and foo-foo-v1.0.0 for their v1.0.0 tag?) we'd prefer not having to worry about. That is what I assumed. Would some sort of fetch option you could set in gitconfig be acceptable? Or possibly extending code to verify that it doesn't do something too silly? Like extend it to verify that the per-section path is actually the same? Today you can get a similar issue with refs/tags/*:refs/tags/foo/* It just is easier to fix since they're all name-spaced into foo instead. The issue I have is that I only want to download specific tag format, instead of all tags.. Maybe there is a better way to handle this? Basically, the automated build software that my team uses (I don't have a choice here) generates tags like scm_063015_050528 and then I re-generate human readable tags based on information so they look like driver-name-0-16-3 I don't want to ever pull scm_* tags because these are useless. I have tried very much to change the behavior of the team running the build software, but they refuse to budge (as this software works against CVS/SVN etc, and supports some older work, and they don't want to special case it if they can avoid it). Basically, all I see in my git tags are useless scm tags, and I want to not download them ever. (so that they don't show up as possible refs at all) Is there a possible better solution to this? Regards, Jake -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: suboptimal behavior of fast-import in some cases with from
Mike Hommey m...@glandium.org writes: One of the first things parse_from does is unconditionally throw away the tree for the given branch, and then the from tree is loaded. So when the from commit is the current head of the branch, that make fast-import do more work than necessary. If it is very common that the next commit the input stream wants to create is often on top of the commit that was just created, and if it is very common that the input stream producer knows what the commit object name of the commit that was just created, then optimising for that case does not sound too bad. It really depends on two factors: - How likely is it that other people make the same mistake? - How bad the damage to parse_from() would be if we wanted to optimize for this case? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: refspecs with '*' as part of pattern
Jacob Keller jacob.kel...@gmail.com writes: I've been looking at the refspecs for git fetch, and noticed that globs are partially supported. I wanted to use something like: refs/tags/some-prefix-*:refs/tags/some-prefix-* as a refspec, so that I can fetch only tags which have a specific prefix. I know that I could use namespaces to separate tags, but unfortunately, I am unable to fix the tag format. The specific repository in question is also generating several tags which are not relevant to me, in formats that are not really useful for human consumption. I am also not able to fix this less than useful practice. However, I noticed that refspecs only support * as a single component. The match algorithm works perfectly fine, as documented in abd2bde78bd9 (Support '*' in the middle of a refspec) What is the reason for not allowing slightly more arbitrary expressions? Obviously no more than one *... I cannot seem to be able to find related discussions around that patch, so this is only my guess, but I suspect that this is to discourage people from doing something like: refs/tags/*:refs/tags/foo-* which would open can of worms (e.g. imagine you fetch with that pathspec and then push with refs/tags/*:refs/tags/* back there; would you now get foo-v1.0.0 and foo-foo-v1.0.0 for their v1.0.0 tag?) we'd prefer not having to worry about. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
suboptimal behavior of fast-import in some cases with from
Hi, I did something stupid with a script using fast-import, and that made the whole process ~20% slower on Linux and 400~500% slower on Mac. The reason this happened is that the script was essentially adding a from to every commit command, even when the from commit is the current head of the branch. One of the first things parse_from does is unconditionally throw away the tree for the given branch, and then the from tree is loaded. So when the from commit is the current head of the branch, that make fast-import do more work than necessary. Even more so when the pack flush code in gfi_unpack_entry is triggered, which, on mac, is extra slow (and explains the huge slow down there). Now, I do understand that my script is doing something stupid. So the question is whether it's worth fixing in fast-import or not. I already fixed my script anyways. Mike -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: refspecs with '*' as part of pattern
On Mon, 6 Jul 2015, Junio C Hamano wrote: Jacob Keller jacob.kel...@gmail.com writes: I've been looking at the refspecs for git fetch, and noticed that globs are partially supported. I wanted to use something like: refs/tags/some-prefix-*:refs/tags/some-prefix-* as a refspec, so that I can fetch only tags which have a specific prefix. I know that I could use namespaces to separate tags, but unfortunately, I am unable to fix the tag format. The specific repository in question is also generating several tags which are not relevant to me, in formats that are not really useful for human consumption. I am also not able to fix this less than useful practice. However, I noticed that refspecs only support * as a single component. The match algorithm works perfectly fine, as documented in abd2bde78bd9 (Support '*' in the middle of a refspec) What is the reason for not allowing slightly more arbitrary expressions? Obviously no more than one *... I cannot seem to be able to find related discussions around that patch, so this is only my guess, but I suspect that this is to discourage people from doing something like: refs/tags/*:refs/tags/foo-* which would open can of worms (e.g. imagine you fetch with that pathspec and then push with refs/tags/*:refs/tags/* back there; would you now get foo-v1.0.0 and foo-foo-v1.0.0 for their v1.0.0 tag?) we'd prefer not having to worry about. That wouldn't be it, since refs/tags/*:refs/tags/foo/* would have the same problem, assuming you didn't set up the push refspec carefully. I think it was mostly that it would be too easy to accidentally do something you don't want by having some other character instead of a slash, like refs/heads/*:refs/heads-*. Aside from the increased risk of hard-to-spot typos leading to very weird behavior, nothing actually goes wrong; in fact, I've been using git with that check removed for ages because I wanted a refspec like refs/heads/something-*:refs/heads/*. And it works fine as a local patch, since you don't need your refspec handling to interoperate with other repositories. -Daniel *This .sig left intentionally blank* -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/23] worktree: add: auto-vivify new branch when branch is omitted
On Mon, Jul 06, 2015 at 12:19:14PM -0700, Junio C Hamano wrote: Eric Sunshine sunsh...@sunshineco.com writes: diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 377ae0f..da71f50 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] [-b new-branch] path branch +'git worktree add' [-f] [--detach] [-b new-branch] path [branch] Ahh, OK, this answers my previous question. Right. I considered squashing this patch with the previous one, in which case the synopsis question wouldn't have come up, but kept them separate since they are (or can be) conceptually distinct, and the one patch builds upon the other (and keeping them separate makes them a bit easier to review). + if (ac 2 !new_branch !new_branch_force) { + int n; + const char *s = worktree_basename(path, n); + new_branch = xstrndup(s, n); + } + and because this is new_branch, not new_branch_force, we will not accidentally clobber an existing branch. The hotfix time is when the end-user tends to be less careful, and it is a good thing to make sure git worktree add ../hotfix will not clobber an unrelated hotfix branch. Good. Which may be something we would want to have a test for, though. Good idea. How about the following as a squash-in? --- 8 --- From: Eric Sunshine sunsh...@sunshineco.com Subject: [PATCH] fixup! worktree: add: auto-vivify new branch when branch is omitted --- t/t2025-worktree-add.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 8fe242f..ead8aa2 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -150,4 +150,13 @@ test_expect_success 'add with branch omitted' ' test_cmp_rev HEAD bat ' +test_expect_success 'add auto-vivify does not clobber existing branch' ' + test_commit c1 + test_commit c2 + git branch precious HEAD~1 + test_must_fail git worktree add precious + test_cmp_rev HEAD~1 precious + test_path_is_missing precious +' + test_done -- 2.5.0.rc1.201.ga12d9f8 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: refspecs with '*' as part of pattern
On Mon, Jul 6, 2015 at 7:20 PM, Daniel Barkalow barka...@iabervon.iabervon.org wrote: On Mon, 6 Jul 2015, Junio C Hamano wrote: Jacob Keller jacob.kel...@gmail.com writes: I've been looking at the refspecs for git fetch, and noticed that globs are partially supported. I wanted to use something like: refs/tags/some-prefix-*:refs/tags/some-prefix-* as a refspec, so that I can fetch only tags which have a specific prefix. I know that I could use namespaces to separate tags, but unfortunately, I am unable to fix the tag format. The specific repository in question is also generating several tags which are not relevant to me, in formats that are not really useful for human consumption. I am also not able to fix this less than useful practice. However, I noticed that refspecs only support * as a single component. The match algorithm works perfectly fine, as documented in abd2bde78bd9 (Support '*' in the middle of a refspec) What is the reason for not allowing slightly more arbitrary expressions? Obviously no more than one *... I cannot seem to be able to find related discussions around that patch, so this is only my guess, but I suspect that this is to discourage people from doing something like: refs/tags/*:refs/tags/foo-* which would open can of worms (e.g. imagine you fetch with that pathspec and then push with refs/tags/*:refs/tags/* back there; would you now get foo-v1.0.0 and foo-foo-v1.0.0 for their v1.0.0 tag?) we'd prefer not having to worry about. That wouldn't be it, since refs/tags/*:refs/tags/foo/* would have the same problem, assuming you didn't set up the push refspec carefully. I think it was mostly that it would be too easy to accidentally do something you don't want by having some other character instead of a slash, like refs/heads/*:refs/heads-*. Aside from the increased risk of hard-to-spot typos leading to very weird behavior, nothing actually goes wrong; in fact, I've been using git with that check removed for ages because I wanted a refspec like refs/heads/something-*:refs/heads/*. And it works fine as a local patch, since you don't need your refspec handling to interoperate with other repositories. -Daniel *This .sig left intentionally blank* Which is why I suggested a patch that adds this behavior conditionally and only for fetch. This way you don't have to use a local patch, and it wouldn't hit normal users. Ideally we can add a flag that only enables it for refspecs that don't interoperate. Does this seem ok? If so I will go ahead and try to work up a patch Regards, Jake -- To unsubscribe from this list: send the line unsubscribe 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] index-pack: fix allocation of sorted_by_pos array
On Sun, Jul 5, 2015 at 5:30 AM, Junio C Hamano gits...@pobox.com wrote: When c6458e60 (index-pack: kill union delta_base to save memory, 2015-04-18) attempted to reduce the memory footprint of index-pack, one of the key thing it did was to keep track of ref-deltas and ofs-deltas separately. In fix_unresolved_deltas(), however it forgot that it now wants to look only at ref deltas in one place. The code allocated an array for nr_unresolved, which is sum of number of ref- and ofs-deltas minus nr_resolved, which may be larger or smaller than the number ref-deltas. Depending on nr_resolved, this was either under or over allocating. It's either that or we could put back if (real_type != OBJ_REF_DELTA) continue; in the sorted_by_pos population loop. Resolved deltas can't have real_type == OBJ_REF_DELTA, so if we allocate nr_unresolved, it's guaranteed over-allocation, never under-allocation. But I guess your approach would make the code easier to read. I keep tripping over this real_type vs type in this code. What do you think about renaming type field to in_pack_type and real_type to canon_type (or final_type)? Real does not really say anything in this context.. -- 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 v5] pull: allow dirty tree when rebase.autostash enabled
On Mon, Jul 06, 2015 at 01:39:47PM -0700, Junio C Hamano wrote: Kevin Daudt m...@ikke.info writes: rebase learned to stash changes when it encounters a dirty work tree, but git pull --rebase does not. Only verify if the working tree is dirty when rebase.autostash is not enabled. Signed-off-by: Kevin Daudt m...@ikke.info Helped-by: Paul Tan pyoka...@gmail.com --- I applied it, tried to run today's integration cycle, and then ended up ejecting it from my tree for now, as this seemed to break 5520 when merged to 'pu' X-. Well, that is partly expected, as Paul's builtin/pull.c does not know about it (yet). Yeah, sorry about that. Here's a modified patch for the C code. Regards, Paul --- 8 --- From: Kevin Daudt m...@ikke.info Date: Sat, 4 Jul 2015 23:42:38 +0200 rebase learned to stash changes when it encounters a dirty work tree, but git pull --rebase does not. Only verify if the working tree is dirty when rebase.autostash is not enabled. Signed-off-by: Kevin Daudt m...@ikke.info Signed-off-by: Paul Tan pyoka...@gmail.com --- builtin/pull.c | 6 +- t/t5520-pull.sh | 11 +++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index 722a83c..b7bc1ff 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -823,10 +823,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix) hashclr(orig_head); if (opt_rebase) { + int autostash = 0; + if (is_null_sha1(orig_head) !is_cache_unborn()) die(_(Updating an unborn branch with changes added to the index.)); - die_on_unclean_work_tree(prefix); + git_config_get_bool(rebase.autostash, autostash); + if (!autostash) + die_on_unclean_work_tree(prefix); if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) hashclr(rebase_fork_point); diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index f4a7193..a0013ee 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -245,6 +245,17 @@ test_expect_success '--rebase fails with multiple branches' ' test modified = $(git show HEAD:file) ' +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' + test_config rebase.autostash true + git reset --hard before-rebase + echo dirty new_file + git add new_file + git pull --rebase . copy + test_cmp_rev HEAD^ copy + test $(cat new_file) = dirty + test $(cat file) = modified again +' + test_expect_success 'pull.rebase' ' git reset --hard before-rebase test_config pull.rebase true -- 2.5.0.rc1.21.gbd65f2d.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 v2] l10n: de.po: translate 65 new messages
Translate 65 new messages came from git.pot update in 64f23b0 (l10n: git.pot: v2.5.0 round 1 (65 new, 15 removed)). Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/de.po | 192 --- 1 file changed, 98 insertions(+), 94 deletions(-) diff --git a/po/de.po b/po/de.po index d52184c..9c04ebf 100644 --- a/po/de.po +++ b/po/de.po @@ -539,19 +539,19 @@ msgstr #: diff.c:3570 #, c-format msgid Failed to parse --submodule option parameter: '%s' msgstr Fehler beim Parsen des --submodule Optionsparameters: '%s' #: dir.c:1852 msgid failed to get kernel name and information -msgstr +msgstr Fehler beim Sammeln von Namen und Informationen zum Kernel #: dir.c:1945 msgid Untracked cache is disabled on this system. -msgstr +msgstr Cache für unversionierte Dateien ist auf diesem System deaktiviert. #: gpg-interface.c:129 gpg-interface.c:200 msgid could not run gpg. msgstr konnte gpg nicht ausführen #: gpg-interface.c:141 msgid gpg did not accept the data @@ -589,19 +589,19 @@ msgstr '%s': read() zu kurz %s #: help.c:207 #, c-format msgid available git commands in '%s' msgstr Vorhandene Git-Kommandos in '%s' #: help.c:214 msgid git commands available from elsewhere on your $PATH -msgstr Vorhandene Git-Kommandos irgendwo in Ihrem $PATH +msgstr Vorhandene Git-Kommandos anderswo in Ihrem $PATH #: help.c:246 msgid These are common Git commands used in various situations: -msgstr +msgstr Allgemeine Git-Kommandos, verwendet in verschiedenen Situationen: #: help.c:311 #, c-format msgid '%s' appears to be a git command, but we were not\n able to execute it. Maybe git-%s is broken? msgstr @@ -1089,55 +1089,56 @@ msgid Internal error msgstr Interner Fehler #: remote.c:1723 remote.c:1766 msgid HEAD does not point to a branch msgstr HEAD zeigt auf keinen Branch #: remote.c:1732 -#, fuzzy, c-format +#, c-format msgid no such branch: '%s' -msgstr Kein solcher Branch '%s' +msgstr Kein solcher Branch: '%s' #: remote.c:1735 -#, fuzzy, c-format +#, c-format msgid no upstream configured for branch '%s' msgstr Kein Upstream-Branch für Branch '%s' konfiguriert. #: remote.c:1741 -#, fuzzy, c-format +#, c-format msgid upstream branch '%s' not stored as a remote-tracking branch -msgstr Upstream-Branch '%s' ist nicht als Remote-Tracking-Branch gespeichert +msgstr Upstream-Branch '%s' nicht als Remote-Tracking-Branch gespeichert #: remote.c:1756 #, c-format msgid push destination '%s' on remote '%s' has no local tracking branch -msgstr +msgstr Ziel für \push\ '%s' auf Remote-Repository '%s' hat keinen lokal +gefolgten Branch #: remote.c:1771 -#, fuzzy, c-format +#, c-format msgid branch '%s' has no remote for pushing msgstr Branch '%s' hat keinen Upstream-Branch gesetzt #: remote.c:1782 #, c-format msgid push refspecs for '%s' do not include '%s' -msgstr +msgstr Push-Refspecs für '%s' beinhalten nicht '%s' #: remote.c:1795 msgid push has no destination (push.default is 'nothing') -msgstr +msgstr kein Ziel für \push\ (push.default ist 'nothing') #: remote.c:1817 msgid cannot resolve 'simple' push to a single destination -msgstr +msgstr kann einzelnes Ziel für \push\ im Modus 'simple' nicht auflösen #: remote.c:2124 #, c-format msgid Your branch is based on '%s', but the upstream is gone.\n -msgstr Ihr Branch basiert auf '%s', aber Upstream-Branch wurde entfernt.\n +msgstr Ihr Branch basiert auf '%s', aber der Upstream-Branch wurde entfernt.\n #: remote.c:2128 msgid (use \git branch --unset-upstream\ to fixup)\n msgstr (benutzen Sie \git branch --unset-upstream\ zum Beheben)\n #: remote.c:2131 #, c-format @@ -1463,17 +1464,17 @@ msgid Can't revert as initial commit msgstr Kann nicht als allerersten Commit einen Revert ausführen. #: sequencer.c:1124 msgid Can't cherry-pick into empty head msgstr Kann nicht als allerersten Commit einen Cherry-Pick ausführen. #: setup.c:243 -#, fuzzy, c-format +#, c-format msgid failed to read %s -msgstr Fehler beim Löschen von %s +msgstr Fehler beim Lesen von %s #: sha1_name.c:453 msgid Git normally never creates a ref that ends with 40 hex characters\n because it will be ignored when you just specify 40-hex. These refs\n may be created by mistake. For example,\n \n @@ -1604,27 +1605,27 @@ msgid no such user msgstr kein solcher Benutzer #: wrapper.c:564 msgid unable to get current working directory msgstr Konnte aktuelles Arbeitsverzeichnis nicht bekommen. #: wrapper.c:575 -#, fuzzy, c-format +#, c-format msgid could not open %s for writing msgstr Konnte '%s' nicht zum Schreiben öffnen. #: wrapper.c:587 -#, fuzzy, c-format +#, c-format msgid could not write to %s -msgstr Konnte nicht nach %s schreiben +msgstr Konnte nicht nach '%s' schreiben. #: wrapper.c:593 -#, fuzzy, c-format +#, c-format msgid could not close %s -msgstr konnte %s nicht parsen +msgstr Konnte '%s' nicht schließen. #: wt-status.c:150
send-pack silently tries to recreate funny remote refs
Today I learned that git push will silently try to recreate a remote funny ref if you push to it. Hypothetically ... lets say I have a reimplementation of Git called JGit. Lets say its on a server somewhere on the network. Lets say its a bit more liberal in what it accepts... $ git push origin HEAD^:refs/wtf Counting objects: 3, done. Writing objects: 100% (3/3), 206 bytes | 0 bytes/s, done. Total 3 (delta 0), reused 0 (delta 0) To ...blahblah.../wtf * [new branch] HEAD^ - refs/wtf Oky. We are the proud new owners of $ git ls-remote origin 4da022e5c7e2c33025374ddd1c3aeeb082d7b394refs/wtf Yea. Great. Here is the WTF. git push will silently send a creation command instead of an update. $ GIT_TRACE_PACKET=1 git push origin HEAD:refs/wtf 22:48:09.277446 pkt-line.c:46 packet: push 4da022e5c7e2c33025374ddd1c3aeeb082d7b394 refs/wtf\0 side-band-64k delete-refs report-status quiet ofs-delta agent=JGit/4-... 22:48:09.277522 pkt-line.c:46 packet: push 22:48:09.277595 pkt-line.c:46 packet: push f721fb36771dc587d45e8a95ef9f7edcc3849236 refs/wtf\0 report-status side-band-64k agent=git/2.4.3.573.g4eafbef 22:48:09.277605 pkt-line.c:46 packet: push Counting objects: 6, done. Delta compression using up to 12 threads. Compressing objects: 100% (2/2), done. Writing objects: 100% (6/6), 409 bytes | 0 bytes/s, done. Total 6 (delta 0), reused 0 (delta 0) 22:48:09.419038 pkt-line.c:46 packet: push \1000eunpack ok001fng refs/wtf already exists 22:48:09.419066 pkt-line.c:46 packet: push 22:48:09.419073 pkt-line.c:46 packet: push unpack ok 22:48:09.419082 pkt-line.c:46 packet: push ng refs/wtf already exists 22:48:09.419092 pkt-line.c:46 packet: push To rpc://user/sop/wtf ! [remote rejected] HEAD - refs/wtf (already exists) error: failed to push some refs to '...blahblah.../wtf' I think the cause is get_remote_heads() gets passed REF_NORMAL during send-pack, which filters out that refs/wtf advertisement from the server. Once its filtered out send-pack thinks the ref is new and happily sends along the command to create it. In git-core this rejection of the funny ref is done at the server. This JGit server was fine with the funny ref. It didn't care earlier when git push created it. Now git push isn't so happy with that funny ref on the RHS, because it always forgets it in the advertisement. This makes it hard to update the ref. Or delete it remotely. If we don't want that ref in the advertisement, push should refuse it in the RHS. If we want to let the remote decide, send-pack should honor the advertisement. -- To unsubscribe from this list: send the line unsubscribe 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] l10n: de.po: translate 65 new messages
Hi Phillip, Thanks for review! 2015-07-05 15:21 GMT+02:00 Phillip Sz phillip.sze...@gmail.com: #: help.c:214 msgid git commands available from elsewhere on your $PATH msgstr Vorhandene Git-Kommandos irgendwo in Ihrem $PATH What do you think about Git-Kommandos sind anderwo verfügbar in Ihrem $PATH? This message is the title of a listing of git commands, that are available from $PATH but not located in git's exec path. Vorhandene Git-Kommandos in '/usr/libexec/git-core' add merge-octopus ... Vorhandene Git-Kommandos irgendwo in Ihrem $PATH imerge ... What about changing it to Vorhandene Git-Kommandos anderswo in Ihrem $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: end-of-line diff checkout direction dependence problem
On 2015-07-02 16.00, Thomas Vieten wrote: [] see the file attachend to the end of this message Thanks for the info It may be, that you need to nornalize your repo: in principle we know all this. What is remarkable that we are able to checkout a version of master which is not consistent with the repo, and more, dependent from the checkout direction (if the direction is the positive or negative history in time). And on the other hand we can checkout a version of master which is in sync with the master. Normally such conflicts with not normalised repos appear immediately also in positive history direction. And then it is possible to detect them. The other way around - negative history and diffs - it causes a big questionmark. On the other hand this would lead to the mandatory work flow advice: Always normalize the repository after changes within the gitattributes file Yes, I think this sounds reasonable, and I think the documentations says this. But it is a manual step, which is typically done only once. It there anything which can be improved here ? And then: Should this then not be automatically be done somehow in the background by git ? This could make sense, but the word background should mean visible to the user ? Recently we got the untracked cache into Git, which keeps track about the .gitignore files A similar logic can be used to keep track of the .gitattributes file(s) (There can be more than one) Patches are welcome. Reasoning: if the git machine is causing this behaviour systematically, shouldn't the machine itself have compensation, correction? Question 1: The documentation should be clear enough: Whenever someone introduces .gitattributes, the repo should be normalized. Depending on your point of view this could be seen as a bug. I have run into this myself, but never in a reproducable way. The day I can reproduce it, I may send a patch. Or, somebody else sends a patch before that day. There is also a big question open: will normalisation really help ? Because there must be one commit with the new gitattributes and then you normalize. To my experience it does. Normalizing means one single commit (not 2), including the .gitattributes and the normalized files. Question 2: Is there something in the documentation that could be improved. But the wrong diff is in the repo and will cause the problem when going back to master in the negative direction. This is how understand it up to now. But at this point git is complex and we are not really the experts. best regards Thomas V. Thanks for the report(s) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Wed, Jul 1, 2015 at 4:49 PM, Junio C Hamano gits...@pobox.com wrote: 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. Ok, I understand your proposal now, thank you. I will drop this documentation patch from this series, and abandon https://git.eclipse.org/r/51071 in JGit. I am not volunteering to rewrite push cert handling in git-core though ;) 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
Re: [PATCH v6 1/7] refs.c: add err arguments to reflog functions
On 06/29/2015 10:17 PM, David Turner wrote: Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Update of a patch by Ronnie Sahlberg. First a general comment: we have a convention, not yet very well adhered to, that error messages should start with lower-case letters. I know that in many cases you are just carrying along pre-existing error messages that are capitalized. But at a minimum, please avoid adding new error messages that are capitalized. And if you are feeling ambitious, feel free to lower-case some existing ones :-) Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: David Turner dtur...@twopensource.com --- builtin/checkout.c | 8 ++-- refs.c | 111 - refs.h | 4 +- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c [...] diff --git a/refs.c b/refs.c index fb568d7..c97ca02 100644 --- a/refs.c +++ b/refs.c [...] @@ -3216,26 +3217,25 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, result = log_ref_write_fd(logfd, old_sha1, new_sha1, git_committer_info(0), msg); if (result) { - int save_errno = errno; close(logfd); - error(Unable to append to %s, log_file); - errno = save_errno; + strbuf_addf(err, Unable to append to %s. %s, log_file, + strerror(errno)); return -1; Above, the call to close(logfd) could clobber errno. It would be better to exchange the calls. } if (close(logfd)) { - int save_errno = errno; - error(Unable to append to %s, log_file); - errno = save_errno; + strbuf_addf(err, Unable to append to %s. %s, log_file, + strerror(errno)); return -1; } return 0; } static int log_ref_write(const char *refname, const unsigned char *old_sha1, - const unsigned char *new_sha1, const char *msg) + const unsigned char *new_sha1, const char *msg, + struct strbuf *err) { struct strbuf sb = STRBUF_INIT; - int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, sb); + int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, sb, err); strbuf_release(sb); return ret; } [...] @@ -3288,12 +3290,15 @@ static int write_ref_to_lockfile(struct ref_lock *lock, * necessary, using the specified lockmsg (which can be NULL). */ static int commit_ref_update(struct ref_lock *lock, - const unsigned char *sha1, const char *logmsg) + const unsigned char *sha1, const char *logmsg, + struct strbuf *err) { clear_loose_ref_cache(ref_cache); - if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg) 0 || + if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg, err) 0 || (strcmp(lock-ref_name, lock-orig_ref_name) - log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg) 0)) { + log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, logmsg, err) 0)) { + strbuf_addf(err, Cannot update the ref '%s'., + lock-ref_name); unlock_ref(lock); return -1; } Since you are passing err into log_ref_write(), I assume that it would already have an error message written to it by the time you enter into this block. Yet in the block you append more text to it. It seems to me that you either want to clear err and replace it with your own message, or create a new message that looks like Cannot update the ref '%s': %s where the second %s is replaced with the error from log_ref_write(). @@ -3317,7 +3322,8 @@ static int commit_ref_update(struct ref_lock *lock, head_sha1, head_flag); if (head_ref (head_flag REF_ISSYMREF) !strcmp(head_ref, lock-ref_name)) - log_ref_write(HEAD, lock-old_oid.hash, sha1, logmsg); + log_ref_write(HEAD, lock-old_oid.hash, sha1, logmsg, + err); Here you don't check for errors from log_ref_write(). So it might or might not have written something to err. If it has, is that error handled
Git merge commit message issue
Dear list: In the past, when we do the merge in git, if conflicts occurred, when we commit, the conflict list will be appended to the default commit message automatically. eg: Conflicts: a.java b.java Today, after using Git 2.3.0 for a merge, it seems now the conflict list was commented out by default. I just searched a bit in the release notes, don't know whether below item has to do with this change. git interpret-trailers learned to properly handle the Conflicts: block at the end. We quite rely on the default generated conflict list to reminder us about the history. Is this the default behavior now (conflict list commented out)? Thanks Best Regards -- To unsubscribe from this list: send the line unsubscribe 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:49 PM, Junio C Hamano gits...@pobox.com wrote: 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. This is madness. For all the reasons Dave points out later in the thread. You can't store and make sense of the push cert without the LF record delimiters. push cert format is like commit or tag format. You need those LFs. We can't just go declare them optional because of the way pkt-line read function is implemented in git-core. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git merge commit message issue
Andrey Hsiao andreyhs...@gmail.com writes: Dear list: In the past, when we do the merge in git, if conflicts occurred, when we commit, the conflict list will be appended to the default commit message automatically. eg: Conflicts: a.java b.java Today, after using Git 2.3.0 for a merge, it seems now the conflict list was commented out by default. Yes, this changed here: https://github.com/git/git/commit/261f315bebfa9af2d09f20211960100ff06f87cb -- 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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Mon, Jul 6, 2015 at 11:22 AM, Dave Borowitz dborow...@google.com wrote: b. Write a parser in some other clever way, e.g. parsing the entire cert in reverse might work. ...as long as is illegal in nonce and pushee values, which it may be but is not explicitly documented. I still have no desire to write such a parser. -- To unsubscribe from this list: send the line unsubscribe 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 Mon, Jul 6, 2015 at 11:27 AM, Dave Borowitz dborow...@google.com wrote: On Mon, Jul 6, 2015 at 11:22 AM, Dave Borowitz dborow...@google.com wrote: b. Write a parser in some other clever way, e.g. parsing the entire cert in reverse might work. ...as long as is illegal in nonce and pushee values, which it may be but is not explicitly documented. I still have no desire to write such a parser. TBQH at this point I would prefer, as a protocol implementor, to restore the original proposal of this patch, which is to require \n in push certificates. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git merge commit message issue
Dear list: In the past, when we do the merge in git, if conflicts occurred, when we commit, the conflict list will be appended to the default commit message automatically. eg: Conflicts: a.java b.java Today, after using Git 2.3.0 for a merge, it seems now the conflict list was commented out by default. I just searched a bit in the release notes, don't know whether below item has to do with this change. git interpret-trailers learned to properly handle the Conflicts: block at the end. We quite rely on the default generated conflict list to reminder us about the history. Is this the default behavior now (conflict list commented out)? Thanks Best Regards -- To unsubscribe from this list: send the line unsubscribe 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 Mon, Jul 6, 2015 at 10:46 AM, Dave Borowitz dborow...@google.com wrote: On Wed, Jul 1, 2015 at 4:49 PM, Junio C Hamano gits...@pobox.com wrote: 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. Ok, I understand your proposal now, thank you. I will drop this documentation patch from this series, and abandon https://git.eclipse.org/r/51071 in JGit. I am not volunteering to rewrite push cert handling in git-core though ;) Unfortunately, optional LFs still make the stored certs for later auditing and parsing a bit illegible. This is one way in which push certs are fundamentally different from the rest of the wire protocol, which is not intended to be persisted. The corner case I pointed out before where nonce runs into commands is not the only one. Consider the following cert fragment: 001fpushee git://localhost/repo 0029nonce 1433954361-bde756572d665bba81d8 A naive cert storage/auditing implementation would store the raw payload that needs to be verified, without the pkt-line framing. In this case: pushee git://localhost/repononce 1433954361-bde756572d665bba81d8 A naive parser that wants to find the pushee would look for pushee urlish, which would be wrong in this case. (To say nothing of the fact that pushee might actually be -0700pushee.) The alternatives for someone writing a parser are: a. Store the original pkt-line framing. b. Write a parser in some other clever way, e.g. parsing the entire cert in reverse might work. Neither of these is very satisfying, and both reduce human legibility of the stored payload. 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
Re: [PATCH v6 2/7] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs
On 06/29/2015 10:17 PM, David Turner wrote: Instead of directly writing to and reading from files in $GIT_DIR, use ref API to interact with CHERRY_PICK_HEAD and REVERT_HEAD. Signed-off-by: David Turner dtur...@twopensource.com --- branch.c | 4 ++-- builtin/commit.c | 6 +++--- builtin/merge.c | 2 +- contrib/completion/git-prompt.sh | 4 ++-- git-gui/lib/commit.tcl | 2 +- sequencer.c | 39 --- t/t7509-commit.sh| 4 ++-- wt-status.c | 6 ++ 8 files changed, 33 insertions(+), 34 deletions(-) [...] diff --git a/sequencer.c b/sequencer.c index f8421a8..44c43e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -158,21 +158,22 @@ static void free_message(struct commit *commit, struct commit_message *msg) unuse_commit_buffer(commit, msg-message); } -static void write_cherry_pick_head(struct commit *commit, const char *pseudoref) +static void write_cherry_pick_head(struct commit *commit, const char *ref) { - const char *filename; - int fd; - struct strbuf buf = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + void *transaction; - strbuf_addf(buf, %s\n, sha1_to_hex(commit-object.sha1)); + transaction = ref_transaction_begin(err); + if (!transaction) + die(_(Could not create transaction: %s), err.buf); - filename = git_path(%s, pseudoref); - fd = open(filename, O_WRONLY | O_CREAT, 0666); - if (fd 0) - die_errno(_(Could not open '%s' for writing), filename); - if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd)) - die_errno(_(Could not write to '%s'), filename); - strbuf_release(buf); + if (ref_transaction_update(transaction, ref, commit-object.sha1, +NULL, REF_NODEREF, NULL, +err)) + die(_(Could not write ref %s: %s), ref, err.buf); + + if (ref_transaction_commit(transaction, err)) + die(_(Could not commit ref write %s: %s), ref, err.buf); } I didn't check all the details, but this code looks a lot like what update_ref() does. Maybe you can use that function? static void print_advice(int show_hint, struct replay_opts *opts) @@ -186,7 +187,7 @@ static void print_advice(int show_hint, struct replay_opts *opts) [...] Otherwise, looks good to me. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html