* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Jeff King @ 2012-11-11 17:15 UTC (permalink / raw)
To: A Large Angry SCM; +Cc: Felipe Contreras, Michael J Gruber, Git Mailing List
In-Reply-To: <509FD9BC.7050204@gmail.com>
On Sun, Nov 11, 2012 at 12:00:44PM -0500, A Large Angry SCM wrote:
> >>>a) Leave the name conversion to the export tools, and when they miss
> >>>some weird corner case, like 'Author<email', let the user face the
> >>>consequences, perhaps after an hour of the process.
> [...]
> >>>b) Do the name conversion in fast-import itself, perhaps optionally,
> >>>so if a tool missed some weird corner case, the user does not have to
> >>>face the consequences.
> [...]
> >>c) Do the name conversion, and whatever other cleanup and manipulations
> >>you're interesting in, in a filter between the exporter and git-fast-import.
> >
> >Such a filter would probably be quite complicated, and would decrease
> >performance.
> >
>
> Really?
>
> The fast import stream protocol is pretty simple. All the filter
> really needs to do is pass through everything that isn't a 'commit'
> command. And for the 'commit' command, it only needs to do something
> with the 'author' and 'committer' lines; passing through everything
> else.
>
> I agree that an additional filter _may_ decrease performance somewhat
> if you are already CPU constrained. But I suspect that the effect
> would be negligible compared to the all of the SHA-1 calculations.
It might be measurable, as you are passing every byte of every version
of every file in the repo through an extra pipe. But more importantly, I
don't think it helps.
If there is not a standard filter for fixing up names, we do not need to
care. The user can use "sed" or whatever and pay the performance penalty
(and deal with the possibility of errors from being lazy about parsing
the fast-import stream).
If there is a standard filter, then what is the advantage in doing it as
a pipe? Why not just teach fast-import the same trick (and possibly make
it optional)? That would be simpler, more efficient, and it would make
it easier for remote helpers to turn it on (they use a command-line
switch rather than setting up an extra process).
But what I don't understand is: what would such a standard filter look
like? Fast-import (or a filter) would already receive the exporter's
best attempt at a git-like ident string. We can clean up and normalize
things like whitespace (and we probably should if we do not do so
already). But beyond that, we have no context about the name; only the
exporter has that.
So if we receive:
Foo Bar<foo.bar@example.com> <none@none>
or:
Foo Bar<foo.bar@example.com <none@none>
or:
Foo Bar<foo.bar@example.com
what do we do with it? Is the first part a malformed name/email pair,
and the second part is crap added by a lazy exporter? Or does the
exporter want to keep the angle brackets as part of the name field? Is
there a malformed email in the last one, or no email at all?
The exporter is the only program that actually knows where the data came
from, how it should be broken down, and what is appropriate for pulling
data out of its particular source system. For that reason, the exporter
has to be the place where we come up with a syntactically correct and
unambiguous ident.
I am not opposed to adding a mailmap-like feature to fast-import to map
identities, but it has to start with sane, unambiguous output from the
exporter.
-Peff
^ permalink raw reply
* Re: Reviews on mailing-list
From: Krzysztof Mazur @ 2012-11-11 17:14 UTC (permalink / raw)
To: Thiago Farina
Cc: Felipe Contreras, Deniz Türkoglu, git, Junio C Hamano,
Shawn Pearce
In-Reply-To: <CACnwZYfP4=Fvt8T0GOEzjTOaEyYF_Ao6e_f02eqO_5_DyzM0Zw@mail.gmail.com>
On Sun, Nov 11, 2012 at 11:09:36AM -0200, Thiago Farina wrote:
> On Sun, Nov 11, 2012 at 10:14 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > Requiring everyone to use a web browser would limit the amount of ways
> > people can review patches.
> I don't see that as a limitation as I think everyone has access to a
> web browser these days, don't have?
Almost everyone, but not everyone likes using web browser.
With emails you can use your favorite interface: favorite MUA and in good
MUA also your favorite editor (program for writing text, not some
editor-like product in web browser). With web-based system you have only
ONE interface (or few), not chosen by you, you cannot change it, web admin
may change it in way you not like.
Krzysiek
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Ramkumar Ramachandra @ 2012-11-11 17:12 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <1352653610-2090-1-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
Use sendemail.from. The email sender doesn't necessarily have to be the author.
Ram
^ permalink raw reply
* [PATCH] Add new git-cc-cmd helper to contrib
From: Felipe Contreras @ 2012-11-11 17:10 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
You can run it like format-patch:
% git cc-cmd master..my-branch
And you'll get relevant people to Cc.
The code finds the changes in each commit in the list, runs 'git blame'
to see which other commits are relevant to those lines, and then adds
the author and signer to the list.
Finally, it calculates what percentage of the total relevant commits
each person was involved in, and if it passes the threshold, it goes in.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
This cannot be used as cc-cmd option for 'git send-email' but it can as
series-cc-cmd for which I just send some patches.
An example output:
% git cc-cmd --role-stat v1.8.0^^1..v1.8.0^^2
Vietor Liu <vietor@vxwo.org> (author: 12%)
"Shawn O. Pearce" <spearce@spearce.org> (signer: 37%)
Alexandre Erwin Ittner <alexandre@ittner.com.br> (author: 12%)
Peter Krefting <peter@softwolves.pp.se> (author: 12%)
Pat Thoyts <patthoyts@users.sourceforge.net> (signer: 37%, author: 12%)
Christian Stimming <stimming@tuhh.de> (author: 12%)
Heiko Voigt <hvoigt@hvoigt.net> (author: 25%)
Heiko Voigt <heiko.voigt@mahr.de> (signer: 12%)
Bert Wesarg <bert.wesarg@googlemail.com> (author: 12%)
Cheers.
contrib/cc-cmd/git-cc-cmd | 186 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 186 insertions(+)
create mode 100755 contrib/cc-cmd/git-cc-cmd
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
new file mode 100755
index 0000000..17b14d4
--- /dev/null
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -0,0 +1,186 @@
+#!/usr/bin/env ruby
+
+require 'optparse'
+
+$since = '3-years-ago'
+$min_percent = 5
+$role_stats = false
+
+begin
+ OptionParser.new do |opts|
+ opts.on('--role-stats') do |v|
+ $role_stats = v
+ end
+ opts.on('--min-percent N', Integer) do |v|
+ $min_percent = v
+ end
+ opts.on('--since DATE') do |v|
+ $since = v
+ end
+ end.parse!
+rescue OptionParser::InvalidOption
+end
+
+revs = []
+File.popen(%w[git rev-parse --revs-only --default=HEAD --symbolic] + ARGV).each do |rev|
+ revs << rev.chomp
+end
+
+case revs.size
+when 0
+ committish = '%s..HEAD' % '@{upstream}'
+when 1
+ committish = '%s..HEAD' % revs[0]
+else
+ committish = revs.join(' ')
+end
+
+$alias_file = "~/.mutt/aliases"
+$aliases = {}
+
+if $alias_file
+ open(File.expand_path($alias_file)).each do |line|
+ if line =~ /^\s*alias\s+(?:-group\s+\S+\s+)*(\S+)\s+(.*)$/
+ key, addresses = $1, $2.split(', ')
+ addresses.each do |address|
+ $aliases[address] = key
+ end
+ end
+ end
+end
+
+class Commit
+
+ attr_reader :id
+ attr_accessor :author, :roles
+
+ def initialize(id)
+ @id = id
+ @roles = []
+ end
+
+ def self.parse(data)
+ id = author = msg = nil
+ roles = {}
+ data.each_line do |line|
+ if not msg
+ case line
+ when /^commit (.+)$/
+ id = $1
+ when /^author ([^<>]+) <(\S+)>$/
+ author = $1, $2
+ roles[author] = 'author'
+ when /^$/
+ msg = true
+ end
+ else
+ if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\S+?)>$/
+ person = $2, $3
+ roles[person] = 'signer' if person != author
+ end
+ end
+ end
+ roles = roles.map do |person, role|
+ address = "%s <%s>" % person
+ if $aliases.include?(address)
+ person = nil, $aliases[address]
+ end
+ [person, role]
+ end
+ [id, author, roles]
+ end
+
+end
+
+class Commits
+
+ attr_reader :items
+
+ def initialize(items = {})
+ @items = items
+ import
+ end
+
+ def size
+ @items.size
+ end
+
+ def import
+ return if @items.empty?
+ ids = @items.keys.join(' ')
+ format = [
+ 'commit %H',
+ 'author %an <%ae>',
+ '', '%B' ].join('%n')
+ open("|git show -z -s --format='format:#{format}' #{ids}").each("\0") do |data|
+ next if data == "\0" # bug in git show?
+ id, author, roles = Commit.parse(data)
+ commit = @items[id]
+ commit.author = author
+ commit.roles = roles
+ end
+ end
+
+ def each_person_role
+ commit_roles = @items.values.map { |commit| commit.roles }.flatten(1)
+ commit_roles.group_by { |person, role| person }.each do |person, commit_roles|
+ commit_roles.group_by { |person, role| role }.each do |role, commit_roles|
+ yield person, role, commit_roles.size
+ end
+ end
+ end
+
+ def self.from_blame(committish)
+ main_commits = {}
+ items = {}
+ open("|git rev-list --reverse #{committish}").each do |e|
+ id = e.chomp
+ main_commits[id] = true
+ open("|git --no-pager show -C --oneline #{id}").each do |e|
+ case e
+ when /^---\s+(\S+)/
+ @source = $1 != '/dev/null' ? $1[2..-1] : nil
+ when /^@@\s-(\d+),(\d+)/
+ next unless @source
+ open("|git blame --incremental -C -L #{$1},+#{$2} --since=#{$since} #{id}^ -- #{@source}").each do |e|
+ if e =~ /^(\h{40})/
+ items[$1] = Commit.new($1) if not main_commits.include?($1)
+ end
+ end
+ end
+ end
+ end
+ self.new(items)
+ end
+
+end
+
+commits = Commits.from_blame(committish)
+
+# hash of hashes
+persons = Hash.new { |hash, key| hash[key] = {} }
+
+commits.each_person_role do |person, role, count|
+ persons[person][role] = count
+end
+
+persons.each do |person, roles|
+ roles = roles.map do |role, count|
+ percent = count.to_f * 100 / commits.size
+ next if percent < $min_percent
+ "%s: %u%%" % [role, percent]
+ end.compact
+ next if roles.empty?
+ name, email = person
+
+ # has must quote chars?
+ name = '"%s"' % name if name =~ /[^\w \-]/i
+
+ person = name ? "%s <%s>" % [name, email] : email
+
+ if $role_stats
+ puts "%s (%s)" % [person, roles.join(', ')]
+ else
+ puts person
+ end
+end
--
1.8.0
^ permalink raw reply related
* [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-11 17:06 UTC (permalink / raw)
To: git; +Cc: Thomas Rast, Junio C Hamano, Jonathan Nieder, Felipe Contreras
There's no point in asking this over and over if the user already
properly configured his/her name and email.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
git-send-email.perl | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..65b8328 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,6 +748,17 @@ if (!$force) {
}
}
+my $name = Git::config('user.name');
+my $email = Git::config('user.email');
+
+if (defined $email) {
+ if (defined $name) {
+ $sender = sprintf("%s <%s>", $name, $email);
+ } else {
+ $sender = $email;
+ }
+}
+
my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
--
1.8.0
^ permalink raw reply related
* [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-11 17:04 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Joe Perches, Jonathan Nieder, Pascal Obry,
Felipe Contreras
In-Reply-To: <1352653463-1923-1-git-send-email-felipe.contreras@gmail.com>
cc-cmd is only per-file, and many times receipients get lost without
seing the full patch series.
So, add an option for series-cc-cmd, which receives as an argument
rev-list options, just like format-patch.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/git-send-email.txt | 7 +++++++
contrib/completion/git-completion.bash | 2 +-
git-send-email.perl | 9 ++++++++-
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3241170..1161d3e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -218,6 +218,13 @@ Automating
Output of this command must be single email address per line.
Default is the value of 'sendemail.cccmd' configuration value.
+--series-cc-cmd=<command>::
+ Specify a command to execute to generate "Cc:" entries for the whole
+ patch series.
+ The arguments would be the same rev-list options the user specified.
+ Output of this command must be single email address per line.
+ Default is the value of 'sendemail.seriescccmd' configuration value.
+
--[no-]chain-reply-to::
If this is set, each email will be sent as a reply to the previous
email sent. If disabled with "--no-chain-reply-to", all emails after
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..0b04229 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1555,7 +1555,7 @@ _git_send_email ()
return
;;
--*)
- __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
+ __gitcomp "--annotate --bcc --cc --cc-cmd --series-cc-cmd --chain-reply-to
--compose --confirm= --dry-run --envelope-sender
--from --identity
--in-reply-to --no-chain-reply-to --no-signed-off-by-cc
diff --git a/git-send-email.perl b/git-send-email.perl
index 26d4477..da89ac6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -191,7 +191,7 @@ sub do_edit {
# Variables with corresponding config settings
my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
-my ($to_cmd, $cc_cmd);
+my ($to_cmd, $cc_cmd, $series_cc_cmd);
my ($smtp_server, $smtp_server_port, @smtp_server_options);
my ($smtp_authuser, $smtp_encryption);
my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
@@ -224,6 +224,7 @@ my %config_settings = (
"tocmd" => \$to_cmd,
"cc" => \@initial_cc,
"cccmd" => \$cc_cmd,
+ "seriescccmd" => \$series_cc_cmd,
"aliasfiletype" => \$aliasfiletype,
"bcc" => \@bcclist,
"suppresscc" => \@suppress_cc,
@@ -305,6 +306,7 @@ my $rc = GetOptions("h" => \$help,
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
+ "series-cc-cmd=s" => \$series_cc_cmd,
"suppress-from!" => \$suppress_from,
"suppress-cc=s" => \@suppress_cc,
"signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
@@ -766,6 +768,11 @@ if (!@initial_to && !defined $to_cmd) {
$prompting++;
}
+if (@rev_list_opts) {
+ push @initial_cc, recipients_cmd("series-cc-cmd", "cc", $series_cc_cmd, @rev_list_opts)
+ if defined $series_cc_cmd;
+}
+
sub expand_aliases {
return map { expand_one_alias($_) } @_;
}
--
1.8.0
^ permalink raw reply related
* [PATCH 1/2] send-email: refactor recipients_cmd()
From: Felipe Contreras @ 2012-11-11 17:04 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Joe Perches, Jonathan Nieder, Pascal Obry,
Felipe Contreras
In-Reply-To: <1352653463-1923-1-git-send-email-felipe.contreras@gmail.com>
If we pass the full command to run, we should be able to use it in more
extensible ways--see next patch.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-send-email.perl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..26d4477 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1373,11 +1373,11 @@ foreach my $t (@files) {
# Execute a command (e.g. $to_cmd) to get a list of email addresses
# and return a results array
sub recipients_cmd {
- my ($prefix, $what, $cmd, $file) = @_;
+ my ($prefix, $what, $cmd, @args) = @_;
my $sanitized_sender = sanitize_address($sender);
my @addresses = ();
- open my $fh, "$cmd \Q$file\E |"
+ open my $fh, "-|", $cmd, @args
or die "($prefix) Could not execute '$cmd'";
while (my $address = <$fh>) {
$address =~ s/^\s*//g;
--
1.8.0
^ permalink raw reply related
* [PATCH 0/2] send-email: new series-cc-cmd option
From: Felipe Contreras @ 2012-11-11 17:04 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Joe Perches, Jonathan Nieder, Pascal Obry,
Felipe Contreras
cc-cmd is probably not that useful, what we really want is to make a list of
people to Cc on a per-patch-series basis.
This patch series allows just that.
Felipe Contreras (2):
send-email: refactor recipients_cmd()
send-email: add series-cc-cmd option
Documentation/git-send-email.txt | 7 +++++++
contrib/completion/git-completion.bash | 2 +-
git-send-email.perl | 13 ++++++++++---
3 files changed, 18 insertions(+), 4 deletions(-)
--
1.8.0
^ permalink raw reply
* [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
From: Paul Fox @ 2012-11-11 16:55 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111163100.GB13188@sigill.intra.peff.net>
The user's editor likely catches SIGINT (ctrl-C). but if
the user spawns a command from the editor and uses ctrl-C to
kill that command, the SIGINT will likely also kill git
itself (depending on the editor, this can leave the terminal
in an unusable state).
Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>
Signed-off-by: Jeff King <peff@peff.net>
---
editor.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/editor.c b/editor.c
index 842f782..28aae85 100644
--- a/editor.c
+++ b/editor.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "strbuf.h"
#include "run-command.h"
+#include "sigchain.h"
#ifndef DEFAULT_EDITOR
#define DEFAULT_EDITOR "vi"
@@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
if (strcmp(editor, ":")) {
const char *args[] = { editor, path, NULL };
struct child_process p;
+ int ret;
memset(&p, 0, sizeof(p));
p.argv = args;
@@ -46,7 +48,10 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
if (start_command(&p) < 0)
return error("unable to start editor '%s'", editor);
- if (finish_command(&p))
+ sigchain_push(SIGINT, SIG_IGN);
+ ret = finish_command(&p);
+ sigchain_pop(SIGINT);
+ if (ret)
return error("There was a problem with the editor '%s'.",
editor);
}
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: A Large Angry SCM @ 2012-11-11 17:00 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Michael J Gruber, Git Mailing List, Jeff King
In-Reply-To: <CAMP44s1dsEU=E8tdgMYxWFyFw+F03bstdb5o7Ww_-RCQPd3R0w@mail.gmail.com>
On 11/11/2012 07:41 AM, Felipe Contreras wrote:
> On Sat, Nov 10, 2012 at 8:25 PM, A Large Angry SCM<gitzilla@gmail.com> wrote:
>> On 11/10/2012 01:43 PM, Felipe Contreras wrote:
>
>>> So, the options are:
>>>
>>> a) Leave the name conversion to the export tools, and when they miss
>>> some weird corner case, like 'Author<email', let the user face the
>>> consequences, perhaps after an hour of the process.
>>>
>>> We know there are sources of data that don't have git-formatted author
>>> names, so we know every tool out there must do this checking.
>>>
>>> In addition to that, let the export tool decide what to do when one of
>>> these bad names appear, which in many cases probably means do nothing,
>>> so the user would not even see that such a bad name was there, which
>>> might not be what they want.
>>>
>>> b) Do the name conversion in fast-import itself, perhaps optionally,
>>> so if a tool missed some weird corner case, the user does not have to
>>> face the consequences.
>>>
>>> The tool writers don't have to worry about this, so we would not have
>>> tools out there doing a half-assed job of this.
>>>
>>> And what happens when such bad names end up being consistent: warning,
>>> a scaffold mapping of bad names, etc.
>>>
>>>
>>> One is bad for the users, and the tools writers, only disadvantages,
>>> the other is good for the users and the tools writers, only
>>> advantages.
>>>
>>
>> c) Do the name conversion, and whatever other cleanup and manipulations
>> you're interesting in, in a filter between the exporter and git-fast-import.
>
> Such a filter would probably be quite complicated, and would decrease
> performance.
>
Really?
The fast import stream protocol is pretty simple. All the filter really
needs to do is pass through everything that isn't a 'commit' command.
And for the 'commit' command, it only needs to do something with the
'author' and 'committer' lines; passing through everything else.
I agree that an additional filter _may_ decrease performance somewhat if
you are already CPU constrained. But I suspect that the effect would be
negligible compared to the all of the SHA-1 calculations.
^ permalink raw reply
* [PATCH 3/3] submodule: display summary header in bold
From: Ramkumar Ramachandra @ 2012-11-11 16:59 UTC (permalink / raw)
To: Git List; +Cc: Jens Lehmann, Jeff King
In-Reply-To: <1352653146-3932-1-git-send-email-artagnon@gmail.com>
Currently, 'git diff --submodule' displays output with a bold diff
header for non-submodules. So this part is in bold:
diff --git a/file1 b/file1
index 30b2f6c..2638038 100644
--- a/file1
+++ b/file1
For submodules, the header looks like this:
Submodule submodule1 012b072..248d0fd:
Unfortunately, it's easy to miss in the output because it's not bold.
Change this.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
diff.c | 2 +-
submodule.c | 8 ++++----
submodule.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index b486070..51c0d6c 100644
--- a/diff.c
+++ b/diff.c
@@ -2267,7 +2267,7 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one ? one->path : two->path,
one->sha1, two->sha1, two->dirty_submodule,
- del, add, reset);
+ set, del, add, reset);
return;
}
diff --git a/submodule.c b/submodule.c
index e3e0b45..c10182e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -258,7 +258,7 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
void show_submodule_summary(FILE *f, const char *path,
unsigned char one[20], unsigned char two[20],
- unsigned dirty_submodule,
+ unsigned dirty_submodule, const char *set,
const char *del, const char *add, const char *reset)
{
struct rev_info rev;
@@ -292,15 +292,15 @@ void show_submodule_summary(FILE *f, const char *path,
return;
}
- strbuf_addf(&sb, "Submodule %s %s..", path,
+ strbuf_addf(&sb, "%sSubmodule %s %s..", set, path,
find_unique_abbrev(one, DEFAULT_ABBREV));
if (!fast_backward && !fast_forward)
strbuf_addch(&sb, '.');
strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
if (message)
- strbuf_addf(&sb, " %s\n", message);
+ strbuf_addf(&sb, " %s%s\n", message, reset);
else
- strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");
+ strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);
fwrite(sb.buf, sb.len, 1, f);
if (!message) {
diff --git a/submodule.h b/submodule.h
index f2e8271..997fd06 100644
--- a/submodule.h
+++ b/submodule.h
@@ -20,7 +20,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
void show_submodule_summary(FILE *f, const char *path,
unsigned char one[20], unsigned char two[20],
- unsigned dirty_submodule,
+ unsigned dirty_submodule, const char *set,
const char *del, const char *add, const char *reset);
void set_config_fetch_recurse_submodules(int value);
void check_for_new_submodule_commits(unsigned char new_sha1[20]);
--
1.7.8.1.362.g5d6df.dirty
^ permalink raw reply related
* [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Ramkumar Ramachandra @ 2012-11-11 16:59 UTC (permalink / raw)
To: Git List; +Cc: Jens Lehmann, Jeff King
In-Reply-To: <1352653146-3932-1-git-send-email-artagnon@gmail.com>
Introduce a diff.submodule configuration variable corresponding to the
'--submodule' command-line option of 'git diff'.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/diff-config.txt | 7 ++++++
Documentation/diff-options.txt | 3 +-
cache.h | 1 +
diff.c | 40 ++++++++++++++++++++++++++++++++++---
t/t4041-diff-submodule-option.sh | 30 +++++++++++++++++++++++++++-
5 files changed, 75 insertions(+), 6 deletions(-)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index decd370..89dd634 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -107,6 +107,13 @@ diff.suppressBlankEmpty::
A boolean to inhibit the standard behavior of printing a space
before each empty output line. Defaults to false.
+diff.submodule::
+ Specify the format in which differences in submodules are
+ shown. The "log" format lists the commits in the range like
+ linkgit:git-submodule[1] `summary` does. The "short" format
+ format just shows the names of the commits at the beginning
+ and end of the range. Defaults to short.
+
diff.wordRegex::
A POSIX Extended Regular Expression used to determine what is a "word"
when performing word-by-word difference calculations. Character
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index cf4b216..f4f7e25 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -170,7 +170,8 @@ any of those replacements occurred.
the commits in the range like linkgit:git-submodule[1] `summary` does.
Omitting the `--submodule` option or specifying `--submodule=short`,
uses the 'short' format. This format just shows the names of the commits
- at the beginning and end of the range.
+ at the beginning and end of the range. Can be tweaked via the
+ `diff.submodule` configuration variable.
--color[=<when>]::
Show colored diff.
diff --git a/cache.h b/cache.h
index dbd8018..4d9dbc7 100644
--- a/cache.h
+++ b/cache.h
@@ -1221,6 +1221,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags);
/* diff.c */
extern int diff_auto_refresh_index;
+extern const char *submodule_format_cfg;
/* match-trees.c */
void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
diff --git a/diff.c b/diff.c
index e89a201..b486070 100644
--- a/diff.c
+++ b/diff.c
@@ -30,6 +30,7 @@ static int diff_use_color_default = -1;
static int diff_context_default = 3;
static const char *diff_word_regex_cfg;
static const char *external_diff_cmd_cfg;
+const char *submodule_format_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
static int diff_no_prefix;
@@ -123,6 +124,20 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
return ret;
}
+static int parse_submodule_params(struct diff_options *options, const char *value,
+ struct strbuf *errmsg)
+{
+ if (!strcmp(value, "log"))
+ DIFF_OPT_SET(options, SUBMODULE_LOG);
+ else if (!strcmp(value, "short"))
+ DIFF_OPT_CLR(options, SUBMODULE_LOG);
+ else {
+ strbuf_addf(errmsg, _("'%s'"), value);
+ return 1;
+ }
+ return 0;
+}
+
static int git_config_rename(const char *var, const char *value)
{
if (!value)
@@ -223,6 +238,15 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
return 0;
}
+ if (!strcmp(var, "diff.submodule")) {
+ struct strbuf errmsg = STRBUF_INIT;
+ if (parse_submodule_params(&default_diff_options, value, &errmsg))
+ warning(_("Unknown value for 'diff.submodule' config variable: %s"),
+ errmsg.buf);
+ strbuf_release(&errmsg);
+ return 0;
+ }
+
if (!prefixcmp(var, "submodule."))
return parse_submodule_config_option(var, value);
@@ -3475,6 +3499,16 @@ static int parse_dirstat_opt(struct diff_options *options, const char *params)
return 1;
}
+static int parse_submodule_opt(struct diff_options *options, const char *params)
+{
+ struct strbuf errmsg = STRBUF_INIT;
+ if (parse_submodule_params(options, params, &errmsg))
+ die(_("Failed to parse --submodule option parameter: %s"),
+ errmsg.buf);
+ strbuf_release(&errmsg);
+ return 1;
+}
+
int diff_opt_parse(struct diff_options *options, const char **av, int ac)
{
const char *arg = av[0];
@@ -3655,10 +3689,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
handle_ignore_submodules_arg(options, arg + 20);
} else if (!strcmp(arg, "--submodule"))
DIFF_OPT_SET(options, SUBMODULE_LOG);
- else if (!prefixcmp(arg, "--submodule=")) {
- if (!strcmp(arg + 12, "log"))
- DIFF_OPT_SET(options, SUBMODULE_LOG);
- }
+ else if (!prefixcmp(arg, "--submodule="))
+ return parse_submodule_opt(options, arg + 12);
/* misc options */
else if (!strcmp(arg, "-z"))
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 6c01d0c..e401814 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -33,6 +33,7 @@ test_create_repo sm1 &&
add_file . foo >/dev/null
head1=$(add_file sm1 foo1 foo2)
+fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
test_expect_success 'added submodule' "
git add sm1 &&
@@ -43,6 +44,34 @@ EOF
test_cmp expected actual
"
+test_expect_success 'added submodule, set diff.submodule' "
+ git config diff.submodule log &&
+ git add sm1 &&
+ git diff --cached >actual &&
+ cat >expected <<-EOF &&
+Submodule sm1 0000000...$head1 (new submodule)
+EOF
+ git config --unset diff.submodule &&
+ test_cmp expected actual
+"
+
+test_expect_success '--submodule=short overrides diff.submodule' "
+ git config diff.submodule log &&
+ git add sm1 &&
+ git diff --submodule=short --cached >actual &&
+ cat >expected <<-EOF &&
+diff --git a/sm1 b/sm1
+new file mode 160000
+index 0000000..a2c4dab
+--- /dev/null
++++ b/sm1
+@@ -0,0 +1 @@
++Subproject commit $fullhead1
+EOF
+ git config --unset diff.submodule &&
+ test_cmp expected actual
+"
+
commit_file sm1 &&
head2=$(add_file sm1 foo3)
@@ -73,7 +102,6 @@ EOF
test_cmp expected actual
"
-fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
test_expect_success 'modified submodule(forward) --submodule=short' "
git diff --submodule=short >actual &&
--
1.7.8.1.362.g5d6df.dirty
^ permalink raw reply related
* [PATCH 1/3] Documentation: move diff.wordRegex from config.txt to diff-config.txt
From: Ramkumar Ramachandra @ 2012-11-11 16:59 UTC (permalink / raw)
To: Git List; +Cc: Jens Lehmann, Jeff King
In-Reply-To: <1352653146-3932-1-git-send-email-artagnon@gmail.com>
19299a8 (Documentation: Move diff.<driver>.* from config.txt to
diff-config.txt, 2011-04-07) moved the diff configuration options to
diff-config.txt, but forgot about diff.wordRegex, which was left
behind in config.txt. Fix this.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/config.txt | 6 ------
Documentation/diff-config.txt | 6 ++++++
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9a0544c..e70216d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -962,12 +962,6 @@ difftool.<tool>.cmd::
difftool.prompt::
Prompt before each invocation of the diff tool.
-diff.wordRegex::
- A POSIX Extended Regular Expression used to determine what is a "word"
- when performing word-by-word difference calculations. Character
- sequences that match the regular expression are "words", all other
- characters are *ignorable* whitespace.
-
fetch.recurseSubmodules::
This option can be either set to a boolean value or to 'on-demand'.
Setting it to a boolean changes the behavior of fetch and pull to
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 75ab8a5..decd370 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -107,6 +107,12 @@ diff.suppressBlankEmpty::
A boolean to inhibit the standard behavior of printing a space
before each empty output line. Defaults to false.
+diff.wordRegex::
+ A POSIX Extended Regular Expression used to determine what is a "word"
+ when performing word-by-word difference calculations. Character
+ sequences that match the regular expression are "words", all other
+ characters are *ignorable* whitespace.
+
diff.<driver>.command::
The custom diff driver command. See linkgit:gitattributes[5]
for details.
--
1.7.8.1.362.g5d6df.dirty
^ permalink raw reply related
* [PATCH v3 0/3] Introduce diff.submodule
From: Ramkumar Ramachandra @ 2012-11-11 16:59 UTC (permalink / raw)
To: Git List; +Cc: Jens Lehmann, Jeff King
v1 is here: http://mid.gmane.org/1349196670-2844-1-git-send-email-artagnon@gmail.com
v2 is here: http://mid.gmane.org/1351766630-4837-1-git-send-email-artagnon@gmail.com
This version was prepared in response to Peff's review of v2. As
suggested, I've created a separate function which both '--submodule'
and 'diff.submodule' use to set/ unset SUBMODULE_OPT.
Ram
Ramkumar Ramachandra (3):
Documentation: move diff.wordRegex from config.txt to diff-config.txt
diff: introduce diff.submodule configuration variable
submodule: display summary header in bold
Documentation/config.txt | 6 -----
Documentation/diff-config.txt | 13 +++++++++++
Documentation/diff-options.txt | 3 +-
cache.h | 1 +
diff.c | 42 +++++++++++++++++++++++++++++++++----
submodule.c | 8 +++---
submodule.h | 2 +-
t/t4041-diff-submodule-option.sh | 30 ++++++++++++++++++++++++++-
8 files changed, 87 insertions(+), 18 deletions(-)
--
1.7.8.1.362.g5d6df.dirty
^ permalink raw reply
* [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
From: Jeff King @ 2012-11-11 16:58 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111163100.GB13188@sigill.intra.peff.net>
From: Paul Fox <pgf@foxharp.boston.ma.us>
The user's editor likely catches SIGINT (ctrl-C). but if
the user spawns a command from the editor and uses ctrl-C to
kill that command, the SIGINT will likely also kill git
itself (depending on the editor, this can leave the terminal
in an unusable state).
Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>
Signed-off-by: Jeff King <peff@peff.net>
---
Whoops, my original sending of this actually had Paul in the email's
From field, not in the pseudo-header of the commit. Apologies if you
receive an extra forged copy.
editor.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/editor.c b/editor.c
index 842f782..28aae85 100644
--- a/editor.c
+++ b/editor.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "strbuf.h"
#include "run-command.h"
+#include "sigchain.h"
#ifndef DEFAULT_EDITOR
#define DEFAULT_EDITOR "vi"
@@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
if (strcmp(editor, ":")) {
const char *args[] = { editor, path, NULL };
struct child_process p;
+ int ret;
memset(&p, 0, sizeof(p));
p.argv = args;
@@ -46,7 +48,10 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
if (start_command(&p) < 0)
return error("unable to start editor '%s'", editor);
- if (finish_command(&p))
+ sigchain_push(SIGINT, SIG_IGN);
+ ret = finish_command(&p);
+ sigchain_pop(SIGINT);
+ if (ret)
return error("There was a problem with the editor '%s'.",
editor);
}
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCH 5/5] launch_editor: propagate SIGINT from editor to git
From: Jeff King @ 2012-11-11 16:57 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111163100.GB13188@sigill.intra.peff.net>
We block SIGINT while the editor runs so that git is not
killed accidentally by a stray "^C" meant for the editor or
its subprocesses. This works because most editors ignore
SIGINT.
However, some editor wrappers, like emacsclient, expect to
die due to ^C. We detect the signal death in the editor and
properly exit, but not before writing a useless error
message to stderr. Instead, let's notice when the editor was
killed by SIGINT and just raise the signal on ourselves.
This skips the message and looks to our parent like we
received SIGINT ourselves.
The end effect is that if the user's editor ignores SIGINT,
we will, too. And if it does not, then we will behave as if
we did not ignore it. That should make all users happy.
Note that in the off chance that another part of git has
ignored SIGINT while calling launch_editor, we will still
properly detect and propagate the failed return code from
the editor (i.e., the worst case is that we generate the
useless error, not fail to notice the editor's death).
Signed-off-by: Jeff King <peff@peff.net>
---
editor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/editor.c b/editor.c
index 28aae85..1275527 100644
--- a/editor.c
+++ b/editor.c
@@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
sigchain_push(SIGINT, SIG_IGN);
ret = finish_command(&p);
sigchain_pop(SIGINT);
+ if (WIFSIGNALED(ret) && WTERMSIG(ret) == SIGINT)
+ raise(SIGINT);
if (ret)
return error("There was a problem with the editor '%s'.",
editor);
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCH 4/5] run-command: do not warn about child death by SIGINT
From: Jeff King @ 2012-11-11 16:56 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111163100.GB13188@sigill.intra.peff.net>
SIGINT is not generally an interesting signal to the user,
since it is typically caused by them hitting "^C" or
otherwise telling their terminal to send the signal.
Signed-off-by: Jeff King <peff@peff.net>
---
I thought about making this an optional parameter for run-command, but
it seems like everybody would want this (and most callsites will
complain about a failed command separately, anyway, so it is not like
errors would go unnoticed).
run-command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/run-command.c b/run-command.c
index 3aae270..0527c61 100644
--- a/run-command.c
+++ b/run-command.c
@@ -242,7 +242,8 @@ static int wait_or_whine(pid_t pid, const char *argv0)
error("waitpid is confused (%s)", argv0);
} else if (WIFSIGNALED(status)) {
code = WTERMSIG(status);
- error("%s died of signal %d", argv0, code);
+ if (code != SIGINT)
+ error("%s died of signal %d", argv0, code);
/*
* This return value is chosen so that code & 0xff
* mimics the exit code that a POSIX shell would report for
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users
From: Sven Strickroth @ 2012-11-11 16:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jakub Narebski
In-Reply-To: <7vmwzzqwud.fsf@alter.siamese.dyndns.org>
git-svn reads usernames and other user queries from an interactive
terminal. This cause GUIs (w/o STDIN connected) to hang waiting forever
for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).
This change extends the Git::prompt helper, so that it can also be used
for non password queries, and makes use of it instead of using
hand-rolled prompt-response code that only works with the interactive
terminal.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 28 +++++++++++++++++-----------
perl/Git/SVN/Prompt.pm | 16 +++++++---------
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 0a0fe91..3200f4d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -511,18 +511,19 @@ C<git --html-path>). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
-=item prompt ( PROMPT )
+=item prompt ( PROMPT , ISPASSWORD )
Query user C<PROMPT> and return answer from user.
Honours GIT_ASKPASS, SSH_ASKPASS environment variables for querying
the user. If no *_ASKPASS variable is set or an error occoured,
the terminal is tried as a fallback.
+If C<ISPASSWORD> is set and true, the terminal disables echo.
=cut
sub prompt {
- my ($prompt) = @_;
+ my ($prompt, $isPassword) = @_;
my $ret;
if (exists $ENV{'GIT_ASKPASS'}) {
$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
@@ -533,16 +534,20 @@ sub prompt {
if (!defined $ret) {
print STDERR $prompt;
STDERR->flush;
- require Term::ReadKey;
- Term::ReadKey::ReadMode('noecho');
- $ret = '';
- while (defined(my $key = Term::ReadKey::ReadKey(0))) {
- last if $key =~ /[\012\015]/; # \n\r
- $ret .= $key;
+ if (defined $isPassword && $isPassword) {
+ require Term::ReadKey;
+ Term::ReadKey::ReadMode('noecho');
+ $ret = '';
+ while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+ last if $key =~ /[\012\015]/; # \n\r
+ $ret .= $key;
+ }
+ Term::ReadKey::ReadMode('restore');
+ print STDERR "\n";
+ STDERR->flush;
+ } else {
+ chomp($ret = <STDIN>);
}
- Term::ReadKey::ReadMode('restore');
- print STDERR "\n";
- STDERR->flush;
}
return $ret;
}
@@ -550,6 +555,7 @@ sub prompt {
sub _prompt {
my ($askpass, $prompt) = @_;
return unless length $askpass;
+ $prompt =~ s/\n/ /g;
my $ret;
open my $fh, "-|", $askpass, $prompt or return;
$ret = <$fh>;
diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
index a2cbcc8..74daa7a 100644
--- a/perl/Git/SVN/Prompt.pm
+++ b/perl/Git/SVN/Prompt.pm
@@ -62,16 +62,16 @@ sub ssl_server_trust {
issuer_dname fingerprint);
my $choice;
prompt:
- print STDERR $may_save ?
+ my $options = $may_save ?
"(R)eject, accept (t)emporarily or accept (p)ermanently? " :
"(R)eject or accept (t)emporarily? ";
STDERR->flush;
- $choice = lc(substr(<STDIN> || 'R', 0, 1));
- if ($choice =~ /^t$/i) {
+ $choice = lc(substr(Git::prompt("Certificate problem.\n" . $options) || 'R', 0, 1));
+ if ($choice eq 't') {
$cred->may_save(undef);
- } elsif ($choice =~ /^r$/i) {
+ } elsif ($choice eq 'r') {
return -1;
- } elsif ($may_save && $choice =~ /^p$/i) {
+ } elsif ($may_save && $choice eq 'p') {
$cred->may_save($may_save);
} else {
goto prompt;
@@ -109,9 +109,7 @@ sub username {
if (defined $_username) {
$username = $_username;
} else {
- print STDERR "Username: ";
- STDERR->flush;
- chomp($username = <STDIN>);
+ $username = Git::prompt("Username: ");
}
$cred->username($username);
$cred->may_save($may_save);
@@ -120,7 +118,7 @@ sub username {
sub _read_password {
my ($prompt, $realm) = @_;
- my $password = Git::prompt($prompt);
+ my $password = Git::prompt($prompt, 1);
$password;
}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 3/5] run-command: drop silent_exec_failure arg from wait_or_whine
From: Jeff King @ 2012-11-11 16:55 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111163100.GB13188@sigill.intra.peff.net>
We do not actually use this parameter; instead we complain
from the child itself (for fork/exec) or from start_command
(if we are using spawn on Windows).
Signed-off-by: Jeff King <peff@peff.net>
---
Just a cleanup I noticed while in the area.
run-command.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/run-command.c b/run-command.c
index 3b982e4..3aae270 100644
--- a/run-command.c
+++ b/run-command.c
@@ -226,7 +226,7 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
}
-static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
+static int wait_or_whine(pid_t pid, const char *argv0)
{
int status, code = -1;
pid_t waiting;
@@ -432,8 +432,7 @@ fail_pipe:
* At this point we know that fork() succeeded, but execvp()
* failed. Errors have been reported to our stderr.
*/
- wait_or_whine(cmd->pid, cmd->argv[0],
- cmd->silent_exec_failure);
+ wait_or_whine(cmd->pid, cmd->argv[0]);
failed_errno = errno;
cmd->pid = -1;
}
@@ -538,7 +537,7 @@ fail_pipe:
int finish_command(struct child_process *cmd)
{
- return wait_or_whine(cmd->pid, cmd->argv[0], cmd->silent_exec_failure);
+ return wait_or_whine(cmd->pid, cmd->argv[0]);
}
int run_command(struct child_process *cmd)
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCH 1/5] launch_editor: refactor to use start/finish_command
From: Jeff King @ 2012-11-11 16:55 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111163100.GB13188@sigill.intra.peff.net>
The launch_editor function uses the convenient run_command_*
interface. Let's use the more flexible start_command and
finish_command functions, which will let us manipulate the
parent state while we're waiting for the child to finish.
Signed-off-by: Jeff King <peff@peff.net>
---
editor.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/editor.c b/editor.c
index d834003..842f782 100644
--- a/editor.c
+++ b/editor.c
@@ -37,8 +37,16 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
if (strcmp(editor, ":")) {
const char *args[] = { editor, path, NULL };
+ struct child_process p;
- if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env))
+ memset(&p, 0, sizeof(p));
+ p.argv = args;
+ p.env = env;
+ p.use_shell = 1;
+ if (start_command(&p) < 0)
+ return error("unable to start editor '%s'", editor);
+
+ if (finish_command(&p))
return error("There was a problem with the editor '%s'.",
editor);
}
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-11 16:54 UTC (permalink / raw)
To: René Scharfe; +Cc: Jean-Jacques Lafay, msysgit, Git List, Philip Oakley
In-Reply-To: <509FD668.20609@lsrfire.ath.cx>
On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
> >However, I couldn't reproduce it on Linux : where the windows
> >implementations crashes at a ~32000 depth (*not* exactly 32768, mind
> >you), on linux it happily went through 100000 commits. I didn't take
> >time to look much further, but maybe on my 64 bit Linux VM, the process
> >can afford to reserve a much bigger address range for the stack of each
> >thread than the 1Mb given to 32 bit processes on windows.
> >Jean-Jacques.
>
> I can reproduce it on Linux (Debian testing amd64) with ulimit -s
> 1000 to reduce the stack size from its default value of 8MB.
>
> After reverting ffc4b8012d9a4f92ef238ff72c0d15e9e1b402ed (tag: speed
> up --contains calculation) the test passes even with the smaller
> stack, but it makes "git tag --contains" take thrice the time as
> before.
Right, I am not too surprised. That function replaced the original
algorithm with a much faster depth-first recursive one. I haven't looked
closely yet at Jean-Jacques' iterative adaptation, but that direction
seems like a good fix for now.
Ultimately, I have some ideas for doing this in a breadth-first way,
which would make it more naturally iterative. It would involve having N
bits of storage per commit to check N tags, but it would mean that we
could get accurate answers in the face of clock skew (like the
merge-base calculation, it would merely get slower in the face of skew).
But since I haven't worked on that at all, fixing the depth-first
algorithm to be iterative makes sense to me.
-Peff
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Sven Strickroth @ 2012-11-11 16:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jakub Narebski
In-Reply-To: <7vmwzzqwud.fsf@alter.siamese.dyndns.org>
git-svn reads passwords from an interactive terminal or by using
GIT_ASKPASS helper tool. But if GIT_ASKPASS environment variable is not
set, git-svn does not try to use SSH_ASKPASS as git-core does. This
cause GUIs (w/o STDIN connected) to hang waiting forever for git-svn to
complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).
Commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795 also tried to solve
this issue, but was incomplete as described above.
Instead of using hand-rolled prompt-response code that only works with the
interactive terminal, a reusable prompt() method is introduced in this commit.
This change also adds a fallback to SSH_ASKPASS.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
perl/Git/SVN/Prompt.pm | 20 +-------------------
2 files changed, 48 insertions(+), 20 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 497f420..0a0fe91 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -58,7 +58,7 @@ require Exporter;
command_output_pipe command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
- remote_refs
+ remote_refs prompt
temp_acquire temp_release temp_reset temp_path);
@@ -511,6 +511,52 @@ C<git --html-path>). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+=item prompt ( PROMPT )
+
+Query user C<PROMPT> and return answer from user.
+
+Honours GIT_ASKPASS, SSH_ASKPASS environment variables for querying
+the user. If no *_ASKPASS variable is set or an error occoured,
+the terminal is tried as a fallback.
+
+=cut
+
+sub prompt {
+ my ($prompt) = @_;
+ my $ret;
+ if (exists $ENV{'GIT_ASKPASS'}) {
+ $ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
+ }
+ if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {
+ $ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);
+ }
+ if (!defined $ret) {
+ print STDERR $prompt;
+ STDERR->flush;
+ require Term::ReadKey;
+ Term::ReadKey::ReadMode('noecho');
+ $ret = '';
+ while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+ last if $key =~ /[\012\015]/; # \n\r
+ $ret .= $key;
+ }
+ Term::ReadKey::ReadMode('restore');
+ print STDERR "\n";
+ STDERR->flush;
+ }
+ return $ret;
+}
+
+sub _prompt {
+ my ($askpass, $prompt) = @_;
+ return unless length $askpass;
+ my $ret;
+ open my $fh, "-|", $askpass, $prompt or return;
+ $ret = <$fh>;
+ $ret =~ s/[\015\012]//g; # strip \r\n, chomp does not work on all systems (i.e. windows) as expected
+ close ($fh);
+ return $ret;
+}
=item repo_path ()
diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
index 3a6f8af..a2cbcc8 100644
--- a/perl/Git/SVN/Prompt.pm
+++ b/perl/Git/SVN/Prompt.pm
@@ -120,25 +120,7 @@ sub username {
sub _read_password {
my ($prompt, $realm) = @_;
- my $password = '';
- if (exists $ENV{GIT_ASKPASS}) {
- open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
- $password = <PH>;
- $password =~ s/[\012\015]//; # \n\r
- close(PH);
- } else {
- print STDERR $prompt;
- STDERR->flush;
- require Term::ReadKey;
- Term::ReadKey::ReadMode('noecho');
- while (defined(my $key = Term::ReadKey::ReadKey(0))) {
- last if $key =~ /[\012\015]/; # \n\r
- $password .= $key;
- }
- Term::ReadKey::ReadMode('restore');
- print STDERR "\n";
- STDERR->flush;
- }
+ my $password = Git::prompt($prompt);
$password;
}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 0/2] second try
From: Sven Strickroth @ 2012-11-11 16:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jakub Narebski
In-Reply-To: <7vmwzzqwud.fsf@alter.siamese.dyndns.org>
Hi,
Am 06.10.2012 20:28 schrieb Junio C Hamano:
> It is either that it was simply forgotten, or after I wrote the part
> you quoted early in January there were discussions later that showed
> the patch was not desirable for some reason. I do not recall which.
I noticed no threads about possible problems, so I try again.
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: René Scharfe @ 2012-11-11 16:46 UTC (permalink / raw)
To: Jean-Jacques Lafay; +Cc: msysgit, Git List, Philip Oakley, Jeff King
In-Reply-To: <d9843c09-3ca9-406e-9df8-78603235d985@googlegroups.com>
Am 10.11.2012 22:13, schrieb Jean-Jacques Lafay:
> Le samedi 10 novembre 2012 21:00:10 UTC+1, Philip Oakley a écrit :
>
> From: "Jean-Jacques Lafay" <jeanjacq...@gmail.com <javascript:>>
> Sent: Saturday,
> November 10, 2012 5:36 PM
> > In large repos, the recursion implementation of contains(commit,
> > commit_list)
> > may result in a stack overflow. Replace the recursion with a loop to
> > fix it.
> >
> > Signed-off-by: Jean-Jacques Lafay <jeanjacq...@gmail.com
> <javascript:>>
>
> This is a change to the main git code so it is better to submit it to
> the main git list at g...@vger.kernel.org <javascript:> (plain text,
> no HTML, first
> post usually has a delay ;-)
>
> It sounds reasonable but others may have opinions and comments.
>
> Have you actually suffered from the stack overflow issue? You only
> suggest it as a possibility, rather than a real problem.
>
> Philip
>
>
> Yes, it actually crashed on me, and since the call is made by
> GitExtension while browsing the commit history, it was quickly annoying
> to have windows popping a "git.exe stopped working" message box at my
> face every time I clicked on a line of the history ;-) (well, you can
> sort of work around it by having the "file tree" or "diff" tab active)
>
> Coincidentally, as I was having a glance at the recent topics, I saw
> that someone else experienced it.
>
> However, I couldn't reproduce it on Linux : where the windows
> implementations crashes at a ~32000 depth (*not* exactly 32768, mind
> you), on linux it happily went through 100000 commits. I didn't take
> time to look much further, but maybe on my 64 bit Linux VM, the process
> can afford to reserve a much bigger address range for the stack of each
> thread than the 1Mb given to 32 bit processes on windows.
> Jean-Jacques.
I can reproduce it on Linux (Debian testing amd64) with ulimit -s 1000
to reduce the stack size from its default value of 8MB.
After reverting ffc4b8012d9a4f92ef238ff72c0d15e9e1b402ed (tag: speed up
--contains calculation) the test passes even with the smaller stack, but
it makes "git tag --contains" take thrice the time as before.
René
^ permalink raw reply
* [PATCH] Re:gitweb: add readme to overview page
From: Heinrich Schuchardt @ 2012-11-11 16:40 UTC (permalink / raw)
To: git; +Cc: xypron.glpk
In-Reply-To: <74707c20a5448989963314a02858c304017b02e3>
In this version of the patch the formatting has been corrected.
Warnings for double / in filenames are avoided.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
gitweb/gitweb.perl | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..699ffac 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6369,6 +6369,18 @@ sub git_project_list {
}
git_project_search_form($searchtext, $search_use_regexp);
+ # If XSS prevention is on, we don't include README.html.
+ # TODO: Allow a readme in some safe format.
+ my $path = "";
+ if (defined $project_filter) {
+ $path = "/$project_filter";
+ }
+ if (!$prevent_xss && -s "$projectroot$path/README.html") {
+ print "<div class=\"title\">readme</div>\n" .
+ "<div class=\"readme\">\n";
+ insert_file("$projectroot$path/README.html");
+ print "\n</div>\n"; # class="readme"
+ }
git_project_list_body(\@list, $order);
git_footer_html();
}
--
1.7.10.4
^ 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