* [PATCH v7 11/16] i18n: add--interactive: mark status words for translation
From: Vasco Almeida @ 2016-12-14 12:54 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar,
Junio C Hamano
In-Reply-To: <20161005172110.30801-1-vascomalmeida@sapo.pt>
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 <vascomalmeida@sapo.pt>
---
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 d3785e8d7..4e0ab5a9b 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.11.0.44.g7d42c6c
^ permalink raw reply related
* [PATCH v7 06/16] i18n: add--interactive: mark plural strings
From: Vasco Almeida @ 2016-12-14 12:54 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar,
Junio C Hamano
In-Reply-To: <20161005172110.30801-1-vascomalmeida@sapo.pt>
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 <vascomalmeida@sapo.pt>
---
Makefile | 3 ++-
git-add--interactive.perl | 27 ++++++++++++++++++---------
perl/Git/I18N.pm | 10 +++++++++-
t/t0202/test.pl | 11 ++++++++++-
4 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/Makefile b/Makefile
index f53fcc90d..fdef1dd94 100644
--- a/Makefile
+++ b/Makefile
@@ -2114,7 +2114,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 d05ac608e..cd617837b 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 f889fd6da..617d8c2a1 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\n", "commited %d files\n", $files), $files;
+
=head1 DESCRIPTION
Git's internal Perl interface to gettext via L<Locale::Messages>. If
@@ -87,6 +91,10 @@ it.
L<Locale::Messages>'s gettext function if all goes well, otherwise our
passthrough fallback function.
+=head2 __n($$$)
+
+L<Locale::Messages>'s ngettext function or passthrough fallback function.
+
=head1 AUTHOR
E<AElig>var ArnfjE<ouml>rE<eth> Bjarmason <avarab@gmail.com>
diff --git a/t/t0202/test.pl b/t/t0202/test.pl
index 2c10cb469..4101833a8 100755
--- a/t/t0202/test.pl
+++ b/t/t0202/test.pl
@@ -4,7 +4,7 @@ use lib (split(/:/, $ENV{GITPERLLIB}));
use strict;
use warnings;
use POSIX qw(:locale_h);
-use Test::More tests => 8;
+use Test::More tests => 11;
use Git::I18N;
my $has_gettext_library = $Git::I18N::__HAS_LIBRARY;
@@ -31,6 +31,7 @@ is_deeply(\@Git::I18N::EXPORT, \@Git::I18N::EXPORT_OK, "sanity: Git::I18N export
# more gettext wrapper functions.
my %prototypes = (qw(
__ $
+ __n $$$
));
while (my ($sub, $proto) = each %prototypes) {
is(prototype(\&{"Git::I18N::$sub"}), $proto, "sanity: $sub has a $proto prototype");
@@ -46,6 +47,14 @@ is_deeply(\@Git::I18N::EXPORT, \@Git::I18N::EXPORT_OK, "sanity: Git::I18N export
my ($got, $expect) = (('TEST: A Perl test string') x 2);
is(__($got), $expect, "Passing a string through __() in the C locale works");
+
+ my ($got_singular, $got_plural, $expect_singular, $expect_plural) =
+ (('TEST: 1 file', 'TEST: n files') x 2);
+
+ is(__n($got_singular, $got_plural, 1), $expect_singular,
+ "Get singular string through __n() in C locale");
+ is(__n($got_singular, $got_plural, 2), $expect_plural,
+ "Get plural string through __n() in C locale");
}
# Test a basic message on different locales
--
2.11.0.44.g7d42c6c
^ permalink raw reply related
* [PATCH v7 02/16] i18n: add--interactive: mark strings for translation
From: Vasco Almeida @ 2016-12-14 12:54 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar,
Junio C Hamano
In-Reply-To: <20161005172110.30801-1-vascomalmeida@sapo.pt>
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 <vascomalmeida@sapo.pt>
---
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 ee3d81269..cf216ecb6 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 {
my $response = $1;
my $no = $ix > 10 ? $ix - 10 : 0;
while ($response eq '') {
- my $extra = "";
$no = display_hunks(\@hunk, $no);
if ($no < $num) {
- $extra = " (<ret> to see more)";
+ print __("go to which hunk (<ret> to see more)? ");
+ } else {
+ print __("go to which hunk? ");
}
- print "go to which hunk$extra? ";
$response = <STDIN>;
if (!defined $response) {
$response = '';
@@ -1439,7 +1447,7 @@ sub patch_update_file {
elsif ($line =~ m|^/(.*)|) {
my $regex = $1;
if ($1 eq "") {
- print colored $prompt_color, "search for regex? ";
+ print colored $prompt_color, __("search for regex? ");
$regex = <STDIN>;
if (defined $regex) {
chomp $regex;
@@ -1462,7 +1470,7 @@ sub patch_update_file {
$iy++;
$iy = 0 if ($iy >= $num);
if ($ix == $iy) {
- error_msg "No hunk matches the given pattern\n";
+ error_msg __("No hunk matches the given pattern\n");
last;
}
}
@@ -1474,7 +1482,7 @@ sub patch_update_file {
$ix--;
}
else {
- error_msg "No previous hunk\n";
+ error_msg __("No previous hunk\n");
}
next;
}
@@ -1483,7 +1491,7 @@ sub patch_update_file {
$ix++;
}
else {
- error_msg "No next hunk\n";
+ error_msg __("No next hunk\n");
}
next;
}
@@ -1496,13 +1504,13 @@ sub patch_update_file {
}
}
else {
- error_msg "No previous hunk\n";
+ error_msg __("No previous hunk\n");
}
next;
}
elsif ($line =~ /^j/) {
if ($other !~ /j/) {
- error_msg "No next hunk\n";
+ error_msg __("No next hunk\n");
next;
}
}
@@ -1560,18 +1568,18 @@ sub diff_cmd {
my @mods = list_modified('index-only');
@mods = grep { !($_->{BINARY}) } @mods;
return if (!@mods);
- my (@them) = list_and_choose({ PROMPT => 'Review diff',
+ my (@them) = list_and_choose({ PROMPT => __('Review diff'),
IMMEDIATE => 1,
HEADER => $status_head, },
@mods);
return if (!@them);
- my $reference = is_initial_commit() ? get_empty_tree() : 'HEAD';
+ my $reference = (is_initial_commit()) ? get_empty_tree() : 'HEAD';
system(qw(git diff -p --cached), $reference, '--',
map { $_->{VALUE} } @them);
}
sub quit_cmd {
- print "Bye.\n";
+ print __("Bye.\n");
exit(0);
}
@@ -1594,32 +1602,32 @@ sub process_args {
if ($1 eq 'reset') {
$patch_mode = 'reset_head';
$patch_mode_revision = 'HEAD';
- $arg = shift @ARGV or die "missing --";
+ $arg = shift @ARGV or die __("missing --");
if ($arg ne '--') {
$patch_mode_revision = $arg;
$patch_mode = ($arg eq 'HEAD' ?
'reset_head' : 'reset_nothead');
- $arg = shift @ARGV or die "missing --";
+ $arg = shift @ARGV or die __("missing --");
}
} elsif ($1 eq 'checkout') {
- $arg = shift @ARGV or die "missing --";
+ $arg = shift @ARGV or die __("missing --");
if ($arg eq '--') {
$patch_mode = 'checkout_index';
} else {
$patch_mode_revision = $arg;
$patch_mode = ($arg eq 'HEAD' ?
'checkout_head' : 'checkout_nothead');
- $arg = shift @ARGV or die "missing --";
+ $arg = shift @ARGV or die __("missing --");
}
} elsif ($1 eq 'stage' or $1 eq 'stash') {
$patch_mode = $1;
- $arg = shift @ARGV or die "missing --";
+ $arg = shift @ARGV or die __("missing --");
} else {
die "unknown --patch mode: $1";
}
} else {
$patch_mode = 'stage';
- $arg = shift @ARGV or die "missing --";
+ $arg = shift @ARGV or die __("missing --");
}
die "invalid argument $arg, expecting --"
unless $arg eq "--";
@@ -1641,10 +1649,10 @@ sub main_loop {
[ 'help', \&help_cmd, ],
);
while (1) {
- my ($it) = list_and_choose({ PROMPT => 'What now',
+ my ($it) = list_and_choose({ PROMPT => __('What now'),
SINGLETON => 1,
LIST_FLAT => 4,
- HEADER => '*** Commands ***',
+ HEADER => __('*** Commands ***'),
ON_EOF => \&quit_cmd,
IMMEDIATE => 1 }, @cmd);
if ($it) {
--
2.11.0.44.g7d42c6c
^ permalink raw reply related
* Re: [PATCH v2 28/34] run_command_opt(): optionally hide stderr when the command succeeds
From: Jeff King @ 2016-12-14 12:53 UTC (permalink / raw)
To: Johannes Sixt
Cc: Johannes Schindelin, git, Junio C Hamano, Kevin Daudt,
Dennis Kaarsemaker
In-Reply-To: <2637bed1-c36f-32f6-b255-ea32da76d792@kdbg.org>
On Wed, Dec 14, 2016 at 09:34:20AM +0100, Johannes Sixt wrote:
> I wanted to see what it would look like if we make it the caller's
> responsibility to throw away stderr. The patch is below, as fixup
> of patch 29/34. The change is gross, but the end result is not that
> bad, though not really a delightful read, either, mostly due to the
> strange cleanup semantics of the start_command/finish_command combo,
> so... I dunno.
I don't have a strong opinion on the patches under discussion, but here
are a few pointers on the run-command interface:
> + struct child_process cmd = CHILD_PROCESS_INIT;
> [...]
> env = read_author_script();
> if (!env) {
The child_process struct comes with its own internal env array. So you
can do:
read_author_script(&cmd.env_array);
if (!cmd.env_array.argc)
...
and then you don't have to worry about free-ing env, as it happens
automatically as part of the child cleanup (I suspect the refactoring
may also reduce some of the confusing memory handling in
read_author_script()).
> + if (cmd.stdout_to_stderr) {
> + /* hide stderr on success */
> + cmd.err = -1;
> + rc = -1;
> + if (start_command(&cmd) < 0)
> + goto cleanup;
> +
> + if (strbuf_read(&errout, cmd.err, 0) < 0) {
> + close(cmd.err);
> + finish_command(&cmd); /* throw away exit code */
> + goto cleanup;
> + }
> +
> + close(cmd.err);
> + rc = finish_command(&cmd);
> + if (rc)
> + fputs(errout.buf, stderr);
> + } else {
> + rc = run_command(&cmd);
> + }
We have a helper function for capturing output, so I think you can write
this as:
if (cmd.err == -1) {
struct strbuf errout = STRBUF_INIT;
int rc = pipe_command(&cmd,
NULL, 0, /* stdin */
NULL, 0, /* stdout */
&errout, 0);
if (rc)
fputs(errout.buf, stderr);
strbuf_release(&errout);
} else
rc = run_command(&cmd);
and drop the cleanup goto entirely (if you do the "env" thing above, you
could even drop "rc" and just return directly from each branch of the
conditional).
> - rc = run_command_v_opt_cd_env(array.argv, opt, NULL,
> - (const char *const *)env);
> - argv_array_clear(&array);
> +cleanup:
> + child_process_clear(&cmd);
> + strbuf_release(&errout);
> free(env);
Even if you do keep the goto here, I think this child_process_clear() is
unnecessary. It should be done automatically either by finish_command(),
or by start_command() when it returns an error.
-Peff
^ permalink raw reply
* RE: Creating remote git repository?
From: Randall S. Becker @ 2016-12-14 10:57 UTC (permalink / raw)
To: 'essam Ganadily', git
In-Reply-To: <CADo08-pBPVShFDSbOuk-12KUQL7t7ajG17-A6=UCrUVk+hcNtA@mail.gmail.com>
On December 14, 2016 1:01 AM, essam Ganadily wrote:
> given that git is an old and mature product i wounder why there is no
> command line (git.exe in windows) way of creating a remote git repository?
>
> "git remote create repo myreponame"
Why not run the commands mkdir myreponame; cd myreponame; git init .... under an SSH command on the destination host. That should get you what you want.
Cheers,
Randall
-- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(211288444200000000)
-- In my real life, I talk too much.
^ permalink raw reply
* Re: [RFC/PATCHv2] Makefile: add cppcheck target
From: Jeff King @ 2016-12-14 11:24 UTC (permalink / raw)
To: Chris Packham; +Cc: git, stefan.naewe, gitter.spiros
In-Reply-To: <20161214092731.29076-1-judge.packham@gmail.com>
On Wed, Dec 14, 2016 at 10:27:31PM +1300, Chris Packham wrote:
> Changes in v2:
> - only run over actual git source files.
> - omit any files in t/
I actually wonder if FIND_SOURCE_FILES should be taking care of the "t/"
thing. I think "make tags" finds tags in t4051/appended1.c, which is
just silly.
> - introduce CPPCHECK_FLAGS which can be overridden in the make command
> line. This also uses a GNU make-ism to allow CPPCHECK_ADD to specify
> additional checks to be enabled.
The GNU-ism is fine; we already require GNU make to build.
The patch itself is OK to me, I guess. The interesting part will be
whether people start actually _using_ cppcheck and squelching the false
positives. I'm not sure how I feel about the in-code annotations. I'd
have to see a patch first.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] Makefile: add cppcheck target
From: Jeff King @ 2016-12-14 11:18 UTC (permalink / raw)
To: Chris Packham; +Cc: GIT, Elia Pinto
In-Reply-To: <CAFOYHZBRq=5FGswQZSYbn0JdrEv+xqLm6gdpD_nE+1L_CfPHEw@mail.gmail.com>
On Wed, Dec 14, 2016 at 09:33:59PM +1300, Chris Packham wrote:
> > I do see a few real problems, but many false positives, too.
> > Unfortunately, one of the false positives is:
> >
> > int foo = foo;
>
> On I side note I have often wondered how this actually works to avoid
> the uninitialised-ness of foo. I can see how some compilers may be
> fooled into thinking that foo has been set but that doesn't actually
> end up with foo having a deterministic value.
Right, this is only used to shut up the compiler when it incorrectly
thinks the variable is uninitialized.
-Peff
^ permalink raw reply
* [RFC/PATCHv2] Makefile: add cppcheck target
From: Chris Packham @ 2016-12-14 9:27 UTC (permalink / raw)
To: git; +Cc: stefan.naewe, peff, gitter.spiros, Chris Packham
In-Reply-To: <20161213092225.15299-1-judge.packham@gmail.com>
Add cppcheck target to Makefile. Cppcheck is a static
analysis tool for C/C++ code. Cppcheck primarily detects
the types of bugs that the compilers normally do not detect.
It is an useful target for doing QA analysis.
To run the default set of checks run
make cppcheck
Additional checks can be enabled by specifying CPPCHECK_ADD. This is a
comma separated list which is passed to cppcheck's --enable option. To
enable style and warning checks run
make cppcheck CPPCHECK_ADD=style,warning
Based-on-patch-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v2:
- only run over actual git source files.
- omit any files in t/
- print cppcheck version to allow for better comparison between
different build environments
- introduce CPPCHECK_FLAGS which can be overridden in the make command
line. This also uses a GNU make-ism to allow CPPCHECK_ADD to specify
additional checks to be enabled.
Makefile | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Makefile b/Makefile
index f53fcc90d..e5c86decf 100644
--- a/Makefile
+++ b/Makefile
@@ -2635,3 +2635,12 @@ cover_db: coverage-report
cover_db_html: cover_db
cover -report html -outputdir cover_db_html cover_db
+.PHONY: cppcheck
+
+CPPCHECK_FLAGS = --force --quiet --inline-suppr $(if $(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD))
+
+cppcheck:
+ @cppcheck --version
+ $(FIND_SOURCE_FILES) | \
+ grep -v '^t/t' | \
+ xargs cppcheck $(CPPCHECK_FLAGS)
--
2.11.0.24.ge6920cf
^ permalink raw reply related
* [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'
From: Chris Packham @ 2016-12-14 8:37 UTC (permalink / raw)
To: git; +Cc: mah, peff, jacob.keller, gitster, Chris Packham
In-Reply-To: <20161213084859.13426-1-judge.packham@gmail.com>
Teach 'git merge' the --continue option which allows 'continuing' a
merge by completing it. The traditional way of completing a merge after
resolving conflicts is to use 'git commit'. Now with commands like 'git
rebase' and 'git cherry-pick' having a '--continue' option adding such
an option to 'git merge' presents a consistent UI.
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v2:
- add --continue to builtin_merge_usage
- verify that no other arguments are present when --continue is used.
- add basic test
Changes in v3:
- check for other options in addtion to arguments, add test for this case
- re-instate references to 'git commit' that were removed in v2
- re-work documentation
Documentation/git-merge.txt | 8 ++++++++
builtin/merge.c | 21 +++++++++++++++++++++
t/t7600-merge.sh | 9 +++++++++
3 files changed, 38 insertions(+)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d5556..ca3c27b88 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
'git merge' <msg> HEAD <commit>...
'git merge' --abort
+'git merge' --continue
DESCRIPTION
-----------
@@ -61,6 +62,8 @@ reconstruct the original (pre-merge) changes. Therefore:
discouraged: while possible, it may leave you in a state that is hard to
back out of in the case of a conflict.
+The fourth syntax ("`git merge --continue`") can only be run after the
+merge has resulted in conflicts.
OPTIONS
-------
@@ -99,6 +102,11 @@ commit or stash your changes before running 'git merge'.
'git merge --abort' is equivalent to 'git reset --merge' when
`MERGE_HEAD` is present.
+--continue::
+ After a 'git merge' stops due to conflicts you can conclude the
+ merge by running 'git merge --continue' (see "HOW TO RESOLVE
+ CONFLICTS" section below).
+
<commit>...::
Commits, usually other branch heads, to merge into our branch.
Specifying more than one commit will create a merge with
diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa87..836ec281b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
N_("git merge [<options>] [<commit>...]"),
N_("git merge [<options>] <msg> HEAD <commit>"),
N_("git merge --abort"),
+ N_("git merge --continue"),
NULL
};
@@ -65,6 +66,7 @@ static int option_renormalize;
static int verbosity;
static int allow_rerere_auto;
static int abort_current_merge;
+static int continue_current_merge;
static int allow_unrelated_histories;
static int show_progress = -1;
static int default_to_upstream = 1;
@@ -223,6 +225,8 @@ static struct option builtin_merge_options[] = {
OPT__VERBOSITY(&verbosity),
OPT_BOOL(0, "abort", &abort_current_merge,
N_("abort the current in-progress merge")),
+ OPT_BOOL(0, "continue", &continue_current_merge,
+ N_("continue the current in-progress merge")),
OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
N_("allow merging unrelated histories")),
OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
@@ -1125,6 +1129,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
const char *best_strategy = NULL, *wt_strategy = NULL;
struct commit_list *remoteheads, *p;
void *branch_to_free;
+ int orig_argc = argc;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_merge_usage, builtin_merge_options);
@@ -1166,6 +1171,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
goto done;
}
+ if (continue_current_merge) {
+ int nargc = 1;
+ const char *nargv[] = {"commit", NULL};
+
+ if (orig_argc != 2)
+ usage_msg_opt("--continue expects no arguments",
+ builtin_merge_usage, builtin_merge_options);
+
+ if (!file_exists(git_path_merge_head()))
+ die(_("There is no merge in progress (MERGE_HEAD missing)."));
+
+ /* Invoke 'git commit' */
+ ret = cmd_commit(nargc, nargv, prefix);
+ goto done;
+ }
+
if (read_cache_unmerged())
die_resolve_conflict("merge");
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 85248a14b..682139c4e 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -154,6 +154,8 @@ test_expect_success 'test option parsing' '
test_must_fail git merge -s foobar c1 &&
test_must_fail git merge -s=foobar c1 &&
test_must_fail git merge -m &&
+ test_must_fail git merge --continue foobar &&
+ test_must_fail git merge --continue --quiet &&
test_must_fail git merge
'
@@ -763,4 +765,11 @@ test_expect_success 'merge nothing into void' '
)
'
+test_expect_success 'merge can be completed with --continue' '
+ git reset --hard c0 &&
+ git merge --no-ff --no-commit c1 &&
+ git merge --continue &&
+ verify_parents $c0 $c1
+'
+
test_done
--
2.11.0.24.ge6920cf
^ permalink raw reply related
* [PATCH 3/3] merge: Ensure '--abort' option takes no arguments
From: Chris Packham @ 2016-12-14 8:37 UTC (permalink / raw)
To: git; +Cc: mah, peff, jacob.keller, gitster, Chris Packham
In-Reply-To: <20161214083757.26412-1-judge.packham@gmail.com>
Like '--continue', the '--abort' option doesn't make any sense with
other options or arguments to 'git merge' so ensure that none are
present.
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v3:
- new
builtin/merge.c | 4 ++++
t/t7600-merge.sh | 2 ++
2 files changed, 6 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index 836ec281b..668aaffb8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1163,6 +1163,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
int nargc = 2;
const char *nargv[] = {"reset", "--merge", NULL};
+ if (orig_argc != 2)
+ usage_msg_opt("--abort expects no arguments",
+ builtin_merge_usage, builtin_merge_options);
+
if (!file_exists(git_path_merge_head()))
die(_("There is no merge to abort (MERGE_HEAD missing)."));
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 682139c4e..2ebda509a 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -154,6 +154,8 @@ test_expect_success 'test option parsing' '
test_must_fail git merge -s foobar c1 &&
test_must_fail git merge -s=foobar c1 &&
test_must_fail git merge -m &&
+ test_must_fail git merge --abort foobar &&
+ test_must_fail git merge --abort --quiet &&
test_must_fail git merge --continue foobar &&
test_must_fail git merge --continue --quiet &&
test_must_fail git merge
--
2.11.0.24.ge6920cf
^ permalink raw reply related
* [PATCH 2/3] completion: add --continue option for merge
From: Chris Packham @ 2016-12-14 8:37 UTC (permalink / raw)
To: git; +Cc: mah, peff, jacob.keller, gitster, Chris Packham
In-Reply-To: <20161214083757.26412-1-judge.packham@gmail.com>
Add 'git merge --continue' option when completing.
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v2:
- new
Changes in v3:
- none
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..1f97ffae1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1552,7 +1552,7 @@ _git_merge ()
case "$cur" in
--*)
__gitcomp "$__git_merge_options
- --rerere-autoupdate --no-rerere-autoupdate --abort"
+ --rerere-autoupdate --no-rerere-autoupdate --abort --continue"
return
esac
__gitcomp_nl "$(__git_refs)"
--
2.11.0.24.ge6920cf
^ permalink raw reply related
* Re: [PATCH v2 28/34] run_command_opt(): optionally hide stderr when the command succeeds
From: Johannes Sixt @ 2016-12-14 8:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <1e82aeabb906a35175362418b2b4957fae50c3b0.1481642927.git.johannes.schindelin@gmx.de>
Am 13.12.2016 um 16:32 schrieb Johannes Schindelin:
> This will be needed to hide the output of `git commit` when the
> sequencer handles an interactive rebase's script.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> run-command.c | 23 +++++++++++++++++++++++
> run-command.h | 1 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/run-command.c b/run-command.c
> index ca905a9e80..5bb957afdd 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -589,6 +589,29 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
> cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
> cmd.dir = dir;
> cmd.env = env;
> +
> + if (opt & RUN_HIDE_STDERR_ON_SUCCESS) {
> + struct strbuf buf = STRBUF_INIT;
> + int res;
> +
> + cmd.err = -1;
> + if (start_command(&cmd) < 0)
> + return -1;
> +
> + if (strbuf_read(&buf, cmd.err, 0) < 0) {
> + close(cmd.err);
> + finish_command(&cmd); /* throw away exit code */
> + return -1;
> + }
> +
> + close(cmd.err);
> + res = finish_command(&cmd);
> + if (res)
> + fputs(buf.buf, stderr);
> + strbuf_release(&buf);
> + return res;
> + }
> +
> return run_command(&cmd);
> }
Clearly, this is a bolted-on feature. It's not the worst move that you
did not advertise the flag in Documentation/technical/api-run-command.txt...
I wanted to see what it would look like if we make it the caller's
responsibility to throw away stderr. The patch is below, as fixup
of patch 29/34. The change is gross, but the end result is not that
bad, though not really a delightful read, either, mostly due to the
strange cleanup semantics of the start_command/finish_command combo,
so... I dunno.
diff --git a/sequencer.c b/sequencer.c
index 41be4cde16..f375880bd7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -660,15 +660,16 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
int cleanup_commit_message)
{
char **env = NULL;
- struct argv_array array;
- int opt = RUN_GIT_CMD, rc;
+ int rc;
const char *value;
+ struct strbuf errout = STRBUF_INIT;
+ struct child_process cmd = CHILD_PROCESS_INIT;
+
+ cmd.git_cmd = 1;
if (is_rebase_i(opts)) {
- if (!edit) {
- opt |= RUN_COMMAND_STDOUT_TO_STDERR;
- opt |= RUN_HIDE_STDERR_ON_SUCCESS;
- }
+ if (!edit)
+ cmd.stdout_to_stderr = 1;
env = read_author_script();
if (!env) {
@@ -679,36 +680,58 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
}
}
- argv_array_init(&array);
- argv_array_push(&array, "commit");
- argv_array_push(&array, "-n");
+ argv_array_push(&cmd.args, "commit");
+ argv_array_push(&cmd.args, "-n");
if (amend)
- argv_array_push(&array, "--amend");
+ argv_array_push(&cmd.args, "--amend");
if (opts->gpg_sign)
- argv_array_pushf(&array, "-S%s", opts->gpg_sign);
+ argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
if (opts->signoff)
- argv_array_push(&array, "-s");
+ argv_array_push(&cmd.args, "-s");
if (defmsg)
- argv_array_pushl(&array, "-F", defmsg, NULL);
+ argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
if (cleanup_commit_message)
- argv_array_push(&array, "--cleanup=strip");
+ argv_array_push(&cmd.args, "--cleanup=strip");
if (edit)
- argv_array_push(&array, "-e");
+ argv_array_push(&cmd.args, "-e");
else if (!cleanup_commit_message &&
!opts->signoff && !opts->record_origin &&
git_config_get_value("commit.cleanup", &value))
- argv_array_push(&array, "--cleanup=verbatim");
+ argv_array_push(&cmd.args, "--cleanup=verbatim");
if (allow_empty)
- argv_array_push(&array, "--allow-empty");
+ argv_array_push(&cmd.args, "--allow-empty");
if (opts->allow_empty_message)
- argv_array_push(&array, "--allow-empty-message");
+ argv_array_push(&cmd.args, "--allow-empty-message");
+
+ cmd.env = (const char *const *)env;
+
+ if (cmd.stdout_to_stderr) {
+ /* hide stderr on success */
+ cmd.err = -1;
+ rc = -1;
+ if (start_command(&cmd) < 0)
+ goto cleanup;
+
+ if (strbuf_read(&errout, cmd.err, 0) < 0) {
+ close(cmd.err);
+ finish_command(&cmd); /* throw away exit code */
+ goto cleanup;
+ }
+
+ close(cmd.err);
+ rc = finish_command(&cmd);
+ if (rc)
+ fputs(errout.buf, stderr);
+ } else {
+ rc = run_command(&cmd);
+ }
- rc = run_command_v_opt_cd_env(array.argv, opt, NULL,
- (const char *const *)env);
- argv_array_clear(&array);
+cleanup:
+ child_process_clear(&cmd);
+ strbuf_release(&errout);
free(env);
return rc;
--
2.11.0.79.g263f27a
^ permalink raw reply related
* Re: [RFC/PATCH] Makefile: add cppcheck target
From: Chris Packham @ 2016-12-14 8:33 UTC (permalink / raw)
To: Jeff King; +Cc: GIT, Elia Pinto
In-Reply-To: <20161213121510.5o5axuwzztbxcvfd@sigill.intra.peff.net>
On Wed, Dec 14, 2016 at 1:15 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Dec 13, 2016 at 10:22:25PM +1300, Chris Packham wrote:
>
>> $ make cppcheck
>> cppcheck --force --quiet --inline-suppr .
>> [compat/nedmalloc/malloc.c.h:4093]: (error) Possible null pointer dereference: sp
>> [compat/nedmalloc/malloc.c.h:4106]: (error) Possible null pointer dereference: sp
>> [compat/nedmalloc/nedmalloc.c:551]: (error) Expression '*(&p.mycache)=TlsAlloc(),TLS_OUT_OF_INDEXES==*(&p.mycache)' depends on order of evaluation of side effects
>> [compat/regex/regcomp.c:3086]: (error) Memory leak: sbcset
>> [compat/regex/regcomp.c:3634]: (error) Memory leak: sbcset
>> [compat/regex/regcomp.c:3086]: (error) Memory leak: mbcset
>> [compat/regex/regcomp.c:3634]: (error) Memory leak: mbcset
>> [compat/regex/regcomp.c:2802]: (error) Uninitialized variable: table_size
>> [compat/regex/regcomp.c:2805]: (error) Uninitialized variable: table_size
>> [compat/regex/regcomp.c:532]: (error) Memory leak: fastmap
>> [t/t4051/appended1.c:3]: (error) Invalid number of character '{' when these macros are defined: ''.
>> [t/t4051/appended2.c:35]: (error) Invalid number of character '{' when these macros are defined: ''.
>>
>> The last 2 are just false positives from test data. I haven't looked
>> into any of the others.
>
> I think these last two are a good sign that we need to be feeding the
> list of source files to cppcheck. I tried your patch and it also started
> looking in t/perf/build, which are old versions of git built to serve
> the performance-testing suite.
>
> See the way that the "tags" target is handled for a possible approach.
>
> My main complaint with any static checker is how we can handle false
> positives. I think our use of "-Wall -Werror" is successful because it's
> not too hard to keep the normal state to zero warnings. Looking at the
> output of cppcheck on my system (which is different than on yours!),
I think you get a similar class of problems with different compilers
(different gcc versions, clang, msvc). Although this appears to be
mitigated already with the diverse developers in the git community.
> I do see a few real problems, but many false positives, too.
> Unfortunately, one of the false positives is:
>
> int foo = foo;
On I side note I have often wondered how this actually works to avoid
the uninitialised-ness of foo. I can see how some compilers may be
fooled into thinking that foo has been set but that doesn't actually
end up with foo having a deterministic value.
> to silence -Wuninitialized, which causes cppcheck to complain that "foo"
> is uninitialized. I'm worried we will end up with two static checkers
> fighting each other, and no good way to please both.
>
> -Peff
^ permalink raw reply
* Re: [RFC/PATCH] Makefile: add cppcheck target
From: Chris Packham @ 2016-12-14 8:23 UTC (permalink / raw)
To: Jeff King; +Cc: GIT, Elia Pinto
In-Reply-To: <20161213122854.pphyp342tstxbbqe@sigill.intra.peff.net>
On Wed, Dec 14, 2016 at 1:28 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Dec 13, 2016 at 07:15:10AM -0500, Jeff King wrote:
>
>> I think these last two are a good sign that we need to be feeding the
>> list of source files to cppcheck. I tried your patch and it also started
>> looking in t/perf/build, which are old versions of git built to serve
>> the performance-testing suite.
>>
>> See the way that the "tags" target is handled for a possible approach.
>
> Maybe something like this:
>
> diff --git a/Makefile b/Makefile
> index 8b5976d88..e7684ae63 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2638,4 +2638,6 @@ cover_db_html: cover_db
> .PHONY: cppcheck
>
> cppcheck:
> - cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) .
> + $(FIND_SOURCE_FILES) |\
> + grep -v ^t/t |\
> + xargs cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD)
Will look at something like this for v2.
>
>> My main complaint with any static checker is how we can handle false
>> positives. [...]
<snip>
> So I think it is capable of finding real problems, but I think we'd need
> some way of squelching false positives, preferably in a way that carries
> forward as the code changes (so not just saying "foo.c:1234 is a false
> positive", which will break when it becomes "foo.c:1235").
If we're prepared to wear them, the --inline-suppr will let us
annotate the code to avoid the false-positives. Suppressions can also
be specified with --suppressions-list=file-with-suppressions but that
would suffer from the moving target problem although you can specify
the file without the line number to squash a class of warning for a
whole file.
^ permalink raw reply
* [PATCHv1 1/1] git-p4: avoid crash adding symlinked directory
From: Luke Diamand @ 2016-12-14 8:17 UTC (permalink / raw)
To: git; +Cc: Vinicius Kursancew, larsxschneider, aoakley, Duy Nguyen,
Luke Diamand
In-Reply-To: <20161214081724.16663-1-luke@diamand.org>
When submitting to P4, if git-p4 came across a symlinked
directory, then during the generation of the submit diff, it would
try to open it as a normal file and fail.
Spot this case, and instead output the target of the symlink in
the submit diff.
Add a test case.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
git-p4.py | 26 ++++++++++++++++++++------
t/t9830-git-p4-symlink-dir.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 6 deletions(-)
create mode 100755 t/t9830-git-p4-symlink-dir.sh
diff --git a/git-p4.py b/git-p4.py
index fd5ca5246..d2f59bd91 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -25,6 +25,7 @@ import stat
import zipfile
import zlib
import ctypes
+import errno
try:
from subprocess import CalledProcessError
@@ -1538,7 +1539,7 @@ class P4Submit(Command, P4UserMap):
if response == 'n':
return False
- def get_diff_description(self, editedFiles, filesToAdd):
+ def get_diff_description(self, editedFiles, filesToAdd, symlinks):
# diff
if os.environ.has_key("P4DIFF"):
del(os.environ["P4DIFF"])
@@ -1553,10 +1554,17 @@ class P4Submit(Command, P4UserMap):
newdiff += "==== new file ====\n"
newdiff += "--- /dev/null\n"
newdiff += "+++ %s\n" % newFile
- f = open(newFile, "r")
- for line in f.readlines():
- newdiff += "+" + line
- f.close()
+
+ try:
+ f = open(newFile, "r")
+ for line in f.readlines():
+ newdiff += "+" + line
+ f.close()
+ except IOError as e:
+ if e.errno == errno.EISDIR and newFile in symlinks:
+ newdiff += "+%s\n" % os.readlink(newFile)
+ else:
+ raise
return (diff + newdiff).replace('\r\n', '\n')
@@ -1574,6 +1582,7 @@ class P4Submit(Command, P4UserMap):
filesToDelete = set()
editedFiles = set()
pureRenameCopy = set()
+ symlinks = set()
filesToChangeExecBit = {}
for line in diff:
@@ -1590,6 +1599,11 @@ class P4Submit(Command, P4UserMap):
filesToChangeExecBit[path] = diff['dst_mode']
if path in filesToDelete:
filesToDelete.remove(path)
+
+ dst_mode = int(diff['dst_mode'], 8)
+ if dst_mode == 0120000:
+ symlinks.add(path)
+
elif modifier == "D":
filesToDelete.add(path)
if path in filesToAdd:
@@ -1727,7 +1741,7 @@ class P4Submit(Command, P4UserMap):
separatorLine = "######## everything below this line is just the diff #######\n"
if not self.prepare_p4_only:
submitTemplate += separatorLine
- submitTemplate += self.get_diff_description(editedFiles, filesToAdd)
+ submitTemplate += self.get_diff_description(editedFiles, filesToAdd, symlinks)
(handle, fileName) = tempfile.mkstemp()
tmpFile = os.fdopen(handle, "w+b")
diff --git a/t/t9830-git-p4-symlink-dir.sh b/t/t9830-git-p4-symlink-dir.sh
new file mode 100755
index 000000000..3dc528bb1
--- /dev/null
+++ b/t/t9830-git-p4-symlink-dir.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='git p4 symlinked directories'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'symlinked directory' '
+ (
+ cd "$cli" &&
+ : >first_file.t &&
+ p4 add first_file.t &&
+ p4 submit -d "first change"
+ ) &&
+ git p4 clone --dest "$git" //depot &&
+ (
+ cd "$git" &&
+ mkdir -p some/sub/directory &&
+ mkdir -p other/subdir2 &&
+ : > other/subdir2/file.t &&
+ (cd some/sub/directory && ln -s ../../../other/subdir2 .) &&
+ git add some other &&
+ git commit -m "symlinks" &&
+ git config git-p4.skipSubmitEdit true &&
+ git p4 submit -v
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ test -L some/sub/directory/subdir2
+ test_path_is_file some/sub/directory/subdir2/file.t
+ )
+
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
2.11.0.274.g0ea315c
^ permalink raw reply related
* [PATCHv1 0/1] git-p4: handle symlinked directories
From: Luke Diamand @ 2016-12-14 8:17 UTC (permalink / raw)
To: git; +Cc: Vinicius Kursancew, larsxschneider, aoakley, Duy Nguyen,
Luke Diamand
There's a long-standing bug around handling of symlinked directories
in git-p4.
If in git you create a directory, and then symlink it, when git-p4
tries to create the diff-summary of what it's about to do, it
tries to open the symlink as a regular file, and fails.
Luke Diamand (1):
git-p4: avoid crash adding symlinked directory
git-p4.py | 26 ++++++++++++++++++++------
t/t9830-git-p4-symlink-dir.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 6 deletions(-)
create mode 100755 t/t9830-git-p4-symlink-dir.sh
--
2.11.0.274.g0ea315c
^ permalink raw reply
* some updates for public-inbox readers
From: Eric Wong @ 2016-12-14 7:59 UTC (permalink / raw)
To: git
First, bad news:
There's been some hardware problems the past week or so and a
few minutes of scattered downtime and high latency as a result.
Hopefully no lost messages, as SMTP should retry.
Getting things migrated to different hardware in a few
minutes, so maybe the new hardware will have fewer gremlins
(but we could always end up with more gremlins...)
To encourage independently-run mirrors and decentralization:
I will never guarantee the continued availability of any
service I run.
The (maybe) good news:
I'm probably a day or so away from deploying RFC 4685 support
for threading the Atom feeds at
http://hjrcffqmbrq6wope.onion/git/new.atom
http://czquwvybam4bgbro.onion/git/new.atom
https://public-inbox.org/git/new.atom
The implementation is here:
http://hjrcffqmbrq6wope.onion/meta/20161213023330.6104-1-e@80x24.org/
http://czquwvybam4bgbro.onion/meta/20161213023330.6104-1-e@80x24.org/
https://public-inbox.org/meta/20161213023330.6104-1-e@80x24.org/
But I'm not sure if there's any widely-used feed readers capable
of doing proper threading like a good mail/news client can, yet.
Perhaps this will be a step to get feed reader authors
to implement threading support (and find bugs in my generator!).
I don't believe this feature will break existing feed readers;
at least not the command-line ones I've tried. But maybe it
does and I'll have to put threading in a different endpoint
(that would hurt cache hit rates :<)
I'll also increase the size of the feed from the measly 25
entries to something more at some point.... Hopefully it won't
cause somebody's feed reader to fall over when the feed grows; but
25 entries is not enough for higher-traffic lists, and a recent
change means buffering big feeds for slow clients is now bounded
to one message (instead of potentially using all space):
http://hjrcffqmbrq6wope.onion/meta/20161203015045.9398-1-e@80x24.org/
http://czquwvybam4bgbro.onion/meta/20161203015045.9398-1-e@80x24.org/
https://public-inbox.org/meta/20161203015045.9398-1-e@80x24.org/
Anyways, if you always want to be up-to-date; don't feel bad
about hammering the Atom feed every few seconds, or even running
"git fetch" every minute. The servers can handle it; and we'll
find out if they don't!
And of course you can also clone the code:
git clone http://hjrcffqmbrq6wope.onion/public-inbox
git clone https://public-inbox.org/
and run everything yourself off your list subscription
(Maybe it's near time to do a proper release;
I guess the project is 10% "complete" at this point)
Tor hidden service mirrors remain on different (better) hardware
and networks, and have historically higher reliability:
http://czquwvybam4bgbro.onion/git/
http://hjrcffqmbrq6wope.onion/git/
nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
But it's winter and storms are coming...
^ permalink raw reply
* Re: [PATCH v2 00/34] Teach the sequencer to act as rebase -i's backend
From: Johannes Sixt @ 2016-12-14 7:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>
Am 13.12.2016 um 16:29 schrieb Johannes Schindelin:
> base-commit: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
> Published-As: https://github.com/dscho/git/releases/tag/sequencer-i-v2
> Fetch-It-Via: git fetch https://github.com/dscho/git sequencer-i-v2
Thank you so much!
I would appreciate if you could publish a branch that contains the end
game so that I can test it, too. Currently I am still using
git://github.com/dscho/git interactive-rebase (fca871a3cf4d)
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 05/34] sequencer (rebase -i): learn about the 'verbose' mode
From: Junio C Hamano @ 2016-12-14 6:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqq37hr1orb.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> ...
>> + if (opts->verbose) {
>> + const char *argv[] = {
>> + "diff-tree", "--stat", NULL, NULL
>> + };
>> + ...
>> + run_command_v_opt(argv, RUN_GIT_CMD);
>> + strbuf_reset(&buf);
>> + }
>> + strbuf_release(&buf);
>> }
>
> It's a bit curious that the previous step avoided running a separate
> process and instead did "diff-tree -p" all in C, but this one does not.
>
> I think it is because this one is outside the loop?
Nah, this guess of mine "The patch file generation done in 03/34
avoids spawn because it is in a loop" is off the mark. It is done
before "edit" gives control back to the end user and it is not like
we write one patch file per iteration of the loop we want to get
maximum speed out of.
^ permalink raw reply
* Re: Creating remote git repository?
From: Konstantin Khomoutov @ 2016-12-14 6:36 UTC (permalink / raw)
To: essam Ganadily; +Cc: git
In-Reply-To: <CADo08-pBPVShFDSbOuk-12KUQL7t7ajG17-A6=UCrUVk+hcNtA@mail.gmail.com>
On Wed, 14 Dec 2016 09:00:42 +0300
essam Ganadily <doctoresam@gmail.com> wrote:
> given that git is an old and mature product i wounder why there is no
> command line (git.exe in windows) way of creating a remote git
> repository?
>
> "git remote create repo myreponame"
>
> frankly speaking i know that our friends in the linux kernel project
> never felt the need to create remote repository because they probably
> rarely need that but i guess their merciful hearts could have some
> feeling for the rest of us?
Also asked and answered (by me) over there at git-users [1].
1. https://groups.google.com/d/msg/git-users/twgkOYV6kX4/FED559TPDQAJ
^ permalink raw reply
* Creating remote git repository?
From: essam Ganadily @ 2016-12-14 6:00 UTC (permalink / raw)
To: git
given that git is an old and mature product i wounder why there is no
command line (git.exe in windows) way of creating a remote git
repository?
"git remote create repo myreponame"
frankly speaking i know that our friends in the linux kernel project
never felt the need to create remote repository because they probably
rarely need that but i guess their merciful hearts could have some
feeling for the rest of us?
^ permalink raw reply
* [PATCH v9 5/5] transport: add from_user parameter to is_transport_allowed
From: Brandon Williams @ 2016-12-14 1:40 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, gitster, jrnieder
In-Reply-To: <1481679637-133137-1-git-send-email-bmwill@google.com>
Add the from_user parameter to the 'is_transport_allowed' function.
This allows callers to query if a transport protocol is allowed, given
that the caller knows that the protocol is coming from the user (1) or
not from the user (0) such as redirects in libcurl. If unknown a -1
should be provided which falls back to reading `GIT_PROTOCOL_FROM_USER`
to determine if the protocol came from the user.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
http.c | 14 +++++++-------
transport.c | 8 +++++---
transport.h | 13 ++++++++++---
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/http.c b/http.c
index f7c488a..2208269 100644
--- a/http.c
+++ b/http.c
@@ -489,17 +489,17 @@ static void set_curl_keepalive(CURL *c)
}
#endif
-static long get_curl_allowed_protocols(void)
+static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
- if (is_transport_allowed("http"))
+ if (is_transport_allowed("http", from_user))
allowed_protocols |= CURLPROTO_HTTP;
- if (is_transport_allowed("https"))
+ if (is_transport_allowed("https", from_user))
allowed_protocols |= CURLPROTO_HTTPS;
- if (is_transport_allowed("ftp"))
+ if (is_transport_allowed("ftp", from_user))
allowed_protocols |= CURLPROTO_FTP;
- if (is_transport_allowed("ftps"))
+ if (is_transport_allowed("ftps", from_user))
allowed_protocols |= CURLPROTO_FTPS;
return allowed_protocols;
@@ -588,9 +588,9 @@ static CURL *get_curl_handle(void)
#endif
#if LIBCURL_VERSION_NUM >= 0x071304
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
- get_curl_allowed_protocols());
+ get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
- get_curl_allowed_protocols());
+ get_curl_allowed_protocols(-1));
#else
warning("protocol restrictions not applied to curl redirects because\n"
"your curl version is too old (>= 7.19.4)");
diff --git a/transport.c b/transport.c
index fbd799d..f50c31a 100644
--- a/transport.c
+++ b/transport.c
@@ -676,7 +676,7 @@ static enum protocol_allow_config get_protocol_config(const char *type)
return PROTOCOL_ALLOW_USER_ONLY;
}
-int is_transport_allowed(const char *type)
+int is_transport_allowed(const char *type, int from_user)
{
const struct string_list *whitelist = protocol_whitelist();
if (whitelist)
@@ -688,7 +688,9 @@ int is_transport_allowed(const char *type)
case PROTOCOL_ALLOW_NEVER:
return 0;
case PROTOCOL_ALLOW_USER_ONLY:
- return git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
+ if (from_user < 0)
+ from_user = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
+ return from_user;
}
die("BUG: invalid protocol_allow_config type");
@@ -696,7 +698,7 @@ int is_transport_allowed(const char *type)
void transport_check_allowed(const char *type)
{
- if (!is_transport_allowed(type))
+ if (!is_transport_allowed(type, -1))
die("transport '%s' not allowed", type);
}
diff --git a/transport.h b/transport.h
index 3396e1d..4f1c801 100644
--- a/transport.h
+++ b/transport.h
@@ -142,10 +142,17 @@ struct transport {
struct transport *transport_get(struct remote *, const char *);
/*
- * Check whether a transport is allowed by the environment. Type should
- * generally be the URL scheme, as described in Documentation/git.txt
+ * Check whether a transport is allowed by the environment.
+ *
+ * Type should generally be the URL scheme, as described in
+ * Documentation/git.txt
+ *
+ * from_user specifies if the transport was given by the user. If unknown pass
+ * a -1 to read from the environment to determine if the transport was given by
+ * the user.
+ *
*/
-int is_transport_allowed(const char *type);
+int is_transport_allowed(const char *type, int from_user);
/*
* Check whether a transport is allowed by the environment,
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v9 4/5] http: create function to get curl allowed protocols
From: Brandon Williams @ 2016-12-14 1:40 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, gitster, jrnieder
In-Reply-To: <1481679637-133137-1-git-send-email-bmwill@google.com>
Move the creation of an allowed protocols whitelist to a helper
function.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
http.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/http.c b/http.c
index 034426e..f7c488a 100644
--- a/http.c
+++ b/http.c
@@ -489,10 +489,25 @@ static void set_curl_keepalive(CURL *c)
}
#endif
+static long get_curl_allowed_protocols(void)
+{
+ long allowed_protocols = 0;
+
+ if (is_transport_allowed("http"))
+ allowed_protocols |= CURLPROTO_HTTP;
+ if (is_transport_allowed("https"))
+ allowed_protocols |= CURLPROTO_HTTPS;
+ if (is_transport_allowed("ftp"))
+ allowed_protocols |= CURLPROTO_FTP;
+ if (is_transport_allowed("ftps"))
+ allowed_protocols |= CURLPROTO_FTPS;
+
+ return allowed_protocols;
+}
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
- long allowed_protocols = 0;
if (!result)
die("curl_easy_init failed");
@@ -572,16 +587,10 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_POST301, 1);
#endif
#if LIBCURL_VERSION_NUM >= 0x071304
- if (is_transport_allowed("http"))
- allowed_protocols |= CURLPROTO_HTTP;
- if (is_transport_allowed("https"))
- allowed_protocols |= CURLPROTO_HTTPS;
- if (is_transport_allowed("ftp"))
- allowed_protocols |= CURLPROTO_FTP;
- if (is_transport_allowed("ftps"))
- allowed_protocols |= CURLPROTO_FTPS;
- curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_protocols);
- curl_easy_setopt(result, CURLOPT_PROTOCOLS, allowed_protocols);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
+ get_curl_allowed_protocols());
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS,
+ get_curl_allowed_protocols());
#else
warning("protocol restrictions not applied to curl redirects because\n"
"your curl version is too old (>= 7.19.4)");
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v9 3/5] http: always warn if libcurl version is too old
From: Brandon Williams @ 2016-12-14 1:40 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, gitster, jrnieder
In-Reply-To: <1481679637-133137-1-git-send-email-bmwill@google.com>
Now that there are default "known-good" and "known-bad" protocols which
are allowed/disallowed by 'is_transport_allowed' we should always warn
the user that older versions of libcurl can't respect the allowed
protocols for redirects.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
http.c | 5 ++---
transport.c | 5 -----
transport.h | 6 ------
3 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/http.c b/http.c
index 5cd3ffd..034426e 100644
--- a/http.c
+++ b/http.c
@@ -583,9 +583,8 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_protocols);
curl_easy_setopt(result, CURLOPT_PROTOCOLS, allowed_protocols);
#else
- if (transport_restrict_protocols())
- warning("protocol restrictions not applied to curl redirects because\n"
- "your curl version is too old (>= 7.19.4)");
+ warning("protocol restrictions not applied to curl redirects because\n"
+ "your curl version is too old (>= 7.19.4)");
#endif
if (getenv("GIT_CURL_VERBOSE"))
diff --git a/transport.c b/transport.c
index e1ba78b..fbd799d 100644
--- a/transport.c
+++ b/transport.c
@@ -700,11 +700,6 @@ void transport_check_allowed(const char *type)
die("transport '%s' not allowed", type);
}
-int transport_restrict_protocols(void)
-{
- return !!protocol_whitelist();
-}
-
struct transport *transport_get(struct remote *remote, const char *url)
{
const char *helper;
diff --git a/transport.h b/transport.h
index c681408..3396e1d 100644
--- a/transport.h
+++ b/transport.h
@@ -153,12 +153,6 @@ int is_transport_allowed(const char *type);
*/
void transport_check_allowed(const char *type);
-/*
- * Returns true if the user has attempted to turn on protocol
- * restrictions at all.
- */
-int transport_restrict_protocols(void);
-
/* Transport options which apply to git:// and scp-style URLs */
/* The program to use on the remote side to send a pack */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v9 2/5] transport: add protocol policy config option
From: Brandon Williams @ 2016-12-14 1:40 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, gitster, jrnieder
In-Reply-To: <1481679637-133137-1-git-send-email-bmwill@google.com>
Previously the `GIT_ALLOW_PROTOCOL` environment variable was used to
specify a whitelist of protocols to be used in clone/fetch/push
commands. This patch introduces new configuration options for more
fine-grained control for allowing/disallowing protocols. This also has
the added benefit of allowing easier construction of a protocol
whitelist on systems where setting an environment variable is
non-trivial.
Now users can specify a policy to be used for each type of protocol via
the 'protocol.<name>.allow' config option. A default policy for all
unconfigured protocols can be set with the 'protocol.allow' config
option. If no user configured default is made git will allow known-safe
protocols (http, https, git, ssh, file), disallow known-dangerous
protocols (ext), and have a default policy of `user` for all other
protocols.
The supported policies are `always`, `never`, and `user`. The `user`
policy can be used to configure a protocol to be usable when explicitly
used by a user, while disallowing it for commands which run
clone/fetch/push commands without direct user intervention (e.g.
recursive initialization of submodules). Commands which can potentially
clone/fetch/push from untrusted repositories without user intervention
can export `GIT_PROTOCOL_FROM_USER` with a value of '0' to prevent
protocols configured to the `user` policy from being used.
Fix remote-ext tests to use the new config to allow the ext
protocol to be tested.
Based on a patch by Jeff King <peff@peff.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/config.txt | 46 ++++++++++++++
Documentation/git.txt | 38 +++++-------
git-submodule.sh | 12 ++--
t/lib-proto-disable.sh | 130 +++++++++++++++++++++++++++++++++++++--
t/t5509-fetch-push-namespaces.sh | 1 +
t/t5802-connect-helper.sh | 1 +
transport.c | 75 +++++++++++++++++++++-
7 files changed, 264 insertions(+), 39 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8153336..50d3d06 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2260,6 +2260,52 @@ pretty.<name>::
Note that an alias with the same name as a built-in format
will be silently ignored.
+protocol.allow::
+ If set, provide a user defined default policy for all protocols which
+ don't explicitly have a policy (`protocol.<name>.allow`). By default,
+ if unset, known-safe protocols (http, https, git, ssh, file) have a
+ default policy of `always`, known-dangerous protocols (ext) have a
+ default policy of `never`, and all other protocols have a default
+ policy of `user`. Supported policies:
++
+--
+
+* `always` - protocol is always able to be used.
+
+* `never` - protocol is never able to be used.
+
+* `user` - protocol is only able to be used when `GIT_PROTOCOL_FROM_USER` is
+ either unset or has a value of 1. This policy should be used when you want a
+ protocol to be directly usable by the user but don't want it used by commands which
+ execute clone/fetch/push commands without user input, e.g. recursive
+ submodule initialization.
+
+--
+
+protocol.<name>.allow::
+ Set a policy to be used by protocol `<name>` with clone/fetch/push
+ commands. See `protocol.allow` above for the available policies.
++
+The protocol names currently used by git are:
++
+--
+ - `file`: any local file-based path (including `file://` URLs,
+ or local paths)
+
+ - `git`: the anonymous git protocol over a direct TCP
+ connection (or proxy, if configured)
+
+ - `ssh`: git over ssh (including `host:path` syntax,
+ `ssh://`, etc).
+
+ - `http`: git over http, both "smart http" and "dumb http".
+ Note that this does _not_ include `https`; if you want to configure
+ both, you must do so individually.
+
+ - any external helpers are named by their protocol (e.g., use
+ `hg` to allow the `git-remote-hg` helper)
+--
+
pull.ff::
By default, Git does not create an extra merge commit when merging
a commit that is a descendant of the current commit. Instead, the
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 923aa49..d9fb937 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1129,30 +1129,20 @@ of clones and fetches.
cloning a repository to make a backup).
`GIT_ALLOW_PROTOCOL`::
- If set, provide a colon-separated list of protocols which are
- allowed to be used with fetch/push/clone. This is useful to
- restrict recursive submodule initialization from an untrusted
- repository. Any protocol not mentioned will be disallowed (i.e.,
- this is a whitelist, not a blacklist). If the variable is not
- set at all, all protocols are enabled. The protocol names
- currently used by git are:
-
- - `file`: any local file-based path (including `file://` URLs,
- or local paths)
-
- - `git`: the anonymous git protocol over a direct TCP
- connection (or proxy, if configured)
-
- - `ssh`: git over ssh (including `host:path` syntax,
- `ssh://`, etc).
-
- - `http`: git over http, both "smart http" and "dumb http".
- Note that this does _not_ include `https`; if you want both,
- you should specify both as `http:https`.
-
- - any external helpers are named by their protocol (e.g., use
- `hg` to allow the `git-remote-hg` helper)
-
+ If set to a colon-separated list of protocols, behave as if
+ `protocol.allow` is set to `never`, and each of the listed
+ protocols has `protocol.<name>.allow` set to `always`
+ (overriding any existing configuration). In other words, any
+ protocol not mentioned will be disallowed (i.e., this is a
+ whitelist, not a blacklist). See the description of
+ `protocol.allow` in linkgit:git-config[1] for more details.
+
+`GIT_PROTOCOL_FROM_USER`::
+ Set to 0 to prevent protocols used by fetch/push/clone which are
+ configured to the `user` state. This is useful to restrict recursive
+ submodule initialization from an untrusted repository or for programs
+ which feed potentially-untrusted URLS to git commands. See
+ linkgit:git-config[1] for more details.
Discussion[[Discussion]]
------------------------
diff --git a/git-submodule.sh b/git-submodule.sh
index 78fdac9..fc44076 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -22,14 +22,10 @@ require_work_tree
wt_prefix=$(git rev-parse --show-prefix)
cd_to_toplevel
-# Restrict ourselves to a vanilla subset of protocols; the URLs
-# we get are under control of a remote repository, and we do not
-# want them kicking off arbitrary git-remote-* programs.
-#
-# If the user has already specified a set of allowed protocols,
-# we assume they know what they're doing and use that instead.
-: ${GIT_ALLOW_PROTOCOL=file:git:http:https:ssh}
-export GIT_ALLOW_PROTOCOL
+# Tell the rest of git that any URLs we get don't come
+# directly from the user, so it can apply policy as appropriate.
+GIT_PROTOCOL_FROM_USER=0
+export GIT_PROTOCOL_FROM_USER
command=
branch=
diff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh
index be88e9a..02f49cb 100644
--- a/t/lib-proto-disable.sh
+++ b/t/lib-proto-disable.sh
@@ -1,10 +1,7 @@
# Test routines for checking protocol disabling.
-# test cloning a particular protocol
-# $1 - description of the protocol
-# $2 - machine-readable name of the protocol
-# $3 - the URL to try cloning
-test_proto () {
+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist
+test_whitelist () {
desc=$1
proto=$2
url=$3
@@ -62,6 +59,129 @@ test_proto () {
test_must_fail git clone --bare "$url" tmp.git
)
'
+
+ test_expect_success "clone $desc (env var has precedence)" '
+ rm -rf tmp.git &&
+ (
+ GIT_ALLOW_PROTOCOL=none &&
+ export GIT_ALLOW_PROTOCOL &&
+ test_must_fail git -c protocol.allow=always clone --bare "$url" tmp.git &&
+ test_must_fail git -c protocol.$proto.allow=always clone --bare "$url" tmp.git
+ )
+ '
+}
+
+test_config () {
+ desc=$1
+ proto=$2
+ url=$3
+
+ # Test clone/fetch/push with protocol.<type>.allow config
+ test_expect_success "clone $desc (enabled with config)" '
+ rm -rf tmp.git &&
+ git -c protocol.$proto.allow=always clone --bare "$url" tmp.git
+ '
+
+ test_expect_success "fetch $desc (enabled)" '
+ git -C tmp.git -c protocol.$proto.allow=always fetch
+ '
+
+ test_expect_success "push $desc (enabled)" '
+ git -C tmp.git -c protocol.$proto.allow=always push origin HEAD:pushed
+ '
+
+ test_expect_success "push $desc (disabled)" '
+ test_must_fail git -C tmp.git -c protocol.$proto.allow=never push origin HEAD:pushed
+ '
+
+ test_expect_success "fetch $desc (disabled)" '
+ test_must_fail git -C tmp.git -c protocol.$proto.allow=never fetch
+ '
+
+ test_expect_success "clone $desc (disabled)" '
+ rm -rf tmp.git &&
+ test_must_fail git -c protocol.$proto.allow=never clone --bare "$url" tmp.git
+ '
+
+ # Test clone/fetch/push with protocol.user.allow and its env var
+ test_expect_success "clone $desc (enabled)" '
+ rm -rf tmp.git &&
+ git -c protocol.$proto.allow=user clone --bare "$url" tmp.git
+ '
+
+ test_expect_success "fetch $desc (enabled)" '
+ git -C tmp.git -c protocol.$proto.allow=user fetch
+ '
+
+ test_expect_success "push $desc (enabled)" '
+ git -C tmp.git -c protocol.$proto.allow=user push origin HEAD:pushed
+ '
+
+ test_expect_success "push $desc (disabled)" '
+ (
+ cd tmp.git &&
+ GIT_PROTOCOL_FROM_USER=0 &&
+ export GIT_PROTOCOL_FROM_USER &&
+ test_must_fail git -c protocol.$proto.allow=user push origin HEAD:pushed
+ )
+ '
+
+ test_expect_success "fetch $desc (disabled)" '
+ (
+ cd tmp.git &&
+ GIT_PROTOCOL_FROM_USER=0 &&
+ export GIT_PROTOCOL_FROM_USER &&
+ test_must_fail git -c protocol.$proto.allow=user fetch
+ )
+ '
+
+ test_expect_success "clone $desc (disabled)" '
+ rm -rf tmp.git &&
+ (
+ GIT_PROTOCOL_FROM_USER=0 &&
+ export GIT_PROTOCOL_FROM_USER &&
+ test_must_fail git -c protocol.$proto.allow=user clone --bare "$url" tmp.git
+ )
+ '
+
+ # Test clone/fetch/push with protocol.allow user defined default
+ test_expect_success "clone $desc (enabled)" '
+ rm -rf tmp.git &&
+ git config --global protocol.allow always &&
+ git clone --bare "$url" tmp.git
+ '
+
+ test_expect_success "fetch $desc (enabled)" '
+ git -C tmp.git fetch
+ '
+
+ test_expect_success "push $desc (enabled)" '
+ git -C tmp.git push origin HEAD:pushed
+ '
+
+ test_expect_success "push $desc (disabled)" '
+ git config --global protocol.allow never &&
+ test_must_fail git -C tmp.git push origin HEAD:pushed
+ '
+
+ test_expect_success "fetch $desc (disabled)" '
+ test_must_fail git -C tmp.git fetch
+ '
+
+ test_expect_success "clone $desc (disabled)" '
+ rm -rf tmp.git &&
+ test_must_fail git clone --bare "$url" tmp.git
+ '
+}
+
+# test cloning a particular protocol
+# $1 - description of the protocol
+# $2 - machine-readable name of the protocol
+# $3 - the URL to try cloning
+test_proto () {
+ test_whitelist "$@"
+
+ test_config "$@"
}
# set up an ssh wrapper that will access $host/$repo in the
diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
index bc44ac3..75c570a 100755
--- a/t/t5509-fetch-push-namespaces.sh
+++ b/t/t5509-fetch-push-namespaces.sh
@@ -4,6 +4,7 @@ test_description='fetch/push involving ref namespaces'
. ./test-lib.sh
test_expect_success setup '
+ git config --global protocol.ext.allow user &&
test_tick &&
git init original &&
(
diff --git a/t/t5802-connect-helper.sh b/t/t5802-connect-helper.sh
index b7a7f9d..c6c2661 100755
--- a/t/t5802-connect-helper.sh
+++ b/t/t5802-connect-helper.sh
@@ -4,6 +4,7 @@ test_description='ext::cmd remote "connect" helper'
. ./test-lib.sh
test_expect_success setup '
+ git config --global protocol.ext.allow user &&
test_tick &&
git commit --allow-empty -m initial &&
test_tick &&
diff --git a/transport.c b/transport.c
index 41eb82c..e1ba78b 100644
--- a/transport.c
+++ b/transport.c
@@ -617,10 +617,81 @@ static const struct string_list *protocol_whitelist(void)
return enabled ? &allowed : NULL;
}
+enum protocol_allow_config {
+ PROTOCOL_ALLOW_NEVER = 0,
+ PROTOCOL_ALLOW_USER_ONLY,
+ PROTOCOL_ALLOW_ALWAYS
+};
+
+static enum protocol_allow_config parse_protocol_config(const char *key,
+ const char *value)
+{
+ if (!strcasecmp(value, "always"))
+ return PROTOCOL_ALLOW_ALWAYS;
+ else if (!strcasecmp(value, "never"))
+ return PROTOCOL_ALLOW_NEVER;
+ else if (!strcasecmp(value, "user"))
+ return PROTOCOL_ALLOW_USER_ONLY;
+
+ die("unknown value for config '%s': %s", key, value);
+}
+
+static enum protocol_allow_config get_protocol_config(const char *type)
+{
+ char *key = xstrfmt("protocol.%s.allow", type);
+ char *value;
+
+ /* first check the per-protocol config */
+ if (!git_config_get_string(key, &value)) {
+ enum protocol_allow_config ret =
+ parse_protocol_config(key, value);
+ free(key);
+ free(value);
+ return ret;
+ }
+ free(key);
+
+ /* if defined, fallback to user-defined default for unknown protocols */
+ if (!git_config_get_string("protocol.allow", &value)) {
+ enum protocol_allow_config ret =
+ parse_protocol_config("protocol.allow", value);
+ free(value);
+ return ret;
+ }
+
+ /* fallback to built-in defaults */
+ /* known safe */
+ if (!strcmp(type, "http") ||
+ !strcmp(type, "https") ||
+ !strcmp(type, "git") ||
+ !strcmp(type, "ssh") ||
+ !strcmp(type, "file"))
+ return PROTOCOL_ALLOW_ALWAYS;
+
+ /* known scary; err on the side of caution */
+ if (!strcmp(type, "ext"))
+ return PROTOCOL_ALLOW_NEVER;
+
+ /* unknown; by default let them be used only directly by the user */
+ return PROTOCOL_ALLOW_USER_ONLY;
+}
+
int is_transport_allowed(const char *type)
{
- const struct string_list *allowed = protocol_whitelist();
- return !allowed || string_list_has_string(allowed, type);
+ const struct string_list *whitelist = protocol_whitelist();
+ if (whitelist)
+ return string_list_has_string(whitelist, type);
+
+ switch (get_protocol_config(type)) {
+ case PROTOCOL_ALLOW_ALWAYS:
+ return 1;
+ case PROTOCOL_ALLOW_NEVER:
+ return 0;
+ case PROTOCOL_ALLOW_USER_ONLY:
+ return git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
+ }
+
+ die("BUG: invalid protocol_allow_config type");
}
void transport_check_allowed(const char *type)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox