* 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
* Re: [PATCH v5 01/15] fast-export: avoid importing blob marks
From: Jeff King @ 2012-11-11 16:38 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Felipe Contreras, git, Junio C Hamano, Johannes Schindelin,
Max Horn, Sverre Rabbelier, Brandon Casey, Brandon Casey,
Jonathan Nieder, Ilari Liusvaara, Pete Wyckoff, Ben Walton,
Matthieu Moy, Julian Phillips
In-Reply-To: <509FD425.5030702@web.de>
On Sun, Nov 11, 2012 at 05:36:53PM +0100, Torsten Bögershausen wrote:
> On 11.11.12 14:59, Felipe Contreras wrote:
> > test_expect_success 'test biridectionality' '
> > + echo -n > marks-cur &&
> > + echo -n > marks-new &&
> Unless I messed up the patch:
>
> Minor issue: still a typo "biridectionality"
> Major issue: "echo -n" is still not portable.
>
> Could we simply use
>
> touch marks-cur &&
> touch marks-new
Yes, "echo -n" is definitely not portable. Our preferred way of
creating an empty file is just ">file".
-Peff
^ permalink raw reply
* Re: [PATCH v5 01/15] fast-export: avoid importing blob marks
From: Torsten Bögershausen @ 2012-11-11 16:36 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Johannes Schindelin, Max Horn, Jeff King,
Sverre Rabbelier, Brandon Casey, Brandon Casey, Jonathan Nieder,
Ilari Liusvaara, Pete Wyckoff, Ben Walton, Matthieu Moy,
Julian Phillips
In-Reply-To: <1352642392-28387-2-git-send-email-felipe.contreras@gmail.com>
On 11.11.12 14:59, Felipe Contreras wrote:
> test_expect_success 'test biridectionality' '
> + echo -n > marks-cur &&
> + echo -n > marks-new &&
Unless I messed up the patch:
Minor issue: still a typo "biridectionality"
Major issue: "echo -n" is still not portable.
Could we simply use
touch marks-cur &&
touch marks-new
/Torsten
^ permalink raw reply
* [PATCH 0/5] ignore SIGINT while editor runs
From: Jeff King @ 2012-11-11 16:31 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <20121111154846.GA13188@sigill.intra.peff.net>
On Sun, Nov 11, 2012 at 10:48:46AM -0500, Jeff King wrote:
> Silly me. When I thought through the impact of Paul's patch, I knew that
> we would notice signal death of the editor. But I totally forgot to
> consider that the blocked signal is inherited by the child process. I
> think we just need to move the signal() call to after we've forked. Like
> this (on top of Paul's patch):
> [...]
> Note that this will give you a slightly verbose message from git.
> Potentially we could notice editor death due to SIGINT and suppress the
> message, under the assumption that the user hit ^C and does not need to
> be told.
Here's a series that I think should resolve the situation for everybody.
[1/5]: launch_editor: refactor to use start/finish_command
The cleanup I sent out a few minutes ago.
[2/5]: launch_editor: ignore SIGINT while the editor has control
Paul's patch rebased on my 1/5.
[3/5]: run-command: drop silent_exec_failure arg from wait_or_whine
[4/5]: run-command: do not warn about child death by SIGINT
[5/5]: launch_editor: propagate SIGINT from editor to git
Act more like current git when the editor dies from SIGINT.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)
From: Jeff King @ 2012-11-11 15:48 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <87wqxs4o6f.fsf@Niukka.kon.iki.fi>
On Sun, Nov 11, 2012 at 09:02:48AM +0200, Kalle Olavi Niemitalo wrote:
> If git did the same thing as cvs here, i.e. ignore the signals in
> the parent process only and check the exit status of the editor,
> I think that would be OK.
Silly me. When I thought through the impact of Paul's patch, I knew that
we would notice signal death of the editor. But I totally forgot to
consider that the blocked signal is inherited by the child process. I
think we just need to move the signal() call to after we've forked. Like
this (on top of Paul's patch):
diff --git a/editor.c b/editor.c
index 3ca361b..0ed23ce 100644
--- a/editor.c
+++ b/editor.c
@@ -38,11 +38,20 @@ 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;
+ p.env = env;
+ p.use_shell = 1;
+ if (start_command(&p) < 0)
+ return error("unable to start editor '%s'", editor);
+
sigchain_push(SIGINT, SIG_IGN);
- ret = run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env);
+ ret = finish_command(&p);
sigchain_pop(SIGINT);
+
if (ret)
return error("There was a problem with the editor '%s'.",
editor);
Note that this will give you a slightly verbose message from git.
Potentially we could notice editor death due to SIGINT and suppress the
message, under the assumption that the user hit ^C and does not need to
be told.
-Peff
^ permalink raw reply related
* [PATCH] gitweb: add readme to overview page
From: Heinrich Schuchardt @ 2012-11-11 15:32 UTC (permalink / raw)
To: git; +Cc: xypron.glpk
For repositories it is possible to maintain a README.html which will
be shown on the summary page. This is not possible for the server
root.
German law requires to provide contact data on the web server. This
data could easily be entered in the overview page using a README.html.
Furthermore it is possible to put the repositories not directly into
the root directory but into a subdirectory. Here also a README.html
would be helpful to indicate what the subdirectory is about.
The patch introduces README.html functionality for the root directory
and all subdirectories.
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..30cd028 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
* Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Jeff King @ 2012-11-11 15:09 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jens Lehmann
In-Reply-To: <CALkWK0=zTCXki2c=ugRXE485ps2=OWag7mdzVJW93cnsypwkiA@mail.gmail.com>
On Sun, Nov 11, 2012 at 08:20:27PM +0530, Ramkumar Ramachandra wrote:
> >> diff --git a/builtin/diff.c b/builtin/diff.c
> >> index 9650be2..6d00311 100644
> >> --- a/builtin/diff.c
> >> +++ b/builtin/diff.c
> >> @@ -297,6 +297,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
> >> DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
> >> DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
> >>
> >> + /* Set SUBMODULE_LOG if diff.submodule config var was set */
> >> + if (submodule_format_cfg && !strcmp(submodule_format_cfg, "log"))
> >> + DIFF_OPT_SET(&rev.diffopt, SUBMODULE_LOG);
> >> +
> >
> > Yuck. Why is this parsing happening in cmd_diff?
>
> Blame Jens- see this thread |
> http://thread.gmane.org/gmane.comp.version-control.git/206816/focus=206815
I don't think that is the right path, as at means that the option can
only ever affect diff, not other porcelains. I was thinking something
more like this (completely untested):
diff --git a/diff.c b/diff.c
index e89a201..74f4fc6 100644
--- a/diff.c
+++ b/diff.c
@@ -37,6 +37,13 @@ static int diff_stat_graph_width;
static int diff_dirstat_permille_default = 30;
static struct diff_options default_diff_options;
+/*
+ * 0 for "short", 1 for "log". This should probably just be an enum, and
+ * SUBMODULE_LOG lifted up from being a bit in the options to being its own
+ * struct member.
+ */
+static int diff_submodule_default;
+
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_NORMAL, /* PLAIN */
@@ -178,6 +185,19 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "diff.ignoresubmodules"))
handle_ignore_submodules_arg(&default_diff_options, value);
+ if (!strcmp(var, "diff.submodule")) {
+ /* XXX this should be factored out from the command-line parser */
+ if (!value)
+ return config_error_nonbool(var);
+ else if (!strcmp(var, "short"))
+ diff_submodule_default = 0;
+ else if (!strcmp(var, "log"))
+ diff_submodule_default = 1;
+ else
+ return error("unknown %s value: %s", var, value);
+ return 0;
+ }
+
if (git_color_config(var, value, cb) < 0)
return -1;
@@ -3193,6 +3213,9 @@ void diff_setup(struct diff_options *options)
options->a_prefix = "a/";
options->b_prefix = "b/";
}
+
+ if (diff_submodule_default)
+ DIFF_OPT_SET(options, SUBMODULE_LOG);
}
void diff_setup_done(struct diff_options *options)
^ permalink raw reply related
* Re: Help requested - trying to build a tool doing whole-tree commits
From: Eric S. Raymond @ 2012-11-11 14:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlie9pf96.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> Perhaps not exactly what you are looking for, but don't we have
> import-tar somewhere in contrib/fast-import hierarchy (sorry, not on
> a machine yet, and I cannot give more details).
If I recall correctly, that can only be used for original import.
I think Andreas Schwab's suggestion (git add -A && git write-tree &&
git commit-tree && git update-ref) is probably the right thing. I'm
going to try that.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-11 15:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann, Nahor
In-Reply-To: <7vzk2oo2d2.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]
On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote:
> The change seems to think "branch" is the _only_ thing the user
> might want to record per submodule upon "git submodule add".
I felt that earlier floating/tracking submodule patches were biting
off more than they could chew, so I was looking for a lightweight fix
to make the tracking workflow easier. It seems like I ended up with
something that is too lightweight ;).
> On the other hand, if this were one small part of a series to define
> the "tip following mode" where (at least)
>
> (1) "git submodule update [$path]" makes sure that the checkout of
> the submodule at $path matches the commit at the tip of the
> branch named by submodule.$name.branch in .gitmodules of the
> superproject, instead of the commit that is recorded in the
> index of the superproject; and
As I mentioned earlier, I think
$ git submodule update [$path]
should keep its current “checkout the already-registered SHA”
functionality, with
$ git submodule update --pull [$path]
pulling the tracked branch. I'll add a patch implementing this to v4.
In order to avoid losing (or creating) local-only submodule commits,
I'll probably bail (with an error) on non-fast-forward pulls. Can
anyone else think of other safety concerns?
This means that I'll probably drop Phil's $submodule_* export in v4,
because the only explicit use we have for it is this branch tracking.
I still think it is a useful idea, but it may not be useful enough to
be worth the complexity.
> (2) "git diff [$path]" and friends in the superproject compares the
> HEAD of the checkout of the submodule at $path with the tip of
> the branch named by submodule.$name.branch in .gitmodules of
> the superproject, instead of the commit that is recorded in the
> index of the superproject.
>
Hmm. “git diff” compares the working tree with the local HEAD (just a
SHA for submodules), so I don't think it should care about the status
of a remote branch. This sounds like you want something like:
$ git submodule foreach 'git diff origin/$submodule_branch'
Perhaps this is enough motivation for keeping $submodule_* exports?
> and the option were called something like "--follow-branch=$branch",
> …
I'll replace -r/--record with --follow-branch in v4.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Ramkumar Ramachandra @ 2012-11-11 14:50 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, Jens Lehmann
In-Reply-To: <20121108205110.GB8376@sigill.intra.peff.net>
Jeff King wrote:
> On Thu, Nov 01, 2012 at 04:13:49PM +0530, Ramkumar Ramachandra wrote:
>
>> diff --git a/builtin/diff.c b/builtin/diff.c
>> index 9650be2..6d00311 100644
>> --- a/builtin/diff.c
>> +++ b/builtin/diff.c
>> @@ -297,6 +297,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>> DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
>> DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
>>
>> + /* Set SUBMODULE_LOG if diff.submodule config var was set */
>> + if (submodule_format_cfg && !strcmp(submodule_format_cfg, "log"))
>> + DIFF_OPT_SET(&rev.diffopt, SUBMODULE_LOG);
>> +
>
> Yuck. Why is this parsing happening in cmd_diff?
Blame Jens- see this thread |
http://thread.gmane.org/gmane.comp.version-control.git/206816/focus=206815
> Then you can keep the parsing logic for "log" in diff.c. And you should
> factor it out into a function so that the command-line option and the
> config parser can share the same code. I know it's only one line now,
> but anybody who wants to add an option will have to update both places.
> See the parsing of diff.dirstat for an example.
Done. I'll send out the new series shortly.
Ram
^ permalink raw reply
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