* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: Junio C Hamano @ 2013-02-05 17:40 UTC (permalink / raw)
To: Dale R. Worley; +Cc: git
In-Reply-To: <201302051700.r15H0GXx031004@freeze.ariadne.com>
worley@alum.mit.edu (Dale R. Worley) writes:
> I have found a situation where "git log" produces (apparently)
> endless output. Presumably this is a bug. Following is a (Linux)
> script that reliably reproduces the error for me (on Fedora 16):
Wasn't this fixed at v1.8.1.1~13 or is this a different issue?
^ permalink raw reply
* [PATCHv4] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-05 18:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <87k3qmr8yc.fsf@lifelogs.com>
Changes since PATCHv3:
- simple tests in Makefile
- support multiple files, code refactored
- documentation and comments updated
- fix IO::File for GPG pipe
- exit peacefully in almost every situation, die on bad invocation or query
- use log_verbose() and -v for logging for the user
- use log_debug() and -d for logging for the developer
- use Net::Netrc parser and `man netrc' to improve parsing
- ignore 'default' and 'macdef' netrc entries
- require 'machine' token in netrc lines
- ignore netrc files with bad permissions or owner (from Net::Netrc)
Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
contrib/credential/netrc/Makefile | 10 +
contrib/credential/netrc/git-credential-netrc | 423 +++++++++++++++++++++++++
2 files changed, 433 insertions(+), 0 deletions(-)
create mode 100644 contrib/credential/netrc/Makefile
create mode 100755 contrib/credential/netrc/git-credential-netrc
diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..ee8c5f0
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,10 @@
+test_netrc:
+ @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
+ @echo "-> Silent invocation... nothing should show up here with a missing file"
+ @echo "bad data" | ./git-credential-netrc -f A get
+ @echo "-> Back to noisy: -v and -d used below, missing file"
+ echo "bad data" | ./git-credential-netrc -f A -d -v get
+ @echo "-> Look for any entry in the default file set"
+ echo "" | ./git-credential-netrc -d -v get
+ @echo "-> Look for github.com in the default file set"
+ echo "host=google.com" | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..6946217
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,423 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+ help => 0,
+ debug => 0,
+ verbose => 0,
+ file => [],
+
+ # identical token maps, e.g. host -> host, will be inserted later
+ tmap => {
+ port => 'protocol',
+ machine => 'host',
+ path => 'path',
+ login => 'username',
+ user => 'username',
+ password => 'password',
+ }
+ );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+ $options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file. Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+ push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+ "help|h",
+ "debug|d",
+ "verbose|v",
+ "file|f=s@",
+ );
+
+if ($options{help}) {
+ my $shortname = basename($0);
+ $shortname =~ s/git-credential-//;
+
+ print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] get
+
+Version $VERSION by tzz\@lifelogs.com. License: BSD.
+
+Options:
+
+ -f|--file AUTHFILE : specify netrc-style files. Files with the .gpg extension
+ will be decrypted by GPG before parsing. Multiple -f
+ arguments are OK, and the order is respected.
+
+ -d|--debug : turn on debugging (developer info)
+
+ -v|--verbose : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+ git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+ git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+ git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper. It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+ The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+ The remote hostname for a network credential. (host=X)
+
+ 'path':
+ The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+ The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+port https login tzz
+
+OR
+
+protocol https login tzz
+
+OR... etc. acceptable tokens as listed above. Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol".
+
+Again, note that the first matching entry from all the AUTHFILEs is used.
+
+Tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+ exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files)
+{
+ my @candidates = qw[
+ ~/.authinfo.gpg
+ ~/.netrc.gpg
+ ~/.authinfo
+ ~/.netrc
+ ];
+
+ $files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files)
+{
+ unless (-r $file)
+ {
+ log_verbose("Unable to read $file; skipping it");
+ next FILE;
+ }
+
+ # the following check is copied from Net::Netrc
+ # OS/2 and Win32 do not handle stat in a way compatable with this check :-(
+ unless ($^O eq 'os2'
+ || $^O eq 'MSWin32'
+ || $^O eq 'MacOS'
+ || $^O =~ /^cygwin/)
+ {
+ my @stat = stat($file);
+
+ if (@stat) {
+ if ($stat[2] & 077) {
+ log_verbose("Insecure $file (mode=%04o); skipping it",
+ $stat[2] & 07777);
+ next FILE;
+ }
+ if ($stat[4] != $<) {
+ log_verbose("Not owner of $file; skipping it");
+ next FILE;
+ }
+ }
+ }
+
+ my $mode = (stat($file))[2];
+ if ($mode & 077)
+ {
+ log_verbose("Insecure $file (mode=%04o); skipping it",
+ $mode & 07777);
+ next FILE;
+ }
+
+ my @entries = load_netrc($file);
+
+ unless (scalar @entries)
+ {
+ if ($!)
+ {
+ log_verbose("Unable to open $file: $!");
+ }
+ else
+ {
+ log_verbose("No netrc entries found in $file");
+ }
+
+ next FILE;
+ }
+
+ my $entry = find_netrc_entry($query, @entries);
+ if ($entry)
+ {
+ print_credential_data($entry, $query);
+ # we're done!
+ last FILE;
+ }
+}
+
+exit 0;
+
+sub load_netrc
+{
+ my $file = shift @_;
+
+ my $io;
+ if ($file =~ m/\.gpg$/) {
+ log_verbose("Using GPG to open $file");
+ # GPG doesn't work well with 2- or 3-argument open
+ $io = new IO::File("gpg --decrypt $file|");
+ }
+ else {
+ log_verbose("Opening $file...");
+ $io = new IO::File($file, '<');
+ }
+
+ # nothing to do if the open failed (we log the error later)
+ return unless $io;
+
+ # Net::Netrc does this, but the functionality is merged with the file
+ # detection logic, so we have to extract just the part we need
+ my @netrc_entries = net_netrc_loader($io);
+
+ # these entries will use the credential helper protocol token names
+ my @entries;
+
+ foreach my $nentry (@netrc_entries) {
+ my %entry;
+ my $num_port;
+
+ if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+ $num_port = $nentry->{port};
+ delete $nentry->{port};
+ }
+
+ # create the new entry for the credential helper protocol
+ $entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+ # for "host X port Y" where Y is an integer (captured by
+ # $num_port above), set the host to "X:Y"
+ if (defined $entry{host} && defined $num_port) {
+ $entry{host} = join(':', $entry{host}, $num_port);
+ }
+
+ push @entries, \%entry;
+ }
+
+ return @entries;
+}
+
+sub net_netrc_loader
+{
+ my $fh = shift @_;
+ my @entries;
+ my ($mach, $macdef, $tok, @tok) = (0, 0);
+
+ LINE:
+ while (<$fh>) {
+ undef $macdef if /\A\n\Z/;
+
+ if ($macdef) {
+ push(@$macdef, $_);
+ next LINE;
+ }
+
+ s/^\s*//;
+ chomp;
+
+ while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+ (my $tok = $+) =~ s/\\(.)/$1/g;
+ push(@tok, $tok);
+ }
+
+ TOKEN:
+ while (@tok) {
+ $tok = shift(@tok);
+
+ if ($tok eq "machine") {
+ my $host = shift @tok;
+ $mach = { machine => $host };
+ push @entries, $mach;
+ }
+ elsif (exists $options{tmap}->{$tok}) {
+ unless ($mach) {
+ log_debug("Skipping token $tok because no machine was given");
+ next TOKEN;
+ }
+
+ my $value = shift @tok;
+ unless (defined $value) {
+ log_debug("Token $tok had no value, skipping it.");
+ next TOKEN;
+ }
+
+ # Following line added by rmerrell to remove '/' escape char in .netrc
+ $value =~ s/\/\\/\\/g;
+ $mach->{$tok} = $value;
+ }
+ elsif ($tok eq "macdef") { # we ignore macros
+ next TOKEN unless $mach;
+ my $value = shift @tok;
+ $mach->{macdef} = {} unless exists $mach->{macdef};
+ $macdef = $mach->{machdef}{$value} = [];
+ }
+ }
+ }
+
+ return @entries;
+}
+
+sub read_credential_data_from_stdin
+{
+ # the query: start with every token with no value
+ my %q = map { $_ => undef } values(%{$options{tmap}});
+
+ while (<STDIN>) {
+ next unless m/^([^=]+)=(.+)/;
+
+ my ($token, $value) = ($1, $2);
+ die "Unknown search token $token" unless exists $q{$token};
+ $q{$token} = $value;
+ log_debug("We were given search token $token and value $value");
+ }
+
+ foreach (sort keys %q) {
+ log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+ }
+
+ return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry
+{
+ my $query = shift @_;
+
+ ENTRY:
+ foreach my $entry (@_)
+ {
+ my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+ foreach my $check (sort keys %$query) {
+ if (defined $query->{$check}) {
+ log_debug("compare %s [%s] to [%s] (entry: %s)",
+ $check,
+ $entry->{$check},
+ $query->{$check},
+ $entry_text);
+ unless ($query->{$check} eq $entry->{$check}) {
+ next ENTRY;
+ }
+ }
+ else {
+ log_debug("OK: any value satisfies check $check");
+ }
+ }
+
+ return $entry;
+ }
+
+ # nothing was found
+ return;
+}
+
+sub print_credential_data
+{
+ my $entry = shift @_;
+ my $query = shift @_;
+
+ log_debug("entry has passed all the search checks");
+ TOKEN:
+ foreach my $git_token (sort keys %$entry) {
+ log_debug("looking for useful token $git_token");
+ # don't print unknown (to the credential helper protocol) tokens
+ next TOKEN unless exists $query->{$git_token};
+
+ # don't print things asked in the query (the entry matches them)
+ next TOKEN if defined $query->{$git_token};
+
+ log_debug("FOUND: $git_token=$entry->{$git_token}");
+ printf "%s=%s\n", $git_token, $entry->{$git_token};
+ }
+}
+sub log_verbose {
+ return unless $options{verbose};
+ printf STDERR @_;
+ printf STDERR "\n";
+}
+
+sub log_debug {
+ return unless $options{debug};
+ printf STDERR @_;
+ printf STDERR "\n";
+}
--
1.7.9.rc2
^ permalink raw reply related
* Re* [PATCH 3/3] apply: diagnose incomplete submodule object name better
From: Junio C Hamano @ 2013-02-05 19:19 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Martin von Zweigbergk
In-Reply-To: <1359693125-22357-4-git-send-email-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> We could read from the payload part of the patch to learn the full
> object name of the commit, but the primary user "git rebase" has
> been fixed to give us a full object name, so this should suffice
> for now.
And the patch on top to do so looks like this. With this patch in
place, we could drop 1/3 of this series, but there is no strong
reason to do so. After all, 1/3 is all about internal implementation
details.
-- >8 --
Subject: [PATCH 4/3] apply: verify submodule commit object name better
A textual patch also records the submodule commit object name in
full. Make the parsing more robust by reading from there and
verifying the (possibly abbreviated) name on the index line matches.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/apply.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 1f78e2c..e0f1474 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3587,6 +3587,40 @@ static int get_current_sha1(const char *path, unsigned char *sha1)
return 0;
}
+static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char sha1[20])
+{
+ /*
+ * A usable gitlink patch has only one fragment (hunk) that looks like:
+ * @@ -1 +1 @@
+ * -Subproject commit <old sha1>
+ * +Subproject commit <new sha1>
+ * or
+ * @@ -1 +0,0 @@
+ * -Subproject commit <old sha1>
+ * for a removal patch.
+ */
+ struct fragment *hunk = p->fragments;
+ static const char heading[] = "-Subproject commit ";
+ char *preimage;
+
+ if (/* does the patch have only one hunk? */
+ hunk && !hunk->next &&
+ /* is its preimage one line? */
+ hunk->oldpos == 1 && hunk->oldlines == 1 &&
+ /* does preimage begin with the heading? */
+ (preimage = memchr(hunk->patch, '\n', hunk->size)) != NULL &&
+ !prefixcmp(++preimage, heading) &&
+ /* does it record full SHA-1? */
+ !get_sha1_hex(preimage + sizeof(heading) - 1, sha1) &&
+ preimage[sizeof(heading) + 40 - 1] == '\n' &&
+ /* does the abbreviated name on the index line agree with it? */
+ !prefixcmp(preimage + sizeof(heading) - 1, p->old_sha1_prefix))
+ return 0; /* it all looks fine */
+
+ /* we may have full object name on the index line */
+ return get_sha1_hex(p->old_sha1_prefix, sha1);
+}
+
/* Build an index that contains the just the files needed for a 3way merge */
static void build_fake_ancestor(struct patch *list, const char *filename)
{
@@ -3607,8 +3641,10 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
continue;
if (S_ISGITLINK(patch->old_mode)) {
- if (get_sha1_hex(patch->old_sha1_prefix, sha1))
- die("submoule change for %s without full index name",
+ if (!preimage_sha1_in_gitlink_patch(patch, sha1))
+ ; /* ok, the textual part looks sane */
+ else
+ die("sha1 information is lacking or useless for submoule %s",
name);
} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
--
1.8.1.2.639.g9428735
^ permalink raw reply related
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-05 19:47 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87k3qmr8yc.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> JCH> Oh, another thing. 'default' is like 'machine' followed by any
> JCH> machine name, so the above while loop that reads two tokens
> JCH> pair-wise needs to be aware that 'default' is not followed by a
> JCH> value. I think the loop will fail to parse this:
>
> JCH> default login anonymous password me@home
> JCH> machine k.org login me password mysecret
>
> I'd prefer to ignore "default" because it should not be used for the Git
> credential helpers (its only use case is for anonymous services AFAIK).
> So I'll add a case to ignore it in PATCHv4, if that's OK.
You still need to parse a file that has a "default" entry correctly;
otherwise the users won't be able to share existing .netrc files
with other applications e.g. ftp, which is the whole point of this
series. Not using values from the "default" entry is probably fine,
though.
Thanks.
^ permalink raw reply
* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-05 19:53 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87fw1ar3og.fsf_-_@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> Changes since PATCHv3:
>
> - simple tests in Makefile
> - support multiple files, code refactored
> - documentation and comments updated
> - fix IO::File for GPG pipe
> - exit peacefully in almost every situation, die on bad invocation or query
> - use log_verbose() and -v for logging for the user
> - use log_debug() and -d for logging for the developer
> - use Net::Netrc parser and `man netrc' to improve parsing
> - ignore 'default' and 'macdef' netrc entries
> - require 'machine' token in netrc lines
> - ignore netrc files with bad permissions or owner (from Net::Netrc)
Please place the above _after_ the three-dashes.
The space here, above "---", is to justify why this change is a good
idea to people who see this patch for the first time who never saw
the earlier rounds of this patch, e.g. reading "git log" output 6
months down the road (see Documentation/SubmittingPatches "(2)
Describe your changes well").
>
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
> ---
> contrib/credential/netrc/Makefile | 10 +
> contrib/credential/netrc/git-credential-netrc | 423 +++++++++++++++++++++++++
> 2 files changed, 433 insertions(+), 0 deletions(-)
> create mode 100644 contrib/credential/netrc/Makefile
> create mode 100755 contrib/credential/netrc/git-credential-netrc
>
> diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
> new file mode 100644
> index 0000000..ee8c5f0
> --- /dev/null
> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,10 @@
> +test_netrc:
> + @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
> + @echo "-> Silent invocation... nothing should show up here with a missing file"
Avoid starting an argument to "echo" with a dash; some
implementations choke with "unknown option".
> + @echo "bad data" | ./git-credential-netrc -f A get
> + @echo "-> Back to noisy: -v and -d used below, missing file"
> + echo "bad data" | ./git-credential-netrc -f A -d -v get
> + @echo "-> Look for any entry in the default file set"
> + echo "" | ./git-credential-netrc -d -v get
> + @echo "-> Look for github.com in the default file set"
> + echo "host=google.com" | ./git-credential-netrc -d -v get
> diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
> new file mode 100755
> index 0000000..6946217
> --- /dev/null
> +++ b/contrib/credential/netrc/git-credential-netrc
> @@ -0,0 +1,423 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;
> +
> +use Getopt::Long;
> +use File::Basename;
> +
> +my $VERSION = "0.1";
> +
> +my %options = (
> + help => 0,
> + debug => 0,
> + verbose => 0,
> + file => [],
Looks like there is some funny indentation going on here.
> +
> + # identical token maps, e.g. host -> host, will be inserted later
> + tmap => {
> + port => 'protocol',
> + machine => 'host',
> + path => 'path',
> + login => 'username',
> + user => 'username',
> + password => 'password',
> + }
> + );
> +
> +# Map each credential protocol token to itself on the netrc side.
> +foreach (values %{$options{tmap}}) {
> + $options{tmap}->{$_} = $_;
> +}
> +
> +# Now, $options{tmap} has a mapping from the netrc format to the Git credential
> +# helper protocol.
> +
> +# Next, we build the reverse token map.
> +
> +# When $rmap{foo} contains 'bar', that means that what the Git credential helper
> +# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file. Keys in
> +# %rmap are what we expect to read from the netrc/authinfo file.
> +
> +my %rmap;
> +foreach my $k (keys %{$options{tmap}}) {
> + push @{$rmap{$options{tmap}->{$k}}}, $k;
> +}
> +
> +Getopt::Long::Configure("bundling");
> +
> +# TODO: maybe allow the token map $options{tmap} to be configurable.
> +GetOptions(\%options,
> + "help|h",
> + "debug|d",
> + "verbose|v",
> + "file|f=s@",
> + );
> +
> +if ($options{help}) {
> + my $shortname = basename($0);
> + $shortname =~ s/git-credential-//;
> +
> + print <<EOHIPPUS;
> +
> +$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] get
> +
> +Version $VERSION by tzz\@lifelogs.com. License: BSD.
> +
> +Options:
> +
> + -f|--file AUTHFILE : specify netrc-style files. Files with the .gpg extension
> + will be decrypted by GPG before parsing. Multiple -f
> + arguments are OK, and the order is respected.
Saying "order is respected" without mentioning the collision
resolution rules is not helpful to the users when deciding in what
order they should give these files. First one wins, or last one
wins? Later you say "looks for the first entry", but it will be
much easier to read the above to mention it here as well.
> + -d|--debug : turn on debugging (developer info)
> +
> + -v|--verbose : be more verbose (show files and information found)
> +
> +To enable this credential helper:
> +
> + git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
> +
> +(Note that Git will prepend "git-credential-" to the helper name and look for it
> +in the path.)
> +
> +...and if you want lots of debugging info:
> +
> + git config credential.helper '$shortname -f AUTHFILE -d'
> +
> +...or to see the files opened and data found:
> +
> + git config credential.helper '$shortname -f AUTHFILE -v'
> +
> +Only "get" mode is supported by this credential helper. It opens every AUTHFILE
> +and looks for the first entry that matches the requested search criteria:
> +
> + 'port|protocol':
> + The protocol that will be used (e.g., https). (protocol=X)
> +
> + 'machine|host':
> + The remote hostname for a network credential. (host=X)
> +
> + 'path':
> + The path with which the credential will be used. (path=X)
> +
> + 'login|user|username':
> + The credential’s username, if we already have one. (username=X)
> +
> +Thus, when we get this query on STDIN:
> +
> +protocol=https
> +username=tzz
> +
> +this credential helper will look for the first entry in every AUTHFILE that
> +matches
> +
> +port https login tzz
> +
> +OR
> +
> +protocol https login tzz
> +
> +OR... etc. acceptable tokens as listed above. Any unknown tokens are
> +simply ignored.
> +
> +Then, the helper will print out whatever tokens it got from the entry, including
> +"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
> +back to "protocol".
Isn't "hostname" typically what users expect to see? It is somewhat
unnerving to see an example that throws the same password back to
any host you happen to have an accoutn "tzz" on, even though that is
not technically an invalid way to use this helper.
> +Again, note that the first matching entry from all the AUTHFILEs is used.
> +
> +Tokens can be quoted as 'STRING' or "STRING".
> +
> +No caching is performed by this credential helper.
> +
> +EOHIPPUS
Otherwise, nice write-up.
> +my $mode = shift @ARGV;
> +
> +# Credentials must get a parameter, so die if it's missing.
> +die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
> +
> +# Only support 'get' mode; with any other unsupported ones we just exit.
> +exit 0 unless $mode eq 'get';
> +
> +my $files = $options{file};
> +
> +# if no files were given, use a predefined list.
> +# note that .gpg files come first
> +unless (scalar @$files)
> +{
> + my @candidates = qw[
> + ~/.authinfo.gpg
> + ~/.netrc.gpg
> + ~/.authinfo
> + ~/.netrc
> + ];
> +
> + $files = $options{file} = [ map { glob $_ } @candidates ];
> +}
> +
> +my $query = read_credential_data_from_stdin();
> +
> +FILE:
> +foreach my $file (@$files)
> +{
> + unless (-r $file)
> + {
> + log_verbose("Unable to read $file; skipping it");
> + next FILE;
> + }
> +
> + # the following check is copied from Net::Netrc
> + # OS/2 and Win32 do not handle stat in a way compatable with this check :-(
> + unless ($^O eq 'os2'
> + || $^O eq 'MSWin32'
> + || $^O eq 'MacOS'
> + || $^O =~ /^cygwin/)
> + {
> + my @stat = stat($file);
> +
> + if (@stat) {
> + if ($stat[2] & 077) {
> + log_verbose("Insecure $file (mode=%04o); skipping it",
> + $stat[2] & 07777);
Nice touch, although I am not sure rejecting world or group readable
encrypted file is absolutely necessary.
> + next FILE;
> + }
> + if ($stat[4] != $<) {
> + log_verbose("Not owner of $file; skipping it");
> + next FILE;
OK. A group of local users may share the same account at the
remote, but that would be unusual.
> + }
> + }
> + }
> +
> + my $mode = (stat($file))[2];
> + if ($mode & 077)
> + {
> + log_verbose("Insecure $file (mode=%04o); skipping it",
> + $mode & 07777);
Again? Didn't you just do this?
> + next FILE;
> + }
> +
> + my @entries = load_netrc($file);
> +
> + unless (scalar @entries)
> + {
> + if ($!)
> + {
> + log_verbose("Unable to open $file: $!");
> + }
> + else
> + {
> + log_verbose("No netrc entries found in $file");
> + }
I think the prevalent style is to
if (condition) {
do this;
} elsif (another condition) {
do that
} else {
do that other thing;
}
(this comment applies to all if/elsif/else cascades in this patch).
> +
> + next FILE;
Isn't this outermost loop, by the way? What the motivation to have
an explicit label everywhere (not complaining---it could be your own
discipline thing---just wondering).
> + }
> +
> + my $entry = find_netrc_entry($query, @entries);
> + if ($entry)
> + {
> + print_credential_data($entry, $query);
> + # we're done!
> + last FILE;
> + }
> +}
> +
> +exit 0;
> +
> +sub load_netrc
> +{
> + my $file = shift @_;
> +
> + my $io;
> + if ($file =~ m/\.gpg$/) {
> + log_verbose("Using GPG to open $file");
> + # GPG doesn't work well with 2- or 3-argument open
If that is the case, please quote $file properly against shell
munging it.
The only thing you do on $io is to read from it via "while (<$io>)",
so I would personally have written this part like this without
having to use IO::File(), though:
$io = open("-|", qw(gpg --decrypt), $file);
Similarly for the plain file:
$io = open("<", $file);
> + $io = new IO::File("gpg --decrypt $file|");
> + }
> + else {
> + log_verbose("Opening $file...");
> + $io = new IO::File($file, '<');
> + }
> +
> + # nothing to do if the open failed (we log the error later)
> + return unless $io;
> +
> + # Net::Netrc does this, but the functionality is merged with the file
> + # detection logic, so we have to extract just the part we need
> + my @netrc_entries = net_netrc_loader($io);
> +
> + # these entries will use the credential helper protocol token names
> + my @entries;
> +
> + foreach my $nentry (@netrc_entries) {
> + my %entry;
> + my $num_port;
> +
> + if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
> + $num_port = $nentry->{port};
> + delete $nentry->{port};
> + }
> +
> + # create the new entry for the credential helper protocol
> + $entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
> +
> + # for "host X port Y" where Y is an integer (captured by
> + # $num_port above), set the host to "X:Y"
> + if (defined $entry{host} && defined $num_port) {
> + $entry{host} = join(':', $entry{host}, $num_port);
> + }
> +
> + push @entries, \%entry;
> + }
> +
> + return @entries;
> +}
> +
> +sub net_netrc_loader
> +{
> + my $fh = shift @_;
> + my @entries;
> + my ($mach, $macdef, $tok, @tok) = (0, 0);
I think you meant to use $mach as a reference to a hash and $macdef
as a reference to an array; do you want to initialize them to
numeric zeros?
(The remainder of the patch unsnipped for others' reference).
Thanks.
> + LINE:
> + while (<$fh>) {
> + undef $macdef if /\A\n\Z/;
> +
> + if ($macdef) {
> + push(@$macdef, $_);
> + next LINE;
> + }
> +
> + s/^\s*//;
> + chomp;
> +
> + while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
> + (my $tok = $+) =~ s/\\(.)/$1/g;
> + push(@tok, $tok);
> + }
> +
> + TOKEN:
> + while (@tok) {
> + $tok = shift(@tok);
> +
> + if ($tok eq "machine") {
> + my $host = shift @tok;
> + $mach = { machine => $host };
> + push @entries, $mach;
> + }
> + elsif (exists $options{tmap}->{$tok}) {
> + unless ($mach) {
> + log_debug("Skipping token $tok because no machine was given");
> + next TOKEN;
> + }
> +
> + my $value = shift @tok;
> + unless (defined $value) {
> + log_debug("Token $tok had no value, skipping it.");
> + next TOKEN;
> + }
> +
> + # Following line added by rmerrell to remove '/' escape char in .netrc
> + $value =~ s/\/\\/\\/g;
> + $mach->{$tok} = $value;
> + }
> + elsif ($tok eq "macdef") { # we ignore macros
> + next TOKEN unless $mach;
> + my $value = shift @tok;
> + $mach->{macdef} = {} unless exists $mach->{macdef};
> + $macdef = $mach->{machdef}{$value} = [];
> + }
> + }
> + }
> +
> + return @entries;
> +}
> +
> +sub read_credential_data_from_stdin
> +{
> + # the query: start with every token with no value
> + my %q = map { $_ => undef } values(%{$options{tmap}});
> +
> + while (<STDIN>) {
> + next unless m/^([^=]+)=(.+)/;
> +
> + my ($token, $value) = ($1, $2);
> + die "Unknown search token $token" unless exists $q{$token};
> + $q{$token} = $value;
> + log_debug("We were given search token $token and value $value");
> + }
> +
> + foreach (sort keys %q) {
> + log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
> + }
> +
> + return \%q;
> +}
> +
> +# takes the search tokens and then a list of entries
> +# each entry is a hash reference
> +sub find_netrc_entry
> +{
> + my $query = shift @_;
> +
> + ENTRY:
> + foreach my $entry (@_)
> + {
> + my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
> + foreach my $check (sort keys %$query) {
> + if (defined $query->{$check}) {
> + log_debug("compare %s [%s] to [%s] (entry: %s)",
> + $check,
> + $entry->{$check},
> + $query->{$check},
> + $entry_text);
> + unless ($query->{$check} eq $entry->{$check}) {
> + next ENTRY;
> + }
> + }
> + else {
> + log_debug("OK: any value satisfies check $check");
> + }
> + }
> +
> + return $entry;
> + }
> +
> + # nothing was found
> + return;
> +}
> +
> +sub print_credential_data
> +{
> + my $entry = shift @_;
> + my $query = shift @_;
> +
> + log_debug("entry has passed all the search checks");
> + TOKEN:
> + foreach my $git_token (sort keys %$entry) {
> + log_debug("looking for useful token $git_token");
> + # don't print unknown (to the credential helper protocol) tokens
> + next TOKEN unless exists $query->{$git_token};
> +
> + # don't print things asked in the query (the entry matches them)
> + next TOKEN if defined $query->{$git_token};
> +
> + log_debug("FOUND: $git_token=$entry->{$git_token}");
> + printf "%s=%s\n", $git_token, $entry->{$git_token};
> + }
> +}
> +sub log_verbose {
> + return unless $options{verbose};
> + printf STDERR @_;
> + printf STDERR "\n";
> +}
> +
> +sub log_debug {
> + return unless $options{debug};
> + printf STDERR @_;
> + printf STDERR "\n";
> +}
^ permalink raw reply
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-05 20:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vip66sftf.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 11:47:56 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
JCH> Oh, another thing. 'default' is like 'machine' followed by any
JCH> machine name, so the above while loop that reads two tokens
JCH> pair-wise needs to be aware that 'default' is not followed by a
JCH> value. I think the loop will fail to parse this:
>>
JCH> default login anonymous password me@home
JCH> machine k.org login me password mysecret
>>
>> I'd prefer to ignore "default" because it should not be used for the Git
>> credential helpers (its only use case is for anonymous services AFAIK).
>> So I'll add a case to ignore it in PATCHv4, if that's OK.
JCH> You still need to parse a file that has a "default" entry correctly;
JCH> otherwise the users won't be able to share existing .netrc files
JCH> with other applications e.g. ftp, which is the whole point of this
JCH> series. Not using values from the "default" entry is probably fine,
JCH> though.
OK; done in PATCHv4.
Ted
^ permalink raw reply
* Re: Is anyone working on a next-gen Git protocol (Re: [PATCH v3 0/8] Hiding refs)
From: Ævar Arnfjörð Bjarmason @ 2013-02-05 20:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Shawn Pearce, Johannes Schindelin
In-Reply-To: <7vfw1avjcw.fsf@alter.siamese.dyndns.org>
On Tue, Feb 5, 2013 at 5:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Do you have any plans for something that *does* have the reduction of
>> network bandwidth as a primary goal?
>
> Uncluttering gives reduction of bandwidth anyway, so I do not see
> much point in the distinction you seem to be making.
Doing this work wouldn't only give us a way to specify which refs we
want, but if done correctly would future-proof the protocol in case we
want to add any other extensions down the line in a
backwards-compatible fashion without having the server first spew all
his refs at us.
Anyway, an implementation that allows a client to say "I want X" is
simpler than an implementation where a server has to anticipate in
advance which X the clients will ask for.
>> Is this what you've been working on? Because if so I misunderstood you
>> thinking you were going to work on something that gave clients the
>> ability specify what they wanted before the initial ref advertisement.
>> ...
>> 4. http://thread.gmane.org/gmane.comp.version-control.git/207190
>
> "Who speaks first" mentioned in 4. above, was primarily about
> "delaying ref advertisement", which would be a larger protocol
> change. Nobody seems to have attacked it since it was discussed,
> and I was tired of hearing nothing but complaints and whines. This
> "hiding refs" series was done as a cheaper way to solve a related
> issue, without having to wait for the solution of "delaying
> advertisement", which is an orthogonal issue.
Oh sure. I just wanted to know if you were working on delaying ref
advertisement to avoid duplicating efforts. I had the impression you
were given your earlier E-Mail, but obviously we had a
misunderstanding.
^ permalink raw reply
* Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters
From: Jeff King @ 2013-02-05 20:11 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, git
In-Reply-To: <5111317E.8060906@drmicha.warpmail.net>
On Tue, Feb 05, 2013 at 05:21:18PM +0100, Michael J Gruber wrote:
> Thanks Jeff, that helps a lot! It covers "grep expr" and "grep expr rev
> -- path" just fine. I'll look into "grep expr rev:path" which does not
> work yet because of an empty driver.
>
> I also have "show --textconv" covered and a suggestion for "cat-file
> --textconv" (to work without a textconv filter).
>
> Expect a mini-series soon :)
Cool, I'm glad it helped. It would be great if diff_filespec and
grep_source could grow together into a unified object. One of the gross
things about the patch I posted is that we will now sometimes read the
file/blob data via grep_source_load, and sometimes via
diff_populate_filespec. They _should_ be equivalent, but in an ideal
world, they would be the same code path.
That may be too much to tackle for your series, though (I wanted to do
it when I factored out grep_source, but backed off for the same reason).
The "grep expr rev:path" fix should look something like this:
diff --git a/builtin/grep.c b/builtin/grep.c
index 915c8ef..cdc7d32 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -820,13 +820,17 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
unsigned char sha1[20];
+ struct object_context oc;
/* Is it a rev? */
- if (!get_sha1(arg, sha1)) {
+ if (!get_sha1_with_context(arg, sha1, &oc)) {
struct object *object = parse_object(sha1);
if (!object)
die(_("bad object %s"), arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
+ /* oops, we need something that will remember oc.path
+ * here, so that we can pass it along to
+ * grep_source_init */
add_object_array(object, arg, &list);
continue;
}
But you'll have to replace the object_array with something more
featureful, I think.
-Peff
^ permalink raw reply related
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-05 20:23 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87bobyr0ju.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> JCH> You still need to parse a file that has a "default" entry correctly;
> JCH> otherwise the users won't be able to share existing .netrc files
> JCH> with other applications e.g. ftp, which is the whole point of this
> JCH> series. Not using values from the "default" entry is probably fine,
> JCH> though.
>
> OK; done in PATCHv4.
Hmph.
Didn't you remove that from your version of net_netrc_loader when
you borrowed the bulk of the code from Net::Netrc::_readrc? I see
"default" token handled at the beginning of "TOKEN: while (@tok)"
loop in the original but not in your version I see in v4.
^ permalink raw reply
* [PATCH] t4038: add tests for "diff --cc --raw <trees>"
From: John Keeping @ 2013-02-05 20:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Antoine Pelisse
In-Reply-To: <7vip696i3v.fsf@alter.siamese.dyndns.org>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Sun, Feb 03, 2013 at 04:24:52PM -0800, Junio C Hamano wrote:
> Ideally it should also have test cases
> to show "git diff --cc --raw blob1 blob2...blob$n" for n=4 and n=40
> (or any two values clearly below and above the old hardcoded limit)
> behave sensibly, exposing the old breakage, which I'll leave as a
> LHF (low-hanging-fruit). Hint, hint...
Hint taken ;-)
git-diff uses a different code path for blobs, so I've had to use trees
to trigger this. The last test fails without
jc/combine-diff-many-parents and passes with it.
t/t4038-diff-combined.sh | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 40277c7..a0701bc 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -89,4 +89,33 @@ test_expect_success 'diagnose truncated file' '
grep "diff --cc file" out
'
+test_expect_success 'setup for --cc --raw' '
+ blob=$(echo file |git hash-object --stdin -w) &&
+ base_tree=$(echo "100644 blob $blob file" | git mktree) &&
+ trees= &&
+ for i in `test_seq 1 40`
+ do
+ blob=$(echo file$i |git hash-object --stdin -w) &&
+ trees="$trees $(echo "100644 blob $blob file" |git mktree)"
+ done
+'
+
+test_expect_success 'check --cc --raw with four trees' '
+ four_trees=$(echo "$trees" |awk -e "{
+ print \$1
+ print \$2
+ print \$3
+ print \$4
+ }") &&
+ git diff --cc --raw $four_trees $base_tree >out &&
+ # Check for four leading colons in the output:
+ grep "^::::[^:]" out
+'
+
+test_expect_success 'check --cc --raw with forty trees' '
+ git diff --cc --raw $trees $base_tree >out &&
+ # Check for forty leading colons in the output:
+ grep "^::::::::::::::::::::::::::::::::::::::::[^:]" out
+'
+
test_done
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-02-05 20:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Jonathan Nieder, git, kraai
In-Reply-To: <7v4ni2y1fm.fsf@alter.siamese.dyndns.org>
On 27.01.13 18:34, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Back to the which:
>> ...
>> and running "make test" gives the following, at least in my system:
>> ...
> I think everybody involved in this discussion already knows that;
> the point is that it can easily give false negative, without the
> scripts working very hard to do so.
>
> If we did not care about incurring runtime performance cost, we
> could arrange:
>
> - the test framework to define a variable $TEST_ABORT that has a
> full path to a file that is in somewhere test authors cannot
> touch unless they really try hard to (i.e. preferrably outside
> $TRASH_DIRECTORY, as it is not uncommon for to tests to do "rm *"
> there). This location should be per $(basename "$0" .sh) to allow
> running multiple tests in paralell;
>
> - the test framework to "rm -f $TEST_ABORT" at the beginning of
> test_expect_success/failure;
>
> - test_expect_success/failure to check $TEST_ABORT and if it
> exists, abort the execution, showing the contents of the file as
> an error message.
>
> Then you can wrap commands whose use we want to limit, perhaps like
> this, in the test framework:
>
> which () {
> cat >"$TEST_ABORT" <<-\EOF
> Do not use unportable 'which' in the test script.
> "if type $cmd" is a good way to see if $cmd exists.
> EOF
> }
>
> sed () {
> saw_wantarg= must_abort=
> for arg
> do
> if test -n "$saw_wantarg"
> then
> saw_wantarg=
> continue
> fi
> case "$arg" in
> --) break ;; # end of options
> -i) echo >"$TEST_ABORT" "Do not use 'sed -i'"
> must_abort=yes
> break ;;
> -e) saw_wantarg=yes ;; # skip next arg
> -*) continue ;; # options without arg
> *) break ;; # filename
> esac
> done
> if test -z "$must_abort"
> sed "$@"
> fi
> }
>
> Then you can check that TEST_ABORT does not appear in test scripts
> (ensuring that they do not attempt to circumvent the mechanis) and
> catch use of unwanted commands or unwanted extended features of
> commands at runtime.
>
> But this will incur runtime performace hit, so I am not sure it
> would be worth it.
Thanks for the detailed suggestion.
Instead of using a file for putting out non portable syntax,
can we can use a similar logic as test_failure ?
I did some benchmarking, the test suite on a Laptop is 37..38 minutes,
including make clean && make both on next, pu, master or with the patch below.
I couldn't find a measurable impact on the execution time.
What do we think about a patch like this
(Not sure if this cut-and-paste data applies, it's for review)
[PATCH] test-lib: which, echo -n and sed -i are not portable
The posix version of sed command supports options -n -e -f
The gnu extension -i to edit a file in place is not
available on all systems.
To catch the usage of non-posix options with sed a
wrapper function is added in test-lib.sh.
The wrapper checks that only -n -e -f are used.
The short form "-ne" for "-n -e" is allowed as well.
echo -n is not portable and not available on all systems,
printf can be used instead.
Add a wrapper to catch echo -n
which is not portable, the output differs between different
implementations, and the return code may not be reliable.
Add a function test_bad_syntax_ in test-lib.sh, which increments
the variable test_bad_syntax and works similar to test_failure_
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/test-lib.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a6c4ab..248ed34 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -266,6 +266,7 @@ else
exec 4>/dev/null 3>/dev/null
fi
+test_bad_syntax=0
test_failure=0
test_count=0
test_fixed=0
@@ -300,6 +301,12 @@ test_ok_ () {
say_color "" "ok $test_count - $@"
}
+test_bad_syntax_ () {
+ test_bad_syntax=$(($test_bad_syntax + 1))
+ say_color error "$@"
+ test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+}
+
test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error "not ok $test_count - $1"
@@ -402,10 +409,15 @@ test_done () {
fixed $test_fixed
broken $test_broken
failed $test_failure
+ bad_syntax $test_bad_syntax
EOF
fi
+ if test "$test_bad_syntax" != 0
+ then
+ say_color error "# $test_bad_syntax non portable shell syntax"
+ fi
if test "$test_fixed" != 0
then
say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
@@ -645,6 +657,40 @@ yes () {
done
}
+
+# which is not portable
+which () {
+ test_bad_syntax_ "Do not use unportable 'which' in the test script." \
+ "'if type $1' is a good way to see if '$1' exists."
+ return 1
+}
+
+# catch non-portable usage of sed
+sed () {
+ for arg
+ do
+ case "$arg" in
+ -[efn]) continue ;; # allowed posix options
+ -ne) continue ;; # tolerated
+ -*)test_bad_syntax_ "Do not use 'sed "$arg"'. Valid options for 'sed' are -n -e -f"
+ return 1
+ ;;
+ *) continue ;;
+ esac
+ done
+ command sed "$@"
+}
+
+# catch non portable echo -n
+echo () {
+ if test "$1" = -n
+ then
+ test_bad_syntax_ "Do not use 'echo -n'. Use printf instead"
+ else
+ command echo "$@"
+ fi
+}
+
# Fix some commands on Windows
case $(uname -s) in
*MINGW*)
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-05 20:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vhalqsfkf.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 11:53:20 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> Changes since PATCHv3:
>>
>> - simple tests in Makefile
>> - support multiple files, code refactored
>> - documentation and comments updated
>> - fix IO::File for GPG pipe
>> - exit peacefully in almost every situation, die on bad invocation or query
>> - use log_verbose() and -v for logging for the user
>> - use log_debug() and -d for logging for the developer
>> - use Net::Netrc parser and `man netrc' to improve parsing
>> - ignore 'default' and 'macdef' netrc entries
>> - require 'machine' token in netrc lines
>> - ignore netrc files with bad permissions or owner (from Net::Netrc)
JCH> Please place the above _after_ the three-dashes.
JCH> The space here, above "---", is to justify why this change is a good
JCH> idea to people who see this patch for the first time who never saw
JCH> the earlier rounds of this patch, e.g. reading "git log" output 6
JCH> months down the road (see Documentation/SubmittingPatches "(2)
JCH> Describe your changes well").
Will do in PATCHv5.
JCH> Avoid starting an argument to "echo" with a dash; some
JCH> implementations choke with "unknown option".
Nice, thanks. It's purely decorative so I use '=>' now.
>> +my %options = (
>> + help => 0,
>> + debug => 0,
>> + verbose => 0,
>> + file => [],
JCH> Looks like there is some funny indentation going on here.
Fixed.
>> + -f|--file AUTHFILE : specify netrc-style files. Files with the .gpg extension
>> + will be decrypted by GPG before parsing. Multiple -f
>> + arguments are OK, and the order is respected.
JCH> Saying "order is respected" without mentioning the collision
JCH> resolution rules is not helpful to the users when deciding in what
JCH> order they should give these files. First one wins, or last one
JCH> wins? Later you say "looks for the first entry", but it will be
JCH> much easier to read the above to mention it here as well.
Right. Reworded.
>> +Thus, when we get this query on STDIN:
>> +
>> +protocol=https
>> +username=tzz
>> +
>> +this credential helper will look for the first entry in every AUTHFILE that
>> +matches
>> +
>> +port https login tzz
>> +
>> +OR
>> +
>> +protocol https login tzz
>> +
>> +OR... etc. acceptable tokens as listed above. Any unknown tokens are
>> +simply ignored.
>> +
>> +Then, the helper will print out whatever tokens it got from the entry, including
>> +"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
>> +back to "protocol".
JCH> Isn't "hostname" typically what users expect to see? It is somewhat
JCH> unnerving to see an example that throws the same password back to
JCH> any host you happen to have an accoutn "tzz" on, even though that is
JCH> not technically an invalid way to use this helper.
Yeah, I changed it to show "machine" in the query (which would be more typical).
>> + if ($stat[2] & 077) {
>> + log_verbose("Insecure $file (mode=%04o); skipping it",
>> + $stat[2] & 07777);
JCH> Nice touch, although I am not sure rejecting world or group readable
JCH> encrypted file is absolutely necessary.
Right. Fixed.
>> + if ($stat[4] != $<) {
>> + log_verbose("Not owner of $file; skipping it");
>> + next FILE;
JCH> OK. A group of local users may share the same account at the
JCH> remote, but that would be unusual.
I added --insecure/-k to override this check.
>> + if ($mode & 077)
JCH> Again? Didn't you just do this?
Damn, sorry.
JCH> I think the prevalent style is to
JCH> if (condition) {
JCH> do this;
JCH> } elsif (another condition) {
JCH> do that
JCH> } else {
JCH> do that other thing;
JCH> }
JCH> (this comment applies to all if/elsif/else cascades in this patch).
Yup. I was working with Net::Netrc code and forgot to reformat it. Sorry.
>> +
>> + next FILE;
JCH> Isn't this outermost loop, by the way? What the motivation to have
JCH> an explicit label everywhere (not complaining---it could be your own
JCH> discipline thing---just wondering).
I think it's more readable with large loops, and it actually makes sense
when you read the code. Not a big deal to me either, I just felt for
this particular script it was OK.
>> + if ($file =~ m/\.gpg$/) {
>> + log_verbose("Using GPG to open $file");
>> + # GPG doesn't work well with 2- or 3-argument open
JCH> If that is the case, please quote $file properly against shell
JCH> munging it.
Ahhh that gets ugly. OK, quoted.
JCH> The only thing you do on $io is to read from it via "while (<$io>)",
JCH> so I would personally have written this part like this without
JCH> having to use IO::File(), though:
JCH> $io = open("-|", qw(gpg --decrypt), $file);
That doesn't work for me, unfortunately. I'm trying to avoid the IPC::*
modules and such. Please test it yourself with GPG. I'm on Perl
5.14.2.
I think it's OK with the quoting, as you'll see in PATCHv5.
JCH> Similarly for the plain file:
JCH> $io = open("<", $file);
You mean "open $io, '<', $file". open() returns nonzero on success and
undef on failure. OK, I'll use open() instead of IO::File.
>> + my ($mach, $macdef, $tok, @tok) = (0, 0);
JCH> I think you meant to use $mach as a reference to a hash and $macdef
JCH> as a reference to an array; do you want to initialize them to
JCH> numeric zeros?
That actually came from Net::Netrc. The $mach default is OK either way;
I left it undefined so it's clearer. I think the $macdef initial value
is a bug (which, I guess, shows macros are very rare); I just made it a
boolean for our purposes.
Ted
^ permalink raw reply
* Re: [PATCH] t4038: add tests for "diff --cc --raw <trees>"
From: Junio C Hamano @ 2013-02-05 20:48 UTC (permalink / raw)
To: John Keeping; +Cc: git, Antoine Pelisse
In-Reply-To: <20130205202558.GX1342@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
> ...
> diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
> index 40277c7..a0701bc 100755
> --- a/t/t4038-diff-combined.sh
> +++ b/t/t4038-diff-combined.sh
> @@ -89,4 +89,33 @@ test_expect_success 'diagnose truncated file' '
> grep "diff --cc file" out
> '
>
> +test_expect_success 'setup for --cc --raw' '
> + blob=$(echo file |git hash-object --stdin -w) &&
> + base_tree=$(echo "100644 blob $blob file" | git mktree) &&
> + trees= &&
> + for i in `test_seq 1 40`
> + do
> + blob=$(echo file$i |git hash-object --stdin -w) &&
> + trees="$trees $(echo "100644 blob $blob file" |git mktree)"
Please have a SP after each of these '|' pipes.
If you collect trees this way:
trees="$trees$(echo ... | git mktree)$LF"
then ...
> + done
> +'
> +
> +test_expect_success 'check --cc --raw with four trees' '
> + four_trees=$(echo "$trees" |awk -e "{
> + print \$1
> + print \$2
> + print \$3
> + print \$4
> + }") &&
(What's "awk -e"?)
... you can do
echo "$trees" | sed -e 4q
which is less repetitive.
Thanks.
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-02-05 20:52 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jonathan Nieder, git, kraai
In-Reply-To: <51116D3E.3070409@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> Thanks for the detailed suggestion.
> Instead of using a file for putting out non portable syntax,
> can we can use a similar logic as test_failure ?
Your test_bad_syntax_ function can be called from a subshell, and
its "exit 1" will not exit, no?
test_expect_success 'prepare a blob with incomplete line' '
(
echo first line
echo second line
echo -n final and incomplete line
) >incomplete.txt
'
^ permalink raw reply
* [PATCHv5] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-05 20:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vhalqsfkf.fsf@alter.siamese.dyndns.org>
Add Git credential helper that can parse netrc/authinfo files.
This credential helper support multiple files, returning the first one
that matches. It checks file permissions and owner. For *.gpg files,
it will run GPG to decrypt the file.
Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Changes since PATCHv4:
- indentation and brace fixes
- test makefile uses "=>" as the decorative prefix
- documentation fixes about order and show query with "hostname"
- add --insecure to ignore owner and permission checks
- check permissions just once and only for unencrypted files
- change IO::File to simple open() and quote $file
- fixed macdef buglet from Net::Netrc
- ignore 'default' entries
contrib/credential/netrc/Makefile | 12 +
contrib/credential/netrc/git-credential-netrc | 424 +++++++++++++++++++++++++
2 files changed, 436 insertions(+), 0 deletions(-)
create mode 100644 contrib/credential/netrc/Makefile
create mode 100755 contrib/credential/netrc/git-credential-netrc
diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..18a924f
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+ @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
+ @echo "=> Silent invocation... nothing should show up here with a missing file"
+ @echo "bad data" | ./git-credential-netrc -f A get
+ @echo "=> Back to noisy: -v and -d used below, missing file"
+ echo "bad data" | ./git-credential-netrc -f A -d -v get
+ @echo "=> Look for any entry in the default file set"
+ echo "" | ./git-credential-netrc -d -v get
+ @echo "=> Look for github.com in the default file set"
+ echo "host=google.com" | ./git-credential-netrc -d -v get
+ @echo "=> Look for a nonexistent machine in the default file set"
+ echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..8298564
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,424 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+ help => 0,
+ debug => 0,
+ verbose => 0,
+ insecure => 0,
+ file => [],
+
+ # identical token maps, e.g. host -> host, will be inserted later
+ tmap => {
+ port => 'protocol',
+ machine => 'host',
+ path => 'path',
+ login => 'username',
+ user => 'username',
+ password => 'password',
+ }
+ );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+ $options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file. Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+ push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+ "help|h",
+ "debug|d",
+ "insecure|k",
+ "verbose|v",
+ "file|f=s@",
+ );
+
+if ($options{help}) {
+ my $shortname = basename($0);
+ $shortname =~ s/git-credential-//;
+
+ print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com. License: BSD.
+
+Options:
+
+ -f|--file AUTHFILE : specify netrc-style files. Files with the .gpg extension
+ will be decrypted by GPG before parsing. Multiple -f
+ arguments are OK. They are processed in order, and the
+ first matching entry found is returned via the credential
+ helper protocol (see below).
+
+ -k|--insecure : ignore bad file ownership or permissions
+
+ -d|--debug : turn on debugging (developer info)
+
+ -v|--verbose : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+ git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+ git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+ git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper. It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+ The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+ The remote hostname for a network credential. (host=X)
+
+ 'path':
+ The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+ The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+host=github.com
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+machine github.com port https login tzz
+
+OR
+
+machine github.com protocol https login tzz
+
+OR... etc. acceptable tokens as listed above. Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol". Any redundant entry tokens (part of the original query) are
+skipped.
+
+Again, note that only the first matching entry from all the AUTHFILEs, processed
+in the sequence given on the command line, is used.
+
+Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+ exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files) {
+ my @candidates = qw[
+ ~/.authinfo.gpg
+ ~/.netrc.gpg
+ ~/.authinfo
+ ~/.netrc
+ ];
+
+ $files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files) {
+ my $gpgmode = $file =~ m/\.gpg$/;
+ unless (-r $file) {
+ log_verbose("Unable to read $file; skipping it");
+ next FILE;
+ }
+
+ # the following check is copied from Net::Netrc, for non-GPG files
+ # OS/2 and Win32 do not handle stat in a way compatable with this check :-(
+ unless ($gpgmode || $options{insecure} ||
+ $^O eq 'os2'
+ || $^O eq 'MSWin32'
+ || $^O eq 'MacOS'
+ || $^O =~ /^cygwin/) {
+ my @stat = stat($file);
+
+ if (@stat) {
+ if ($stat[2] & 077) {
+ log_verbose("Insecure $file (mode=%04o); skipping it",
+ $stat[2] & 07777);
+ next FILE;
+ }
+
+ if ($stat[4] != $<) {
+ log_verbose("Not owner of $file; skipping it");
+ next FILE;
+ }
+ }
+ }
+
+ my @entries = load_netrc($file, $gpgmode);
+
+ unless (scalar @entries) {
+ if ($!) {
+ log_verbose("Unable to open $file: $!");
+ }
+ else {
+ log_verbose("No netrc entries found in $file");
+ }
+
+ next FILE;
+ }
+
+ my $entry = find_netrc_entry($query, @entries);
+ if ($entry) {
+ print_credential_data($entry, $query);
+ # we're done!
+ last FILE;
+ }
+}
+
+exit 0;
+
+sub load_netrc {
+ my $file = shift @_;
+ my $gpgmode = shift @_;
+
+ my $io;
+ if ($gpgmode) {
+ # typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
+ my $f = $file;
+ $f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
+ # GPG doesn't work well with 2- or 3-argument open
+ my $cmd = "gpg --decrypt $f";
+ log_verbose("Using GPG to open $file: [$cmd]");
+ open $io, "$cmd|";
+ }
+ else {
+ log_verbose("Opening $file...");
+ open $io, '<', $file;
+ }
+
+ # nothing to do if the open failed (we log the error later)
+ return unless $io;
+
+ # Net::Netrc does this, but the functionality is merged with the file
+ # detection logic, so we have to extract just the part we need
+ my @netrc_entries = net_netrc_loader($io);
+
+ # these entries will use the credential helper protocol token names
+ my @entries;
+
+ foreach my $nentry (@netrc_entries) {
+ my %entry;
+ my $num_port;
+
+ if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+ $num_port = $nentry->{port};
+ delete $nentry->{port};
+ }
+
+ # create the new entry for the credential helper protocol
+ $entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+ # for "host X port Y" where Y is an integer (captured by
+ # $num_port above), set the host to "X:Y"
+ if (defined $entry{host} && defined $num_port) {
+ $entry{host} = join(':', $entry{host}, $num_port);
+ }
+
+ push @entries, \%entry;
+ }
+
+ return @entries;
+}
+
+sub net_netrc_loader {
+ my $fh = shift @_;
+ my @entries;
+ my ($mach, $macdef, $tok, @tok);
+
+ LINE:
+ while (<$fh>) {
+ undef $macdef if /\A\n\Z/;
+
+ if ($macdef) {
+ next LINE;
+ }
+
+ s/^\s*//;
+ chomp;
+
+ while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+ (my $tok = $+) =~ s/\\(.)/$1/g;
+ push(@tok, $tok);
+ }
+
+ TOKEN:
+ while (@tok) {
+ if ($tok[0] eq "default") {
+ shift(@tok);
+ undef $mach; # ignore 'default' lines
+
+ next TOKEN;
+ }
+
+ $tok = shift(@tok);
+
+ if ($tok eq "machine") {
+ my $host = shift @tok;
+ $mach = { machine => $host };
+ push @entries, $mach;
+ }
+ elsif (exists $options{tmap}->{$tok}) {
+ unless ($mach) {
+ log_debug("Skipping token $tok because no machine was given");
+ next TOKEN;
+ }
+
+ my $value = shift @tok;
+ unless (defined $value) {
+ log_debug("Token $tok had no value, skipping it.");
+ next TOKEN;
+ }
+
+ # Following line added by rmerrell to remove '/' escape char in .netrc
+ $value =~ s/\/\\/\\/g;
+ $mach->{$tok} = $value;
+ }
+ elsif ($tok eq "macdef") { # we ignore macros
+ next TOKEN unless $mach;
+ my $value = shift @tok;
+ $macdef = 1;
+ }
+ }
+ }
+
+ return @entries;
+}
+
+sub read_credential_data_from_stdin {
+ # the query: start with every token with no value
+ my %q = map { $_ => undef } values(%{$options{tmap}});
+
+ while (<STDIN>) {
+ next unless m/^([^=]+)=(.+)/;
+
+ my ($token, $value) = ($1, $2);
+ die "Unknown search token $token" unless exists $q{$token};
+ $q{$token} = $value;
+ log_debug("We were given search token $token and value $value");
+ }
+
+ foreach (sort keys %q) {
+ log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+ }
+
+ return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry {
+ my $query = shift @_;
+
+ ENTRY:
+ foreach my $entry (@_)
+ {
+ my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+ foreach my $check (sort keys %$query) {
+ if (defined $query->{$check}) {
+ log_debug("compare %s [%s] to [%s] (entry: %s)",
+ $check,
+ $entry->{$check},
+ $query->{$check},
+ $entry_text);
+ unless ($query->{$check} eq $entry->{$check}) {
+ next ENTRY;
+ }
+ }
+ else {
+ log_debug("OK: any value satisfies check $check");
+ }
+ }
+
+ return $entry;
+ }
+
+ # nothing was found
+ return;
+}
+
+sub print_credential_data {
+ my $entry = shift @_;
+ my $query = shift @_;
+
+ log_debug("entry has passed all the search checks");
+ TOKEN:
+ foreach my $git_token (sort keys %$entry) {
+ log_debug("looking for useful token $git_token");
+ # don't print unknown (to the credential helper protocol) tokens
+ next TOKEN unless exists $query->{$git_token};
+
+ # don't print things asked in the query (the entry matches them)
+ next TOKEN if defined $query->{$git_token};
+
+ log_debug("FOUND: $git_token=$entry->{$git_token}");
+ printf "%s=%s\n", $git_token, $entry->{$git_token};
+ }
+}
+sub log_verbose {
+ return unless $options{verbose};
+ printf STDERR @_;
+ printf STDERR "\n";
+}
+
+sub log_debug {
+ return unless $options{debug};
+ printf STDERR @_;
+ printf STDERR "\n";
+}
--
1.7.9.rc2
^ permalink raw reply related
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-05 21:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vd2wese6z.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 12:23:00 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
JCH> You still need to parse a file that has a "default" entry correctly;
JCH> otherwise the users won't be able to share existing .netrc files
JCH> with other applications e.g. ftp, which is the whole point of this
JCH> series. Not using values from the "default" entry is probably fine,
JCH> though.
>>
>> OK; done in PATCHv4.
JCH> Hmph.
JCH> Didn't you remove that from your version of net_netrc_loader when
JCH> you borrowed the bulk of the code from Net::Netrc::_readrc? I see
JCH> "default" token handled at the beginning of "TOKEN: while (@tok)"
JCH> loop in the original but not in your version I see in v4.
Damn, I accidentally moved that check to /dev/null. OK, I added it in
PATCHv5. Thanks for catching that.
Ted
^ permalink raw reply
* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: Matthieu Moy @ 2013-02-05 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dale R. Worley, git
In-Reply-To: <7vtxpqslpm.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> worley@alum.mit.edu (Dale R. Worley) writes:
>
>> I have found a situation where "git log" produces (apparently)
>> endless output. Presumably this is a bug. Following is a (Linux)
>> script that reliably reproduces the error for me (on Fedora 16):
>
> Wasn't this fixed at v1.8.1.1~13 or is this a different issue?
In any case, I can't reproduce with 1.8.1.2.526.gf51a757: I don't get
undless output. On the other hand, I get a slightly misformatted output:
* commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 2c1e6a36f4b712e914fac994463da7d0fdb2bc6d)
|\ Merge: 2c1e6a3 33e70e7
| | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
| | Date: Tue Feb 5 22:05:33 2013 +0100
| |
| | Commit S
| |
| | diff --git a/file b/file
| | index 6bb4d3e..afd2e75 100644
| | --- a/file
| | +++ b/file
| | @@ -1,4 +1,5 @@
| | 1
| | 1a
| | 2
| | +2a
| | 3
| |
commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
| | Merge: 2c1e6a3 33e70e7
| | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
| | Date: Tue Feb 5 22:05:33 2013 +0100
The second "commit" line (diff with second parent) doesn't have the
"| |" prefix, I don't think this is intentional.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [PATCH v2] t4038: add tests for "diff --cc --raw <trees>"
From: John Keeping @ 2013-02-05 21:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Antoine Pelisse
In-Reply-To: <7v8v72sczp.fsf@alter.siamese.dyndns.org>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
Changes since v1:
- more spaces around '|'
- create trees with line feeds and use 'sed -e 4q'
---
t/t4038-diff-combined.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 40277c7..614425a 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -89,4 +89,28 @@ test_expect_success 'diagnose truncated file' '
grep "diff --cc file" out
'
+test_expect_success 'setup for --cc --raw' '
+ blob=$(echo file | git hash-object --stdin -w) &&
+ base_tree=$(echo "100644 blob $blob file" | git mktree) &&
+ trees= &&
+ for i in `test_seq 1 40`
+ do
+ blob=$(echo file$i | git hash-object --stdin -w) &&
+ trees="$trees$(echo "100644 blob $blob file" | git mktree)$LF"
+ done
+'
+
+test_expect_success 'check --cc --raw with four trees' '
+ four_trees=$(echo "$trees" | sed -e 4q) &&
+ git diff --cc --raw $four_trees $base_tree >out &&
+ # Check for four leading colons in the output:
+ grep "^::::[^:]" out
+'
+
+test_expect_success 'check --cc --raw with forty trees' '
+ git diff --cc --raw $trees $base_tree >out &&
+ # Check for forty leading colons in the output:
+ grep "^::::::::::::::::::::::::::::::::::::::::[^:]" out
+'
+
test_done
--
1.8.1.2.689.g36c777b
^ permalink raw reply related
* importing two different trees into a fresh git repo
From: Constantine A. Murenin @ 2013-02-05 21:46 UTC (permalink / raw)
To: git
Hi,
I have two distinct trees that were not managed by any RCS, and I'd
like to import them into a single repository into two separate orphan
branches, then make sense of what's in there, merge, and unify into
'master'.
(To give some context, it's /etc/nginx config files from nginx/1.0.12
on Debian 6 and nginx/1.2.2 on OpenBSD 5.2.)
I've encountered two problems so far:
0. After initialising the repository, I was unable to `git checkout
--orphan Debian-6.0.4-nginx-1.0.12` -- presumably it doesn't work when
the repo is empty? This sounds like a bug or an artefact of
implementation. I presume this can be worked around by committing
into master instead, and then doing `git checkout -b
Debian-6.0.4-nginx-1.0.12`, and then force-fixing the master somehow
later on.
1. After making a mistake on my first commit (my first commit into
OpenBSD-5.2-nginx-1.2.2 orphan branch ended up including a directory
from master by mistake), I am now unable to rebase and "fixup" the
changes -- `git rebase --interactive HEAD~2` doesn't work, which, from
one perspective, makes perfect sense (indeed there's no prior
revision), but, from another, it's not immediately obvious how to
quickly work around it.
Any suggestions?
It would seem like making some kind of a dummy first commit into
master would be the best workaround for both of these problems. Is
that basically the suggested approach?
Best regards,
Constantine.
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-02-05 21:56 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jonathan Nieder, git, kraai
In-Reply-To: <7v4nhqsctb.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Thanks for the detailed suggestion.
>> Instead of using a file for putting out non portable syntax,
>> can we can use a similar logic as test_failure ?
>
> Your test_bad_syntax_ function can be called from a subshell, and
> its "exit 1" will not exit, no?
What is more important is that the increment to $test_bad_syntax
done in that function will not be propagated up to the main process
that runs the test framework.
Of course, that is why I mentioned communicating using the
filesystem.
^ permalink raw reply
* Re: [PATCHv5] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-05 22:24 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <8738xaqy40.fsf_-_@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> + unless (scalar @entries) {
> + if ($!) {
> + log_verbose("Unable to open $file: $!");
> + }
> + else {
} else {
> + log_verbose("No netrc entries found in $file");
> + }
> +
> + if ($gpgmode) {
> + # typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
> + my $f = $file;
> + $f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
Yuck. If you really have to quote, it is often far simpler to take
advantage of the fact that quoting rule for shell is much simpler
inside '', i.e.
sub sq {
my ($string) = @_;
$string =~ s|'|'\\''|g;
return "'$string'";
}
The only thing you have to worry about is a single quote, which
would close the single quote you want to be in, and you express it
by first closing the single quote you opened, put the single quote
by emitting a backslash and a single quote, and then immediately
open another single quote.
> + # GPG doesn't work well with 2- or 3-argument open
I think I commented on this in a separate message.
> + # Net::Netrc does this, but the functionality is merged with the file
> + # detection logic, so we have to extract just the part we need
> + my @netrc_entries = net_netrc_loader($io);
> +
> + # these entries will use the credential helper protocol token names
> + my @entries;
> +
> + foreach my $nentry (@netrc_entries) {
> + my %entry;
> + my $num_port;
> +
> + if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
> + $num_port = $nentry->{port};
> + delete $nentry->{port};
> + }
> +
> + # create the new entry for the credential helper protocol
> + $entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
> +
> + # for "host X port Y" where Y is an integer (captured by
> + # $num_port above), set the host to "X:Y"
> + if (defined $entry{host} && defined $num_port) {
> + $entry{host} = join(':', $entry{host}, $num_port);
> + }
> +
> + push @entries, \%entry;
> + }
> +
> + return @entries;
> +}
I'll leave this part to Peff to comment on.
> +sub net_netrc_loader {
> + my $fh = shift @_;
> + my @entries;
> + my ($mach, $macdef, $tok, @tok);
> +
> + LINE:
> + while (<$fh>) {
> + undef $macdef if /\A\n\Z/;
> +
> + if ($macdef) {
> + next LINE;
> + }
> +
> + s/^\s*//;
> + chomp;
> +
> + while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
> + (my $tok = $+) =~ s/\\(.)/$1/g;
> + push(@tok, $tok);
> + }
> +
> + TOKEN:
> + while (@tok) {
> + if ($tok[0] eq "default") {
> + shift(@tok);
> + undef $mach; # ignore 'default' lines
I think it is saner to do something like this instead here:
$mach = { machine => undef }
Otherwise your log_debug() will be filled by the tokens used for the
default entry, and also this "undef $mach" here will break your
macdef skipping logic if the default entry has a macdef, I think.
You can ignore an entry with undefined "machine" in the loop at the
end of load_netrc.
> + next TOKEN;
> + }
> +
> + $tok = shift(@tok);
> +
> + if ($tok eq "machine") {
> + my $host = shift @tok;
> + $mach = { machine => $host };
> + push @entries, $mach;
> + }
> + elsif (exists $options{tmap}->{$tok}) {
> + unless ($mach) {
> + log_debug("Skipping token $tok because no machine was given");
> + next TOKEN;
> + }
> +
> + my $value = shift @tok;
> + unless (defined $value) {
> + log_debug("Token $tok had no value, skipping it.");
> + next TOKEN;
> + }
> +
> + # Following line added by rmerrell to remove '/' escape char in .netrc
> + $value =~ s/\/\\/\\/g;
> + $mach->{$tok} = $value;
> + }
> + elsif ($tok eq "macdef") { # we ignore macros
> + next TOKEN unless $mach;
> + my $value = shift @tok;
> + $macdef = 1;
> + }
> + }
> + }
> +
> + return @entries;
> +}
> +
> +sub read_credential_data_from_stdin {
> + # the query: start with every token with no value
> + my %q = map { $_ => undef } values(%{$options{tmap}});
> +
> + while (<STDIN>) {
> + next unless m/^([^=]+)=(.+)/;
> +
> + my ($token, $value) = ($1, $2);
> + die "Unknown search token $token" unless exists $q{$token};
> + $q{$token} = $value;
> + log_debug("We were given search token $token and value $value");
> + }
> +
> + foreach (sort keys %q) {
> + log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
> + }
> +
> + return \%q;
> +}
> +
> +# takes the search tokens and then a list of entries
> +# each entry is a hash reference
> +sub find_netrc_entry {
> + my $query = shift @_;
> +
> + ENTRY:
> + foreach my $entry (@_)
> + {
> + my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
> + foreach my $check (sort keys %$query) {
> + if (defined $query->{$check}) {
> + log_debug("compare %s [%s] to [%s] (entry: %s)",
> + $check,
> + $entry->{$check},
> + $query->{$check},
> + $entry_text);
> + unless ($query->{$check} eq $entry->{$check}) {
> + next ENTRY;
> + }
> + }
> + else {
> + log_debug("OK: any value satisfies check $check");
> + }
> + }
> +
> + return $entry;
> + }
> +
> + # nothing was found
> + return;
> +}
I'll leave this part to Peff to comment on.
> +
> +sub print_credential_data {
> + my $entry = shift @_;
> + my $query = shift @_;
> +
> + log_debug("entry has passed all the search checks");
> + TOKEN:
> + foreach my $git_token (sort keys %$entry) {
> + log_debug("looking for useful token $git_token");
> + # don't print unknown (to the credential helper protocol) tokens
> + next TOKEN unless exists $query->{$git_token};
> +
> + # don't print things asked in the query (the entry matches them)
> + next TOKEN if defined $query->{$git_token};
> +
> + log_debug("FOUND: $git_token=$entry->{$git_token}");
> + printf "%s=%s\n", $git_token, $entry->{$git_token};
> + }
> +}
> +sub log_verbose {
> + return unless $options{verbose};
> + printf STDERR @_;
> + printf STDERR "\n";
> +}
> +
> +sub log_debug {
> + return unless $options{debug};
> + printf STDERR @_;
> + printf STDERR "\n";
> +}
Otherwise, looks almost ready to me.
Thanks.
^ permalink raw reply
* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
From: Junio C Hamano @ 2013-02-05 22:09 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <877gmmqyho.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Tue, 05 Feb 2013 11:53:20 -0800 Junio C Hamano <gitster@pobox.com> wrote:
>
> I think it's more readable with large loops, and it actually makes sense
> when you read the code. Not a big deal to me either, I just felt for
> this particular script it was OK.
>
>>> + if ($file =~ m/\.gpg$/) {
>>> + log_verbose("Using GPG to open $file");
>>> + # GPG doesn't work well with 2- or 3-argument open
>
> JCH> If that is the case, please quote $file properly against shell
> JCH> munging it.
>
> Ahhh that gets ugly. OK, quoted.
>
> JCH> The only thing you do on $io is to read from it via "while (<$io>)",
> JCH> so I would personally have written this part like this without
> JCH> having to use IO::File(), though:
>
> JCH> $io = open("-|", qw(gpg --decrypt), $file);
>
> That doesn't work for me, unfortunately. I'm trying to avoid the IPC::*
> modules and such. Please test it yourself with GPG. I'm on Perl
> 5.14.2.
This works for me as expected (sorry for that open $io syntax
gotcha).
-- cut here -- >8 -- cut here --
#!/usr/bin/perl
my $io;
open $io, "-|", qw(gpg --decrypt), $ARGV[0]
or die "$!: gpg open";
while (<$io>) {
print;
}
close $io
or die "$!: gpg close";
-- cut here -- 8< -- cut here --
$ perl --version
This is perl, v5.10.1 (*) built for x86_64-linux-gnu-thread-multi
^ permalink raw reply
* Re: [PATCH v2] t4038: add tests for "diff --cc --raw <trees>"
From: Junio C Hamano @ 2013-02-05 22:27 UTC (permalink / raw)
To: John Keeping; +Cc: git, Antoine Pelisse
In-Reply-To: <20130205213949.GY1342@serenity.lan>
Thanks.
^ permalink raw reply
* Re: importing two different trees into a fresh git repo
From: Junio C Hamano @ 2013-02-05 22:29 UTC (permalink / raw)
To: Constantine A. Murenin; +Cc: git
In-Reply-To: <CAPKkNb6+ojb+uvBW+AkhGrhjR85LrJEbmR0KmvaKYb2Cj5Aa4g@mail.gmail.com>
"Constantine A. Murenin" <mureninc@gmail.com> writes:
> I have two distinct trees that were not managed by any RCS, and I'd
> like to import them into a single repository into two separate orphan
> branches, then make sense of what's in there, merge, and unify into
> 'master'.
>
> (To give some context, it's /etc/nginx config files from nginx/1.0.12
> on Debian 6 and nginx/1.2.2 on OpenBSD 5.2.)
As these come from two totally separate sources, I'd find it more
natural to do two repositories, deb-nginx-conf and obsd-nginx-conf,
each with one commit and then pull one into the other (or pull both
to master-nginx-conf if you really wanted to), to me.
^ permalink raw reply
* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
From: Ted Zlatanov @ 2013-02-05 22:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vpq0equo9.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 14:09:58 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> open $io, "-|", qw(gpg --decrypt), $ARGV[0]
OK, the below will be in PATCHv6 (I'll wait on mailing it until after
you've reviewed the rest of PATCHv5). Thanks for checking... I must
have had a typo or a missing comma or something, I could swear I tried
exactly what you show.
Ted
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
index 8298564..5f91630 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -230,13 +230,9 @@ sub load_netrc {
my $io;
if ($gpgmode) {
- # typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
- my $f = $file;
- $f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
- # GPG doesn't work well with 2- or 3-argument open
- my $cmd = "gpg --decrypt $f";
- log_verbose("Using GPG to open $file: [$cmd]");
- open $io, "$cmd|";
+ my @cmd = (qw(gpg --decrypt), $file);
+ log_verbose("Using GPG to open $file: [@cmd]");
+ open $io, "-|", @cmd;
}
else {
log_verbose("Opening $file...");
^ 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