Re: %C(auto) not working as expected
On Mon, Oct 10, 2016 at 6:46 AM, Jeff King wrote: > On Sun, Oct 09, 2016 at 03:24:17PM +0200, René Scharfe wrote: > >> Offering a way to enable terminal-detection for all color codes of a >> format would be useful, but using the existing "auto," prefix for that >> would be a behaviour change that could surprise users. I wonder if we made a mistake associating terminal-detection with %C(auto,...). The more likely use case is enable or disable all colors, not "the next tag". > Yeah. In retrospect, it probably would have been saner to make %C(red) a > noop when --color is not in effect (either because of --no-color, or > more likely when --color=auto is in effect and stdout is not a > terminal). But that ship has long since sailed, I think. > > If we do a revamp of the pretty-formats to bring them more in line with > ref-filter (e.g., something like "%(color:red)") maybe that would be an > opportunity to make minor adjustments. Though, hmm, it looks like > for-each-ref already knows "%(color:red)", and it's unconditional. > So perhaps we would need to go through some deprecation period or > other transition. We could add some new tag to change the behavior of all following %C tags. Something like %C(tty) maybe (probably a bad name), then discourage the use if "%C(auto" for terminal detection? -- Duy
Re: Bug? git worktree fails with master on bare repo
On Sun, Oct 9, 2016 at 8:42 PM, Michael Tutty wrote: > Dennis, > Thanks for the great response, and for spending time on my issue. > I'll try that first patch and see what happens. > > In the meantime, it got weirder... > > I created a brand-new (bare) repo Elaboration needed here. If I create a bare _clone_, then "HEAD" could be detached, or point to some branch, depending on where "HEAD" is in the source repo. If source repo's HEAD is "master", I got the same behavior (worktree add fails). If it's detached or points to some other branch, it's ok. If this is "git init --bare" then I got "fatal: invalid reference: master". > and was able to git add worktree > /path master. I was able to do this repeatedly, even using the > worktree to merge other branches to master. I didn't find any > condition or step that caused some kind of orphan master work tree, > which was what I thought the underlying problem might be. -- Duy
git merge deletes my changes
Hello Git experts, A week ago, I've reset a state of 'ceph-ansible' folder in %current% branch with code from corresponding branch (that tracks an upstream from github): # git read-tree --prefix=ceph-ansible/ -u ceph_ansible Then I've committed several changes, including: 1. Renamed file and commited: # git mv site.yml.sample site.yml 2. Made some changes and committed 3. Pulled updates from original branch by: # git merge -s subtree --squash ceph_ansible It said: Auto-merging ceph-ansible/site.yml.sample blablabla Squash commit -- not updating HEAD Automatic merge went well; stopped before committing as requested I can see that "my" ceph-ansible/site.yml is deleted (as well as few new files added by me), ceph-ansible/site.yml.sample is restored and doesn't contain my changes (it's just restored to the state before my changes). `git log ceph_ansible site.yml.sample` shows the latest change on ceph_ansible branch done from 26th of August, 2016, my changes in %current% branch was this October, so it shouldn't be any conflict either (even if it was). I believe there is some obvious explanation for this behavior. Could you help me please? What information can I provide in order to troubleshoot this issue? A post on SO: http://stackoverflow.com/questions/39954265/git-merge-deletes-my-changes With best regards Eduard Egorov
Re: [PATCH v2 1/2] files_read_raw_ref: avoid infinite loop on broken symlinks
On 10/06/2016 09:41 PM, Jeff King wrote: > [...] > Subject: files_read_raw_ref: avoid infinite loop on broken symlinks > [...] This patch is Reviewed-by: Michael Haggerty Michael
Re: [PATCH 2/2] files_read_raw_ref: prevent infinite retry loops in general
On 10/06/2016 06:48 PM, Jeff King wrote: > Limit the number of retries to 3. That should be adequate to > prevent any races, while preventing the possibility of > infinite loops if the logic fails to handle any other > possible error modes correctly. > > After the fix in the previous commit, there's no known way > to trigger an infinite loop, but I did manually verify that > this fixes the test in that commit even when the code change > is not applied. > [...] This patch is Reviewed-by: Michael Haggerty Michael
[PATCH v4 00/14] Mark strings in Perl scripts for translation
Mark messages in some perl scripts for translation. Fix minor stuff and follow Jakub Narębski's suggestion to use N__() instead of __() in the hash tables. Interdiff included below. Vasco Almeida (14): i18n: add--interactive: mark strings for translation i18n: add--interactive: mark simple here-documents for translation i18n: add--interactive: mark strings with interpolation for translation i18n: clean.c: match string with git-add--interactive.perl i18n: add--interactive: mark plural strings i18n: add--interactive: mark patch prompt for translation i18n: add--interactive: i18n of help_patch_cmd i18n: add--interactive: mark edit_hunk_manually message for translation i18n: add--interactive: remove %patch_modes entries i18n: add--interactive: mark status words for translation i18n: send-email: mark strings for translation i18n: send-email: mark warnings and errors for translation i18n: send-email: mark string with interpolation for translation i18n: difftool: mark warnings for translation Makefile | 3 +- builtin/clean.c | 10 +- git-add--interactive.perl | 322 ++ git-difftool.perl | 22 ++-- git-send-email.perl | 176 + perl/Git/I18N.pm | 17 ++- t/t0202/test.pl | 14 +- 7 files changed, 353 insertions(+), 211 deletions(-) diff --git a/Makefile b/Makefile index 4ef0344..9dc95cb 100644 --- a/Makefile +++ b/Makefile @@ -2112,7 +2112,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \ --keyword=gettextln --keyword=eval_gettextln XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \ - --keyword=__ --keyword="__n:1,2" + --keyword=__ --keyword=N__ --keyword="__n:1,2" LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH = $(SCRIPT_SH) LOCALIZED_SH += git-parse-remote.sh diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 0b4a27c..4754104 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -601,7 +601,7 @@ sub list_and_choose { } } if ($opts->{SINGLETON} && $bottom != $top) { - error_msg sprintf(__("Huh (%s)?"), $choice); + error_msg sprintf(__("Huh (%s)?\n"), $choice); next TOPLOOP; } for ($i = $bottom-1; $i <= $top-1; $i++) { @@ -704,7 +704,7 @@ sub revert_cmd { $_->{INDEX_ADDDEL} eq 'create') { system(qw(git update-index --force-remove --), $_->{VALUE}); - print "note: $_->{VALUE} is untracked now.\n"; + printf(__("note: %s is untracked now.\n"), $_->{VALUE}); } } } @@ -1038,25 +1038,25 @@ sub color_diff { } my %edit_hunk_manually_modes = ( - stage => __( + stage => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for staging."), - stash => __( + stash => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for stashing."), - reset_head => __( + reset_head => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for unstaging."), - reset_nothead => __( + reset_nothead => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for applying."), - checkout_index => __( + checkout_index => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for discarding"), - checkout_head => __( + checkout_head => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for discarding."), - checkout_nothead => __( + checkout_nothead => N__( "# If the patch applies cleanly, the edited hunk will immediately be # marked for applying."), ); @@ -1078,7 +1078,7 @@ sub edit_hunk_manually { # To remove '%s' lines, delete them. # Lines starting with # will be removed. #\n"), $remove_minus, $remove_plus), -$edit_hunk_manually_modes{$patch_mode}, __( +__($edit_hunk_manually_modes{$patch_mode}), __( # TRANSLATORS: 'it' refers to the patch mentioned in the previous messages. " If it does not apply cleanly, you will be given # an opportunity to edit again. If all lines of the hunk are removed, @@ -1192,43 +1192,43 @@ sub edit_hunk_loop { } my %help_patch_modes = ( - stage => __( + stage => N__( "y - stage this hunk n - do not stage this hunk q - quit; do not stage this hunk or any of the remaining ones a - stage this hunk and all later hunks in
[PATCH v4 07/14] i18n: add--interactive: i18n of help_patch_cmd
Mark help message of help_patch_cmd for translation. The message must be unfolded to be free of variables so we can have high quality translations. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 54 --- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index b7d382b..045b847 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -1189,15 +1189,53 @@ sub edit_hunk_loop { } } +my %help_patch_modes = ( + stage => N__( +"y - stage this hunk +n - do not stage this hunk +q - quit; do not stage this hunk or any of the remaining ones +a - stage this hunk and all later hunks in the file +d - do not stage this hunk or any of the later hunks in the file"), + stash => N__( +"y - stash this hunk +n - do not stash this hunk +q - quit; do not stash this hunk or any of the remaining ones +a - stash this hunk and all later hunks in the file +d - do not stash this hunk or any of the later hunks in the file"), + reset_head => N__( +"y - unstage this hunk +n - do not unstage this hunk +q - quit; do not unstage this hunk or any of the remaining ones +a - unstage this hunk and all later hunks in the file +d - do not unstage this hunk or any of the later hunks in the file"), + reset_nothead => N__( +"y - apply this hunk to index +n - do not apply this hunk to index +q - quit; do not apply this hunk or any of the remaining ones +a - apply this hunk and all later hunks in the file +d - do not apply this hunk or any of the later hunks in the file"), + checkout_index => N__( +"y - discard this hunk from worktree +n - do not discard this hunk from worktree +q - quit; do not discard this hunk or any of the remaining ones +a - discard this hunk and all later hunks in the file +d - do not discard this hunk or any of the later hunks in the file"), + checkout_head => N__( +"y - discard this hunk from index and worktree +n - do not discard this hunk from index and worktree +q - quit; do not discard this hunk or any of the remaining ones +a - discard this hunk and all later hunks in the file +d - do not discard this hunk or any of the later hunks in the file"), + checkout_nothead => N__( +"y - apply this hunk to index and worktree +n - do not apply this hunk to index and worktree +q - quit; do not apply this hunk or any of the remaining ones +a - apply this hunk and all later hunks in the file +d - do not apply this hunk or any of the later hunks in the file"), +); + sub help_patch_cmd { - my $verb = lc $patch_mode_flavour{VERB}; - my $target = $patch_mode_flavour{TARGET}; - print colored $help_color, <
[PATCH v4 02/14] i18n: add--interactive: mark simple here-documents for translation
Mark messages in here-document without interpolation for translation. The here-document delimiter \EOF, which is the same as 'EOF', indicate that the text is to be treated literally without interpolation of its content. Unfortunately xgettext is not able to extract here documents with delimiter \EOF but it is with delimiter enclosed in single quotes. Then change \EOF to 'EOF', although in this case does not make difference what variation of here-document to use since there is nothing to interpolate. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index cf216ec..5800010 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -639,7 +639,7 @@ sub list_and_choose { } sub singleton_prompt_help_cmd { - print colored $help_color, <<\EOF ; + print colored $help_color, __ <<'EOF' ; Prompt help: 1 - select a numbered item foo- select item based on unique prefix @@ -648,7 +648,7 @@ EOF } sub prompt_help_cmd { - print colored $help_color, <<\EOF ; + print colored $help_color, __ <<'EOF' ; Prompt help: 1 - select a single item 3-5- select a range of items @@ -1584,7 +1584,9 @@ sub quit_cmd { } sub help_cmd { - print colored $help_color, <<\EOF ; +# TRANSLATORS: please do not translate the command names +# 'status', 'update', 'revert', etc. + print colored $help_color, __ <<'EOF' ; status- show paths with changes update- add working tree state to the staged set of changes revert- revert staged set of changes back to the HEAD version -- 2.7.4
[PATCH v4 03/14] i18n: add--interactive: mark strings with interpolation for translation
Since at this point Git::I18N.perl lacks support for Perl i18n placeholder substitution, use of sprintf following die or error_msg is necessary for placeholder substitution take place. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 5800010..d05ac60 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -615,12 +615,12 @@ sub list_and_choose { else { $bottom = $top = find_unique($choice, @stuff); if (!defined $bottom) { - error_msg "Huh ($choice)?\n"; + error_msg sprintf(__("Huh (%s)?\n"), $choice); next TOPLOOP; } } if ($opts->{SINGLETON} && $bottom != $top) { - error_msg "Huh ($choice)?\n"; + error_msg sprintf(__("Huh (%s)?\n"), $choice); next TOPLOOP; } for ($i = $bottom-1; $i <= $top-1; $i++) { @@ -717,7 +717,7 @@ sub revert_cmd { $_->{INDEX_ADDDEL} eq 'create') { system(qw(git update-index --force-remove --), $_->{VALUE}); - print "note: $_->{VALUE} is untracked now.\n"; + printf(__("note: %s is untracked now.\n"), $_->{VALUE}); } } } @@ -1056,7 +1056,7 @@ sub edit_hunk_manually { my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff"; my $fh; open $fh, '>', $hunkfile - or die "failed to open hunk edit file for writing: " . $!; + or die sprintf(__("failed to open hunk edit file for writing: %s"), $!); print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n"; print $fh @$oldtext; my $participle = $patch_mode_flavour{PARTICIPLE}; @@ -1083,7 +1083,7 @@ EOF } open $fh, '<', $hunkfile - or die "failed to open hunk edit file for reading: " . $!; + or die sprintf(__("failed to open hunk edit file for reading: %s"), $!); my @newtext = grep { !/^#/ } <$fh>; close $fh; unlink $hunkfile; @@ -1236,7 +1236,7 @@ sub apply_patch_for_checkout_commit { sub patch_update_cmd { my @all_mods = list_modified($patch_mode_flavour{FILTER}); - error_msg "ignoring unmerged: $_->{VALUE}\n" + error_msg sprintf(__("ignoring unmerged: %s\n"), $_->{VALUE}) for grep { $_->{UNMERGED} } @all_mods; @all_mods = grep { !$_->{UNMERGED} } @all_mods; @@ -1418,7 +1418,8 @@ sub patch_update_file { chomp $response; } if ($response !~ /^\s*\d+\s*$/) { - error_msg "Invalid number: '$response'\n"; + error_msg sprintf(__("Invalid number: '%s'\n"), +$response); } elsif (0 < $response && $response <= $num) { $ix = $response - 1; } else { @@ -1460,7 +1461,7 @@ sub patch_update_file { if ($@) { my ($err,$exp) = ($@, $1); $err =~ s/ at .*git-add--interactive line \d+, line \d+.*$//; - error_msg "Malformed search regexp $exp: $err\n"; + error_msg sprintf(__("Malformed search regexp %s: %s\n"), $exp, $err); next; } my $iy = $ix; @@ -1625,18 +1626,18 @@ sub process_args { $patch_mode = $1; $arg = shift @ARGV or die __("missing --"); } else { - die "unknown --patch mode: $1"; + die sprintf(__("unknown --patch mode: %s"), $1); } } else { $patch_mode = 'stage'; $arg = shift @ARGV or die __("missing --"); } - die "invalid argument $arg, expecting --" - unless $arg eq "--"; + die sprintf(__("invalid argument %s, expecting --"), + $a
[PATCH v4 01/14] i18n: add--interactive: mark strings for translation
Mark simple strings (without interpolation) for translation. Brackets around first parameter of ternary operator is necessary because otherwise xgettext fails to extract strings marked for translation from the rest of the file. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 76 ++- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index ee3d812..cf216ec 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -4,6 +4,7 @@ use 5.008; use strict; use warnings; use Git; +use Git::I18N; binmode(STDOUT, ":raw"); @@ -253,8 +254,9 @@ sub list_untracked { run_cmd_pipe(qw(git ls-files --others --exclude-standard --), @ARGV); } -my $status_fmt = '%12s %12s %s'; -my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path'); +# TRANSLATORS: you can adjust this to align "git add -i" status menu +my $status_fmt = __('%12s %12s %s'); +my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), __('path')); { my $initial; @@ -680,7 +682,7 @@ sub update_cmd { my @mods = list_modified('file-only'); return if (!@mods); - my @update = list_and_choose({ PROMPT => 'Update', + my @update = list_and_choose({ PROMPT => __('Update'), HEADER => $status_head, }, @mods); if (@update) { @@ -692,7 +694,7 @@ sub update_cmd { } sub revert_cmd { - my @update = list_and_choose({ PROMPT => 'Revert', + my @update = list_and_choose({ PROMPT => __('Revert'), HEADER => $status_head, }, list_modified()); if (@update) { @@ -726,13 +728,13 @@ sub revert_cmd { } sub add_untracked_cmd { - my @add = list_and_choose({ PROMPT => 'Add untracked' }, + my @add = list_and_choose({ PROMPT => __('Add untracked') }, list_untracked()); if (@add) { system(qw(git update-index --add --), @add); say_n_paths('added', @add); } else { - print "No untracked files.\n"; + print __("No untracked files.\n"); } print "\n"; } @@ -1166,8 +1168,14 @@ sub edit_hunk_loop { } else { prompt_yesno( - 'Your edited hunk does not apply. Edit again ' - . '(saying "no" discards!) [y/n]? ' + # TRANSLATORS: do not translate [y/n] + # The program will only accept that input + # at this point. + # Consider translating (saying "no" discards!) as + # (saying "n" for "no" discards!) if the translation + # of the word "no" does not start with n. + __('Your edited hunk does not apply. Edit again ' + . '(saying "no" discards!) [y/n]? ') ) or return undef; } } @@ -1213,11 +1221,11 @@ sub apply_patch_for_checkout_commit { run_git_apply 'apply '.$reverse, @_; return 1; } elsif (!$applies_index) { - print colored $error_color, "The selected hunks do not apply to the index!\n"; - if (prompt_yesno "Apply them to the worktree anyway? ") { + print colored $error_color, __("The selected hunks do not apply to the index!\n"); + if (prompt_yesno __("Apply them to the worktree anyway? ")) { return run_git_apply 'apply '.$reverse, @_; } else { - print colored $error_color, "Nothing was applied.\n"; + print colored $error_color, __("Nothing was applied.\n"); return 0; } } else { @@ -1237,9 +1245,9 @@ sub patch_update_cmd { if (!@mods) { if (@all_mods) { - print STDERR "Only binary files changed.\n"; + print STDERR __("Only binary files changed.\n"); } else { - print STDERR "No changes.\n"; + print STDERR __("No changes.\n"); } return 0; } @@ -1247,7 +1255,7 @@ sub patch_update_cmd { @them = @mods; } else { - @them = list_and_choose({ PROMPT => 'Patch update', + @them = list_and_choose({ PROMPT => __('Patch update'), HEADER => $status_head, }, @mods); } @@ -1397,12 +1405,12 @@ sub patch_update_file {
[PATCH v4 04/14] i18n: clean.c: match string with git-add--interactive.perl
Change strings for help to match the ones in git-add--interactive.perl. The strings now represent one entry to translate each rather then two entries each different only by an ending newline character. Signed-off-by: Vasco Almeida --- builtin/clean.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 0371010..d6bc3aa 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -287,11 +287,11 @@ static void pretty_print_menus(struct string_list *menu_list) static void prompt_help_cmd(int singleton) { clean_print_color(CLEAN_COLOR_HELP); - printf_ln(singleton ? + printf(singleton ? _("Prompt help:\n" "1 - select a numbered item\n" "foo- select item based on unique prefix\n" - " - (empty) select nothing") : + " - (empty) select nothing\n") : _("Prompt help:\n" "1 - select a single item\n" "3-5- select a range of items\n" @@ -299,7 +299,7 @@ static void prompt_help_cmd(int singleton) "foo- select item based on unique prefix\n" "-... - unselect specified items\n" "* - choose all items\n" - " - (empty) finish selecting")); + " - (empty) finish selecting\n")); clean_print_color(CLEAN_COLOR_RESET); } @@ -508,7 +508,7 @@ static int parse_choice(struct menu_stuff *menu_stuff, if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top || (is_single && bottom != top)) { clean_print_color(CLEAN_COLOR_ERROR); - printf_ln(_("Huh (%s)?"), (*ptr)->buf); + printf(_("Huh (%s)?\n"), (*ptr)->buf); clean_print_color(CLEAN_COLOR_RESET); continue; } @@ -774,7 +774,7 @@ static int ask_each_cmd(void) static int quit_cmd(void) { string_list_clear(&del_list, 0); - printf_ln(_("Bye.")); + printf(_("Bye.\n")); return MENU_RETURN_NO_LOOP; } -- 2.7.4
[PATCH v4 05/14] i18n: add--interactive: mark plural strings
Mark plural strings for translation. Unfold each action case in one entire sentence. Pass new keyword for xgettext to extract. Update test to include new subroutine __n() for plural strings handling. Update documentation to include a description of the new __n() subroutine. Signed-off-by: Vasco Almeida --- Makefile | 3 ++- git-add--interactive.perl | 27 ++- perl/Git/I18N.pm | 9 - t/t0202/test.pl | 11 ++- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 1aad150..4ef0344 100644 --- a/Makefile +++ b/Makefile @@ -2111,7 +2111,8 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword="Q_:1,2" XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \ --keyword=gettextln --keyword=eval_gettextln -XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl +XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \ + --keyword=__ --keyword="__n:1,2" LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH = $(SCRIPT_SH) LOCALIZED_SH += git-parse-remote.sh diff --git a/git-add--interactive.perl b/git-add--interactive.perl index d05ac60..cd61783 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -669,12 +669,18 @@ sub status_cmd { sub say_n_paths { my $did = shift @_; my $cnt = scalar @_; - print "$did "; - if (1 < $cnt) { - print "$cnt paths\n"; - } - else { - print "one path\n"; + if ($did eq 'added') { + printf(__n("added %d path\n", "added %d paths\n", + $cnt), $cnt); + } elsif ($did eq 'updated') { + printf(__n("updated %d path\n", "updated %d paths\n", + $cnt), $cnt); + } elsif ($did eq 'reverted') { + printf(__n("reverted %d path\n", "reverted %d paths\n", + $cnt), $cnt); + } else { + printf(__n("touched %d path\n", "touched %d paths\n", + $cnt), $cnt); } } @@ -1423,7 +1429,8 @@ sub patch_update_file { } elsif (0 < $response && $response <= $num) { $ix = $response - 1; } else { - error_msg "Sorry, only $num hunks available.\n"; + error_msg sprintf(__n("Sorry, only %d hunk available.\n", + "Sorry, only %d hunks available.\n", $num), $num); } next; } @@ -1518,8 +1525,10 @@ sub patch_update_file { elsif ($other =~ /s/ && $line =~ /^s/) { my @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY}); if (1 < @split) { - print colored $header_color, "Split into ", - scalar(@split), " hunks.\n"; + print colored $header_color, sprintf( + __n("Split into %d hunk.\n", + "Split into %d hunks.\n", + scalar(@split)), scalar(@split)); } splice (@hunk, $ix, 1, @split); $num = scalar @hunk; diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm index f889fd6..3f7ac25 100644 --- a/perl/Git/I18N.pm +++ b/perl/Git/I18N.pm @@ -13,7 +13,7 @@ BEGIN { } } -our @EXPORT = qw(__); +our @EXPORT = qw(__ __n); our @EXPORT_OK = @EXPORT; sub __bootstrap_locale_messages { @@ -44,6 +44,7 @@ BEGIN eval { __bootstrap_locale_messages(); *__ = \&Locale::Messages::gettext; + *__n = \&Locale::Messages::ngettext; 1; } or do { # Tell test.pl that we couldn't load the gettext library. @@ -51,6 +52,7 @@ BEGIN # Just a fall-through no-op *__ = sub ($) { $_[0] }; + *__n = sub ($$$) { $_[2] == 1 ? $_[0] : $_[1] }; }; } @@ -70,6 +72,8 @@ Git::I18N - Perl interface to Git's Gettext localizations printf __("The following error occurred: %s\n"), $error; + printf __n("commited %d file", "commited %d files", $files), $files; + =head1 DESCRIPTION Git's internal Perl interface to gettext via L. If @@ -87,6 +91,9 @@ it. L's gettext function if all goes well, otherwise our passthrough fallback function. +=head2 __n($$$) +L's ngettext function or passthrough fallback function. + =head1 AUTHOR Evar ArnfjErE Bjarmason diff --git a/t/t0
[PATCH v4 06/14] i18n: add--interactive: mark patch prompt for translation
Mark prompt message assembled in place for translation, unfolding each use case for each entry in the %patch_modes hash table. Previously, this script relied on whether $patch_mode was set to run the command patch_update_cmd() or show status and loop the main loop. Now, it uses $cmd to indicate we must run patch_update_cmd() and $patch_mode is used to tell which flavor of the %patch_modes are we on. This is introduced in order to be able to mark and unfold the message prompt knowing in which context we are. The tracking of context was done previously by point %patch_mode_flavour hash table to the correct entry of %patch_modes, focusing only on value of %patch_modes. Now, we are also interested in the key ('staged', 'stash', 'checkout_head', ...). Signed-off-by: Vasco Almeida --- Makefile | 2 +- git-add--interactive.perl | 54 --- perl/Git/I18N.pm | 10 - t/t0202/test.pl | 5 - 4 files changed, 60 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 4ef0344..9dc95cb 100644 --- a/Makefile +++ b/Makefile @@ -2112,7 +2112,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \ --keyword=gettextln --keyword=eval_gettextln XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \ - --keyword=__ --keyword="__n:1,2" + --keyword=__ --keyword=N__ --keyword="__n:1,2" LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH = $(SCRIPT_SH) LOCALIZED_SH += git-parse-remote.sh diff --git a/git-add--interactive.perl b/git-add--interactive.perl index cd61783..b7d382b 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -93,6 +93,7 @@ sub colored { } # command line options +my $cmd; my $patch_mode; my $patch_mode_revision; @@ -173,7 +174,8 @@ my %patch_modes = ( }, ); -my %patch_mode_flavour = %{$patch_modes{stage}}; +$patch_mode = 'stage'; +my %patch_mode_flavour = %{$patch_modes{$patch_mode}}; sub run_cmd_pipe { if ($^O eq 'MSWin32') { @@ -1311,6 +1313,44 @@ sub display_hunks { return $i; } +my %patch_update_prompt_modes = ( + stage => { + mode => N__("Stage mode change [y,n,q,a,d,/%s,?]? "), + deletion => N__("Stage deletion [y,n,q,a,d,/%s,?]? "), + hunk => N__("Stage this hunk [y,n,q,a,d,/%s,?]? "), + }, + stash => { + mode => N__("Stash mode change [y,n,q,a,d,/%s,?]? "), + deletion => N__("Stash deletion [y,n,q,a,d,/%s,?]? "), + hunk => N__("Stash this hunk [y,n,q,a,d,/%s,?]? "), + }, + reset_head => { + mode => N__("Unstage mode change [y,n,q,a,d,/%s,?]? "), + deletion => N__("Unstage deletion [y,n,q,a,d,/%s,?]? "), + hunk => N__("Unstage this hunk [y,n,q,a,d,/%s,?]? "), + }, + reset_nothead => { + mode => N__("Apply mode change to index [y,n,q,a,d,/%s,?]? "), + deletion => N__("Apply deletion to index [y,n,q,a,d,/%s,?]? "), + hunk => N__("Apply this hunk to index [y,n,q,a,d,/%s,?]? "), + }, + checkout_index => { + mode => N__("Discard mode change from worktree [y,n,q,a,d,/%s,?]? "), + deletion => N__("Discard deletion from worktree [y,n,q,a,d,/%s,?]? "), + hunk => N__("Discard this hunk from worktree [y,n,q,a,d,/%s,?]? "), + }, + checkout_head => { + mode => N__("Discard mode change from index and worktree [y,n,q,a,d,/%s,?]? "), + deletion => N__("Discard deletion from index and worktree [y,n,q,a,d,/%s,?]? "), + hunk => N__("Discard this hunk from index and worktree [y,n,q,a,d,/%s,?]? "), + }, + checkout_nothead => { + mode => N__("Apply mode change to index and worktree [y,n,q,a,d,/%s,?]? "), + deletion => N__("Apply deletion to index and worktree [y,n,q,a,d,/%s,?]? "), + hunk => N__("Apply this hunk to index and worktree [y,n,q,a,d,/%s,?]? "), + }, +); + sub patch_update_file { my $quit = 0; my ($ix, $num); @@ -1383,12 +1423,9 @@ sub patch_update_file { for (@{$hunk[$ix]{DISPLAY}}) { print; } - print colored $prompt_color, $patch_mode_flavour{VERB}, - ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' : - $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' : - ' this hunk'), - $patch_mode_flavour{TARGET}, - " [y,n,q,a,d,/$other,?]? "; + print colored $prompt_color, + sprintf(__($patch_update_prompt_modes{$patch_mode}{$hunk[$ix]{TYPE}}), $other); + my $line = prompt_single_character; last unless defined $line; if ($line) { @@ -1644,6
[PATCH v4 09/14] i18n: add--interactive: remove %patch_modes entries
Remove unnecessary entries from %patch_modes. After the i18n conversion, these entries are not used anymore. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 21 - 1 file changed, 21 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 861f7b0..d7a8e0d 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -106,9 +106,6 @@ my %patch_modes = ( DIFF => 'diff-files -p', APPLY => sub { apply_patch 'apply --cached', @_; }, APPLY_CHECK => 'apply --cached', - VERB => 'Stage', - TARGET => '', - PARTICIPLE => 'staging', FILTER => 'file-only', IS_REVERSE => 0, }, @@ -116,9 +113,6 @@ my %patch_modes = ( DIFF => 'diff-index -p HEAD', APPLY => sub { apply_patch 'apply --cached', @_; }, APPLY_CHECK => 'apply --cached', - VERB => 'Stash', - TARGET => '', - PARTICIPLE => 'stashing', FILTER => undef, IS_REVERSE => 0, }, @@ -126,9 +120,6 @@ my %patch_modes = ( DIFF => 'diff-index -p --cached', APPLY => sub { apply_patch 'apply -R --cached', @_; }, APPLY_CHECK => 'apply -R --cached', - VERB => 'Unstage', - TARGET => '', - PARTICIPLE => 'unstaging', FILTER => 'index-only', IS_REVERSE => 1, }, @@ -136,9 +127,6 @@ my %patch_modes = ( DIFF => 'diff-index -R -p --cached', APPLY => sub { apply_patch 'apply --cached', @_; }, APPLY_CHECK => 'apply --cached', - VERB => 'Apply', - TARGET => ' to index', - PARTICIPLE => 'applying', FILTER => 'index-only', IS_REVERSE => 0, }, @@ -146,9 +134,6 @@ my %patch_modes = ( DIFF => 'diff-files -p', APPLY => sub { apply_patch 'apply -R', @_; }, APPLY_CHECK => 'apply -R', - VERB => 'Discard', - TARGET => ' from worktree', - PARTICIPLE => 'discarding', FILTER => 'file-only', IS_REVERSE => 1, }, @@ -156,9 +141,6 @@ my %patch_modes = ( DIFF => 'diff-index -p', APPLY => sub { apply_patch_for_checkout_commit '-R', @_ }, APPLY_CHECK => 'apply -R', - VERB => 'Discard', - TARGET => ' from index and worktree', - PARTICIPLE => 'discarding', FILTER => undef, IS_REVERSE => 1, }, @@ -166,9 +148,6 @@ my %patch_modes = ( DIFF => 'diff-index -R -p', APPLY => sub { apply_patch_for_checkout_commit '', @_ }, APPLY_CHECK => 'apply', - VERB => 'Apply', - TARGET => ' to index and worktree', - PARTICIPLE => 'applying', FILTER => undef, IS_REVERSE => 0, }, -- 2.7.4
[PATCH v4 08/14] i18n: add--interactive: mark edit_hunk_manually message for translation
Mark message of edit_hunk_manually displayed in the editing file when user chooses 'e' option. The message had to be unfolded to allow translation of the $participle verb. Some messages end up being exactly the same for some uses cases, but left it for easier change in the future, e.g., wanting to change wording of one particular use case. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 45 ++--- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 045b847..861f7b0 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -1058,6 +1058,30 @@ sub color_diff { } @_; } +my %edit_hunk_manually_modes = ( + stage => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for staging."), + stash => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for stashing."), + reset_head => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for unstaging."), + reset_nothead => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for applying."), + checkout_index => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for discarding"), + checkout_head => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for discarding."), + checkout_nothead => N__( +"# If the patch applies cleanly, the edited hunk will immediately be +# marked for applying."), +); + sub edit_hunk_manually { my ($oldtext) = @_; @@ -1065,22 +1089,21 @@ sub edit_hunk_manually { my $fh; open $fh, '>', $hunkfile or die sprintf(__("failed to open hunk edit file for writing: %s"), $!); - print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n"; + print $fh __("# Manual hunk edit mode -- see bottom for a quick guide\n"); print $fh @$oldtext; - my $participle = $patch_mode_flavour{PARTICIPLE}; my $is_reverse = $patch_mode_flavour{IS_REVERSE}; my ($remove_plus, $remove_minus) = $is_reverse ? ('-', '+') : ('+', '-'); - print $fh <
[PATCH v4 12/14] i18n: send-email: mark warnings and errors for translation
Mark warnings, errors and other messages for translation. Signed-off-by: Vasco Almeida --- git-send-email.perl | 34 +- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 78eb59b..982c6c0 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -118,20 +118,20 @@ sub format_2822_time { my $localmin = $localtm[1] + $localtm[2] * 60; my $gmtmin = $gmttm[1] + $gmttm[2] * 60; if ($localtm[0] != $gmttm[0]) { - die "local zone differs from GMT by a non-minute interval\n"; + die __("local zone differs from GMT by a non-minute interval\n"); } if ((($gmttm[6] + 1) % 7) == $localtm[6]) { $localmin += 1440; } elsif ((($gmttm[6] - 1) % 7) == $localtm[6]) { $localmin -= 1440; } elsif ($gmttm[6] != $localtm[6]) { - die "local time offset greater than or equal to 24 hours\n"; + die __("local time offset greater than or equal to 24 hours\n"); } my $offset = $localmin - $gmtmin; my $offhour = $offset / 60; my $offmin = abs($offset % 60); if (abs($offhour) >= 24) { - die ("local time offset greater than or equal to 24 hours\n"); + die __("local time offset greater than or equal to 24 hours\n"); } return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d", @@ -199,13 +199,13 @@ sub do_edit { map { system('sh', '-c', $editor.' "$@"', $editor, $_); if (($? & 127) || ($? >> 8)) { - die("the editor exited uncleanly, aborting everything"); + die(__("the editor exited uncleanly, aborting everything")); } } @_; } else { system('sh', '-c', $editor.' "$@"', $editor, @_); if (($? & 127) || ($? >> 8)) { - die("the editor exited uncleanly, aborting everything"); + die(__("the editor exited uncleanly, aborting everything")); } } } @@ -299,7 +299,7 @@ my $help; my $rc = GetOptions("h" => \$help, "dump-aliases" => \$dump_aliases); usage() unless $rc; -die "--dump-aliases incompatible with other options\n" +die __("--dump-aliases incompatible with other options\n") if !$help and $dump_aliases and @ARGV; $rc = GetOptions( "sender|from=s" => \$sender, @@ -362,7 +362,7 @@ unless ($rc) { usage(); } -die "Cannot run git format-patch from outside a repository\n" +die __("Cannot run git format-patch from outside a repository\n") if $format_patch and not $repo; # Now, let's fill any that aren't set in with defaults: @@ -617,7 +617,7 @@ while (defined(my $f = shift @ARGV)) { } if (@rev_list_opts) { - die "Cannot run git format-patch from outside a repository\n" + die __("Cannot run git format-patch from outside a repository\n") unless $repo; push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts); } @@ -638,7 +638,7 @@ if (@files) { print $_,"\n" for (@files); } } else { - print STDERR "\nNo patch files specified!\n\n"; + print STDERR __("\nNo patch files specified!\n\n"); usage(); } @@ -730,7 +730,7 @@ EOT $sender = $1; next; } elsif (/^(?:To|Cc|Bcc):/i) { - print "To/Cc/Bcc fields are not interpreted yet, they have been ignored\n"; + print __("To/Cc/Bcc fields are not interpreted yet, they have been ignored\n"); next; } print $c2 $_; @@ -739,7 +739,7 @@ EOT close $c2; if ($summary_empty) { - print "Summary email is empty, skipping it\n"; + print __("Summary email is empty, skipping it\n"); $compose = -1; } } elsif ($annotate) { @@ -1314,7 +1314,7 @@ Message-Id: $message_id $_ = ask(__("Send this email? ([y]es|[n]o|[q]uit|[a]ll): "), valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i, default => $ask_default); - die "Send this email reply required" unless defined $_; + die __("Send this email reply required") unless defined $_; if (/^n/i) { return 0; } elsif (/^q/i) { @@ -1340,7 +1340,7 @@ Message-Id: $message_id } else { if (!defined $smtp_server) { - die "The required SMTP server is not properly defined." + die __("The required SMTP server is not properly defined.") } if ($smtp_encryptio
[PATCH v4 10/14] i18n: add--interactive: mark status words for translation
Mark words 'nothing', 'unchanged' and 'binary' used to display what has been staged or not, in "git add -i" status command. Alternatively one could mark N__('nothing') no-op in order to xgettext(1) extract the string and then trigger the translation at run time only with __($print->{FILE}), but that has the side effect of triggering retrieval of translations for the changes indicator too (e.g. +2/-1) which may or may not be a problem. To avoid that potential problem, mark only where there is certain to trigger translation only of those words but in this case we must also retrieve the translation for the eq tests, since the value assigned was of the translation, not the English source. Signed-off-by: Vasco Almeida --- git-add--interactive.perl | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index d7a8e0d..4754104 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -295,7 +295,7 @@ sub list_modified { my ($change, $bin); $file = unquote_path($file); if ($add eq '-' && $del eq '-') { - $change = 'binary'; + $change = __('binary'); $bin = 1; } else { @@ -304,7 +304,7 @@ sub list_modified { $data{$file} = { INDEX => $change, BINARY => $bin, - FILE => 'nothing', + FILE => __('nothing'), } } elsif (($adddel, $file) = @@ -320,7 +320,7 @@ sub list_modified { $file = unquote_path($file); my ($change, $bin); if ($add eq '-' && $del eq '-') { - $change = 'binary'; + $change = __('binary'); $bin = 1; } else { @@ -340,7 +340,7 @@ sub list_modified { $file = unquote_path($2); if (!exists $data{$file}) { $data{$file} = +{ - INDEX => 'unchanged', + INDEX => __('unchanged'), BINARY => 0, }; } @@ -355,10 +355,10 @@ sub list_modified { if ($only) { if ($only eq 'index-only') { - next if ($it->{INDEX} eq 'unchanged'); + next if ($it->{INDEX} eq __('unchanged')); } if ($only eq 'file-only') { - next if ($it->{FILE} eq 'nothing'); + next if ($it->{FILE} eq __('nothing')); } } push @return, +{ -- 2.7.4
[PATCH v4 11/14] i18n: send-email: mark strings for translation
Mark strings often displayed to the user for translation. Signed-off-by: Vasco Almeida --- git-send-email.perl | 52 ++-- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index da81be4..78eb59b 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /; use File::Spec::Functions qw(catfile); use Error qw(:try); use Git; +use Git::I18N; Getopt::Long::Configure qw/ pass_through /; @@ -797,12 +798,12 @@ foreach my $f (@files) { } if (!defined $auto_8bit_encoding && scalar %broken_encoding) { - print "The following files are 8bit, but do not declare " . - "a Content-Transfer-Encoding.\n"; + print __("The following files are 8bit, but do not declare " . +"a Content-Transfer-Encoding.\n"); foreach my $f (sort keys %broken_encoding) { print "$f\n"; } - $auto_8bit_encoding = ask("Which 8bit encoding should I declare [UTF-8]? ", + $auto_8bit_encoding = ask(__("Which 8bit encoding should I declare [UTF-8]? "), valid_re => qr/.{4}/, confirm_only => 1, default => "UTF-8"); } @@ -829,7 +830,7 @@ if (defined $sender) { # But it's a no-op to run sanitize_address on an already sanitized address. $sender = sanitize_address($sender); -my $to_whom = "To whom should the emails be sent (if anyone)?"; +my $to_whom = __("To whom should the emails be sent (if anyone)?"); my $prompting = 0; if (!@initial_to && !defined $to_cmd) { my $to = ask("$to_whom ", @@ -859,7 +860,7 @@ sub expand_one_alias { if ($thread && !defined $initial_reply_to && $prompting) { $initial_reply_to = ask( - "Message-ID to be used as In-Reply-To for the first email (if any)? ", + __("Message-ID to be used as In-Reply-To for the first email (if any)? "), default => "", valid_re => qr/\@.*\./, confirm_only => 1); } @@ -918,7 +919,10 @@ sub validate_address { my $address = shift; while (!extract_valid_address($address)) { print STDERR "error: unable to extract a valid address from: $address\n"; - $_ = ask("What to do with this address? ([q]uit|[d]rop|[e]dit): ", + # TRANSLATORS: Make sure to include [q] [d] [e] in your + # translation. The program will only accept English input + # at this point. + $_ = ask(__("What to do with this address? ([q]uit|[d]rop|[e]dit): "), valid_re => qr/^(?:quit|q|drop|d|edit|e)/i, default => 'q'); if (/^d/i) { @@ -1293,17 +1297,21 @@ Message-Id: $message_id if ($needs_confirm eq "inform") { $confirm_unconfigured = 0; # squelch this message for the rest of this run $ask_default = "y"; # assume yes on EOF since user hasn't explicitly asked for confirmation - print "The Cc list above has been expanded by additional\n"; - print "addresses found in the patch commit message. By default\n"; - print "send-email prompts before sending whenever this occurs.\n"; - print "This behavior is controlled by the sendemail.confirm\n"; - print "configuration setting.\n"; - print "\n"; - print "For additional information, run 'git send-email --help'.\n"; - print "To retain the current behavior, but squelch this message,\n"; - print "run 'git config --global sendemail.confirm auto'.\n\n"; + print __( +"The Cc list above has been expanded by additional +addresses found in the patch commit message. By default +send-email prompts before sending whenever this occurs. +This behavior is controlled by the sendemail.confirm +configuration setting. + +For additional information, run 'git send-email --help'. +To retain the current behavior, but squelch this message, +run 'git config --global sendemail.confirm auto'."), "\n\n"; } - $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ", + # TRANSLATORS: Make sure to include [y] [n] [q] [a] in your + # translation. The program will only accept English input + # at this point. + $_ = ask(__("Send this email? ([y]es|[n]o|[q]uit|[a]ll): "), valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i, default => $ask_default); die "Send this email reply required" unless defined $_; @@ -1405,7 +1413,7 @@ Message-Id: $message_id if ($quiet) {
[PATCH v4 13/14] i18n: send-email: mark string with interpolation for translation
Mark warnings, errors and other messages that are interpolated for translation. We call sprintf() before calling die() and in few other circumstances in order to replace the values on the placeholders. Signed-off-by: Vasco Almeida --- git-send-email.perl | 90 - 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 982c6c0..5c01425 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -279,10 +279,13 @@ sub signal_handler { # tmp files from --compose if (defined $compose_filename) { if (-e $compose_filename) { - print "'$compose_filename' contains an intermediate version of the email you were composing.\n"; + printf __("'%s' contains an intermediate version ". + "of the email you were composing.\n"), + $compose_filename; } if (-e ($compose_filename . ".final")) { - print "'$compose_filename.final' contains the composed email.\n" + printf __("'%s.final' contains the composed email.\n"), + $compose_filename; } } @@ -431,7 +434,7 @@ $smtp_encryption = '' unless (defined $smtp_encryption); my(%suppress_cc); if (@suppress_cc) { foreach my $entry (@suppress_cc) { - die "Unknown --suppress-cc field: '$entry'\n" + die sprintf(__("Unknown --suppress-cc field: '%s'\n"), $entry) unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/; $suppress_cc{$entry} = 1; } @@ -460,7 +463,7 @@ my $confirm_unconfigured = !defined $confirm; if ($confirm_unconfigured) { $confirm = scalar %suppress_cc ? 'compose' : 'auto'; }; -die "Unknown --confirm setting: '$confirm'\n" +die sprintf(__("Unknown --confirm setting: '%s'\n"), $confirm) unless $confirm =~ /^(?:auto|cc|compose|always|never)/; # Debugging, print out the suppressions. @@ -492,16 +495,16 @@ my %aliases; sub parse_sendmail_alias { local $_ = shift; if (/"/) { - print STDERR "warning: sendmail alias with quotes is not supported: $_\n"; + printf STDERR __("warning: sendmail alias with quotes is not supported: %s\n"), $_; } elsif (/:include:/) { - print STDERR "warning: `:include:` not supported: $_\n"; + printf STDERR __("warning: `:include:` not supported: %s\n"), $_; } elsif (/[\/|]/) { - print STDERR "warning: `/file` or `|pipe` redirection not supported: $_\n"; + printf STDERR __("warning: `/file` or `|pipe` redirection not supported: %s\n"), $_; } elsif (/^(\S+?)\s*:\s*(.+)$/) { my ($alias, $addr) = ($1, $2); $aliases{$alias} = [ split_addrs($addr) ]; } else { - print STDERR "warning: sendmail line is not recognized: $_\n"; + printf STDERR __("warning: sendmail line is not recognized: %s\n"), $_; } } @@ -582,13 +585,12 @@ sub is_format_patch_arg { if (defined($format_patch)) { return $format_patch; } - die(< $repo->repo_path()) : tempfile(".gitsendemail.msg.XX", DIR => "."))[1]; open my $c, ">", $compose_filename - or die "Failed to open for writing $compose_filename: $!"; + or die sprintf(__("Failed to open for writing %s: %s"), $compose_filename, $!); my $tpl_sender = $sender || $repoauthor || $repocommitter || ''; @@ -692,10 +695,10 @@ EOT } open my $c2, ">", $compose_filename . ".final" - or die "Failed to open $compose_filename.final : " . $!; + or die sprintf(__("Failed to open %s.final : %s"), $compose_filename, $!); open $c, "<", $compose_filename - or die "Failed to open $compose_filename : " . $!; + or die sprintf(__("Failed to open %s : %s"), $compose_filename, $!); my $need_8bit_cte = file_has_nonascii($compose_filename); my $in_body = 0; @@ -769,7 +772,9 @@ sub ask { return $resp; } if ($confirm_only) { - my $yesno = $term->readline("Are you sure you want to use <$resp> [y/N]? "); + my $yesno = $term->readline( + # TRANSLATORS: please keep [y/N] as is. + sprintf(__("Are you sure you want to use <%s> [y/N]? "), $resp)); if (defined $yesno && $yesno =~ /y/i) { return $resp; } @@ -811,9 +816,9 @@ if (!defined $auto_8bit_encoding && scalar %broken_encoding) { if (!$force
[PATCH v4 14/14] i18n: difftool: mark warnings for translation
Signed-off-by: Vasco Almeida --- git-difftool.perl | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index a5790d0..8d3632e 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -22,6 +22,7 @@ use File::Path qw(mkpath rmtree); use File::Temp qw(tempdir); use Getopt::Long qw(:config pass_through); use Git; +use Git::I18N; sub usage { @@ -122,7 +123,7 @@ sub setup_dir_diff my $i = 0; while ($i < $#rawdiff) { if ($rawdiff[$i] =~ /^::/) { - warn << 'EOF'; + warn __ <<'EOF'; Combined diff formats ('-c' and '--cc') are not supported in directory diff mode ('-d' and '--dir-diff'). EOF @@ -338,7 +339,7 @@ sub main if (length($opts{difftool_cmd}) > 0) { $ENV{GIT_DIFF_TOOL} = $opts{difftool_cmd}; } else { - print "No given for --tool=\n"; + print __("No given for --tool=\n"); usage(1); } } @@ -346,7 +347,7 @@ sub main if (length($opts{extcmd}) > 0) { $ENV{GIT_DIFFTOOL_EXTCMD} = $opts{extcmd}; } else { - print "No given for --extcmd=\n"; + print __("No given for --extcmd=\n"); usage(1); } } @@ -419,11 +420,11 @@ sub dir_diff } if (exists $wt_modified{$file} and exists $tmp_modified{$file}) { - my $errmsg = "warning: Both files modified: "; - $errmsg .= "'$workdir/$file' and '$b/$file'.\n"; - $errmsg .= "warning: Working tree file has been left.\n"; - $errmsg .= "warning:\n"; - warn $errmsg; + warn sprintf(__( + "warning: Both files modified:\n" . + "'%s/%s' and '%s/%s'.\n" . + "warning: Working tree file has been left.\n" . + "warning:\n"), $workdir, $file, $b, $file); $error = 1; } elsif (exists $tmp_modified{$file}) { my $mode = stat("$b/$file")->mode; @@ -435,8 +436,9 @@ sub dir_diff } } if ($error) { - warn "warning: Temporary files exist in '$tmpdir'.\n"; - warn "warning: You may want to cleanup or recover these.\n"; + warn sprintf(__( + "warning: Temporary files exist in '%s'.\n" . + "warning: You may want to cleanup or recover these.\n"), $tmpdir); exit(1); } else { exit_cleanup($tmpdir, $rc); -- 2.7.4
Re: [PATCH v2] gpg-interface: use more status letters
Junio C Hamano venit, vidit, dixit 06.10.2016 23:43: > Junio C Hamano writes: > >> Michael J Gruber writes: >> >>> Also, I'm open to using another letter for EXPKEYSIG but couldn't decide >>> between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid. >>> 'Y' is next to 'X' and contained in 'KEY', it would be my first choice. >> >> Sounds good enough to me. Thanks. > > I really do not want to leave too many topics listed in the "What's > cooking" report to be in undecided / waiting state. How about > squashing this in, with a fully updated log message to replace? Sorry, this got "lost in vacation". Before that, I was looking for an easy way to test expired signatures, but gpg1 and gpg2 behave somewhat differently in that respect (2 does not allow to create already expired signatures). Is there anything I should or could do now? Michael > -- >8 -- > From: Michael J Gruber > Date: Wed, 28 Sep 2016 16:24:13 +0200 > Subject: [PATCH] SQUASH: gpg-interface: use more status letters > > According to gpg2's doc/DETAILS: > > For each signature only one of the codes GOODSIG, BADSIG, > EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted. > > gpg1 ("classic") behaves the same (although doc/DETAILS differs). > > Currently, we parse gpg's status output for GOODSIG, BADSIG and > trust information and translate that into status codes G, B, U, N > for the %G? format specifier. > > git-verify-* returns success in the GOODSIG case only. This is > somewhat in disagreement with gpg, which considers the first 5 of > the 6 above as VALIDSIG, but we err on the very safe side. > > Introduce additional status codes E, X, Y, R for ERRSIG, EXPSIG, > EXPKEYSIG, and REVKEYSIG so that a user of %G? gets more information > about the absence of a 'G' on first glance. > > Requested-by: Alex > Signed-off-by: Michael J Gruber > Signed-off-by: Junio C Hamano > --- > Documentation/pretty-formats.txt | 3 ++- > gpg-interface.c | 2 +- > pretty.c | 1 + > 3 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/Documentation/pretty-formats.txt > b/Documentation/pretty-formats.txt > index c28ff2b919..179c9389aa 100644 > --- a/Documentation/pretty-formats.txt > +++ b/Documentation/pretty-formats.txt > @@ -146,7 +146,8 @@ endif::git-rev-list[] > - '%G?': show "G" for a good (valid) signature, >"B" for a bad signature, >"U" for a good signature with unknown validity, > - "X" for a good expired signature, or good signature made by an expired key, > + "X" for a good signature that has expired, > + "Y" for a good signature made by an expired key, >"R" for a good signature made by a revoked key, >"E" if the signature cannot be checked (e.g. missing key) >and "N" for no signature > diff --git a/gpg-interface.c b/gpg-interface.c > index 6999e7b469..e44cc27da1 100644 > --- a/gpg-interface.c > +++ b/gpg-interface.c > @@ -35,7 +35,7 @@ static struct { > { 'U', "\n[GNUPG:] TRUST_UNDEFINED" }, > { 'E', "\n[GNUPG:] ERRSIG "}, > { 'X', "\n[GNUPG:] EXPSIG "}, > - { 'X', "\n[GNUPG:] EXPKEYSIG "}, > + { 'Y', "\n[GNUPG:] EXPKEYSIG "}, > { 'R', "\n[GNUPG:] REVKEYSIG "}, > }; > > diff --git a/pretty.c b/pretty.c > index 39a36cd825..f98b271069 100644 > --- a/pretty.c > +++ b/pretty.c > @@ -1236,6 +1236,7 @@ static size_t format_commit_one(struct strbuf *sb, /* > in UTF-8 */ > case 'U': > case 'N': > case 'X': > + case 'Y': > case 'R': > strbuf_addch(sb, c->signature_check.result); > } >
Re: Bug? git worktree fails with master on bare repo
> If I create a bare _clone_, then "HEAD" could be detached, or point to some > branch, depending on where "HEAD" is in the source repo I didn't mean a clone, I meant a brand-new (bare) repo. Then I would clone it somewhere, add commits and branches, and push them to the bare repo. > If source repo's HEAD is "master", I got the same behavior (worktree add > fails) So if it's possible for a bare repo to have HEAD pointing at master, is there a safe way for me to change this (e.g., as a cleanup step before doing my actual merge process)? On Mon, Oct 10, 2016 at 4:45 AM, Duy Nguyen wrote: > On Sun, Oct 9, 2016 at 8:42 PM, Michael Tutty wrote: >> Dennis, >> Thanks for the great response, and for spending time on my issue. >> I'll try that first patch and see what happens. >> >> In the meantime, it got weirder... >> >> I created a brand-new (bare) repo > > Elaboration needed here. If I create a bare _clone_, then "HEAD" could > be detached, or point to some branch, depending on where "HEAD" is in > the source repo. If source repo's HEAD is "master", I got the same > behavior (worktree add fails). If it's detached or points to some > other branch, it's ok. If this is "git init --bare" then I got "fatal: > invalid reference: master". > >> and was able to git add worktree >> /path master. I was able to do this repeatedly, even using the >> worktree to merge other branches to master. I didn't find any >> condition or step that caused some kind of orphan master work tree, >> which was what I thought the underlying problem might be. > -- > Duy -- Michael Tutty, CTO e: mtu...@gforgegroup.com t: @mtutty, @gforgegroup v: 515-789-0772 w: http://gforgegroup.com, http://gforge.com
Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
On Sun, Oct 09, 2016 at 07:53:23PM -0700, Jeremy Huddleston Sequoia wrote: > Subject: Re: [PATCH] t4014-format-patch: Adjust git_version regex to better > handle distro changes to DEF_VER > > Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae > Signed-off-by: Jeremy Huddleston Sequoia I see there was a discussion elsewhere on the list about exactly what you are putting into DEF_VAR that causes the problem. Perhaps the commit message here would be a good place to mention that, why the current regex breaks it, and why your new version fixes not only it, but other possible values of DEF_VAR. -Peff
git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
From https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History Now your new project root is what was in the trunk subdirectory each time. > Git will also automatically remove commits that did not affect the > subdirectory. > We have a git repository that looks like sdk/ android/ ios/ unity/ windows/ Which we'd like to split into 4 repositories, 1 for each platform. To start this process (for splitting android out), I ran, git filter-branch -f --prune-empty --subdirectory-filter android -- --all Which rewrote a ton of history and commits, and looked like it worked, but on closer inspection had left a ton of history behind. If I run git log --all -- unity/ It returns a list of commits that happened in the unity/ subfolder of the original root. commit c4ea2797... > Author: tom... > Date: Thu Feb 25 14:20:59 2016 + > > kick off build > > ... > Which only contains an edit to a file with path "unity/tom" relative to the root *before* the filter-branch, doesn't exist any more, and from my understanding of the docs, shouldn't have been taken across. It's also not an isolated instance, if I run the same checks against "ios/", "windows/", any file that existed in a folder other than "android" to the old root, that history is also preserved. I've just about resorted to running multiple other, explicit filters to remove all references to those other folders, but it seems like this would be redoing the job that I understood git filter-branch should have been doing. Any help in this regard is greatly appreciated. seaders.
Re: %C(auto) not working as expected
On Mon, Oct 10, 2016 at 04:26:18PM +0700, Duy Nguyen wrote: > > If we do a revamp of the pretty-formats to bring them more in line with > > ref-filter (e.g., something like "%(color:red)") maybe that would be an > > opportunity to make minor adjustments. Though, hmm, it looks like > > for-each-ref already knows "%(color:red)", and it's unconditional. > > So perhaps we would need to go through some deprecation period or > > other transition. > > We could add some new tag to change the behavior of all following %C > tags. Something like %C(tty) maybe (probably a bad name), then > discourage the use if "%C(auto" for terminal detection? Yeah, adding a "%C(enable-auto-color)" or something would be backwards compatible and less painful than using "%C(auto)" everywhere. I do wonder if anybody actually _wants_ the "always show color, even if --no-color" behavior. I'm having trouble thinking of a good use for it. IOW, I'm wondering if anyone would disagree that the current behavior is simply buggy. Reading the thread at: http://public-inbox.org/git/7v4njkmq07@alter.siamese.dyndns.org/ I don't really see any compelling reason against it (there was some question of which config to use, but we already answered that with "%C(auto)", and use the value from the pretty_ctx). -Peff
Re: How to watch a mailing list & repo for patches which affect a certain area of code?
Ian Kelling wrote: > I've got patches in various projects, and I don't have time to keep up > with the mailing list, but I'd like to help out with maintenance of that > code, or the functions/files it touches. People don't cc me. I figure I > could filter the list, test patches submitted, commits made, mentions of > files/functions, build filters based on the code I have in the repo even > if it's been moved or changed subsequently. I'm wondering what other > people have implemented already for automation around this, or general > thoughts. Web search is not showing me much. For the mailing list, you can try following an Atom feed for any specific search query. https://public-inbox.org/git/?q=FILE_OR_FUNCTION&x=A (the "x=A" makes it an Atom feed) It's all still a work-in-progress but there'll be better filename and diff handling in public-inbox soon. It's all AGPL and the data is only sourced from this mailing list, so 100% reproducible as I'm incapable of running a reliable server :> Clone instructions at the bottom of https://public-inbox.org/git/
[PATCH] pretty: respect color settings for %C placeholders
On Mon, Oct 10, 2016 at 10:28:18AM -0400, Jeff King wrote: > > We could add some new tag to change the behavior of all following %C > > tags. Something like %C(tty) maybe (probably a bad name), then > > discourage the use if "%C(auto" for terminal detection? > > Yeah, adding a "%C(enable-auto-color)" or something would be backwards > compatible and less painful than using "%C(auto)" everywhere. I do > wonder if anybody actually _wants_ the "always show color, even if > --no-color" behavior. I'm having trouble thinking of a good use for it. > > IOW, I'm wondering if anyone would disagree that the current behavior is > simply buggy. Reading the thread at: > > http://public-inbox.org/git/7v4njkmq07@alter.siamese.dyndns.org/ > > I don't really see any compelling reason against it (there was some > question of which config to use, but we already answered that with > "%C(auto)", and use the value from the pretty_ctx). So here's what a patch to do that would look like. I admit that "I can't think of a good use" does not mean there _isn't_ one, but perhaps by posting this, it might provoke other people to think on it, too. And if nobody can come up with, maybe it's a good idea. -- >8 -- Subject: pretty: respect color settings for %C placeholders The color placeholders have traditionally been unconditional, showing colors even when git is not otherwise configured to do so. This was not so bad for their original use, which was on the command-line (and the user could decide at that moment whether to add colors or not). But these days we have configured formats via pretty.*, and those should operate in multiple contexts. In 3082517 (log --format: teach %C(auto,black) to respect color config, 2012-12-17), we gave an extended placeholder that could be used to accomplish this. But it's rather clunky to use, because you have to specify it individually for each color (and their matching resets) in the format. We shied away from just switching the default to auto, because it is technically breaking backwards compatibility. However, there's not really a use case for unconditional colors. The most plausible reason you would want them unconditional is to redirect "git log" output to a file. But there, the right answer is --color=always, as it does the right thing both with custom user-format colors and git-generated colors. So let's switch to the more useful default. In the off-chance that somebody really does find a use for unconditional colors without wanting to enable the rest of git's colors, we can provide %C(always,...) to enable the old behavior. Signed-off-by: Jeff King --- The tests unsurprisingly needed updating, as we're breaking the old behavior. The diff is easier to read read with "-w". Documentation/pretty-formats.txt | 13 +++--- pretty.c | 19 +--- t/t6006-rev-list-format.sh | 94 3 files changed, 70 insertions(+), 56 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index a942d57..7aa1a8b 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -167,11 +167,14 @@ endif::git-rev-list[] - '%Cblue': switch color to blue - '%Creset': reset color - '%C(...)': color specification, as described in color.branch.* config option; - adding `auto,` at the beginning will emit color only when colors are - enabled for log output (by `color.diff`, `color.ui`, or `--color`, and - respecting the `auto` settings of the former if we are going to a - terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring - on the next placeholders until the color is switched again. + By default, colors are shown only when enabled for log output (by + `color.diff`, `color.ui`, or `--color`, and respecting the `auto` + settings of the former if we are going to a terminal). `%C(auto,...)` + is accepted as a historical synonym for the default. Specifying + `%C(always,...) will show the colors always, even when colors are not + otherwise enabled (to enable this behavior for the whole format, use + `--color=always`). `auto` alone (i.e. `%C(auto)`) will turn on auto + coloring on the next placeholders until the color is switched again. - '%m': left (`<`), right (`>`) or boundary (`-`) mark - '%n': newline - '%%': a raw '%' diff --git a/pretty.c b/pretty.c index 25efbca..73e58b5 100644 --- a/pretty.c +++ b/pretty.c @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */ if (!end) return 0; - if (skip_prefix(begin, "auto,", &begin)) { + + if (!skip_prefix(begin, "always,", &begin)) { if (!want_color(c->pretty_ctx->color)) return end - placeholder + 1; } + + /* this is a historical noop */ + skip_prefix(begin, "auto,", &begin); + if (color_parse_
Re: git merge deletes my changes
On Mon, Oct 10, 2016 at 09:39:13AM +, Eduard Egorov wrote: > A week ago, I've reset a state of 'ceph-ansible' folder in %current% > branch with code from corresponding branch (that tracks an upstream > from github): > > # git read-tree --prefix=ceph-ansible/ -u ceph_ansible This pulls in the subtree files, but there's no actual relationship with the commit history in ceph_ansible. So later... > Then I've committed several changes, including: > > 1. Renamed file and commited: > # git mv site.yml.sample site.yml > > 2. Made some changes and committed > > 3. Pulled updates from original branch by: > # git merge -s subtree --squash ceph_ansible > > It said: > Auto-merging ceph-ansible/site.yml.sample > blablabla > Squash commit -- not updating HEAD > Automatic merge went well; stopped before committing as requested When you merge from ceph_ansible, there is no shared history, and git uses the empty tree as a common ancestor. It looks like the other side added site.yml.sample, for instance, because that is a change from the empty tree. > A post on SO: > http://stackoverflow.com/questions/39954265/git-merge-deletes-my-changes As you noted on SO, modern git disallows merges of unrelated history by default, because it's usually a mistake to do so. If you are doing repeated merges into the subtree, you need to somehow tell git how the histories are related. The obvious answer is to do a "git merge -s ours ceph_ansible" after your initial read-tree, so that git knows you've pulled in the changes up to that point. But I'd guess from your use of "--squash" that you don't want to carry the ceph_ansible history in your project. So you need to record the original upstream commit somewhere (probably in the commit message when you commit the read-tree result), and then ask git to use that as the merge-base during subsequent merges (which will require using plumbing codes, as git-merge wants to compute the merge base itself). I believe the git-subtree command (in contrib/subtree of git.git) handles this use case, but I haven't used it myself. -Peff
Re: git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
On Mon, Oct 10, 2016 at 02:42:36PM +0100, Seaders Oloinsigh wrote: > We have a git repository that looks like > > sdk/ > android/ > ios/ > unity/ > windows/ > > Which we'd like to split into 4 repositories, 1 for each platform. To > start this process (for splitting android out), I ran, > > git filter-branch -f --prune-empty --subdirectory-filter android -- --all OK, so that should rewrite each ref to have only the contents of the "android" directory at the top-level. Note that filter-branch saves a copy of the old refs in refs/original. > Which rewrote a ton of history and commits, and looked like it worked, but > on closer inspection had left a ton of history behind. > > If I run > > git log --all -- unity/ > > It returns a list of commits that happened in the unity/ subfolder of the > original root. Here you asked for "--all", which includes refs/original. So you are seeing the original, unwritten commits (and none of your new ones, of course, because they do not have a unity/ directory!). Try: git log --all --source -- unity to see which ref each commit is coming from. Or try: git log --branches --tags -- unity to confirm that your branches and tags do not include that path. Or just: git for-each-ref --format='delete %(refname)' refs/original | git update-ref --stdin to get rid of the backup refs entirely. -Peff
Re: git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
Thanks for the reply, Jeff. Clearing the backups of the branches, those starting with "refs/original" has gotten me closer, but I also needed to do that with "refs/tags" as well, or change my filter-branch command to, git filter-branch -f --prune-empty --tag-name-filter cat --subdirectory-filter android -- --all I still have remnants of that other history, though. Due to the structure of this repo, it looks like there are some branches that never had anything to do with the android/ subdirectory, so they're not getting wiped out. My branch is in a better state to how I want it, but still, if I run your suggestion, git log --all --source -- unity/ I get output like > commit 4853c... refs/heads/unity-sdk-3_1_3 > Author: serg... > Date: Thu Sep 11 16:30:01 2014 +0100 > >Started 3.1.3 Which is basically logs of branches which contain only edits within the unity/ subdirectory of the original root. There are other branches like that for the other platforms / subdirectories of the original root, which if that is the case, I would consider filter-branch with the subdirectory-filter isn't acting as expected, and doesn't get rid of all the history you want it to get rid of. On Mon, Oct 10, 2016 at 4:30 PM, Jeff King wrote: > On Mon, Oct 10, 2016 at 02:42:36PM +0100, Seaders Oloinsigh wrote: > >> We have a git repository that looks like >> >> sdk/ >> android/ >> ios/ >> unity/ >> windows/ >> >> Which we'd like to split into 4 repositories, 1 for each platform. To >> start this process (for splitting android out), I ran, >> >> git filter-branch -f --prune-empty --subdirectory-filter android -- --all > > OK, so that should rewrite each ref to have only the contents of the > "android" directory at the top-level. > > Note that filter-branch saves a copy of the old refs in refs/original. > >> Which rewrote a ton of history and commits, and looked like it worked, but >> on closer inspection had left a ton of history behind. >> >> If I run >> >> git log --all -- unity/ >> >> It returns a list of commits that happened in the unity/ subfolder of the >> original root. > > Here you asked for "--all", which includes refs/original. So you are > seeing the original, unwritten commits (and none of your new ones, of > course, because they do not have a unity/ directory!). > > Try: > > git log --all --source -- unity > > to see which ref each commit is coming from. > > Or try: > > git log --branches --tags -- unity > > to confirm that your branches and tags do not include that path. > > Or just: > > git for-each-ref --format='delete %(refname)' refs/original | > git update-ref --stdin > > to get rid of the backup refs entirely. > > -Peff
Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
> On Oct 10, 2016, at 06:10, Jeff King wrote: > > On Sun, Oct 09, 2016 at 07:53:23PM -0700, Jeremy Huddleston Sequoia wrote: > >> Subject: Re: [PATCH] t4014-format-patch: Adjust git_version regex to better >> handle distro changes to DEF_VER >> >> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae >> Signed-off-by: Jeremy Huddleston Sequoia > > I see there was a discussion elsewhere on the list about exactly what > you are putting into DEF_VAR that causes the problem. Perhaps the commit > message here would be a good place to mention that, why the current > regex breaks it, and why your new version fixes not only it, but other > possible values of DEF_VAR. Thanks, I've added this blurb: For example, git distributed with Apple's Xcode reports a version like: git version 2.9.3 (Apple Git-75) Apple started doing this to help customers distinguish between different versions of their packaged git which have the same base version (eg: with different patches applied). smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
Jeremy Huddleston Sequoia writes: > Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae Please be considerate to future readers of "git log" to help them avoid mistakes earlier commits made that caused you troubles. This line by itself without any explanation of what was broken is quite useless as a commit message. What can the readers do? They'd go back and say "git show 480871e09" and then what? The test added or modified by the commit has been working quite well for others since it was made, so it is very likely the reader wouldn't be able to tell if anything is wrong in it. You would help if you said what is different in _your_ environment from others who have happily been running and passing this test for others to understand and learn from your fix. What is it? The "Adjust ... distro changes" in the title offers some hint but it could be more explicit. Please write something along this line instead. Subject: t4014: git --version can have SP in it 480871e09e ("format-patch: show base info before email signature", 2016-09-07) added a helper function to recreate the signature at the end of the e-mail, i.e. "-- " line followed by the version string of Git, using output from "git --version" and stripping everything before the last SP. Because the default Git version string looks like "git version 2.10.0-1-g480871e09e", this was mostly OK, but people can change this version string to arbitrary thing while compiling, which can break the assumption if they had SP in it. Notably, Apple ships modified Git with " (Apple Git-xx)" appended to its version number. Instead, come up with the version string by stripping the "git version " from the beginning. > Signed-off-by: Jeremy Huddleston Sequoia Good. > CC: Josh Triplett > CC: Junio C Hamano Please don't do this in your log message. These belong to your e-mail headers, not here. > --- > t/t4014-format-patch.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 8d90a6e..33f6940 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -754,7 +754,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream > HEAD' ' > git format-patch --ignore-if-in-upstream HEAD > ' > > -git_version="$(git --version | sed "s/.* //")" > +git_version="$(git --version | sed "s/git version //")" Anchor the fixed prefix to the beginning, so that we can protect ourselves from another distro that would add "git version" in the middle of their arbitrary versioning scheme ;-). I.e. sed "s/^git version //" > > signature() { > printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
Jeff King writes: > On Sun, Oct 09, 2016 at 07:53:23PM -0700, Jeremy Huddleston Sequoia wrote: > >> Subject: Re: [PATCH] t4014-format-patch: Adjust git_version regex to better >> handle distro changes to DEF_VER >> >> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae >> Signed-off-by: Jeremy Huddleston Sequoia > > I see there was a discussion elsewhere on the list about exactly what > you are putting into DEF_VAR that causes the problem. Perhaps the commit > message here would be a good place to mention that, why the current > regex breaks it, and why your new version fixes not only it, but other > possible values of DEF_VAR. Ah, should have known that you'd already be helping with this. Thanks.
Re: [PATCH] t3700-add.sh: Avoid filename collission between tests which could lead to test failure
Jeremy Huddleston Sequoia writes: > Regressed-in: 610d55af0f082f6b866dc858e144c03d8ed4424c > Signed-off-by: Jeremy Huddleston Sequoia > CC: Thomas Gummerer > CC: Junio C Hamano This is also under-explained. Was the test broken for everybody and you are the first to report, or was your environment somehow different that the existing problem was revealed there first?
RE: git merge deletes my changes
Hello again, I've noticed that my git is quite old (1.8.3) and built it from source tarball (2.10.1). Now the output is: # ~/gitbuild/git-2.10.1/git merge -s subtree --squash ceph_ansible fatal: refusing to merge unrelated histories I've checked my command history again: 738 git rm -rf ceph-ansible/ 739 ll 740 ll ceph-ansible/ 741 rm ceph-ansible/purge_ceph_cluster.yml 742 git read-tree --prefix=ceph-ansible/ -u ceph_ansible 743 gs 744 gc "ceph-ansible: reset repo state" A quick googling showed (http://stackoverflow.com/questions/37937984/git-refusing-to-merge-unrelated-histories ) that the default behavior is changed. Adding ' --allow-unrelated-histories' clears the error message but the merge itself is still wrong (my changes are lost). This error message might explain why git can't merge it correctly (since these repos doesn't has any relations, right). Can somebody confirm this please? Doesn't "merge -s subtree" really merges branches? With best regards Eduard Egorov -Original Message- From: Eduard Egorov Sent: Monday, October 10, 2016 12:39 PM To: 'git@vger.kernel.org' Subject: git merge deletes my changes Hello Git experts, A week ago, I've reset a state of 'ceph-ansible' folder in %current% branch with code from corresponding branch (that tracks an upstream from github): # git read-tree --prefix=ceph-ansible/ -u ceph_ansible Then I've committed several changes, including: 1. Renamed file and commited: # git mv site.yml.sample site.yml 2. Made some changes and committed 3. Pulled updates from original branch by: # git merge -s subtree --squash ceph_ansible It said: Auto-merging ceph-ansible/site.yml.sample blablabla Squash commit -- not updating HEAD Automatic merge went well; stopped before committing as requested I can see that "my" ceph-ansible/site.yml is deleted (as well as few new files added by me), ceph-ansible/site.yml.sample is restored and doesn't contain my changes (it's just restored to the state before my changes). `git log ceph_ansible site.yml.sample` shows the latest change on ceph_ansible branch done from 26th of August, 2016, my changes in %current% branch was this October, so it shouldn't be any conflict either (even if it was). I believe there is some obvious explanation for this behavior. Could you help me please? What information can I provide in order to troubleshoot this issue? A post on SO: http://stackoverflow.com/questions/39954265/git-merge-deletes-my-changes With best regards Eduard Egorov
Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
> On Oct 10, 2016, at 09:42, Junio C Hamano wrote: > > Jeremy Huddleston Sequoia writes: > >> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae > > Please be considerate to future readers of "git log" to help them > avoid mistakes earlier commits made that caused you troubles. > > This line by itself without any explanation of what was broken is > quite useless as a commit message. What can the readers do? They'd > go back and say "git show 480871e09" and then what? The test added > or modified by the commit has been working quite well for others > since it was made, so it is very likely the reader wouldn't be able > to tell if anything is wrong in it. > > You would help if you said what is different in _your_ environment > from others who have happily been running and passing this test for > others to understand and learn from your fix. What is it? > > The "Adjust ... distro changes" in the title offers some hint but it > could be more explicit. Please write something along this line > instead. > >Subject: t4014: git --version can have SP in it > >480871e09e ("format-patch: show base info before email >signature", 2016-09-07) added a helper function to recreate the >signature at the end of the e-mail, i.e. "-- " line followed by >the version string of Git, using output from "git --version" and >stripping everything before the last SP. > >Because the default Git version string looks like "git version >2.10.0-1-g480871e09e", this was mostly OK, but people can change >this version string to arbitrary thing while compiling, which >can break the assumption if they had SP in it. Notably, Apple >ships modified Git with " (Apple Git-xx)" appended to its >version number. > >Instead, come up with the version string by stripping the "git >version " from the beginning. Ok, I'll add that explanation to the commit message, thanks. > >> Signed-off-by: Jeremy Huddleston Sequoia > > Good. > >> CC: Josh Triplett >> CC: Junio C Hamano > > Please don't do this in your log message. These belong to your > e-mail headers, not here. Sorry, we do that consistently on other projects during the review process, so git send-email picks them up and adds them automatically for each patch being sent out. It's quite useful, actually. > >> --- >> t/t4014-format-patch.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh >> index 8d90a6e..33f6940 100755 >> --- a/t/t4014-format-patch.sh >> +++ b/t/t4014-format-patch.sh >> @@ -754,7 +754,7 @@ test_expect_success 'format-patch >> --ignore-if-in-upstream HEAD' ' >> git format-patch --ignore-if-in-upstream HEAD >> ' >> >> -git_version="$(git --version | sed "s/.* //")" >> +git_version="$(git --version | sed "s/git version //")" > > Anchor the fixed prefix to the beginning, so that we can protect > ourselves from another distro that would add "git version" in the > middle of their arbitrary versioning scheme ;-). I.e. > >sed "s/^git version //" Ok. > >> >> signature() { >> printf "%s\n%s\n\n" "-- " "${1:-$git_version}" smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] remote.c: free previous results when looking for a ref match
On Sat, Oct 8, 2016 at 6:38 AM, René Scharfe wrote: > > Again, this check is not necessary. If I read the code correctly the > pointer could be uninitialized at that point, though, causing free(3) to > crash. yep, this patch is bogus.
Re: [PATCH] t3700-add.sh: Avoid filename collission between tests which could lead to test failure
> On Oct 10, 2016, at 09:44, Junio C Hamano wrote: > > Jeremy Huddleston Sequoia writes: > >> Regressed-in: 610d55af0f082f6b866dc858e144c03d8ed4424c >> Signed-off-by: Jeremy Huddleston Sequoia >> CC: Thomas Gummerer >> CC: Junio C Hamano > > This is also under-explained. Was the test broken for everybody and > you are the first to report, or was your environment somehow different > that the existing problem was revealed there first? There is nothing obviously different about my environment. I'd expect this test to fail for everyone that runs the entire test because xfoo3 is not cleaned up after this earlier test: test_expect_success \ 'git update-index --add: Test that executable bit is not used...' \ 'git config core.filemode 0 && test_ln_s_add xfoo2 xfoo3 && # runs git update-index --add test_mode_in_index 12 xfoo3' If anyone ran the test by itself or otherwise skipped the 'git update-index --add: Test that executable bit is not used...' test, they would not encounter the issue. However, the normal case is to run all of the tests, so I would expect everyone to be tripping over this issue. If they're not, then that itself might be indicative of another bug because clearly that previous test just added xfoo3 as a symlink. --Jeremy smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] pretty: respect color settings for %C placeholders
Am 10.10.2016 um 17:15 schrieb Jeff King: On Mon, Oct 10, 2016 at 10:28:18AM -0400, Jeff King wrote: We could add some new tag to change the behavior of all following %C tags. Something like %C(tty) maybe (probably a bad name), then discourage the use if "%C(auto" for terminal detection? Yeah, adding a "%C(enable-auto-color)" or something would be backwards compatible and less painful than using "%C(auto)" everywhere. I do wonder if anybody actually _wants_ the "always show color, even if --no-color" behavior. I'm having trouble thinking of a good use for it. IOW, I'm wondering if anyone would disagree that the current behavior is simply buggy. Reading the thread at: http://public-inbox.org/git/7v4njkmq07@alter.siamese.dyndns.org/ I don't really see any compelling reason against it (there was some question of which config to use, but we already answered that with "%C(auto)", and use the value from the pretty_ctx). So here's what a patch to do that would look like. I admit that "I can't think of a good use" does not mean there _isn't_ one, but perhaps by posting this, it might provoke other people to think on it, too. And if nobody can come up with, maybe it's a good idea. Color tags that respect the config and the --color option make the most sense to me as well. Nevertheless a possible counterpoint: Coloring is used in commands that are intended for human consumption. Most of the time there is no need to to conserve the output in a file. But even then, and of course with pipes, once you look at it using less -R or grep you still get nice colored lines and not a monochrome wall of text. -- >8 -- Subject: pretty: respect color settings for %C placeholders The color placeholders have traditionally been unconditional, showing colors even when git is not otherwise configured to do so. This was not so bad for their original use, which was on the command-line (and the user could decide at that moment whether to add colors or not). But these days we have configured formats via pretty.*, and those should operate in multiple contexts. In 3082517 (log --format: teach %C(auto,black) to respect color config, 2012-12-17), we gave an extended placeholder that could be used to accomplish this. But it's rather clunky to use, because you have to specify it individually for each color (and their matching resets) in the format. We shied away from just switching the default to auto, because it is technically breaking backwards compatibility. However, there's not really a use case for unconditional colors. The most plausible reason you would want them unconditional is to redirect "git log" output to a file. But there, the right answer is --color=always, as it does the right thing both with custom user-format colors and git-generated colors. So let's switch to the more useful default. In the off-chance that somebody really does find a use for unconditional colors without wanting to enable the rest of git's colors, we can provide %C(always,...) to enable the old behavior. Signed-off-by: Jeff King --- The tests unsurprisingly needed updating, as we're breaking the old behavior. The diff is easier to read read with "-w". Documentation/pretty-formats.txt | 13 +++--- pretty.c | 19 +--- t/t6006-rev-list-format.sh | 94 3 files changed, 70 insertions(+), 56 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index a942d57..7aa1a8b 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -167,11 +167,14 @@ endif::git-rev-list[] - '%Cblue': switch color to blue - '%Creset': reset color - '%C(...)': color specification, as described in color.branch.* config option; - adding `auto,` at the beginning will emit color only when colors are - enabled for log output (by `color.diff`, `color.ui`, or `--color`, and - respecting the `auto` settings of the former if we are going to a - terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring - on the next placeholders until the color is switched again. + By default, colors are shown only when enabled for log output (by + `color.diff`, `color.ui`, or `--color`, and respecting the `auto` + settings of the former if we are going to a terminal). `%C(auto,...)` + is accepted as a historical synonym for the default. Specifying + `%C(always,...) will show the colors always, even when colors are not + otherwise enabled (to enable this behavior for the whole format, use + `--color=always`). `auto` alone (i.e. `%C(auto)`) will turn on auto + coloring on the next placeholders until the color is switched again. - '%m': left (`<`), right (`>`) or boundary (`-`) mark - '%n': newline - '%%': a raw '%' diff --git a/pretty.c b/pretty.c index 25efbca..73e58b5 100644 --- a/pretty.c +++ b/pretty.c @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */ if (!
[PATCH v3 01/25] sequencer: use static initializers for replay_opts
This change is not completely faithful: instead of initializing all fields to 0, we choose to initialize command and subcommand to -1 (instead of defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically, it makes no difference at all, but future-proofs the code to require explicit assignments for both fields. Signed-off-by: Johannes Schindelin --- builtin/revert.c | 6 ++ sequencer.h | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 4e69380..7365559 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) int cmd_revert(int argc, const char **argv, const char *prefix) { - struct replay_opts opts; + struct replay_opts opts = REPLAY_OPTS_INIT; int res; - memset(&opts, 0, sizeof(opts)); if (isatty(0)) opts.edit = 1; opts.action = REPLAY_REVERT; @@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char *prefix) int cmd_cherry_pick(int argc, const char **argv, const char *prefix) { - struct replay_opts opts; + struct replay_opts opts = REPLAY_OPTS_INIT; int res; - memset(&opts, 0, sizeof(opts)); opts.action = REPLAY_PICK; git_config(git_default_config, NULL); parse_args(argc, argv, &opts); diff --git a/sequencer.h b/sequencer.h index 5ed5cb1..db425ad 100644 --- a/sequencer.h +++ b/sequencer.h @@ -47,6 +47,7 @@ struct replay_opts { /* Only used by REPLAY_NONE */ struct rev_info *revs; }; +#define REPLAY_OPTS_INIT { -1, -1 } int sequencer_pick_revisions(struct replay_opts *opts); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 00/25] Prepare the sequencer for the upcoming rebase -i patches
This patch series marks the '4' in the countdown to speed up rebase -i by implementing large parts in C. It is based on the `libify-sequencer` patch series that I submitted last week. The patches in this series merely prepare the sequencer code for the next patch series that actually teaches the sequencer to run an interactive rebase. The reason to split these two patch series is simple: to keep them at a sensible size. The two patch series after that are much smaller: a two-patch "series" that switches rebase -i to use the sequencer (except with --root or --preserve-merges), and a couple of patches to move several pretty expensive script processing steps to C (think: autosquash). The end game of this patch series is a git-rebase--helper that makes rebase -i 5x faster on Windows (according to t/perf/p3404). Travis says that even MacOSX and Linux benefit (4x and 3x, respectively). I have been working on this since early February, whenever time allowed, and it is time to put it into the users' hands. To that end, I will most likely submit the remaining three patch series in the next two days, and integrate the whole shebang into Git for Windows 2.10.0. Therefore I would be most grateful for every in-depth review. Changes vs v2: - dramatically simplified the logic to retain do_recursive_merge()'s return value, even when positive (indicating merge conflicts). - ensured that write_message() rolls back locked files in case of errors. - added a patch to downcase all first letters of sequencer's error messages, as suggested by Junio. - replaced 25/25 ("sequencer: remove bogus hint for translators") by a fix to 8/25 ("completely revamp todo parsing"): the first %s is no longer "revert" or "cherry-pick", but a "todo" command. - backed out the sequencer_entrust() function and uglified the code to always duplicate the option values for the sake of being able to properly releasing them afterwards. - CR/LFs are now handled like LFs in todo scripts: they are stripped (thanks to Hannes Sixt for pointing that out). - unexported sequencer_commit() (it can safely remain a static function). While at it, renamed it back to run_git_commit() (the unspecific name does not hurt if it remains private to sequencer.c). - clarified in a code comment what read_author_script() does. - marked for translation the last remaining error message in sequencer.c that was not yet marked for translation. Johannes Schindelin (25): sequencer: use static initializers for replay_opts sequencer: use memoized sequencer directory path sequencer: avoid unnecessary indirection sequencer: future-proof remove_sequencer_state() sequencer: eventually release memory allocated for the option values sequencer: future-proof read_populate_todo() sequencer: completely revamp the "todo" script parsing sequencer: strip CR from the todo script sequencer: avoid completely different messages for different actions sequencer: get rid of the subcommand field sequencer: refactor the code to obtain a short commit name sequencer: remember the onelines when parsing the todo file sequencer: prepare for rebase -i's commit functionality sequencer: introduce a helper to read files written by scripts sequencer: allow editing the commit message on a case-by-case basis sequencer: support amending commits sequencer: support cleaning up commit messages sequencer: do not try to commit when there were merge conflicts sequencer: left-trim lines read from the script sequencer: refactor write_message() sequencer: remove overzealous assumption in rebase -i mode sequencer: mark action_name() for translation sequencer: quote filenames in error messages sequencer: start error messages consistently with lower case sequencer: mark all error messages for translation builtin/commit.c | 2 +- builtin/revert.c | 48 +-- sequencer.c | 690 -- sequencer.h | 23 +- t/t3501-revert-cherry-pick.sh | 2 +- 5 files changed, 503 insertions(+), 262 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v3 Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v3 Interdiff vs v2: diff --git a/builtin/revert.c b/builtin/revert.c index c9ae4dc..0a7b5f4 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -165,6 +165,12 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts) if (argc > 1) usage_with_options(usage_str, options); + /* These option values will be free()d */ + if (opts->gpg_sign) + opts->gpg_sign = xstrdup(opts->gpg_sign); + if (opts->strategy) + opts->strategy = xstrdup(opts->strategy); + if (cmd == 'q') return sequencer_remove_state(opts); if (cmd == 'c') diff --git a/sequencer.c b/sequencer.c index cdff0f1..86
[PATCH v3 02/25] sequencer: use memoized sequencer directory path
Signed-off-by: Johannes Schindelin --- builtin/commit.c | 2 +- sequencer.c | 11 ++- sequencer.h | 5 + 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 1cba3b7..9fddb19 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -183,7 +183,7 @@ static void determine_whence(struct wt_status *s) whence = FROM_MERGE; else if (file_exists(git_path_cherry_pick_head())) { whence = FROM_CHERRY_PICK; - if (file_exists(git_path(SEQ_DIR))) + if (file_exists(git_path_seq_dir())) sequencer_in_use = 1; } else diff --git a/sequencer.c b/sequencer.c index eec8a60..cb16cbd 100644 --- a/sequencer.c +++ b/sequencer.c @@ -21,10 +21,11 @@ const char sign_off_header[] = "Signed-off-by: "; static const char cherry_picked_prefix[] = "(cherry picked from commit "; -static GIT_PATH_FUNC(git_path_todo_file, SEQ_TODO_FILE) -static GIT_PATH_FUNC(git_path_opts_file, SEQ_OPTS_FILE) -static GIT_PATH_FUNC(git_path_seq_dir, SEQ_DIR) -static GIT_PATH_FUNC(git_path_head_file, SEQ_HEAD_FILE) +GIT_PATH_FUNC(git_path_seq_dir, "sequencer") + +static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo") +static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts") +static GIT_PATH_FUNC(git_path_head_file, "sequencer/head") static int is_rfc2822_line(const char *buf, int len) { @@ -112,7 +113,7 @@ static void remove_sequencer_state(void) { struct strbuf seq_dir = STRBUF_INIT; - strbuf_addstr(&seq_dir, git_path(SEQ_DIR)); + strbuf_addstr(&seq_dir, git_path_seq_dir()); remove_dir_recursively(&seq_dir, 0); strbuf_release(&seq_dir); } diff --git a/sequencer.h b/sequencer.h index db425ad..dd4d33a 100644 --- a/sequencer.h +++ b/sequencer.h @@ -1,10 +1,7 @@ #ifndef SEQUENCER_H #define SEQUENCER_H -#define SEQ_DIR"sequencer" -#define SEQ_HEAD_FILE "sequencer/head" -#define SEQ_TODO_FILE "sequencer/todo" -#define SEQ_OPTS_FILE "sequencer/opts" +const char *git_path_seq_dir(void); #define APPEND_SIGNOFF_DEDUP (1u << 0) -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 05/25] sequencer: eventually release memory allocated for the option values
The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves like a one-shot command when it reads its configuration: memory is allocated and released only when the command exits. This is kind of okay for git-cherry-pick, which *is* a one-shot command. All the work to make the sequencer its work horse was done to allow using the functionality as a library function, though, including proper clean-up after use. To remedy that, we now take custody of the option values in question, requiring those values to be malloc()ed or strdup()ed (an alternative approach, to add a list of pointers to malloc()ed data and to ask the sequencer to release all of them in the end, was proposed earlier but rejected during review). Sadly, the current approach makes the code uglier, as we now have to take care to strdup() the values passed via the command-line. Signed-off-by: Johannes Schindelin --- builtin/revert.c | 6 ++ sequencer.c | 32 ++-- sequencer.h | 6 +++--- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 7365559..fce9c75 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -174,6 +174,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) if (argc > 1) usage_with_options(usage_str, options); + + /* These option values will be free()d */ + if (opts->gpg_sign) + opts->gpg_sign = xstrdup(opts->gpg_sign); + if (opts->strategy) + opts->strategy = xstrdup(opts->strategy); } int cmd_revert(int argc, const char **argv, const char *prefix) diff --git a/sequencer.c b/sequencer.c index 8d272fb..22c31c8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -117,6 +117,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, static void remove_sequencer_state(const struct replay_opts *opts) { struct strbuf dir = STRBUF_INIT; + int i; + + free(opts->gpg_sign); + free(opts->strategy); + for (i = 0; i < opts->xopts_nr; i++) + free(opts->xopts[i]); + free(opts->xopts); strbuf_addf(&dir, "%s", get_dir(opts)); remove_dir_recursively(&dir, 0); @@ -280,7 +287,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next, struct merge_options o; struct tree *result, *next_tree, *base_tree, *head_tree; int clean; - const char **xopt; + char **xopt; static struct lock_file index_lock; hold_locked_index(&index_lock, 1); @@ -583,7 +590,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) commit_list_insert(base, &common); commit_list_insert(next, &remotes); - res |= try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts, + res |= try_merge_command(opts->strategy, +opts->xopts_nr, (const char **)opts->xopts, common, sha1_to_hex(head), remotes); free_commit_list(common); free_commit_list(remotes); @@ -802,10 +810,22 @@ static int populate_opts_cb(const char *key, const char *value, void *data) opts->allow_ff = git_config_bool_or_int(key, value, &error_flag); else if (!strcmp(key, "options.mainline")) opts->mainline = git_config_int(key, value); - else if (!strcmp(key, "options.strategy")) - git_config_string(&opts->strategy, key, value); - else if (!strcmp(key, "options.gpg-sign")) - git_config_string(&opts->gpg_sign, key, value); + else if (!strcmp(key, "options.strategy")) { + if (!value) + config_error_nonbool(key); + else { + free(opts->strategy); + opts->strategy = xstrdup(value); + } + } + else if (!strcmp(key, "options.gpg-sign")) { + if (!value) + config_error_nonbool(key); + else { + free(opts->gpg_sign); + opts->gpg_sign = xstrdup(value); + } + } else if (!strcmp(key, "options.strategy-option")) { ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc); opts->xopts[opts->xopts_nr++] = xstrdup(value); diff --git a/sequencer.h b/sequencer.h index dd4d33a..8453669 100644 --- a/sequencer.h +++ b/sequencer.h @@ -34,11 +34,11 @@ struct replay_opts { int mainline; - const char *gpg_sign; + char *gpg_sign; /* Merge strategy */ - const char *strategy; - const char **xopts; + char *strategy; + char **xopts; size_t xopts_nr, xopts_alloc; /* Only used by REPLAY_NONE */ -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 04/25] sequencer: future-proof remove_sequencer_state()
In a couple of commits, we will teach the sequencer to handle the nitty gritty of the interactive rebase, which keeps its state in a different directory. Signed-off-by: Johannes Schindelin --- sequencer.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/sequencer.c b/sequencer.c index c2fbf6f..8d272fb 100644 --- a/sequencer.c +++ b/sequencer.c @@ -27,6 +27,11 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo") static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts") static GIT_PATH_FUNC(git_path_head_file, "sequencer/head") +static const char *get_dir(const struct replay_opts *opts) +{ + return git_path_seq_dir(); +} + static int is_rfc2822_line(const char *buf, int len) { int i; @@ -109,13 +114,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, return 1; } -static void remove_sequencer_state(void) +static void remove_sequencer_state(const struct replay_opts *opts) { - struct strbuf seq_dir = STRBUF_INIT; + struct strbuf dir = STRBUF_INIT; - strbuf_addstr(&seq_dir, git_path_seq_dir()); - remove_dir_recursively(&seq_dir, 0); - strbuf_release(&seq_dir); + strbuf_addf(&dir, "%s", get_dir(opts)); + remove_dir_recursively(&dir, 0); + strbuf_release(&dir); } static const char *action_name(const struct replay_opts *opts) @@ -940,7 +945,7 @@ static int sequencer_rollback(struct replay_opts *opts) } if (reset_for_rollback(sha1)) goto fail; - remove_sequencer_state(); + remove_sequencer_state(opts); strbuf_release(&buf); return 0; fail: @@ -1034,7 +1039,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) * Sequence of picks finished successfully; cleanup by * removing the .git/sequencer directory */ - remove_sequencer_state(); + remove_sequencer_state(opts); return 0; } @@ -1095,7 +1100,7 @@ int sequencer_pick_revisions(struct replay_opts *opts) * one that is being continued */ if (opts->subcommand == REPLAY_REMOVE_STATE) { - remove_sequencer_state(); + remove_sequencer_state(opts); return 0; } if (opts->subcommand == REPLAY_ROLLBACK) -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 08/25] sequencer: strip CR from the todo script
It is not unheard of that editors on Windows write CR/LF even if the file originally had only LF. This is particularly awkward for exec lines of a rebase -i todo sheet. Take for example the insn "exec echo": The shell script parser splits at the LF and leaves the CR attached to "echo", which leads to the unknown command "echo\r". Work around that by stripping CR when reading the todo commands, as we already do for LF. This happens to fix t9903.14 and .15 in MSYS1 environments (with the rebase--helper patches based on this patch series): the todo script constructed in such a setup contains CR/LF thanks to MSYS1 runtime's cleverness. Based on a report and a patch by Johannes Sixt. Signed-off-by: Johannes Schindelin --- sequencer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sequencer.c b/sequencer.c index 678fdf3..cee7e50 100644 --- a/sequencer.c +++ b/sequencer.c @@ -774,6 +774,9 @@ static int parse_insn_buffer(char *buf, struct todo_list *todo_list) next_p = *eol ? eol + 1 /* skip LF */ : eol; + if (p != eol && eol[-1] == '\r') + eol--; /* skip Carriage Return */ + item = append_new_todo(todo_list); item->offset_in_buf = p - todo_list->buf.buf; if (parse_insn_line(item, p, eol)) { -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 06/25] sequencer: future-proof read_populate_todo()
Over the next commits, we will work on improving the sequencer to the point where it can process the todo script of an interactive rebase. To that end, we will need to teach the sequencer to read interactive rebase's todo file. In preparation, we consolidate all places where that todo file is needed to call a function that we will later extend. Signed-off-by: Johannes Schindelin --- sequencer.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/sequencer.c b/sequencer.c index 22c31c8..4e00c5e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -32,6 +32,11 @@ static const char *get_dir(const struct replay_opts *opts) return git_path_seq_dir(); } +static const char *get_todo_path(const struct replay_opts *opts) +{ + return git_path_todo_file(); +} + static int is_rfc2822_line(const char *buf, int len) { int i; @@ -769,25 +774,24 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list, static int read_populate_todo(struct commit_list **todo_list, struct replay_opts *opts) { + const char *todo_file = get_todo_path(opts); struct strbuf buf = STRBUF_INIT; int fd, res; - fd = open(git_path_todo_file(), O_RDONLY); + fd = open(todo_file, O_RDONLY); if (fd < 0) - return error_errno(_("Could not open %s"), - git_path_todo_file()); + return error_errno(_("Could not open %s"), todo_file); if (strbuf_read(&buf, fd, 0) < 0) { close(fd); strbuf_release(&buf); - return error(_("Could not read %s."), git_path_todo_file()); + return error(_("Could not read %s."), todo_file); } close(fd); res = parse_insn_buffer(buf.buf, todo_list, opts); strbuf_release(&buf); if (res) - return error(_("Unusable instruction sheet: %s"), - git_path_todo_file()); + return error(_("Unusable instruction sheet: %s"), todo_file); return 0; } @@ -1077,7 +1081,7 @@ static int sequencer_continue(struct replay_opts *opts) { struct commit_list *todo_list = NULL; - if (!file_exists(git_path_todo_file())) + if (!file_exists(get_todo_path(opts))) return continue_single_pick(); if (read_populate_opts(opts) || read_populate_todo(&todo_list, opts)) -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 07/25] sequencer: completely revamp the "todo" script parsing
When we came up with the "sequencer" idea, we really wanted to have kind of a plumbing equivalent of the interactive rebase. Hence the choice of words: the "todo" script, a "pick", etc. However, when it came time to implement the entire shebang, somehow this idea got lost and the sequencer was used as working horse for cherry-pick and revert instead. So as not to interfere with the interactive rebase, it even uses a separate directory to store its state. Furthermore, it also is stupidly strict about the "todo" script it accepts: while it parses commands in a way that was *designed* to be similar to the interactive rebase, it then goes on to *error out* if the commands disagree with the overall action (cherry-pick or revert). Finally, the sequencer code chose to deviate from the interactive rebase code insofar that when it comes to writing the file with the remaining commands, it *reformats* the "todo" script instead of just writing the part of the parsed script that were not yet processed. This is not only unnecessary churn, but might well lose information that is valuable to the user (i.e. comments after the commands). Let's just bite the bullet and rewrite the entire parser; the code now becomes not only more elegant: it allows us to go on and teach the sequencer how to parse *true* "todo" scripts as used by the interactive rebase itself. In a way, the sequencer is about to grow up to do its older brother's job. Better. In particular, we choose to maintain the list of commands in an array instead of a linked list: this is flexible enough to allow us later on to even implement rebase -i's reordering of fixup!/squash! commits very easily (and with a very nice speed bonus, at least on Windows). While at it, do not stop at the first problem, but list *all* of the problems. This will help the user when the sequencer will do `rebase -i`'s work by allowing to address all issues in one go rather than going back and forth until the todo list is valid. Signed-off-by: Johannes Schindelin --- sequencer.c | 288 +++- 1 file changed, 167 insertions(+), 121 deletions(-) diff --git a/sequencer.c b/sequencer.c index 4e00c5e..678fdf3 100644 --- a/sequencer.c +++ b/sequencer.c @@ -465,7 +465,26 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit) return 1; } -static int do_pick_commit(struct commit *commit, struct replay_opts *opts) +enum todo_command { + TODO_PICK = 0, + TODO_REVERT +}; + +static const char *todo_command_strings[] = { + "pick", + "revert" +}; + +static const char *command_to_string(const enum todo_command command) +{ + if (command < ARRAY_SIZE(todo_command_strings)) + return todo_command_strings[command]; + die("Unknown command: %d", command); +} + + +static int do_pick_commit(enum todo_command command, struct commit *commit, + struct replay_opts *opts) { unsigned char head[20]; struct commit *base, *next, *parent; @@ -524,10 +543,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) return fast_forward_to(commit->object.oid.hash, head, unborn, opts); if (parent && parse_commit(parent) < 0) - /* TRANSLATORS: The first %s will be "revert" or - "cherry-pick", the second %s a SHA1 */ + /* +* TRANSLATORS: The first %s will be a "todo" command like +* "revert" or "pick", the second %s a SHA1. +*/ return error(_("%s: cannot parse parent commit %s"), - action_name(opts), oid_to_hex(&parent->object.oid)); + command_to_string(command), + oid_to_hex(&parent->object.oid)); if (get_message(commit, &msg) != 0) return error(_("Cannot get commit message for %s"), @@ -540,7 +562,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) * reverse of it if we are revert. */ - if (opts->action == REPLAY_REVERT) { + if (command == TODO_REVERT) { base = commit; base_label = msg.label; next = parent; @@ -581,7 +603,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } } - if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) { + if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) { res = do_recursive_merge(base, next, base_label, next_label, head, &msgbuf, opts); if (res < 0) @@ -608,17 +630,17 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) * However, if the merge did not even start, then we don't want to * write it at all.
[PATCH v3 03/25] sequencer: avoid unnecessary indirection
We really do not need the *pointer to a* pointer to the options in the read_populate_opts() function. Signed-off-by: Johannes Schindelin --- sequencer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index cb16cbd..c2fbf6f 100644 --- a/sequencer.c +++ b/sequencer.c @@ -813,7 +813,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data) return 0; } -static int read_populate_opts(struct replay_opts **opts) +static int read_populate_opts(struct replay_opts *opts) { if (!file_exists(git_path_opts_file())) return 0; @@ -823,7 +823,7 @@ static int read_populate_opts(struct replay_opts **opts) * about this case, though, because we wrote that file ourselves, so we * are pretty certain that it is syntactically correct. */ - if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0) + if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0) return error(_("Malformed options sheet: %s"), git_path_opts_file()); return 0; @@ -1054,7 +1054,7 @@ static int sequencer_continue(struct replay_opts *opts) if (!file_exists(git_path_todo_file())) return continue_single_pick(); - if (read_populate_opts(&opts) || + if (read_populate_opts(opts) || read_populate_todo(&todo_list, opts)) return -1; -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 11/25] sequencer: refactor the code to obtain a short commit name
Not only does this DRY up the code (providing a better documentation what the code is about, as well as allowing to change the behavior in a single place), it also makes it substantially shorter to use the same functionality in functions to be introduced when we teach the sequencer to process interactive-rebase's git-rebase-todo file. Signed-off-by: Johannes Schindelin --- sequencer.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/sequencer.c b/sequencer.c index 14b1746..afc494e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -149,13 +149,18 @@ struct commit_message { const char *message; }; +static const char *short_commit_name(struct commit *commit) +{ + return find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV); +} + static int get_message(struct commit *commit, struct commit_message *out) { const char *abbrev, *subject; int subject_len; out->message = logmsg_reencode(commit, NULL, get_commit_output_encoding()); - abbrev = find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV); + abbrev = short_commit_name(commit); subject_len = find_commit_subject(out->message, &subject); @@ -642,8 +647,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, error(command == TODO_REVERT ? _("could not revert %s... %s") : _("could not apply %s... %s"), - find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), - msg.subject); + short_commit_name(commit), msg.subject); print_advice(res == 1, opts); rerere(opts->allow_rerere_auto); goto leave; @@ -910,9 +914,7 @@ static int walk_revs_populate_todo(struct todo_list *todo_list, item->offset_in_buf = todo_list->buf.len; subject_len = find_commit_subject(commit_buffer, &subject); strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string, - find_unique_abbrev(commit->object.oid.hash, - DEFAULT_ABBREV), - subject_len, subject); + short_commit_name(commit), subject_len, subject); unuse_commit_buffer(commit, commit_buffer); } return 0; -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 09/25] sequencer: avoid completely different messages for different actions
Signed-off-by: Johannes Schindelin --- sequencer.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index cee7e50..443a238 100644 --- a/sequencer.c +++ b/sequencer.c @@ -224,11 +224,8 @@ static int error_dirty_index(struct replay_opts *opts) if (read_cache_unmerged()) return error_resolve_conflict(action_name(opts)); - /* Different translation strings for cherry-pick and revert */ - if (opts->action == REPLAY_PICK) - error(_("Your local changes would be overwritten by cherry-pick.")); - else - error(_("Your local changes would be overwritten by revert.")); + error(_("Your local changes would be overwritten by %s."), + action_name(opts)); if (advice_commit_before_merge) advise(_("Commit your changes or stash them to proceed.")); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 19/25] sequencer: left-trim lines read from the script
Interactive rebase's scripts may be indented; we need to handle this case, too, now that we prepare the sequencer to process interactive rebases. Signed-off-by: Johannes Schindelin --- sequencer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sequencer.c b/sequencer.c index 23fe7db..45a3651 100644 --- a/sequencer.c +++ b/sequencer.c @@ -873,6 +873,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) char *end_of_object_name; int i, saved, status, padding; + /* left-trim */ + bol += strspn(bol, " \t"); + for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++) if (skip_prefix(bol, todo_command_strings[i], &bol)) { item->command = i; -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 13/25] sequencer: prepare for rebase -i's commit functionality
In interactive rebases, we commit a little bit differently than the sequencer did so far: we heed the "author-script", the "message" and the "amend" files in the .git/rebase-merge/ subdirectory. Likewise, we may want to edit the commit message *even* when providing a file containing the suggested commit message. Therefore we change the code to not even provide a default message when we do not want any, and to call the editor explicitly. Also, in "interactive rebase" mode we want to skip reading the options in the state directory of the cherry-pick/revert commands. Finally, as interactive rebase's GPG settings are configured differently from how cherry-pick (and therefore sequencer) handles them, we will leave support for that to the next commit. Signed-off-by: Johannes Schindelin --- sequencer.c | 99 ++--- 1 file changed, 89 insertions(+), 10 deletions(-) diff --git a/sequencer.c b/sequencer.c index 7ba5e07..b694ace 100644 --- a/sequencer.c +++ b/sequencer.c @@ -27,6 +27,19 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo") static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts") static GIT_PATH_FUNC(git_path_head_file, "sequencer/head") +/* + * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and + * GIT_AUTHOR_DATE that will be used for the commit that is currently + * being rebased. + */ +static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script") + +/* We will introduce the 'interactive rebase' mode later */ +static inline int is_rebase_i(const struct replay_opts *opts) +{ + return 0; +} + static const char *get_dir(const struct replay_opts *opts) { return git_path_seq_dir(); @@ -370,19 +383,79 @@ static int is_index_unchanged(void) } /* + * Read the author-script file into an environment block, ready for use in + * run_command(), that can be free()d afterwards. + */ +static char **read_author_script(void) +{ + struct strbuf script = STRBUF_INIT; + int i, count = 0; + char *p, *p2, **env; + size_t env_size; + + if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0) + return NULL; + + for (p = script.buf; *p; p++) + if (skip_prefix(p, "'''", (const char **)&p2)) + strbuf_splice(&script, p - script.buf, p2 - p, "'", 1); + else if (*p == '\'') + strbuf_splice(&script, p-- - script.buf, 1, "", 0); + else if (*p == '\n') { + *p = '\0'; + count++; + } + + env_size = (count + 1) * sizeof(*env); + strbuf_grow(&script, env_size); + memmove(script.buf + env_size, script.buf, script.len); + p = script.buf + env_size; + env = (char **)strbuf_detach(&script, NULL); + + for (i = 0; i < count; i++) { + env[i] = p; + p += strlen(p) + 1; + } + env[count] = NULL; + + return env; +} + +/* * If we are cherry-pick, and if the merge did not result in * hand-editing, we will hit this commit and inherit the original * author date and name. + * * If we are revert, or if our cherry-pick results in a hand merge, * we had better say that the current user is responsible for that. + * + * An exception is when run_git_commit() is called during an + * interactive rebase: in that case, we will want to retain the + * author metadata. */ static int run_git_commit(const char *defmsg, struct replay_opts *opts, int allow_empty) { + char **env = NULL; struct argv_array array; int rc; const char *value; + if (is_rebase_i(opts)) { + env = read_author_script(); + if (!env) + return error("You have staged changes in your working " + "tree. If these changes are meant to be\n" + "squashed into the previous commit, run:\n\n" + " git commit --amend $gpg_sign_opt_quoted\n\n" + "If they are meant to go into a new commit, " + "run:\n\n" + " git commit $gpg_sign_opt_quoted\n\n" + "In both cases, once you're done, continue " + "with:\n\n" + " git rebase --continue\n"); + } + argv_array_init(&array); argv_array_push(&array, "commit"); argv_array_push(&array, "-n"); @@ -391,14 +464,13 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_pushf(&array, "-S%s", opts->gpg_sign); if (opts->signoff) argv_array_push(&array, "-s"); - if (!opts->edit) { - argv_array_push(&array, "-F"); - argv_array_push(&array, defmsg); -
[PATCH v3 20/25] sequencer: refactor write_message()
The write_message() function safely writes an strbuf to a file. Sometimes it is inconvenient to require an strbuf, though: the text to be written may not be stored in a strbuf, or the strbuf should not be released after writing. Let's refactor "safely writing string to a file" into write_with_lock_file(), and make write_message() use it. The new function makes it easy to create new convenience function write_file_gently(); as some of the upcoming callers of this new function would want to append a newline character, add a flag for it in write_file_gently(), and thus in write_with_lock_file(). While at it, roll back the locked files in case of failure, as pointed out by Hannes Sixt. Signed-off-by: Johannes Schindelin --- sequencer.c | 31 ++- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index 45a3651..5cca8d8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -234,22 +234,43 @@ static void print_advice(int show_hint, struct replay_opts *opts) } } -static int write_message(struct strbuf *msgbuf, const char *filename) +static int write_with_lock_file(const char *filename, + const void *buf, size_t len, int append_eol) { static struct lock_file msg_file; int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0); if (msg_fd < 0) return error_errno(_("Could not lock '%s'"), filename); - if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0) - return error_errno(_("Could not write to %s"), filename); - strbuf_release(msgbuf); - if (commit_lock_file(&msg_file) < 0) + if (write_in_full(msg_fd, buf, len) < 0) { + rollback_lock_file(&msg_file); + return error_errno(_("Could not write to '%s'"), filename); + } + if (append_eol && write(msg_fd, "\n", 1) < 0) { + rollback_lock_file(&msg_file); + return error_errno(_("Could not write eol to '%s"), filename); + } + if (commit_lock_file(&msg_file) < 0) { + rollback_lock_file(&msg_file); return error(_("Error wrapping up %s."), filename); + } return 0; } +static int write_message(struct strbuf *msgbuf, const char *filename) +{ + int res = write_with_lock_file(filename, msgbuf->buf, msgbuf->len, 0); + strbuf_release(msgbuf); + return res; +} + +static int write_file_gently(const char *filename, +const char *text, int append_eol) +{ + return write_with_lock_file(filename, text, strlen(text), append_eol); +} + /* * Reads a file that was presumably written by a shell script, i.e. * with an end-of-line marker that needs to be stripped. -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 16/25] sequencer: support amending commits
This teaches the run_git_commit() function to take an argument that will allow us to implement "todo" commands that need to amend the commit messages ("fixup", "squash" and "reword"). Signed-off-by: Johannes Schindelin --- sequencer.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index b621f4b..403a4f0 100644 --- a/sequencer.c +++ b/sequencer.c @@ -481,7 +481,7 @@ static char **read_author_script(void) * author metadata. */ static int run_git_commit(const char *defmsg, struct replay_opts *opts, - int allow_empty, int edit) + int allow_empty, int edit, int amend) { char **env = NULL; struct argv_array array; @@ -510,6 +510,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_push(&array, "commit"); argv_array_push(&array, "-n"); + if (amend) + argv_array_push(&array, "--amend"); if (opts->gpg_sign) argv_array_pushf(&array, "-S%s", opts->gpg_sign); if (opts->signoff) @@ -785,7 +787,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } if (!opts->no_commit) res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(), -opts, allow, opts->edit); +opts, allow, opts->edit, 0); leave: free_message(commit, &msg); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 17/25] sequencer: support cleaning up commit messages
The run_git_commit() function already knows how to amend commits, and with this new option, it can also clean up commit messages (i.e. strip out commented lines). This is needed to implement rebase -i's 'fixup' and 'squash' commands as sequencer commands. Signed-off-by: Johannes Schindelin --- sequencer.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index 403a4f0..108bca8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -481,7 +481,8 @@ static char **read_author_script(void) * author metadata. */ static int run_git_commit(const char *defmsg, struct replay_opts *opts, - int allow_empty, int edit, int amend) + int allow_empty, int edit, int amend, + int cleanup_commit_message) { char **env = NULL; struct argv_array array; @@ -518,9 +519,12 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_push(&array, "-s"); if (defmsg) argv_array_pushl(&array, "-F", defmsg, NULL); + if (cleanup_commit_message) + argv_array_push(&array, "--cleanup=strip"); if (edit) argv_array_push(&array, "-e"); - else if (!opts->signoff && !opts->record_origin && + else if (!cleanup_commit_message && +!opts->signoff && !opts->record_origin && git_config_get_value("commit.cleanup", &value)) argv_array_push(&array, "--cleanup=verbatim"); @@ -787,7 +791,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } if (!opts->no_commit) res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(), -opts, allow, opts->edit, 0); +opts, allow, opts->edit, 0, 0); leave: free_message(commit, &msg); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 15/25] sequencer: allow editing the commit message on a case-by-case basis
In the upcoming commits, we will implement more and more of rebase -i's functionality inside the sequencer. One particular feature of the commands to come is that some of them allow editing the commit message while others don't, i.e. we cannot define in the replay_opts whether the commit message should be edited or not. Let's add a new parameter to the run_git_commit() function. Previously, it was the duty of the caller to ensure that the opts->edit setting indicates whether to let the user edit the commit message or not, indicating that it is an "all or nothing" setting, i.e. that the sequencer wants to let the user edit *all* commit message, or none at all. In the upcoming rebase -i mode, it will depend on the particular command that is currently executed, though. Signed-off-by: Johannes Schindelin --- sequencer.c | 48 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/sequencer.c b/sequencer.c index 681552a..b621f4b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -15,6 +15,7 @@ #include "merge-recursive.h" #include "refs.h" #include "argv-array.h" +#include "quote.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -33,6 +34,11 @@ static GIT_PATH_FUNC(git_path_head_file, "sequencer/head") * being rebased. */ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script") +/* + * The following files are written by git-rebase just after parsing the + * command-line (and are only consumed, not modified, by the sequencer). + */ +static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") /* We will introduce the 'interactive rebase' mode later */ static inline int is_rebase_i(const struct replay_opts *opts) @@ -132,6 +138,16 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, return 1; } +static const char *gpg_sign_opt_quoted(struct replay_opts *opts) +{ + static struct strbuf buf = STRBUF_INIT; + + strbuf_reset(&buf); + if (opts->gpg_sign) + sq_quotef(&buf, "-S%s", opts->gpg_sign); + return buf.buf; +} + int sequencer_remove_state(struct replay_opts *opts) { struct strbuf dir = STRBUF_INIT; @@ -465,7 +481,7 @@ static char **read_author_script(void) * author metadata. */ static int run_git_commit(const char *defmsg, struct replay_opts *opts, - int allow_empty) + int allow_empty, int edit) { char **env = NULL; struct argv_array array; @@ -474,17 +490,20 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, if (is_rebase_i(opts)) { env = read_author_script(); - if (!env) + if (!env) { + const char *gpg_opt = gpg_sign_opt_quoted(opts); + return error("You have staged changes in your working " "tree. If these changes are meant to be\n" "squashed into the previous commit, run:\n\n" - " git commit --amend $gpg_sign_opt_quoted\n\n" + " git commit --amend %s\n\n" "If they are meant to go into a new commit, " "run:\n\n" - " git commit $gpg_sign_opt_quoted\n\n" + " git commit %s\n\n" "In both cases, once you're done, continue " "with:\n\n" - " git rebase --continue\n"); + " git rebase --continue\n", gpg_opt, gpg_opt); + } } argv_array_init(&array); @@ -497,7 +516,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_push(&array, "-s"); if (defmsg) argv_array_pushl(&array, "-F", defmsg, NULL); - if (opts->edit) + if (edit) argv_array_push(&array, "-e"); else if (!opts->signoff && !opts->record_origin && git_config_get_value("commit.cleanup", &value)) @@ -766,7 +785,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } if (!opts->no_commit) res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(), -opts, allow); +opts, allow, opts->edit); leave: free_message(commit, &msg); @@ -990,8 +1009,21 @@ static int populate_opts_cb(const char *key, const char *value, void *data) static int read_populate_opts(struct replay_opts *opts) { - if (is_rebase_i(opts)) + if (is_rebase_i(opts)) { + struct strbuf buf = STRBUF_INIT; + + if (read_oneliner(&buf, rebase_path_gpg_sign_opt(), 1)) { + if (!starts_with(buf.buf,
[PATCH v3 14/25] sequencer: introduce a helper to read files written by scripts
As we are slowly teaching the sequencer to perform the hard work for the interactive rebase, we need to read files that were written by shell scripts. These files typically contain a single line and are invariably ended by a line feed (and possibly a carriage return before that). Let's use a helper to read such files and to remove the line ending. Signed-off-by: Johannes Schindelin --- sequencer.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/sequencer.c b/sequencer.c index b694ace..681552a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -234,6 +234,37 @@ static int write_message(struct strbuf *msgbuf, const char *filename) return 0; } +/* + * Reads a file that was presumably written by a shell script, i.e. + * with an end-of-line marker that needs to be stripped. + * + * Returns 1 if the file was read, 0 if it could not be read or does not exist. + */ +static int read_oneliner(struct strbuf *buf, + const char *path, int skip_if_empty) +{ + int orig_len = buf->len; + + if (!file_exists(path)) + return 0; + + if (strbuf_read_file(buf, path, 0) < 0) { + warning_errno(_("could not read '%s'"), path); + return 0; + } + + if (buf->len > orig_len && buf->buf[buf->len - 1] == '\n') { + if (--buf->len > orig_len && buf->buf[buf->len - 1] == '\r') + --buf->len; + buf->buf[buf->len] = '\0'; + } + + if (skip_if_empty && buf->len == orig_len) + return 0; + + return 1; +} + static struct tree *empty_tree(void) { return lookup_tree(EMPTY_TREE_SHA1_BIN); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 18/25] sequencer: do not try to commit when there were merge conflicts
The return value of do_recursive_merge() may be positive (indicating merge conflicts), or 0 (indicating success). It also may be negative, indicating a fatal error that requires us to abort. Now, if the return value indicates that there are merge conflicts, we should not try to commit those changes, of course. Signed-off-by: Johannes Schindelin --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 108bca8..23fe7db 100644 --- a/sequencer.c +++ b/sequencer.c @@ -789,7 +789,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, res = allow; goto leave; } - if (!opts->no_commit) + if (!res && !opts->no_commit) res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(), opts, allow, opts->edit, 0, 0); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 10/25] sequencer: get rid of the subcommand field
The subcommands are used exactly once, at the very beginning of sequencer_pick_revisions(), to determine what to do. This is an unnecessary level of indirection: we can simply call the correct function to begin with. So let's do that. While at it, ensure that the subcommands return an error code so that they do not have to die() all over the place (bad practice for library functions...). Signed-off-by: Johannes Schindelin --- builtin/revert.c | 36 sequencer.c | 35 +++ sequencer.h | 13 - 3 files changed, 31 insertions(+), 53 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index fce9c75..0a7b5f4 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...) die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt); } -static void parse_args(int argc, const char **argv, struct replay_opts *opts) +static int run_sequencer(int argc, const char **argv, struct replay_opts *opts) { const char * const * usage_str = revert_or_cherry_pick_usage(opts); const char *me = action_name(opts); @@ -115,25 +115,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) if (opts->keep_redundant_commits) opts->allow_empty = 1; - /* Set the subcommand */ - if (cmd == 'q') - opts->subcommand = REPLAY_REMOVE_STATE; - else if (cmd == 'c') - opts->subcommand = REPLAY_CONTINUE; - else if (cmd == 'a') - opts->subcommand = REPLAY_ROLLBACK; - else - opts->subcommand = REPLAY_NONE; - /* Check for incompatible command line arguments */ - if (opts->subcommand != REPLAY_NONE) { + if (cmd) { char *this_operation; - if (opts->subcommand == REPLAY_REMOVE_STATE) + if (cmd == 'q') this_operation = "--quit"; - else if (opts->subcommand == REPLAY_CONTINUE) + else if (cmd == 'c') this_operation = "--continue"; else { - assert(opts->subcommand == REPLAY_ROLLBACK); + assert(cmd == 'a'); this_operation = "--abort"; } @@ -156,7 +146,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) "--edit", opts->edit, NULL); - if (opts->subcommand != REPLAY_NONE) { + if (cmd) { opts->revs = NULL; } else { struct setup_revision_opt s_r_opt; @@ -180,6 +170,14 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) opts->gpg_sign = xstrdup(opts->gpg_sign); if (opts->strategy) opts->strategy = xstrdup(opts->strategy); + + if (cmd == 'q') + return sequencer_remove_state(opts); + if (cmd == 'c') + return sequencer_continue(opts); + if (cmd == 'a') + return sequencer_rollback(opts); + return sequencer_pick_revisions(opts); } int cmd_revert(int argc, const char **argv, const char *prefix) @@ -191,8 +189,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix) opts.edit = 1; opts.action = REPLAY_REVERT; git_config(git_default_config, NULL); - parse_args(argc, argv, &opts); - res = sequencer_pick_revisions(&opts); + res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("revert failed")); return res; @@ -205,8 +202,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix) opts.action = REPLAY_PICK; git_config(git_default_config, NULL); - parse_args(argc, argv, &opts); - res = sequencer_pick_revisions(&opts); + res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("cherry-pick failed")); return res; diff --git a/sequencer.c b/sequencer.c index 443a238..14b1746 100644 --- a/sequencer.c +++ b/sequencer.c @@ -119,7 +119,7 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, return 1; } -static void remove_sequencer_state(const struct replay_opts *opts) +int sequencer_remove_state(struct replay_opts *opts) { struct strbuf dir = STRBUF_INIT; int i; @@ -133,6 +133,8 @@ static void remove_sequencer_state(const struct replay_opts *opts) strbuf_addf(&dir, "%s", get_dir(opts)); remove_dir_recursively(&dir, 0); strbuf_release(&dir); + + return 0; } static const char *action_name(const struct replay_opts *opts) @@ -977,7 +979,7 @@ static int rollback_single_pick(void) return reset_for_rollback(head_sha1); } -static in
[PATCH v3 12/25] sequencer: remember the onelines when parsing the todo file
The `git-rebase-todo` file contains a list of commands. Most of those commands have the form The is displayed primarily for the user's convenience, as rebase -i really interprets only the part. However, there are *some* places in interactive rebase where the is used to display messages, e.g. for reporting at which commit we stopped. So let's just remember it when parsing the todo file; we keep a copy of the entire todo file anyway (to write out the new `done` and `git-rebase-todo` file just before processing each command), so all we need to do is remember the begin offsets and lengths. As we will have to parse and remember the command-line for `exec` commands later, we do not call the field "oneline" but rather "arg" (and will reuse that for exec's command-line). Signed-off-by: Johannes Schindelin --- sequencer.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/sequencer.c b/sequencer.c index afc494e..7ba5e07 100644 --- a/sequencer.c +++ b/sequencer.c @@ -708,6 +708,8 @@ static int read_and_refresh_cache(struct replay_opts *opts) struct todo_item { enum todo_command command; struct commit *commit; + const char *arg; + int arg_len; size_t offset_in_buf; }; @@ -759,6 +761,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) status = get_sha1(bol, commit_sha1); *end_of_object_name = saved; + item->arg = end_of_object_name + strspn(end_of_object_name, " \t"); + item->arg_len = (int)(eol - item->arg); + if (status < 0) return -1; @@ -911,6 +916,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list, item->command = command; item->commit = commit; + item->arg = NULL; + item->arg_len = 0; item->offset_in_buf = todo_list->buf.len; subject_len = find_commit_subject(commit_buffer, &subject); strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string, -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 24/25] sequencer: start error messages consistently with lower case
Quite a few error messages touched by this developer during the work to speed up rebase -i started with an upper case letter, violating our current conventions. Instead of sneaking in this fix (and forgetting quite a few error messages), let's just have one wholesale patch fixing all of the error messages in the sequencer. While at it, the funny "error: Error wrapping up..." was changed to a less funny, but more helpful, "error: failed to finalize...". Pointed out by Junio Hamano. Signed-off-by: Johannes Schindelin --- sequencer.c | 68 +-- t/t3501-revert-cherry-pick.sh | 2 +- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/sequencer.c b/sequencer.c index 4596540..676f16c 100644 --- a/sequencer.c +++ b/sequencer.c @@ -241,18 +241,18 @@ static int write_with_lock_file(const char *filename, int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0); if (msg_fd < 0) - return error_errno(_("Could not lock '%s'"), filename); + return error_errno(_("could not lock '%s'"), filename); if (write_in_full(msg_fd, buf, len) < 0) { rollback_lock_file(&msg_file); - return error_errno(_("Could not write to '%s'"), filename); + return error_errno(_("could not write to '%s'"), filename); } if (append_eol && write(msg_fd, "\n", 1) < 0) { rollback_lock_file(&msg_file); - return error_errno(_("Could not write eol to '%s"), filename); + return error_errno(_("could not write eol to '%s"), filename); } if (commit_lock_file(&msg_file) < 0) { rollback_lock_file(&msg_file); - return error(_("Error wrapping up '%s'."), filename); + return error(_("failed to finalize '%s'."), filename); } return 0; @@ -312,11 +312,11 @@ static int error_dirty_index(struct replay_opts *opts) if (read_cache_unmerged()) return error_resolve_conflict(_(action_name(opts))); - error(_("Your local changes would be overwritten by %s."), + error(_("your local changes would be overwritten by %s."), _(action_name(opts))); if (advice_commit_before_merge) - advise(_("Commit your changes or stash them to proceed.")); + advise(_("commit your changes or stash them to proceed.")); return -1; } @@ -425,7 +425,7 @@ static int is_index_unchanged(void) struct commit *head_commit; if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL)) - return error(_("Could not resolve HEAD commit\n")); + return error(_("could not resolve HEAD commit\n")); head_commit = lookup_commit(head_sha1); @@ -445,7 +445,7 @@ static int is_index_unchanged(void) if (!cache_tree_fully_valid(active_cache_tree)) if (cache_tree_update(&the_index, 0)) - return error(_("Unable to update cache tree\n")); + return error(_("unable to update cache tree\n")); return !hashcmp(active_cache_tree->sha1, head_commit->tree->object.oid.hash); } @@ -515,7 +515,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, if (!env) { const char *gpg_opt = gpg_sign_opt_quoted(opts); - return error("You have staged changes in your working " + return error("you have staged changes in your working " "tree. If these changes are meant to be\n" "squashed into the previous commit, run:\n\n" " git commit --amend %s\n\n" @@ -568,12 +568,12 @@ static int is_original_commit_empty(struct commit *commit) const unsigned char *ptree_sha1; if (parse_commit(commit)) - return error(_("Could not parse commit %s\n"), + return error(_("could not parse commit %s\n"), oid_to_hex(&commit->object.oid)); if (commit->parents) { struct commit *parent = commit->parents->item; if (parse_commit(parent)) - return error(_("Could not parse parent commit %s\n"), + return error(_("could not parse parent commit %s\n"), oid_to_hex(&parent->object.oid)); ptree_sha1 = parent->tree->object.oid.hash; } else { @@ -657,7 +657,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, * to work on. */ if (write_cache_as_tree(head, 0, NULL)) - return error(_("Your index file is unmerged.")); + return error(_("your index file is unmerged.")); } else {
[PATCH v3 23/25] sequencer: quote filenames in error messages
This makes the code consistent by fixing quite a couple of error messages. Suggested by Jakub Narębski. Signed-off-by: Johannes Schindelin --- sequencer.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sequencer.c b/sequencer.c index 40ef33c..4596540 100644 --- a/sequencer.c +++ b/sequencer.c @@ -252,7 +252,7 @@ static int write_with_lock_file(const char *filename, } if (commit_lock_file(&msg_file) < 0) { rollback_lock_file(&msg_file); - return error(_("Error wrapping up %s."), filename); + return error(_("Error wrapping up '%s'."), filename); } return 0; @@ -963,16 +963,16 @@ static int read_populate_todo(struct todo_list *todo_list, strbuf_reset(&todo_list->buf); fd = open(todo_file, O_RDONLY); if (fd < 0) - return error_errno(_("Could not open %s"), todo_file); + return error_errno(_("Could not open '%s'"), todo_file); if (strbuf_read(&todo_list->buf, fd, 0) < 0) { close(fd); - return error(_("Could not read %s."), todo_file); + return error(_("Could not read '%s'."), todo_file); } close(fd); res = parse_insn_buffer(todo_list->buf.buf, todo_list); if (res) - return error(_("Unusable instruction sheet: %s"), todo_file); + return error(_("Unusable instruction sheet: '%s'"), todo_file); if (!is_rebase_i(opts)) { enum todo_command valid = @@ -1065,7 +1065,7 @@ static int read_populate_opts(struct replay_opts *opts) * are pretty certain that it is syntactically correct. */ if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0) - return error(_("Malformed options sheet: %s"), + return error(_("Malformed options sheet: '%s'"), git_path_opts_file()); return 0; } @@ -1108,7 +1108,7 @@ static int create_seq_dir(void) return -1; } else if (mkdir(git_path_seq_dir(), 0777) < 0) - return error_errno(_("Could not create sequencer directory %s"), + return error_errno(_("Could not create sequencer directory '%s'"), git_path_seq_dir()); return 0; } @@ -1127,12 +1127,12 @@ static int save_head(const char *head) strbuf_addf(&buf, "%s\n", head); if (write_in_full(fd, buf.buf, buf.len) < 0) { rollback_lock_file(&head_lock); - return error_errno(_("Could not write to %s"), + return error_errno(_("Could not write to '%s'"), git_path_head_file()); } if (commit_lock_file(&head_lock) < 0) { rollback_lock_file(&head_lock); - return error(_("Error wrapping up %s."), git_path_head_file()); + return error(_("Error wrapping up '%s'."), git_path_head_file()); } return 0; } @@ -1177,9 +1177,9 @@ int sequencer_rollback(struct replay_opts *opts) return rollback_single_pick(); } if (!f) - return error_errno(_("cannot open %s"), git_path_head_file()); + return error_errno(_("cannot open '%s'"), git_path_head_file()); if (strbuf_getline_lf(&buf, f)) { - error(_("cannot read %s: %s"), git_path_head_file(), + error(_("cannot read '%s': %s"), git_path_head_file(), ferror(f) ? strerror(errno) : _("unexpected end of file")); fclose(f); goto fail; @@ -1218,7 +1218,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) todo_list->buf.len - offset) < 0) return error_errno(_("Could not write to '%s'"), todo_path); if (commit_lock_file(&todo_lock) < 0) - return error(_("Error wrapping up %s."), todo_path); + return error(_("Error wrapping up '%s'."), todo_path); return 0; } -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 25/25] sequencer: mark all error messages for translation
There was actually only one error message that was not yet marked for translation. Signed-off-by: Johannes Schindelin --- sequencer.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/sequencer.c b/sequencer.c index 676f16c..86d86ce 100644 --- a/sequencer.c +++ b/sequencer.c @@ -515,16 +515,19 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, if (!env) { const char *gpg_opt = gpg_sign_opt_quoted(opts); - return error("you have staged changes in your working " - "tree. If these changes are meant to be\n" - "squashed into the previous commit, run:\n\n" - " git commit --amend %s\n\n" - "If they are meant to go into a new commit, " - "run:\n\n" - " git commit %s\n\n" - "In both cases, once you're done, continue " - "with:\n\n" - " git rebase --continue\n", gpg_opt, gpg_opt); + return error(_("you have staged changes in your " + "working tree. If these changes are " + "meant to be\n" + "squashed into the previous commit, " + "run:\n\n" + " git commit --amend %s\n\n" + "If they are meant to go into a new " + "commit, run:\n\n" + " git commit %s\n\n" + "In both cases, once you're done, " + "continue with:\n\n" + " git rebase --continue\n"), +gpg_opt, gpg_opt); } } -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 22/25] sequencer: mark action_name() for translation
The definition of this function goes back all the way to 043a449 (sequencer: factor code out of revert builtin, 2012-01-11), long before a serious effort was made to translate all the error messages. It is slightly out of the context of the current patch series (whose purpose it is to re-implement the performance critical parts of the interactive rebase in C) to make the error messages in the sequencer translatable, but what the heck. We'll just do it while we're looking at this part of the code. Signed-off-by: Johannes Schindelin --- sequencer.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sequencer.c b/sequencer.c index ffce095..40ef33c 100644 --- a/sequencer.c +++ b/sequencer.c @@ -168,7 +168,7 @@ int sequencer_remove_state(struct replay_opts *opts) static const char *action_name(const struct replay_opts *opts) { - return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick"; + return opts->action == REPLAY_REVERT ? N_("revert") : N_("cherry-pick"); } struct commit_message { @@ -310,10 +310,10 @@ static struct tree *empty_tree(void) static int error_dirty_index(struct replay_opts *opts) { if (read_cache_unmerged()) - return error_resolve_conflict(action_name(opts)); + return error_resolve_conflict(_(action_name(opts))); error(_("Your local changes would be overwritten by %s."), - action_name(opts)); + _(action_name(opts))); if (advice_commit_before_merge) advise(_("Commit your changes or stash them to proceed.")); @@ -331,7 +331,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from, if (checkout_fast_forward(from, to, 1)) return -1; /* the callee should have complained already */ - strbuf_addf(&sb, _("%s: fast-forward"), action_name(opts)); + strbuf_addf(&sb, _("%s: fast-forward"), _(action_name(opts))); transaction = ref_transaction_begin(&err); if (!transaction || @@ -407,7 +407,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next, write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) /* TRANSLATORS: %s will be "revert" or "cherry-pick" */ return error(_("%s: Unable to write new index file"), - action_name(opts)); + _(action_name(opts))); rollback_lock_file(&index_lock); if (opts->signoff) @@ -844,14 +844,14 @@ static int read_and_refresh_cache(struct replay_opts *opts) if (read_index_preload(&the_index, NULL) < 0) { rollback_lock_file(&index_lock); return error(_("git %s: failed to read the index"), - action_name(opts)); + _(action_name(opts))); } refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL); if (the_index.cache_changed && index_fd >= 0) { if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) { rollback_lock_file(&index_lock); return error(_("git %s: failed to refresh the index"), - action_name(opts)); + _(action_name(opts))); } } rollback_lock_file(&index_lock); -- 2.10.0.windows.1.325.ge6089c1
[PATCH v3 21/25] sequencer: remove overzealous assumption in rebase -i mode
The sequencer was introduced to make the cherry-pick and revert functionality available as library function, with the original idea being to extend the sequencer to also implement the rebase -i functionality. The test to ensure that all of the commands in the script are identical to the overall operation does not mesh well with that. Therefore let's disable the test in rebase -i mode. While at it, error out early if the "instruction sheet" (i.e. the todo script) could not be parsed. Signed-off-by: Johannes Schindelin --- sequencer.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index 5cca8d8..ffce095 100644 --- a/sequencer.c +++ b/sequencer.c @@ -971,7 +971,10 @@ static int read_populate_todo(struct todo_list *todo_list, close(fd); res = parse_insn_buffer(todo_list->buf.buf, todo_list); - if (!res) { + if (res) + return error(_("Unusable instruction sheet: %s"), todo_file); + + if (!is_rebase_i(opts)) { enum todo_command valid = opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT; int i; @@ -985,8 +988,6 @@ static int read_populate_todo(struct todo_list *todo_list, return error(_("Cannot revert during a cherry-pick.")); } - if (res) - return error(_("Unusable instruction sheet: %s"), todo_file); return 0; } -- 2.10.0.windows.1.325.ge6089c1
Re: git 2.10.1 test regression in t3700-add.sh
Jeremy Huddleston Sequoia writes: > Actually, looks like that as just a rabbit hole. The real issue > looks to be because an earlier test drops down xfoo3 as a symlink, > which causes this test to fail due to the collision. I'll get out > a patch in a bit. [administrivia: please don't top-post, it is extremely hard to follow what is going on] There indeed is a test that creates xfoo3 as a symbolic link and leaves it in the filesystem pointing at xfoo2 much earlier in the sequence. It seems that later one of the "add ignore-errors" tests (there are two back-to-back) runs "git reset --hard" to make it (and other symbolic links that are similarly named) go away, namely this part of the code: test_expect_success POSIXPERM,SANITY 'git add --ignore-errors' ' git reset --hard && date >foo1 && date >foo2 && chmod 0 foo2 && test_must_fail git add --verbose --ignore-errors . && git ls-files foo1 | grep foo1 ' rm -f foo2 test_expect_success POSIXPERM,SANITY 'git add (add.ignore-errors)' ' git config add.ignore-errors 1 && git reset --hard && date >foo1 && date >foo2 && chmod 0 foo2 && test_must_fail git add --verbose . && git ls-files foo1 | grep foo1 ' rm -f foo2 "git reset --hard" in the first one, because these symbolic links are not in the index at that point in the sequence, would leave them untracked and in the working tree. Then "add" is told to be non-atomic with "--ignore-errors", adding them to the index while reporting a failure. When the test after that runs "git reset --hard" again, because these symlinks are in the index (and not in HEAD), they are removed from the working tree. And that is why normal people won't see xfoo3 in later tests, like the one you had trouble with. Are you running without SANITY by any chance (I am not saying that you are doing a wrong thing--just trying to make sure I am guessing along the correct route)? If that is the issue, then I think the right correction would be to make sure that the files that an individual test expects to be missing from the file system is indeed missing by explicitly removing it, perhaps like this. I also notice that the problematic test uses "chmod 755"; don't we need POSIXPERM prerequisite on this test, too, I wonder? Thanks. -- >8 -- t3700: fix broken test under !SANITY An "add --chmod=+x" test recently added by 610d55af0f ("add: modify already added files when --chmod is given", 2016-09-14) used "xfoo3" as a test file. The paths xfoo[1-3] were used by earlier tests for symbolic links but they were expected to have been removed by the test script reached this new test. The removal with "git reset --hard" however happened in tests that are protected by POSIXPERM,SANITY prerequisites. Platforms and test environments that lacked these would have seen xfoo3 as a leftover symbolic link, pointing somewhere else, and chmod test would have given a wrong result. t/t3700-add.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 924a266126..53c0cb6dea 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -350,6 +350,7 @@ test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' ' ' test_expect_success 'git add --chmod=[+-]x changes index with already added file' ' + rm -f foo3 xfoo3 && echo foo >foo3 && git add foo3 && git add --chmod=+x foo3 &&
Re: [PATCH] pretty: respect color settings for %C placeholders
On Mon, Oct 10, 2016 at 07:09:12PM +0200, René Scharfe wrote: > > diff --git a/pretty.c b/pretty.c > > index 25efbca..73e58b5 100644 > > --- a/pretty.c > > +++ b/pretty.c > > @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in > > UTF-8 */ > > > > if (!end) > > return 0; > > - if (skip_prefix(begin, "auto,", &begin)) { > > + > > + if (!skip_prefix(begin, "always,", &begin)) { > > if (!want_color(c->pretty_ctx->color)) > > return end - placeholder + 1; > > } > > Shouldn't we have an "else" here? I'm not sure what you mean; can you write it out? > > - if (skip_prefix(placeholder + 1, "red", &rest)) > > + if (skip_prefix(placeholder + 1, "red", &rest) && > > + want_color(c->pretty_ctx->color)) > > strbuf_addstr(sb, GIT_COLOR_RED); > > - else if (skip_prefix(placeholder + 1, "green", &rest)) > > + else if (skip_prefix(placeholder + 1, "green", &rest) && > > +want_color(c->pretty_ctx->color)) > > strbuf_addstr(sb, GIT_COLOR_GREEN); > > - else if (skip_prefix(placeholder + 1, "blue", &rest)) > > + else if (skip_prefix(placeholder + 1, "blue", &rest) && > > +want_color(c->pretty_ctx->color)) > > strbuf_addstr(sb, GIT_COLOR_BLUE); > > - else if (skip_prefix(placeholder + 1, "reset", &rest)) > > + else if (skip_prefix(placeholder + 1, "reset", &rest) && > > +want_color(c->pretty_ctx->color)) > > strbuf_addstr(sb, GIT_COLOR_RESET); > > return rest - placeholder; > > } > > Perhaps it's a funtion like add_color(sb, ctx, color) or similar would be > nice? I actually wrote it that way first (I called it "maybe_add_color()"), but it felt a little funny to have a separate function that people might be tempted to reuse (the right solution is generally to check want_color() early as above, but we can't do that here because we have to find the end of each placeholder). What I have here is a little funny, too, though, as it keeps trying other color names if it finds "red" but want_color() returns 0. -Peff
Re: git 2.10.1 test regression in t3700-add.sh
Junio C Hamano writes: > ... > I also notice that the problematic test uses "chmod 755"; don't we > need POSIXPERM prerequisite on this test, too, I wonder? > > Thanks. > > -- >8 -- > t3700: fix broken test under !SANITY > > An "add --chmod=+x" test recently added by 610d55af0f ("add: modify > already added files when --chmod is given", 2016-09-14) used "xfoo3" > as a test file. The paths xfoo[1-3] were used by earlier tests for > symbolic links but they were expected to have been removed by the > test script reached this new test. Eh, "removed by the time test script reached..."
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Jeff King writes: > Having separate exec/shell boolean options just punts the overlap from > the command key to those keys. If you have two mutually exclusive > options, I think the best thing is a single option, like: > > type = > > and then it is obvious that a second appearance of "type" overrides an > earlier one, by our usual "last one wins" convention. As opposed to: > > shell = true > exec = true > > where you have to understand the meaning of each option to know that > "exec" overrides "shell". Good. Duy's "do we want to chdir or stay?" would be an orthogonal axis to "what does the command line look like?" and "how is the command line run?" so it adds one member to the "alias..*" family of variables, I guess.
Re: git merge deletes my changes
On Mon, 2016-10-10 at 10:19 +, Eduard Egorov wrote: > # ~/gitbuild/git-2.10.1/git merge -s subtree --squash ceph_ansible > > Can somebody confirm this please? Doesn't "merge -s subtree" really > merges branches? I think possibly you're not fully understanding what the --squash flag does... that's what's causing your issue here, not the "-s" option. A squash merge takes the commits that would be merged from the origin branch and squashes them into a single patch and applies them to the current branch as a new commit... but this new commit is not a merge commit (that is, when you look at it with "git show" etc. the commit will have only one parent, not two--or more--parents like a normal merge commit). Basically, it's syntactic sugar for a diff plus patch operation plus some Git goodness wrapped around it to make it easier to use. But ultimately once you're done, Git has no idea that this new commit has any relationship whatsoever to the origin branch. So the next time you merge, Git doesn't know that there was a previous merge and it will try to merge everything from scratch rather than starting at the previous common merge point. So either you'll have to use a normal, non-squash merge, or else you'll have to tell Git by hand what the previous common merge point was (as Jeff King's excellent email suggests). Or else, you'll have to live with this behavior.
[PATCH 1/2] submodule: ignore trailing slash on superproject URL
Before 63e95beb0 (2016-04-15, submodule: port resolve_relative_url from shell to C), it did not matter if the superprojects URL had a trailing slash or not. It was just chopped off as one of the first steps (The "remoteurl=${remoteurl%/}" near the beginning of resolve_relative_url(), which was removed in said commit). When porting this to the C version, an off-by-one error was introduced and we did not check the actual last character to be a slash, but the NULL delimiter. Reintroduce the behavior from before 63e95beb0, to ignore the trailing slash. Reported-by: Helped-by: Dennis Kaarsemaker Signed-off-by: Stefan Beller --- builtin/submodule--helper.c | 6 -- t/t0060-path-utils.sh | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 444ec06..a7841a5 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -95,6 +95,8 @@ static int chop_last_dir(char **remoteurl, int is_relative) * NEEDSWORK: This works incorrectly on the domain and protocol part. * remote_url url outcome expectation * http://a.com/b ../c http://a.com/c as is + * http://a.com/b/ ../c http://a.com/c same as previous line, but + * ignore trailing slash in url * http://a.com/b ../../c http://c error out * http://a.com/b ../../../c http:/c error out * http://a.com/b ../../../../chttp:c error out @@ -113,8 +115,8 @@ static char *relative_url(const char *remote_url, struct strbuf sb = STRBUF_INIT; size_t len = strlen(remoteurl); - if (is_dir_sep(remoteurl[len])) - remoteurl[len] = '\0'; + if (is_dir_sep(remoteurl[len-1])) + remoteurl[len-1] = '\0'; if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl)) is_relative = 0; diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index bf2deee..82b98f8 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -319,6 +319,7 @@ test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule" test_submodule_relative_url "../" "foo" "../submodule" "../submodule" test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c" +test_submodule_relative_url "(null)" "../foo/bar/" "../sub/a/b/c" "../foo/sub/a/b/c" test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule" test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule" test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule" -- 2.10.1.382.ga23ca1b.dirty
Re: [PATCH v2] gpg-interface: use more status letters
Michael J Gruber writes: > Sorry, this got "lost in vacation". Before that, I was looking for an > easy way to test expired signatures, but gpg1 and gpg2 behave somewhat > differently in that respect (2 does not allow to create already expired > signatures). > > Is there anything I should or could do now? I dunno. It's your itch. You can say "I'll need more time to figure out the way to test what I am not testing here, so do not merge it to 'next' yet". You can also say "This is good enough for now, so go ahead and merge it to 'next'; more detailed tests can be done as follow-up patches if needed". You can also say "Thinking about it again, there is no strong reason why we need to differentiate EXPSIG and EXPKEYSIG, so don't do this SQUASH and use my original one as-is". I'd be happy with any of the above and there may be other ones I'd be happy with that I haven't thought of ;-)
[PATCH 2/2] submodule: ignore trailing slash in relative url
This is similar to the previous patch, though no user reported a bug and I could not find a regressive behavior. However it is a good thing to be strict on the output and for that we always omit a trailing slash. Signed-off-by: Stefan Beller --- builtin/submodule--helper.c | 2 ++ t/t0060-path-utils.sh | 1 + 2 files changed, 3 insertions(+) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index a7841a5..260f46f 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -149,6 +149,8 @@ static char *relative_url(const char *remote_url, } strbuf_reset(&sb); strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url); + if (ends_with(url, "/")) + strbuf_setlen(&sb, sb.len - 1); free(remoteurl); if (starts_with_dot_slash(sb.buf)) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 82b98f8..25b48e5 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -319,6 +319,7 @@ test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule" test_submodule_relative_url "../" "foo" "../submodule" "../submodule" test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c" +test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c/" "../foo/sub/a/b/c" test_submodule_relative_url "(null)" "../foo/bar/" "../sub/a/b/c" "../foo/sub/a/b/c" test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule" test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule" -- 2.10.1.382.ga23ca1b.dirty
Re: [PATCH] documentation: clarify submodule..[url, path] variables
Stefan Beller writes: > submodule..path:: > + The path within this project for a submodule. This variable is > + kept in the .gitmodules file. See linkgit:git-submodule[1] and > + linkgit:gitmodules[5] for details. What does it exactly mean to be "kept"? Does it mean "never appears in .git/config, and when it appears it will not be used at all"? If so we shouldn't even list it here. I doubt there is any reason for .path to exist in .git/config; where each submodule appears in the working tree is what is recorded in the tree object, and the "identity" (i.e. that which links a submodule in a tree to one of the repositories kept in .git/modules/*) by reverse look-up of submodule..path from in-tree .gitmodules, not from configuration, and it is not something a per-repository configuration should be able to change at the conceptual level. > submodule..url:: > - The path within this project and URL for a submodule. These > - variables are initially populated by 'git submodule init'. See > - linkgit:git-submodule[1] and linkgit:gitmodules[5] for > - details. > + The URL for a submodule. This variable is copied from the .gitmodules > + file to the git config via 'git submodule init'. The user can change > + the configured URL before obtaining the submodule via 'git submodule > + update'. After obtaining the submodule, this variable is kept in the > + config as a boolean flag to indicate whether the submodule is of > + interest to git commands. See linkgit:git-submodule[1] and > + linkgit:gitmodules[5] for details. I think it is great that you are describing that this serves two purposes, but "as a boolean flag" is very misleading. It sounds as if at some point "git submodule $something" command stores true/false there. - It overrides the URL suggested by the project in .gitmodules and replace it with another URL viewed from the local environment (e.g. the project may suggest you to use http://github.com/ while you may have a local mirror elsewhere). - Its presence is also used as a sign that the user is interested in the submodule.
Re: How to watch a mailing list & repo for patches which affect a certain area of code?
W dniu 09.10.2016 o 21:03, Ian Kelling pisze: > I've got patches in various projects, and I don't have time to keep up > with the mailing list, but I'd like to help out with maintenance of that > code, or the functions/files it touches. People don't cc me. I figure I > could filter the list, test patches submitted, commits made, mentions of > files/functions, build filters based on the code I have in the repo even > if it's been moved or changed subsequently. I'm wondering what other > people have implemented already for automation around this, or general > thoughts. Web search is not showing me much. First, the practice on this mailing list is to Cc all, and from what I have seen people tend to do that (well, at least the regular participants). Second, you can read this mailing list (and send emails / respond) with a news reader via the NNTP interface (gmane or public-inbox). News readers usually have good search capability. Hope that helps -- Jakub Narębski
Re: [PATCH] clean up confusing suggestion for commit references
Heiko Voigt writes: > If you want to reference a previous commit in the history of a stable > -branch, use the format "abbreviated sha1 (subject, date)", > +branch, use the format 'abbreviated sha1 ("subject", date)', > with the subject enclosed in a pair of double-quotes, like this: > > Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30) As long as the "like this" example is there close to the sentence "use the format ...", I do not think it matters either way in practice, but I think this change is adding unnecessary confusion. Both the 'subject' and 'date' on that line are meant to be placeholders, so where you see subject, you replace it "with the subject enclosed in a pair of double-quotes" as the next line says. But then you would end up with: Commit f86a374 (""pack-bitmap.c: fix a memleak"", 2015-03-30) which is not what we want to see.
Re: git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
On Mon, Oct 10, 2016 at 05:12:25PM +0100, Seaders Oloinsigh wrote: > Due to the structure of this repo, it looks like there are some > branches that never had anything to do with the android/ subdirectory, > so they're not getting wiped out. My branch is in a better state to > how I want it, but still, if I run your suggestion, > [...] Hmm. Yeah, I think this is an artifact of the way that filter-branch works with pathspec limiting. It keeps a mapping of commits that it has rewritten (including ones that were rewritten only because their ancestors were), and realizes that a branch ref needs updated when the commit it points to was rewritten. But if we don't touch _any_ commits in the history reachable from a branch (because they didn't even show up in our pathspec-limited rev-list), then it doesn't realize we touched the branch's history at all. I agree that the right outcome is for it to delete those branches entirely. I suspect the fix would be pretty tricky, though. In the meantime, I think you can work around it by either: 1. Make a pass beforehand for refs that do not touch your desired paths at all, like: path=android ;# or whatever git for-each-ref --format='%(refname)' | while read ref; do if test "$(git rev-list --count "$ref" -- "$path")" = 0; then echo "delete $ref" fi done | git update-ref --stdin and then filter what's left: git filter-branch --subdirectory-filter $path -- --all or 2. Do the filter-branch, and because you know you specified --all and that your filters would touch all histories, any ref which _wasn't_ touched can be deleted. That list is anything which didn't get a backup entry in refs/original. So something like: git for-each-ref --format='%(refname)' | perl -lne 'print $1 if m{^refs/original/(.*)}' >backups git for-each-ref --format='%(refname)' | grep -v ^refs/original >refs comm -23 refs backups | sed "s/^/delete /" | git update-ref --stdin -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Mon, Oct 10, 2016 at 10:52:26AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > Having separate exec/shell boolean options just punts the overlap from > > the command key to those keys. If you have two mutually exclusive > > options, I think the best thing is a single option, like: > > > > type = > > > > and then it is obvious that a second appearance of "type" overrides an > > earlier one, by our usual "last one wins" convention. As opposed to: > > > > shell = true > > exec = true > > > > where you have to understand the meaning of each option to know that > > "exec" overrides "shell". > > Good. > > Duy's "do we want to chdir or stay?" would be an orthogonal axis to > "what does the command line look like?" and "how is the command line > run?" so it adds one member to the "alias..*" family of > variables, I guess. Yeah, exactly. There may be other orthogonal axes to add, too, but I do not have any in mind myself. My main motive in switching to the "alias.$cmd.key" syntax is that it fixes the ancient mistake of putting arbitrary content into the key (just like pager.*, as we've discussed elsewhere). -Peff
Re: [PATCH] clean up confusing suggestion for commit references
Jeff King writes: > On Fri, Oct 07, 2016 at 11:56:38AM +0200, Heiko Voigt wrote: > >> The description for referencing commits looks as if it is contradicting >> the example, since it is itself enclosed in double quotes. Lets use >> single quotes around the description and include the double quotes in >> the description so it matches the example. >> --- >> Sorry for opening this up again but I just looked up the format and was >> like: "Umm, which one is now the correct one..." >> >> For this makes more sense. What do others think? > > Looking over the threads, I wasn't sure there was consensus[1,2]. So it would > be equally correct to drop the quotes from the example. > > I dunno. I am in favor of no-quotes, myself, so maybe I am just > manufacturing dissent in my mind. :) I no longer have preference either way myself, even though I was in favor of no-quotes simply because I had an alias to produce that format and was used to it.
Re: [PATCH] clean up confusing suggestion for commit references
On Mon, Oct 10, 2016 at 11:24:01AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > On Fri, Oct 07, 2016 at 11:56:38AM +0200, Heiko Voigt wrote: > > > >> The description for referencing commits looks as if it is contradicting > >> the example, since it is itself enclosed in double quotes. Lets use > >> single quotes around the description and include the double quotes in > >> the description so it matches the example. > >> --- > >> Sorry for opening this up again but I just looked up the format and was > >> like: "Umm, which one is now the correct one..." > >> > >> For this makes more sense. What do others think? > > > > Looking over the threads, I wasn't sure there was consensus[1,2]. So it > > would > > be equally correct to drop the quotes from the example. > > > > I dunno. I am in favor of no-quotes, myself, so maybe I am just > > manufacturing dissent in my mind. :) > > I no longer have preference either way myself, even though I was in > favor of no-quotes simply because I had an alias to produce that > format and was used to it. I'll admit that I don't care _that_ much and am happy to leave it up to individual authors, as long as nobody quotes SubmittingPatches at me as some kind of gospel when I use the no-quotes form. -Peff
Re: [PATCH v4 4/4] mergetool: honor -O
David Aguilar writes: > Teach mergetool to pass "-O" down to `git diff` when > specified on the command-line. > > Helped-by: Johannes Sixt > Signed-off-by: David Aguilar > --- > Since v3: > > I missed one last piped invocation of "git mergetool" in the tests, > which has been fixed. I only see 4/4 in v4; am I correct to assume that 1-3/4 of v4 are the same as their counterparts in v3?
Re: [PATCH] contrib: add credential helper for libsecret
On Sun, Oct 09, 2016 at 03:34:17PM +0300, Mantas Mikulėnas wrote: > This is based on the existing gnome-keyring helper, but instead of > libgnome-keyring (which was specific to GNOME and is deprecated), it > uses libsecret which can support other implementations of XDG Secret > Service API. Thanks for working on this; I'm happy to see credential helpers for as many storage APIs as possible. > Passes t0303-credential-external.sh. Thank you for running that, as my first question would have been about its results. :) I don't know much about the Secret Service API, nor do I run a desktop environment which provides the server side of it. But the code looks straightforward and reasonable, it passes t0303, and presumably it is working for you. So this seems worth merging to me. -Peff
Re: [PATCH] documentation: clarify submodule..[url, path] variables
On Mon, Oct 10, 2016 at 11:09 AM, Junio C Hamano wrote: > Stefan Beller writes: >> submodule..path:: >> + The path within this project for a submodule. This variable is >> + kept in the .gitmodules file. See linkgit:git-submodule[1] and >> + linkgit:gitmodules[5] for details. > > What does it exactly mean to be "kept"? > > Does it mean "never appears in .git/config, and when it appears it > will not be used at all"? If so we shouldn't even list it here. I meant: Git wont put it into .git/config on its own. If you really wanted to have it there, you need to do it yourself. I assumed you can overwrite the path via the config. It turns out, the value from the config is ignored, so it doesn't even make sense to put it into the config. Only the value from the .gitmodules file counts. So with that knowledge, I'll just remove it here, but I'll cross check if it is referenced else where enough. > > I doubt there is any reason for .path to exist in .git/config; where > each submodule appears in the working tree is what is recorded in > the tree object, and the "identity" (i.e. that which links a > submodule in a tree to one of the repositories kept in > .git/modules/*) by reverse look-up of submodule..path from > in-tree .gitmodules, not from configuration, and it is not something > a per-repository configuration should be able to change at the > conceptual level. As said above, currently we'd only rely on the .gitmodules file. However I think this may conceptually the wrong thing to do. The name is set in stone and ought to never be changed, unlike the path. But as we only obtain this information from within the repository at all times and not make a copy of it, upstream may go crazy and change the names within the .gitmodules file. Then the downstream user is left with a mess and they have no way of fixing it except by modifying the data in the repository. So it may be a sensible thing to allow the user to create their own path -> name mapping in the config file. We as the Git developers may even think about whether we'd want to have the path -> name lookup all the time from upstream, or rather on initialization want to make a copy of the path->name mapping. Then we can never change the name locally, but we'd need to track upstream for rename detection of submodule. > >> submodule..url:: >> - The path within this project and URL for a submodule. These >> - variables are initially populated by 'git submodule init'. See >> - linkgit:git-submodule[1] and linkgit:gitmodules[5] for >> - details. >> + The URL for a submodule. This variable is copied from the .gitmodules >> + file to the git config via 'git submodule init'. The user can change >> + the configured URL before obtaining the submodule via 'git submodule >> + update'. After obtaining the submodule, this variable is kept in the >> + config as a boolean flag to indicate whether the submodule is of >> + interest to git commands. See linkgit:git-submodule[1] and >> + linkgit:gitmodules[5] for details. > > I think it is great that you are describing that this serves two > purposes, but "as a boolean flag" is very misleading. It sounds as > if at some point "git submodule $something" command stores > true/false there. > > - It overrides the URL suggested by the project in .gitmodules and >replace it with another URL viewed from the local environment >(e.g. the project may suggest you to use http://github.com/ while >you may have a local mirror elsewhere). > > - Its presence is also used as a sign that the user is interested in >the submodule. I see, that makes it clearer, I'll reword this.
Re: Feature request: use relative path in worktree config files
Duy Nguyen writes: >> I think there are some pros and some cons for relative path and absolute >> path. >> Maybe append a "--relative" option with `git worktree add` ? >> >> I've converted all path to relative and all work with success. >> >> What do you think to append this --relative option. > > Patches are welcome. Hmm, are they really welcome? Is an invocation of "git worktree add" really the right point in the workflow to decide if these references to other repositories should be relative or absolute? When you are moving referrer, it is more convenient if the reference uses absolute path to name the referrent. When you are moving both referrer and referrent, it is more convenient to use relative. I somehow doubt that users know which future move they would be making when doing "git worktree add". To me, this almost looks like a need for a new subcommand to "git worktree" that lets you move existing worktree to elsewhere, or turn absolute reference to relative and vice versa.
Re: How to watch a mailing list & repo for patches which affect a certain area of code?
+cc Xiaolong Ye On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron wrote: >> -Original Message- >> From: Ian Kelling >> Sent: Sunday, October 09, 2016 15:03 >> >> I've got patches in various projects, and I don't have time to keep up >> with the mailing list, but I'd like to help out with >> maintenance of that >> code, or the functions/files it touches. People don't cc me. >> I figure I >> could filter the list, test patches submitted, commits made, >> mentions of >> files/functions, build filters based on the code I have in >> the repo even >> if it's been moved or changed subsequently. I'm wondering what other >> people have implemented already for automation around this, or general >> thoughts. Web search is not showing me much. >> > > One thought would be to apply every patch automatically (to the branches of > interest?). Then trigger on the [successful] changed > code. This would simplify the logic to working on the source only and not > parsing the emails. > > -Jason > I think this is currently attempted by some kernel people. However it is very hard to tell where to apply a patch, as it is not formalized. See the series that was merged at 72ce3ff7b51c ('xy/format-patch-base'), which adds a footer to the patch, that tells you where exactly a patch ought to be applied. The intention behind that series was to have some CI system hooked up and report failures to the mailing list as well IIUC. Maybe that helps with your use case, too?
Re: [PATCH] pretty: respect color settings for %C placeholders
Jeff King writes: > So here's what a patch to do that would look like. I admit that "I can't > think of a good use" does not mean there _isn't_ one, but perhaps by > posting this, it might provoke other people to think on it, too. And if > nobody can come up with, maybe it's a good idea. I do not have a fundamental opposition to this approach myself, modulo a minor nit. > + By default, colors are shown only when enabled for log output (by > + `color.diff`, `color.ui`, or `--color`, and respecting the `auto` > + settings of the former if we are going to a terminal). `%C(auto,...)` > + is accepted as a historical synonym for the default. Specifying > + `%C(always,...) will show the colors always, even when colors are not > + otherwise enabled (to enable this behavior for the whole format, use > + `--color=always`). It is not just "for the whole format", but also affects other parts of the output, no? I am thinking about "git log -p --format=...". > diff --git a/pretty.c b/pretty.c > index 25efbca..73e58b5 100644 > --- a/pretty.c > +++ b/pretty.c > @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in > UTF-8 */ > > if (!end) > return 0; > - if (skip_prefix(begin, "auto,", &begin)) { > + > + if (!skip_prefix(begin, "always,", &begin)) { > if (!want_color(c->pretty_ctx->color)) > return end - placeholder + 1; > } As a way to say "when color is not enabled, ignore everything unless it begins with 'always,'", this was a bit hard to read. Perhaps an in-code comment is in order? > + > + /* this is a historical noop */ > + skip_prefix(begin, "auto,", &begin); > + > if (color_parse_mem(begin, end - begin, color) < 0) > die(_("unable to parse --pretty format")); > strbuf_addstr(sb, color); > return end - placeholder + 1; > } > - if (skip_prefix(placeholder + 1, "red", &rest)) > + if (skip_prefix(placeholder + 1, "red", &rest) && > + want_color(c->pretty_ctx->color)) > strbuf_addstr(sb, GIT_COLOR_RED); Hmm. If we are in "no I do not want color" mode and "always,red" was given, we countermanded !want_color() check up above and come here. Then we check want_color() again and refuse to paint it red? I must be reading the patch incorrectly, but I cannot quite tell where I want astray...
RE: How to watch a mailing list & repo for patches which affect a certain area of code?
> -Original Message- > From: Stefan Beller > Sent: Monday, October 10, 2016 14:43 > > +cc Xiaolong Ye > > On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron wrote: > >> -Original Message- > >> From: Ian Kelling > >> Sent: Sunday, October 09, 2016 15:03 > >> > >> I've got patches in various projects, and I don't have > time to keep up > >> with the mailing list, but I'd like to help out with > >> maintenance of that > >> code, or the functions/files it touches. People don't cc me. > >> I figure I > >> could filter the list, test patches submitted, commits made, > >> mentions of > >> files/functions, build filters based on the code I have in > >> the repo even > >> if it's been moved or changed subsequently. I'm wondering > what other > >> people have implemented already for automation around > this, or general > >> thoughts. Web search is not showing me much. > >> > > > > One thought would be to apply every patch automatically (to > the branches of interest?). Then trigger on the [successful] changed > > code. This would simplify the logic to working on the > source only and not parsing the emails. > > > > -Jason > > > > I think this is currently attempted by some kernel people. > However it is very hard to tell where to apply a patch, as it This is one of the reasons why I use bundles instead of format patch. > is not formalized. > See the series that was merged at 72ce3ff7b51c > ('xy/format-patch-base'), > which adds a footer to the patch, that tells you where > exactly a patch ought > to be applied. Cant wait for that. > > The intention behind that series was to have some CI system hooked up > and report failures to the mailing list as well IIUC. Maybe > that helps with > your use case, too? I envisioned that it would try for each head he was interested in.
Re: [PATCH] pretty: respect color settings for %C placeholders
On Mon, Oct 10, 2016 at 11:52:14AM -0700, Junio C Hamano wrote: > > + By default, colors are shown only when enabled for log output (by > > + `color.diff`, `color.ui`, or `--color`, and respecting the `auto` > > + settings of the former if we are going to a terminal). `%C(auto,...)` > > + is accepted as a historical synonym for the default. Specifying > > + `%C(always,...) will show the colors always, even when colors are not > > + otherwise enabled (to enable this behavior for the whole format, use > > + `--color=always`). > > It is not just "for the whole format", but also affects other parts > of the output, no? I am thinking about "git log -p --format=...". True. I consider that a feature and more likely to be pointing the user in the right direction, but it should probably be s/format/output/ in the last sentence. > > diff --git a/pretty.c b/pretty.c > > index 25efbca..73e58b5 100644 > > --- a/pretty.c > > +++ b/pretty.c > > @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in > > UTF-8 */ > > > > if (!end) > > return 0; > > - if (skip_prefix(begin, "auto,", &begin)) { > > + > > + if (!skip_prefix(begin, "always,", &begin)) { > > if (!want_color(c->pretty_ctx->color)) > > return end - placeholder + 1; > > } > > As a way to say "when color is not enabled, ignore everything unless > it begins with 'always,'", this was a bit hard to read. Perhaps an > in-code comment is in order? I'll see what I can do. The most readable one is probably: if (skip_prefix(begin, "auto,", &begin)) { if (!want_color(c->pretty_ctx->color)) return end - placeholder + 1; } else if (skip_prefix(begin, "always,", &begin)) { /* nothing to do; we do not respect want_color at all */ } else { /* the default is now the same as "auto" */ if (!want_color(c->pretty_ctx->color)) return end - placeholder + 1; } I avoided that because of the repetition, but it probably is not too bad. > > - if (skip_prefix(placeholder + 1, "red", &rest)) > > + if (skip_prefix(placeholder + 1, "red", &rest) && > > + want_color(c->pretty_ctx->color)) > > strbuf_addstr(sb, GIT_COLOR_RED); > > Hmm. If we are in "no I do not want color" mode and "always,red" > was given, we countermanded !want_color() check up above and come > here. Then we check want_color() again and refuse to paint it red? > > I must be reading the patch incorrectly, but I cannot quite tell > where I want astray... No, we don't come here from %C() at all. This is for bare "%Cred", which cannot have "always" (or "auto"), as there is no syntactic spot for it. It is mostly historical (it _only_ supports red, green, and blue). We could actually leave this as-is to show the colors unconditionally. I changed it to keep the new behavior consistent, but I doubt anybody cares much either way. -Peff
Re: How to watch a mailing list & repo for patches which affect a certain area of code?
On Mon, Oct 10, 2016 at 11:56 AM, Jason Pyeron wrote: >> -Original Message- >> From: Stefan Beller >> Sent: Monday, October 10, 2016 14:43 >> >> +cc Xiaolong Ye >> >> On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron wrote: >> >> -Original Message- >> >> From: Ian Kelling >> >> Sent: Sunday, October 09, 2016 15:03 >> >> >> >> I've got patches in various projects, and I don't have >> time to keep up >> >> with the mailing list, but I'd like to help out with >> >> maintenance of that >> >> code, or the functions/files it touches. People don't cc me. >> >> I figure I >> >> could filter the list, test patches submitted, commits made, >> >> mentions of >> >> files/functions, build filters based on the code I have in >> >> the repo even >> >> if it's been moved or changed subsequently. I'm wondering >> what other >> >> people have implemented already for automation around >> this, or general >> >> thoughts. Web search is not showing me much. >> >> >> > >> > One thought would be to apply every patch automatically (to >> the branches of interest?). Then trigger on the [successful] changed >> > code. This would simplify the logic to working on the >> source only and not parsing the emails. >> > >> > -Jason >> > >> >> I think this is currently attempted by some kernel people. >> However it is very hard to tell where to apply a patch, as it > > This is one of the reasons why I use bundles instead of format patch. Oh! That sounds interesting for solving the problem where to apply a change, but the big disadvantage of bundles to patches is the inability to just comment on it with an inline response. So I assume you follow a different workflow than git or the kernel do. Which project do you use it for and do you have some documentation/blog that explains that workflow? > >> is not formalized. >> See the series that was merged at 72ce3ff7b51c >> ('xy/format-patch-base'), >> which adds a footer to the patch, that tells you where >> exactly a patch ought >> to be applied. > > Cant wait for that. Well it is found in 2.9 and later. Currently the base footer is opt-in, e.g. you'd need to convince people to run `git config format.useAutoBase true` or to manually add the base to the patch via `format-patch --base=`. > >> >> The intention behind that series was to have some CI system hooked up >> and report failures to the mailing list as well IIUC. Maybe >> that helps with >> your use case, too? > > I envisioned that it would try for each head he was interested in. > Well the test system can be smart enough to differentiate between: * the patch you sent did not even compile on your base, so why are you sending bogus patches? * the patch you sent was fine as you sent it, but in the mean time the target head progressed, and it doesn't compile/test any more. collaboration is hard. * or an extension to the prior point: this patch is fine but is broken by the series xyz that is also in flight, please coordinate with name@email.
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Jeff King writes: > ... My main motive in switching to the > "alias.$cmd.key" syntax is that it fixes the ancient mistake of putting > arbitrary content into the key (just like pager.*, as we've discussed > elsewhere). Yup, we are on the same page. It's not too grave a mistake (we said "these are command names and do not have to be able to contain random letters" and deliberately did without the usual three-level hierarchy; I think the reasoning is even more valid for pager.*), but it does imply case insensitivity not to use three-level names, and I think it is a good move.
Re: [PATCH] documentation: clarify submodule..[url, path] variables
Stefan Beller writes: >> Does it mean "never appears in .git/config, and when it appears it >> will not be used at all"? If so we shouldn't even list it here. > > I meant: > Git wont put it into .git/config on its own. If you really wanted to have > it there, you need to do it yourself. > > I assumed you can overwrite the path via the config. It turns out, > the value from the config is ignored, so it doesn't even make sense > to put it into the config. Only the value from the .gitmodules file counts. > > So with that knowledge, I'll just remove it here, but I'll cross check if it > is referenced else where enough. Good. I do recall we designed this part of the system in such a way that .path is conceptually uncostimizable (it _is_ part of what the containing tree already records), and was wondering why it is there as if it can be overridden.
Re: [PATCH] clean up confusing suggestion for commit references
Jeff King writes: > On Mon, Oct 10, 2016 at 11:24:01AM -0700, Junio C Hamano wrote: > >> I no longer have preference either way myself, even though I was in >> favor of no-quotes simply because I had an alias to produce that >> format and was used to it. > > I'll admit that I don't care _that_ much and am happy to leave it up to > individual authors, as long as nobody quotes SubmittingPatches at me as > some kind of gospel when I use the no-quotes form. ;-). I just do not want to hear "gitk (or was it git-gui) produces quoted form, why are you recommending no-quoted form in SubmittingPatches?" I'd say "use common sense; sometimes it is less confusing to read without quotes and it is perfectly OK to do so if that is the case".
Re: git 2.10.1 test regression in t3700-add.sh
Am 10.10.2016 um 19:41 schrieb Junio C Hamano: I also notice that the problematic test uses "chmod 755"; don't we need POSIXPERM prerequisite on this test, too, I wonder? Good point. Without POSIXPERM the test demonstrate that, since chmod 755 is basically a noop, the following add --chmod=-x does not leave an x bit in the index when there was none there in the first place. I think it does not hurt to keep the test even though it does not quite test the same thing as on POSIXPERM enabled systems. -- Hannes
RE: How to watch a mailing list & repo for patches which affect a certain area of code? [OT]
> -Original Message- > From: Stefan Beller > Sent: Monday, October 10, 2016 15:08 > > On Mon, Oct 10, 2016 at 11:56 AM, Jason Pyeron wrote: > >> -Original Message- > >> From: Stefan Beller > >> Sent: Monday, October 10, 2016 14:43 > >> > >> +cc Xiaolong Ye > >> > >> On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron wrote: > >> >> -Original Message- > >> >> From: Ian Kelling > >> >> Sent: Sunday, October 09, 2016 15:03 > >> >> > >> >> I've got patches in various projects, and I don't have > >> time to keep up > >> >> with the mailing list, but I'd like to help out with > >> >> maintenance of that > >> >> code, or the functions/files it touches. People don't cc me. > >> >> I figure I > >> >> could filter the list, test patches submitted, commits made, > >> >> mentions of > >> >> files/functions, build filters based on the code I have in > >> >> the repo even > >> >> if it's been moved or changed subsequently. I'm wondering > >> what other > >> >> people have implemented already for automation around > >> this, or general > >> >> thoughts. Web search is not showing me much. > >> >> > >> > > >> > One thought would be to apply every patch automatically (to > >> the branches of interest?). Then trigger on the > [successful] changed > >> > code. This would simplify the logic to working on the > >> source only and not parsing the emails. > >> > > >> > -Jason > >> > > >> > >> I think this is currently attempted by some kernel people. > >> However it is very hard to tell where to apply a patch, as it > > > > This is one of the reasons why I use bundles instead of > format patch. > > Oh! That sounds interesting for solving the problem where to apply > a change, but the big disadvantage of bundles to patches is > the inability > to just comment on it with an inline response. Yep. It is a big one. I have a personal project to add a footer to a format patch with the missing "binary" data. The thoughts were for the main cases using a RLE bitmap for the whitespace in the above patch and the remainder of the commit blob data. This would allow minimal duplicate information in the email but pure text changes would be binary perfect so the commit id will still be correct. Sigh, never have enough free time. > So I assume you follow > a different workflow than git or the kernel do. Which project > do you use it > for and do you have some documentation/blog that explains > that workflow? This is used when collaborating cross enterprise. In these situations enterprise A will not give access to enterprise B on their CI system, or git repo, etc... We all have a mailing list in common (encrypted/signed emails when it contains sensitive info). The rules prevent us from using cloud solutions for almost all of our work. I have also worked on git for cross domain (tin foil hat time) source code transfer. As to a blog, never thought about it. Ask questions on the list and I will help. > > > > > >> is not formalized. > >> See the series that was merged at 72ce3ff7b51c > >> ('xy/format-patch-base'), > >> which adds a footer to the patch, that tells you where > >> exactly a patch ought > >> to be applied. > > > > Cant wait for that. > > Well it is found in 2.9 and later. Currently the base footer is > opt-in, e.g. you'd > need to convince people to run `git config format.useAutoBase > true` or to > manually add the base to the patch via `format-patch --base=`. Please make default in 2.10.2 . > > > > >> > >> The intention behind that series was to have some CI > system hooked up > >> and report failures to the mailing list as well IIUC. Maybe > >> that helps with > >> your use case, too? > > > > I envisioned that it would try for each head he was interested in. > > > > Well the test system can be smart enough to differentiate between: For the OP's case Test 1: does it cleanly apply to any head, if no for all raise flag. > * the patch you sent did not even compile on your base, so why >are you sending bogus patches? Test 2: is it in an area I care about, if not stop. Test 3: does it compile for clean application, if no for all raise flag. > * the patch you sent was fine as you sent it, but in the mean time > the target head progressed, and it doesn't compile/test any more. > collaboration is hard. Yes, especially when you have no time, or management is in the way. > * or an extension to the prior point: this patch is fine but is broken > by the series xyz that is also in flight, please coordinate with > name@email.
[PATCHv2] documentation: improve submodule..{url, path} description
Unlike the url variable a user cannot override the the path variable, as it is part of the content together with the gitlink at the given path. To avoid confusion do not mention the .path variable in the config section and rely on the documentation provided in gitmodules[5]. Enhance the description of submodule..url and mention its two use cases separately. Signed-off-by: Stefan Beller --- I think the gitmodules[5] is enough for .path, so let's just do this one instead. Thanks, Stefan Documentation/config.txt | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index e78293b..fd775b4 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2811,12 +2811,13 @@ stash.showStat:: option will show diffstat of the stash. Defaults to true. See description of 'show' command in linkgit:git-stash[1]. -submodule..path:: submodule..url:: - The path within this project and URL for a submodule. These - variables are initially populated by 'git submodule init'. See - linkgit:git-submodule[1] and linkgit:gitmodules[5] for - details. + The URL for a submodule. This variable is copied from the .gitmodules + file to the git config via 'git submodule init'. The user can change + the configured URL before obtaining the submodule via 'git submodule + update'. After obtaining the submodule, the presence of this variable + is used as a sign whether the submodule is of interest to git commands. + See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details. submodule..update:: The default update procedure for a submodule. This variable -- 2.10.1.382.ga23ca1b.dirty
Re: [PATCH v10 13/14] convert: add filter..process option
larsxschnei...@gmail.com writes: > +# Count unique lines in two files and compare them. > +test_cmp_count () { > + for FILE in $@ > + do > + sort $FILE | uniq -c | sed "s/^[ ]*//" >$FILE.tmp > + cat $FILE.tmp >$FILE Unquoted references to $FILE bothers me. Are you relying on them getting split at IFS boundaries? Otherwise write this (and other similar ones) like so: for FILE in "$@" do do-this-to "$FILE" | ... >"$FILE.tmp" && cat "$FILE.tmp" >"$FILE" && rm -f "$FILE.tmp" > + done && > + test_cmp $@ The use of "$@" here is quite pointless, as you _know_ all of them are filenames, and you _know_ that test_cmp takes only two filenames. Be explicit and say test_cmp "$1" "$2" or even test_cmp_count () { expect=$1 actual=$2 for FILE in "$expect" "$actual" do ... done && test_cmp "$expect" "$actual" > +# Count unique lines except clean invocations in two files and compare > +# them. Clean invocations are not counted because their number can vary. > +# c.f. > http://public-inbox.org/git/xmqqshv18i8i@gitster.mtv.corp.google.com/ > +test_cmp_count_except_clean () { > + for FILE in $@ > + do > + sort $FILE | uniq -c | sed "s/^[ ]*//" | > + sed "s/^\([0-9]\) IN: clean/x IN: clean/" >$FILE.tmp > + cat $FILE.tmp >$FILE > + done && > + test_cmp $@ > +} Why do you even _care_ about the number of invocations? While I told you why "clean" could be called multiple times under racy Git as an example, that was not meant to be an exhaustive example. I wouldn't be surprised if we needed to run smudge twice, for example, in some weirdly racy cases in the future. Can we just have the correctness (i.e. "we expect that the working tree file gets this as the result of checking it out, and we made sure that is the case") test without getting into such an implementation detail? Thanks.
Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL
[And now with CC to the list, sorry Stefan] On Mon, 2016-10-10 at 10:56 -0700, Stefan Beller wrote: > Before 63e95beb0 (2016-04-15, submodule: port resolve_relative_url from > shell to C), it did not matter if the superprojects URL had a trailing > slash or not. It was just chopped off as one of the first steps > (The "remoteurl=${remoteurl%/}" near the beginning of > resolve_relative_url(), which was removed in said commit). > > When porting this to the C version, an off-by-one error was introduced > and we did not check the actual last character to be a slash, but the > NULL delimiter. > > Reintroduce the behavior from before 63e95beb0, to ignore the trailing > slash. Looks good to me, and fixes my simple testcase and cloning epiphany with trailing slash. Thanks! D.
Re: [PATCH] pretty: respect color settings for %C placeholders
Am 10.10.2016 um 19:42 schrieb Jeff King: On Mon, Oct 10, 2016 at 07:09:12PM +0200, René Scharfe wrote: diff --git a/pretty.c b/pretty.c index 25efbca..73e58b5 100644 --- a/pretty.c +++ b/pretty.c @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */ if (!end) return 0; - if (skip_prefix(begin, "auto,", &begin)) { + + if (!skip_prefix(begin, "always,", &begin)) { if (!want_color(c->pretty_ctx->color)) return end - placeholder + 1; } Shouldn't we have an "else" here? I'm not sure what you mean; can you write it out? > - if (skip_prefix(begin, "auto,", &begin)) { > + > + if (!skip_prefix(begin, "always,", &begin)) { >if (!want_color(c->pretty_ctx->color)) >return end - placeholder + 1; >} else { /* here */ > + /* this is a historical noop */ > + skip_prefix(begin, "auto,", &begin); } Otherwise "always,auto," would be allowed and mean the same as "always,", which just seems wrong. Not a biggie. - if (skip_prefix(placeholder + 1, "red", &rest)) + if (skip_prefix(placeholder + 1, "red", &rest) && + want_color(c->pretty_ctx->color)) strbuf_addstr(sb, GIT_COLOR_RED); - else if (skip_prefix(placeholder + 1, "green", &rest)) + else if (skip_prefix(placeholder + 1, "green", &rest) && +want_color(c->pretty_ctx->color)) strbuf_addstr(sb, GIT_COLOR_GREEN); - else if (skip_prefix(placeholder + 1, "blue", &rest)) + else if (skip_prefix(placeholder + 1, "blue", &rest) && +want_color(c->pretty_ctx->color)) strbuf_addstr(sb, GIT_COLOR_BLUE); - else if (skip_prefix(placeholder + 1, "reset", &rest)) + else if (skip_prefix(placeholder + 1, "reset", &rest) && +want_color(c->pretty_ctx->color)) strbuf_addstr(sb, GIT_COLOR_RESET); return rest - placeholder; } Perhaps it's a funtion like add_color(sb, ctx, color) or similar would be nice? I actually wrote it that way first (I called it "maybe_add_color()"), but it felt a little funny to have a separate function that people might be tempted to reuse (the right solution is generally to check want_color() early as above, but we can't do that here because we have to find the end of each placeholder). OK. A variable then? Lazy pseudo-code: if (RED) color = red; else if (GREEN) ... if (want_color()) strbuf_addstr(sb, color); What I have here is a little funny, too, though, as it keeps trying other color names if it finds "red" but want_color() returns 0. Oh, missed that somehow.. O_o
Re: [PATCH] pretty: respect color settings for %C placeholders
On Mon, Oct 10, 2016 at 09:59:17PM +0200, René Scharfe wrote: > > > Shouldn't we have an "else" here? > > > > I'm not sure what you mean; can you write it out? > > > - if (skip_prefix(begin, "auto,", &begin)) { > > + > > + if (!skip_prefix(begin, "always,", &begin)) { > > if (!want_color(c->pretty_ctx->color)) > > return end - placeholder + 1; > > } > > else { /* here */ > > > + /* this is a historical noop */ > > + skip_prefix(begin, "auto,", &begin); > > } > > Otherwise "always,auto," would be allowed and mean the same as "always,", > which just seems wrong. Not a biggie. I don't think that will parse "%C(auto,foo)", as we hit the !skip_prefix() of the conditional, and do not look for "auto," at all. I think you'd have to move the check for "auto," inside the if block. I'm leaning towards just writing it out the long way, though, as I did in my reply to Junio. > > > Perhaps it's a funtion like add_color(sb, ctx, color) or similar would be > > > nice? > > > > I actually wrote it that way first (I called it "maybe_add_color()"), > > but it felt a little funny to have a separate function that people might > > be tempted to reuse (the right solution is generally to check > > want_color() early as above, but we can't do that here because we have > > to find the end of each placeholder). > > OK. A variable then? Lazy pseudo-code: > > if (RED) > color = red; > else if (GREEN) > ... > > if (want_color()) > strbuf_addstr(sb, color); Yeah, that is a bit more clear (the final conditional just needs to check that we actually found a color). -Peff
Re: [PATCH v1 2/2] convert.c: stream and early out
tbo...@web.de writes: > -static void gather_stats(const char *buf, unsigned long size, struct > text_stat *stats) > +static void gather_stats_partly(const char *buf, unsigned long len, > + struct text_stat *stats, unsigned earlyout) > { I think it is OK not to rename the function (you'd be passing earlyout=0 for callers that want exact stat, right?). > unsigned long i; > > - memset(stats, 0, sizeof(*stats)); > - > - for (i = 0; i < size; i++) { > + if (!buf || !len) > + return; > + for (i = 0; i < len; i++) { > unsigned char c = buf[i]; > if (c == '\r') { > - if (i+1 < size && buf[i+1] == '\n') { > + stats->stat_bits |= CONVERT_STAT_BITS_ANY_CR; > + if (i+1 < len && buf[i+1] == '\n') { > stats->crlf++; > i++; > - } else > + stats->stat_bits |= CONVERT_STAT_BITS_TXT_CRLF; > + } else { > stats->lonecr++; > + stats->stat_bits |= CONVERT_STAT_BITS_BIN; > + } > continue; > } > if (c == '\n') { > stats->lonelf++; > + stats->stat_bits |= CONVERT_STAT_BITS_TXT_LF; > continue; > } > if (c == 127) > @@ -67,7 +74,7 @@ static void gather_stats(const char *buf, unsigned long > size, struct text_stat * > stats->printable++; > break; > case 0: > - stats->nul++; > + stats->stat_bits |= CONVERT_STAT_BITS_BIN; > /* fall through */ > default: > stats->nonprintable++; So depending on the distribution of the bytes in the file, the bitfields in stats->stat_bits will be filled one bit at a time in random order. > @@ -75,10 +82,12 @@ static void gather_stats(const char *buf, unsigned long > size, struct text_stat * > } > else > stats->printable++; > + if (stats->stat_bits & earlyout) > + break; /* We found what we have been searching for */ But an "earlyout" says that if "any" of the earlyout bit is seen, we can return. It somehow felt a bit too limited to me in my initial reading, but I guess I shouldn't be surprised to see that such a limited interface is sufficient for a file-local helper function ;-). The only caller that the semantics of this exit condition matters is the one that wants to know "do we have NUL or CR anywhere?", so I guess this should be sufficient. > } > > /* If file ends with EOF then don't count this EOF as non-printable. */ > - if (size >= 1 && buf[size-1] == '\032') > + if (len >= 1 && buf[len-1] == '\032') > stats->nonprintable--; This noise is somewhat irritating. Was there a reason why size was a bad name for the variable? > +static const char *convert_stats_ascii(unsigned convert_stats) > { > - unsigned int convert_stats = gather_convert_stats(data, size); > - > + const unsigned mask = CONVERT_STAT_BITS_TXT_LF | > + CONVERT_STAT_BITS_TXT_CRLF; > if (convert_stats & CONVERT_STAT_BITS_BIN) > return "-text"; > - switch (convert_stats) { > + switch (convert_stats & mask) { > case CONVERT_STAT_BITS_TXT_LF: > return "lf"; > case CONVERT_STAT_BITS_TXT_CRLF: Subtle. The caller runs the stat colllection with early-out set to BITS_BIN, so that this can set "-text" early. It knows that without BITS_BIN, the stat was taken for the whole contents and the check lf or crlf can be reliable. I wonder if we can/need to do something to remove this subtleness out of this callchain, which could be a source of confusion. > @@ -132,24 +162,45 @@ static const char *gather_convert_stats_ascii(const > char *data, unsigned long si > } > } > > +static unsigned get_convert_stats_wt(const char *path) > +{ > + struct text_stat stats; > + unsigned earlyout = CONVERT_STAT_BITS_BIN; > + int fd; > + memset(&stats, 0, sizeof(stats)); > + fd = open(path, O_RDONLY); > + if (fd < 0) > + return 0; > + for (;;) { > + char buf[2*1024]; Where is this 2kB come from? Out of thin air?