* Re: Commit message problem of reverting multiple commits
From: Junio C Hamano @ 2012-11-13 16:34 UTC (permalink / raw)
To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6Qn68TFVnd_8LSf6OMqHZduAFgk0Hd46E3vKgFHCjpksQ@mail.gmail.com>
乙酸鋰 <ch3cooli@gmail.com> writes:
> I ran git 1.8.0 command line
>
> git revert --no-commit rev1 rev2
>
> I see a prepared commit message like
>
> Revert "<description from one commit>"
> This reverts commit <SHA1 of one commit>.
>
>
> The actual revert content is correct - it is all the relevant commits
> that were selected. I expect the message to reflect this:
>
> Revert "<description from commit1>", "<description from commit2>"
> This reverts commits <SHA1 of commit1>, <SHA1 of commit2>.
Hrmph. I actually think the revert-content is not correct.
I think the command should not take more than one commit on the
command line when --no-commit is in use in the first place (the same
thing can be said for cherry-pick). After all, "git revert rev1
rev2" is to revert rev1 and then rev2 independently, so unless that
option is spelled "--squash", the resulting history should have two
commits that reverts rev1 and rev2 independently.
^ permalink raw reply
* Re: Notes in format-patch
From: Junio C Hamano @ 2012-11-13 16:29 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <50A2213B.4060505@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Michael J Gruber venit, vidit, dixit 12.11.2012 15:18:
>> 'git replace' parses the revision arguments when it creates replacements
>> (so that a sha1 can be abbreviated, e.g.) but not when deleting
>> replacements.
>>
>> Make it parse the argument to 'replace -d' in the same way.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>
>> Notes:
>> v3 safeguards the hex buffer against reuse
>> builtin/replace.c | 16 ++++++++++------
>> t/t6050-replace.sh | 11 +++++++++++
>> 2 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/replace.c b/builtin/replace.c
>
> By the way - Junio, is that the intented outcome of "format-patch
> --notes"? I would rather put the newline between the note and the
> diffstat...
I do not mind (actually I personally would prefer to see) a blank
line between the three-dash and "Notes:", but I agree that we should
have a blank line before the diffstat block.
^ permalink raw reply
* Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Junio C Hamano @ 2012-11-13 16:27 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Jeff King, Git List, Jens Lehmann
In-Reply-To: <CALkWK0nJwznx36yoAUKXRnyA+32143tBVJHnnrosz-Ht7VhwHw@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Jeff King wrote:
>> On Sun, Nov 11, 2012 at 10:29:05PM +0530, Ramkumar Ramachandra wrote:
>>> @@ -223,6 +238,15 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
>>> return 0;
>>> }
>>>
>>> + if (!strcmp(var, "diff.submodule")) {
>>
>> Shouldn't this be in git_diff_ui_config so it does not affect scripts
>> calling plumbing?
>
> I honestly didn't understand the difference between
> git_diff_basic_config and git_diff_ui_config. The latter function
> calls the former function at the end of its body. Why are they two
> separate functions in the first place?
In case you meant s/didn't/don't/, git_diff_ui_config() should be
called only by human-facing Porcelain commands where their
behaviours can and should be affected by end user configuration
variables.
When a configuration variable should not affect output from plumbing
commands like diff-files, diff-index, and diff-tree, it must not be
read in git_diff_basic_config(), but in git_diff_ui_config().
The output from "git format-patch" is consumed by "git apply" that
expects "Subproject commit %s\n" with fully spelled object name, so
your configuration must not affect the output of format-patch,
either.
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Junio C Hamano @ 2012-11-13 16:13 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, git, Thomas Rast, Jonathan Nieder
In-Reply-To: <20121113074720.GA18746@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Nov 13, 2012 at 07:42:58AM +0100, Felipe Contreras wrote:
> ...
>> 5) GIT_COMMITTER
>>
>> Who should the emails appear to be from? [Felipe Contreras 2nd
>> <felipe.contreras+2@gmail.com>]
>>
>> Whoa, what happened there?
>>
>> Well:
>>
>> $sender = $repoauthor || $repocommitter || '';
>> ($repoauthor) = Git::ident_person(@repo, 'author');
>> % ./git var GIT_AUTHOR_IDENT
>> Felipe Contreras 2nd <felipe.contreras+2@gmail.com> 1352783223 +0100
>>
>> That's right, AUTHOR_IDENT would fall back to the default email and full name.
>
> Yeah, I find that somewhat questionable in the current behavior, and I'd
> consider it a bug. Typically we prefer the committer ident when given a
> choice (e.g., for writing reflog entries).
Just to make sure I follow the discussion correctly, do you mean
that the bug is that we pick a fuzzy AUTHOR when COMMITTER is
specified more concretely and we usually use COMMIITTER for this
kind of thing in the first place but send-email does not in this
case (I do not see "git var GIT_AUTHOR_IDENT" returning value from
the implicit logic as a bug in this case---just making sure).
>> 6.1) GIT_AUTHOR without anything else
>>
>> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
>> var GIT_COMMITTER_IDENT: command returned error: 128
>
> Doesn't that seem like a regression? It used to work.
I do not think we mind a change to favor GIT_COMMITTER setting over
GIT_AUTHOR to be consistent with others too much, but that indeed is
a big change. People who are used to the inconsistency that
send-email favors AUTHOR over COMMITTER will certainly find it as a
regression.
> The explicitness needs to be tied to the specific ident we grabbed.
> Probably adding a "git var GIT_AUTHOR_EXPLICIT" would be enough, or
> alternatively, adding a flag to "git var" to error out rather than
> return a non-explicit ident (this may need to adjust the error
> handling of the "git var" calls from send-email).
Yeah, something like that would be necessary for "git var" users to
make this kind of decision.
> While simultaneously breaking "git commit" for people who are happily
> using the implicit generation. I can see the appeal of doing so; I was
> tempted to suggest it when I cleaned up IDENT_STRICT a few months back.
> But do we have any data on how many people are currently using that
> feature that would be annoyed by it?
As we didn't break "git commit" when you worked on IDENT_STRICT, I
do not think we have any data on it ;-)
> ...
> It may be something I would work on myself in the future, but I have
> other things to work on at the moment, and since you are interested in
> the topic, I thought you would be a good candidate to polish it enough
> to be suitable upstream. But instead I see a lot of push-back on what I
> considered to be a fairly straightforward technical comment on a
> regression.
> ...
> But I feel like I am fighting an uphill battle just to convince you that
> regressions are bad, and that I am having to make the same points
> repeatedly. That makes me frustrated and less excited about reviewing
> your patches; and when I say "it is not my itch", that is my most polite
> way of saying "If that is going to be your attitude, then I do not feel
> like dealing with you anymore on this topic".
For a change with low benefit/cost ratio like this, the best course
of action often is to stop paying attention to the thread that has
become unproductive and venomous, and resurrect the topic after
people forgot about it and doing it right yourself ;-).
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jakub Narębski @ 2012-11-13 15:57 UTC (permalink / raw)
To: Kevin
Cc: Drew Northup, Jeff King, glpk xypron, git, Junio C Hamano,
Jason J Pyeron CTR (US), Andreas Schwab
In-Reply-To: <CAO54GHCzeWv41Bu5By0JOzbBHGuzXV=krdDr0U=QsMBun7PF7A@mail.gmail.com>
On Tue, Nov 13, 2012 at 4:45 PM, Kevin <ikke@ikke.info> wrote:
> The problem with input filtering is that you can only filter for one
> output scenario. What if the the input is going to be output in a wiki
> like environment, or to pdf, or whatever? Then you have to unescape
> the data again, and maybe apply filtering/escaping for those
> environments.
>
> You only know how to escape data when you are going to output it, so
> then is the the best moment to escape it.
Also there are so many ways to evade XSS filtering
https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet
If you can and should escape data (like in our case), it cannot not work;
not possible to evade it.
--
Jakub Narebski
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Kevin @ 2012-11-13 15:45 UTC (permalink / raw)
To: Drew Northup
Cc: Jeff King, glpk xypron, git, Jakub Narębski, Junio C Hamano,
Jason J Pyeron CTR (US), Andreas Schwab
In-Reply-To: <CAM9Z-nkuHj8MWLfWsvY=EqHXCUS+Pk5Ezv6m5J+cnh7cQHNc_g@mail.gmail.com>
The problem with input filtering is that you can only filter for one
output scenario. What if the the input is going to be output in a wiki
like environment, or to pdf, or whatever? Then you have to unescape
the data again, and maybe apply filtering/escaping for those
environments.
You only know how to escape data when you are going to output it, so
then is the the best moment to escape it.
^ permalink raw reply
* Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Ramkumar Ramachandra @ 2012-11-13 15:45 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, Jens Lehmann
In-Reply-To: <20121113053336.GA10995@sigill.intra.peff.net>
Jeff King wrote:
> On Sun, Nov 11, 2012 at 10:29:05PM +0530, Ramkumar Ramachandra wrote:
>> @@ -223,6 +238,15 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
>> return 0;
>> }
>>
>> + if (!strcmp(var, "diff.submodule")) {
>
> Shouldn't this be in git_diff_ui_config so it does not affect scripts
> calling plumbing?
I honestly didn't understand the difference between
git_diff_basic_config and git_diff_ui_config. The latter function
calls the former function at the end of its body. Why are they two
separate functions in the first place?
Btw, I just posted a follow-up.
Ram
^ permalink raw reply
* [PATCH v4 4/4] submodule: display summary header in bold
From: Ramkumar Ramachandra @ 2012-11-13 15:42 UTC (permalink / raw)
To: Git List
In-Reply-To: <1352821367-3611-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 ffaed72..1065978 100644
--- a/diff.c
+++ b/diff.c
@@ -2261,7 +2261,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);
+ meta, del, add, reset);
return;
}
diff --git a/submodule.c b/submodule.c
index e3e0b45..2f55436 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 *meta,
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..", meta, 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..3dc1b3f 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 *meta,
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 v4 2/4] diff: introduce diff.submodule configuration variable
From: Ramkumar Ramachandra @ 2012-11-13 15:42 UTC (permalink / raw)
To: Git List
In-Reply-To: <1352821367-3611-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 ++-
diff.c | 32 ++++++++++++++++++++++++++++----
t/t4041-diff-submodule-option.sh | 30 +++++++++++++++++++++++++++++-
4 files changed, 66 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/diff.c b/diff.c
index e89a201..7f2a255 100644
--- a/diff.c
+++ b/diff.c
@@ -123,6 +123,17 @@ 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)
+{
+ if (!strcmp(value, "log"))
+ DIFF_OPT_SET(options, SUBMODULE_LOG);
+ else if (!strcmp(value, "short"))
+ DIFF_OPT_CLR(options, SUBMODULE_LOG);
+ else
+ return -1;
+ return 0;
+}
+
static int git_config_rename(const char *var, const char *value)
{
if (!value)
@@ -178,6 +189,13 @@ 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")) {
+ if (parse_submodule_params(&default_diff_options, value))
+ warning(_("Unknown value for 'diff.submodule' config variable: '%s'"),
+ value);
+ return 0;
+ }
+
if (git_color_config(var, value, cb) < 0)
return -1;
@@ -3475,6 +3493,14 @@ 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 *value)
+{
+ if (parse_submodule_params(options, value))
+ die(_("Failed to parse --submodule option parameter: '%s'"),
+ value);
+ return 1;
+}
+
int diff_opt_parse(struct diff_options *options, const char **av, int ac)
{
const char *arg = av[0];
@@ -3655,10 +3681,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 v4 3/4] diff: rename "set" variable
From: Ramkumar Ramachandra @ 2012-11-13 15:42 UTC (permalink / raw)
To: Git List
In-Reply-To: <1352821367-3611-1-git-send-email-artagnon@gmail.com>
From: Jeff King <peff@peff.net>
Once upon a time the builtin_diff function used one color, and the color
variables were called "set" and "reset". Nowadays it is a much longer
function and we use several colors (e.g., "add", "del"). Rename "set" to
"meta" to show that it is the color for showing diff meta-info (it still
does not indicate that it is a "color", but at least it matches the
scheme of the other color variables).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
diff.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 7f2a255..ffaed72 100644
--- a/diff.c
+++ b/diff.c
@@ -2240,7 +2240,7 @@ static void builtin_diff(const char *name_a,
mmfile_t mf1, mf2;
const char *lbl[2];
char *a_one, *b_two;
- const char *set = diff_get_color_opt(o, DIFF_METAINFO);
+ const char *meta = diff_get_color_opt(o, DIFF_METAINFO);
const char *reset = diff_get_color_opt(o, DIFF_RESET);
const char *a_prefix, *b_prefix;
struct userdiff_driver *textconv_one = NULL;
@@ -2287,24 +2287,24 @@ static void builtin_diff(const char *name_a,
b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
- strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, set, a_one, b_two, reset);
+ strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, meta, a_one, b_two, reset);
if (lbl[0][0] == '/') {
/* /dev/null */
- strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, set, two->mode, reset);
+ strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, meta, two->mode, reset);
if (xfrm_msg)
strbuf_addstr(&header, xfrm_msg);
must_show_header = 1;
}
else if (lbl[1][0] == '/') {
- strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, set, one->mode, reset);
+ strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, meta, one->mode, reset);
if (xfrm_msg)
strbuf_addstr(&header, xfrm_msg);
must_show_header = 1;
}
else {
if (one->mode != two->mode) {
- strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, set, one->mode, reset);
- strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, set, two->mode, reset);
+ strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, meta, one->mode, reset);
+ strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, meta, two->mode, reset);
must_show_header = 1;
}
if (xfrm_msg)
--
1.7.8.1.362.g5d6df.dirty
^ permalink raw reply related
* [PATCH v4 1/4] Documentation: move diff.wordRegex from config.txt to diff-config.txt
From: Ramkumar Ramachandra @ 2012-11-13 15:42 UTC (permalink / raw)
To: Git List
In-Reply-To: <1352821367-3611-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 v4 0/4] Introduce diff.submodule
From: Ramkumar Ramachandra @ 2012-11-13 15:42 UTC (permalink / raw)
To: Git List
In-Reply-To: <http://mid.gmane.org/1352653146-3932-1-git-send-email-artagnon@gmail.com>
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
v3 is here: http://mid.gmane.org/1352653146-3932-1-git-send-email-artagnon@gmail.com
This version was prepared in response to Peff's review of v3.
What changed:
* Functional code simplified and moved to git_diff_ui_config.
* Peff contributed one additional patch to the series.
Thanks.
Ram
Jeff King (1):
diff: rename "set" variable
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 +-
diff.c | 46 ++++++++++++++++++++++++++++---------
submodule.c | 8 +++---
submodule.h | 2 +-
t/t4041-diff-submodule-option.sh | 30 ++++++++++++++++++++++++-
7 files changed, 84 insertions(+), 24 deletions(-)
--
1.7.8.1.362.g5d6df.dirty
^ permalink raw reply
* Re: [PATCH] update-index/diff-index: use core.preloadindex to improve performance
From: karsten.blees @ 2012-11-13 15:36 UTC (permalink / raw)
To: Jeff King; +Cc: git, msysgit, pro-logic
In-Reply-To: <20121102153800.GE11170@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote on 02.11.2012 16:38:00:
> On Fri, Nov 02, 2012 at 11:26:16AM -0400, Jeff King wrote:
>
> > Still, I don't think we need to worry about performance regressions,
> > because people who don't have a setup suitable for it will not turn on
> > core.preloadindex in the first place. And if they have it on, the more
> > places we use it, probably the better.
>
> BTW, your patch was badly damaged in transit (wrapped, and tabs
> converted to spaces). I was able to fix it up, but please check your
> mailer's settings.
>
Yes, I feared as much, that's why I included the pull URL (the company MTA
only accepts outbound mail from Notes clients, sorry).
Is there a policy for people with broken mailers (send patch as
attachment, add pull URL more prominently, don't include plaintext patch
at all...)?
^ permalink raw reply
* Re: [PATCH] update-index/diff-index: use core.preloadindex to improve performance
From: karsten.blees @ 2012-11-13 15:33 UTC (permalink / raw)
To: Jeff King; +Cc: git, msysgit, pro-logic
In-Reply-To: <20121102152616.GD11170@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote on 02.11.2012 16:26:16:
> On Tue, Oct 30, 2012 at 10:50:42AM +0100, karsten.blees@dcon.de wrote:
>
> > 'update-index --refresh' and 'diff-index' (without --cached) don't
honor
> > the core.preloadindex setting yet. Porcelain commands using these
(such as
> > git [svn] rebase) suffer from this, especially on Windows.
> >
> > Use read_cache_preload to improve performance.
> >
> > Additionally, in builtin/diff.c, don't preload index status if we
don't
> > access the working copy (--cached).
> >
> > Results with msysgit on WebKit repo (2GB in 200k files):
> >
> > | update-index | diff-index | rebase
> > ----------------+--------------+------------+---------
> > msysgit-v1.8.0 | 9.157s | 10.536s | 42.791s
> > + preloadindex | 9.157s | 10.536s | 28.725s
> > + this patch | 2.329s | 2.752s | 15.152s
> > + fscache [1] | 0.731s | 1.171s | 8.877s
>
> Cool numbers. On my quad-core SSD Linux box, I saw a few speedups, too.
> Here are the numbers for "update-index --refresh" on the WebKit repo
> (all are wall clock time, best-of-five):
>
> | before | after
> -----------+--------+--------
> cold cache | 4.513s | 2.059s
> warm cache | 0.252s | 0.164s
>
Great, and thanks for testing on Linux (I only have Linux VMs for testing,
and I couldn't get meaningful performance data from those).
> Not as dramatic, but still nice. I wonder how a spinning disk would fare
> on the cold-cache case, though. I also tried it with all but one CPU
> disabled, and the warm cache case was a little bit slower.
>
Unfortunately, with a 'real' disk, cold cache times increase with the
number of threads. I've played around with '#define MAX_PARALLEL 20' in
preload-index.c, with the following results ('git update-index --refresh'
on WebKit repo, Win7 x64, Core i7 860 (2.8GHz 4 Core HT), WD VelociRaptor
(300G 10krpm 8ms), msysgit 1.8.0 + preload-index patch + fscache patch):
MAX_PARALLEL | cold cache | warm cache
-------------+------------+------------
no preload | 49.938 | 9.204 (*)
1 | 45.412 | 1.622
2 | 55.334 | 1.123
3 | 65.973 | 0.982
4 | 67.579 | 0.889
5 | 76.159 | 0.827
6 | 81.510 | 0.811
7 | 86.269 | 0.858
8 | 85.862 | 0.827
...
10 | 87.953 | 0.717
...
20 | 176.251 | 0.749
(*) core.preloadindex currently also disables fscache, thus the 9s
With more threads, Windows resource monitor also shows increasing disk
queue length and response times.
It seems clear that more concurrent disk seeks hurt cold cache performance
pretty badly. On the other hand, warm cache improvements for #threads >
#cores are only about 10 - 20%.
I don't know if Linux is better at caching / prefetching directory
listings (might depend on file system, too), but perhaps MAX_PARALLEL
should be set to a more reasonable value, or be made configurable (e.g.
core.preloadIndexThreads)?
Karsten
^ permalink raw reply
* checkout from neighbour branch undeletes a path?
From: Peter Vereshagin @ 2012-11-13 15:23 UTC (permalink / raw)
To: git
Hello.
Am wondering if 'checkout branch path' undeletes the files? For the example
below I'd like the 'file00.txt' to be deleted and never checked out from the
previous branch... How can I do that?
$ git init
Initialized empty Git repository in /tmp/repo00/.git/
$ mkdir pathdir
$ echo test00 > pathdir/file00.txt
$ git add pathdir
$ git commit -am 'added file00.txt'
[master (root-commit) d4f7c70] added file00.txt
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 pathdir/file00.txt
$ git branch -m master branch00
$ git branch branch01
$ rm pathdir/file00.txt
$ echo test01 > pathdir/file01.txt
$ git add pathdir
$ git status
$ git commit -am 'added file01.txt; removed file00.txt'
[branch00 c3e78ff] added file01.txt; removed file00.txt
2 files changed, 1 insertions(+), 1 deletions(-)
delete mode 100644 pathdir/file00.txt
create mode 100644 pathdir/file01.txt
$ git checkout branch01
Switched to branch 'branch01'
$ rm -r pathdir
$ git checkout branch00 pathdir
$ find pathdir/
pathdir/
pathdir/file00.txt
pathdir/file01.txt
$
I know about 'merge' and it's not the what I need: to import only the
particular subdirectory from the previous branch.
Thank you.
--
Peter Vereshagin <peter@vereshagin.org> (http://vereshagin.org) pgp: A0E26627
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jakub Narębski @ 2012-11-13 15:19 UTC (permalink / raw)
To: Drew Northup
Cc: Jeff King, glpk xypron, git, Junio C Hamano,
Jason J Pyeron CTR (US), Andreas Schwab
In-Reply-To: <CAM9Z-nkuHj8MWLfWsvY=EqHXCUS+Pk5Ezv6m5J+cnh7cQHNc_g@mail.gmail.com>
On Tue, Nov 13, 2012 at 3:44 PM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Mon, Nov 12, 2012 at 3:24 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Nov 12, 2012 at 01:55:46PM -0500, Drew Northup wrote:
>>> + # No XSS <script></script> inclusions
>>> + if ($input =~ m!(<script>)(.*)(</script>)!){
>>> + return undef;
>>> + }
>> This is the wrong fix for a few reasons:
>>
>> 1. It is on the input-validation side, whereas the real problem is on
>> the output-quoting side. Your patch means I could not access a file
>> called "<script>foo</script>". What we really want is to have the
>> unquoted name internally, but then make sure we quote it when
>> outputting as part of an HTML (or XML) file.
>
> I don't buy the argument that we don't need to clean up the input as
> well. There are scant few of us that are going to name a file
> "<script>alert("Something Awful")</script>" in this world (I am
> probably one of them). Input validation is key to keeping problems
> like this from coming up repeatedly as those writing the guts of
> programs are typically more interested in getting the "assigned task"
> done and reporting the output to the user in a safe manner.
Input cleanup or blacklisting *does not* prevent code injection (XSS
in this case). This is a myth.
Input validation has its place, and is done by gitweb when possible
(see e.g. evaluate_and_validate_params, validate_project, etc.).
But the proposed solution is not input validation.
'<script>alert("Something Awful")</script>' is a perfectly valid filename.
As is more realistic "<<create>>.uml" or "File > Open screenshot.png".
And last and most important you have to escape output anyway;
filename is not HTML. Without escaping it would be rendered incorrectly.
And HTML escaping prevents XSS.
>> I think the right answer is going to be a well-placed call to esc_html.
>> This already happens automatically when we go through the CGI
>> element-building functions, but obviously we failed to make the call
>> when building the output manually. This is a great reason why template
>> languages which default to safe expansion should always be used.
>> Unfortunately, gitweb is living in 1995 in terms of web frameworks.
>
> Escaping the output protects the user, but it DOES NOT protect the
> server. We MUST handle both possibilities.
Errr, what?
If you are thinking about shell injection, we are covered.
Gitweb uses list form of open which is for shell what prepared
statements are for SQL. In one or two cases where we need to
use pipe we do shell escaping.
> Besides, inserting one call to esc_html only fixes one attack path. I
> didn't look to see if all others were already covered.
They should be covered. This case slipped.
--
Jakub Narebski
^ permalink raw reply
* Commit message problem of reverting multiple commits
From: 乙酸鋰 @ 2012-11-13 15:15 UTC (permalink / raw)
To: git
Hi,
I ran git 1.8.0 command line
git revert --no-commit rev1 rev2
I see a prepared commit message like
Revert "<description from one commit>"
This reverts commit <SHA1 of one commit>.
The actual revert content is correct - it is all the relevant commits
that were selected. I expect the message to reflect this:
Revert "<description from commit1>", "<description from commit2>"
This reverts commits <SHA1 of commit1>, <SHA1 of commit2>.
Regards,
ch3cooli
^ permalink raw reply
* Re: Launch separate program for HTTPS prompt
From: Erik Faye-Lund @ 2012-11-13 15:07 UTC (permalink / raw)
To: 乙酸鋰; +Cc: GIT Mailing-list
In-Reply-To: <CAHtLG6Q2LANj8WPMsRzpzKZ=zPbLysSFWs0pxSPNj1j87d_LRw@mail.gmail.com>
Please don't cull the CC list or top-post. Other people might be
interested, and quoting inline will make it easier to understand what
you're replying to in that case. I've restored the CC-list and fixed
the quoting for you this time.
On Tue, Nov 13, 2012 at 3:58 PM, 乙酸鋰 <ch3cooli@gmail.com> wrote:
> 2012/11/13 Erik Faye-Lund <kusmabite@gmail.com>:
>> On Tue, Oct 23, 2012 at 4:48 PM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>> 乙酸鋰 <ch3cooli@gmail.com> writes:
>>>
>>>> Hi,
>>>>
>>>> Could you launch a separate program for HTTPS prompt like SSH_ASKPASS?
>>>> This allows GUI programs to answer the trust or not, not via stdin.
>>>
>>> Read about credential helpers, this is what they are meant for, and they
>>> do launch separate programs.
>>>
>>
>> The prompting-support for credential helpers didn't survive the
>> inclusion; instead we have git_prompt() that consults GIT_ASKPASS and
>> SSH_ASKPASS or falls back to git_terminal_prompt().
>
> Sorry, I don't understand what you mean.
>
> But I am asking for a solution of for HTTPS prompt which is similar to
> the credential part.
Quote from Documentation/technical/api-credentials.txt:
"Credential helpers are programs executed by git to fetch or save
credentials from and to long-term storage (where "long-term" is simply
longer than a single git process; e.g., credentials may be stored
in-memory for a few minutes, or indefinitely on disk)."
In other words, credential helpers are not intended for prompting the
user for a username/password pair. Prompting was included in the first
credential-helper proposal, but it didn't survive into the inclusion.
But you can override credential queries by setting GIT_ASKPASS or
SSH_ASKPASS.
I don't know if the credential-system works or not when using
HTTP/HTTPS, it might be that cURL is the one that does the
credential-negotiating in that case. But that's not really related to
credential-helpers; it affects the whole credential-system.
^ permalink raw reply
* Re: Launch separate program for HTTPS prompt
From: Erik Faye-Lund @ 2012-11-13 14:51 UTC (permalink / raw)
To: Matthieu Moy; +Cc: 乙酸鋰, git
In-Reply-To: <vpq6261nt0x.fsf@grenoble-inp.fr>
On Tue, Oct 23, 2012 at 4:48 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> 乙酸鋰 <ch3cooli@gmail.com> writes:
>
>> Hi,
>>
>> Could you launch a separate program for HTTPS prompt like SSH_ASKPASS?
>> This allows GUI programs to answer the trust or not, not via stdin.
>
> Read about credential helpers, this is what they are meant for, and they
> do launch separate programs.
>
The prompting-support for credential helpers didn't survive the
inclusion; instead we have git_prompt() that consults GIT_ASKPASS and
SSH_ASKPASS or falls back to git_terminal_prompt().
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Drew Northup @ 2012-11-13 14:44 UTC (permalink / raw)
To: Jeff King
Cc: glpk xypron, git, jnareb, Junio C Hamano, Jason J Pyeron CTR (US),
Andreas Schwab
In-Reply-To: <20121112202413.GD4623@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 3:24 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Nov 12, 2012 at 01:55:46PM -0500, Drew Northup wrote:
>
>> On Sun, Nov 11, 2012 at 6:28 PM, glpk xypron <xypron.glpk@gmx.de> wrote:
>> > Gitweb can be used to generate an RSS feed.
>> >
>> > Arbitrary tags can be inserted into the XML document describing
>> > the RSS feed by careful construction of the URL.
>> [...]
>> Something like this may be useful to defuse the "file" parameter, but
>> I presume a more definitive fix is in order...
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 10ed9e5..af93e65 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -1447,6 +1447,10 @@ sub validate_pathname {
>> if ($input =~ m!\0!) {
>> return undef;
>> }
>> + # No XSS <script></script> inclusions
>> + if ($input =~ m!(<script>)(.*)(</script>)!){
>> + return undef;
>> + }
>> return $input;
>> }
>
> This is the wrong fix for a few reasons:
>
> 1. It is on the input-validation side, whereas the real problem is on
> the output-quoting side. Your patch means I could not access a file
> called "<script>foo</script>". What we really want is to have the
> unquoted name internally, but then make sure we quote it when
> outputting as part of an HTML (or XML) file.
I don't buy the argument that we don't need to clean up the input as
well. There are scant few of us that are going to name a file
"<script>alert("Something Awful")</script>" in this world (I am
probably one of them). Input validation is key to keeping problems
like this from coming up repeatedly as those writing the guts of
programs are typically more interested in getting the "assigned task"
done and reporting the output to the user in a safe manner.
> 2. Script tags are only part of the problem. They are what make it
> obviously a security vulnerability, but it is equally incorrect for
> us to show the filename "<b>foo</b>" as bold. I would also not be
> surprised if there are other cross-site attacks one can do without
> using <script>.
Yes, there are. You are typically concerned with anything including
the following:
(1) Executable stuff;
(2) Out of nowhere resources that can reference executable stuff
(style / CSS, iframe, script includes);
(3) Media and other things that activate browser plugins directly.
> 3. Your filter is too simplistic. At the very least, it would not
> filter out "<SCRIPT>". I am not up to date on all of the
> sneaking-around-HTML-filters attacks that are available these days,
> but I wonder if one could also get around it using XML entities or
> similar.
You will note that I said "a more definitive fix is in order" in my
original. In other words, I claimed it to be utterly incomplete to
start with. I wanted to get some thought going about input validation
(in particular since I am not a perl guru of any sort whatsoever--the
fair number of things I've written from scratch or mangled into shape
notwithstanding).
> I think the right answer is going to be a well-placed call to esc_html.
> This already happens automatically when we go through the CGI
> element-building functions, but obviously we failed to make the call
> when building the output manually. This is a great reason why template
> languages which default to safe expansion should always be used.
> Unfortunately, gitweb is living in 1995 in terms of web frameworks.
Escaping the output protects the user, but it DOES NOT protect the
server. We MUST handle both possibilities.
Besides, inserting one call to esc_html only fixes one attack path. I
didn't look to see if all others were already covered.
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: [PATCH 0/5] win32: support echo for terminal-prompt
From: Erik Faye-Lund @ 2012-11-13 14:04 UTC (permalink / raw)
To: git, msysgit; +Cc: peff
In-Reply-To: <1352815288-3996-1-git-send-email-kusmabite@gmail.com>
Sorry, I messed up the subject (lacking RFC-prefix), so I aborted
after sending the cover-letter. I'll resend with a proper prefix right
away.
On Tue, Nov 13, 2012 at 3:01 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> We currently only support getpass, which does not echo at all, for
> git_terminal_prompt on Windows. The Windows console is perfectly
> capable of doing this, so let's make it so.
>
> This implementation tries to reuse the /dev/tty-code as much as
> possible.
>
> The big reason that this becomes a bit hairy is that Ctrl+C needs
> to be handled correctly, so we don't leak the console state to a
> non-echoing setting when a user aborts.
>
> Windows makes this bit a little bit tricky, in that we need to
> implement SIGINT for fgetc. However, I suspect that this is a good
> thing to do in the first place.
>
> An earlier iteration was also breifly discussed here:
> http://mid.gmane.org/CABPQNSaUCEDU4+2N63n0k_XwSXOP_iFZG3GEYSPSBPcSVV8wRQ@mail.gmail.com
>
> The series can also be found here, only with an extra patch that
> makes the (interactive) testing a bit easier:
>
> https://github.com/kusma/git/tree/work/terminal-cleanup
>
> Erik Faye-Lund (5):
> mingw: make fgetc raise SIGINT if apropriate
> compat/terminal: factor out echo-disabling
> compat/terminal: separate input and output handles
> mingw: reuse tty-version of git_terminal_prompt
> mingw: get rid of getpass implementation
>
> compat/mingw.c | 91 +++++++++++++++++++++++++++-----------
> compat/mingw.h | 8 +++-
> compat/terminal.c | 129 ++++++++++++++++++++++++++++++++++++++++--------------
> 3 files changed, 169 insertions(+), 59 deletions(-)
>
> --
> 1.8.0.7.gbeffeda
>
--
*** 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/RFC 5/5] mingw: get rid of getpass implementation
From: Erik Faye-Lund @ 2012-11-13 14:04 UTC (permalink / raw)
To: git, msysgit; +Cc: peff
In-Reply-To: <1352815447-8824-1-git-send-email-kusmabite@gmail.com>
There's no remaining call-sites, and as pointed out in the
previous commit message, it's not quite ideal. So let's just
lose it.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
compat/mingw.c | 15 ---------------
compat/mingw.h | 2 --
2 files changed, 17 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 33ddfdf..5fc14b7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1758,21 +1758,6 @@ int link(const char *oldpath, const char *newpath)
return 0;
}
-char *getpass(const char *prompt)
-{
- struct strbuf buf = STRBUF_INIT;
-
- fputs(prompt, stderr);
- for (;;) {
- char c = _getch();
- if (c == '\r' || c == '\n')
- break;
- strbuf_addch(&buf, c);
- }
- fputs("\n", stderr);
- return strbuf_detach(&buf, NULL);
-}
-
pid_t waitpid(pid_t pid, int *status, int options)
{
HANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
diff --git a/compat/mingw.h b/compat/mingw.h
index 6b9e69a..f494ecb 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -55,8 +55,6 @@ struct passwd {
char *pw_dir;
};
-extern char *getpass(const char *prompt);
-
typedef void (__cdecl *sig_handler_t)(int);
struct sigaction {
sig_handler_t sa_handler;
--
1.8.0.7.gbeffeda
--
*** 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 related
* [PATCH/RFC 4/5] mingw: reuse tty-version of git_terminal_prompt
From: Erik Faye-Lund @ 2012-11-13 14:04 UTC (permalink / raw)
To: git, msysgit; +Cc: peff
In-Reply-To: <1352815447-8824-1-git-send-email-kusmabite@gmail.com>
The getpass-implementation we use on Windows isn't at all ideal;
it works in raw-mode (as opposed to cooked mode), and as a result
does not deal correcly with deletion, arrow-keys etc.
Instead, use cooked mode to read a line at the time, allowing the
C run-time to process the input properly.
Since we set files to be opened in binary-mode by default on
Windows, introduce a FORCE_TEXT macro that expands to the "t"
modifier that forces the terminal to be opened in text-mode so we
do not have to deal with CRLF issues.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
compat/terminal.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/compat/terminal.c b/compat/terminal.c
index 4a1fd3d..ce0fbd9 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -3,8 +3,22 @@
#include "sigchain.h"
#include "strbuf.h"
+#if defined(HAVE_DEV_TTY) || defined(WIN32)
+
+static void restore_term(void);
+
+static void restore_term_on_signal(int sig)
+{
+ restore_term();
+ sigchain_pop(sig);
+ raise(sig);
+}
+
#ifdef HAVE_DEV_TTY
+#define INPUT_PATH "/dev/tty"
+#define OUTPUT_PATH "/dev/tty"
+
static int term_fd = -1;
static struct termios old_term;
@@ -18,13 +32,6 @@ static void restore_term(void)
term_fd = -1;
}
-static void restore_term_on_signal(int sig)
-{
- restore_term();
- sigchain_pop(sig);
- raise(sig);
-}
-
static int disable_echo()
{
struct termios t;
@@ -46,17 +53,61 @@ error:
return -1;
}
+#elif defined(WIN32)
+
+#define INPUT_PATH "CONIN$"
+#define OUTPUT_PATH "CONOUT$"
+#define FORCE_TEXT "t"
+
+static HANDLE hconin = INVALID_HANDLE_VALUE;
+static DWORD cmode;
+
+static void restore_term(void)
+{
+ if (hconin == INVALID_HANDLE_VALUE)
+ return;
+
+ SetConsoleMode(hconin, cmode);
+ CloseHandle(hconin);
+ hconin = INVALID_HANDLE_VALUE;
+}
+
+static int disable_echo(void)
+{
+ hconin = CreateFile("CONIN$", GENERIC_READ | GENERIC_WRITE,
+ FILE_SHARE_READ, NULL, OPEN_EXISTING,
+ FILE_ATTRIBUTE_NORMAL, NULL);
+ if (hconin == INVALID_HANDLE_VALUE)
+ return -1;
+
+ GetConsoleMode(hconin, &cmode);
+ sigchain_push_common(restore_term_on_signal);
+ if (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {
+ CloseHandle(hconin);
+ hconin = INVALID_HANDLE_VALUE;
+ return -1;
+ }
+
+ return 0;
+}
+
+#endif
+
+#ifndef FORCE_TEXT
+#define FORCE_TEXT
+#endif
+
char *git_terminal_prompt(const char *prompt, int echo)
{
static struct strbuf buf = STRBUF_INIT;
int r;
FILE *input_fh, *output_fh;
- input_fh = fopen("/dev/tty", "r");
+ input_fh = fopen(INPUT_PATH, "r" FORCE_TEXT);
if (!input_fh)
return NULL;
- output_fh = fopen("/dev/tty", "w");
+ output_fh = fopen(OUTPUT_PATH, "w" FORCE_TEXT);
if (!output_fh) {
fclose(input_fh);
return NULL;
--
1.8.0.7.gbeffeda
--
*** 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 related
* [PATCH/RFC 3/5] compat/terminal: separate input and output handles
From: Erik Faye-Lund @ 2012-11-13 14:04 UTC (permalink / raw)
To: git, msysgit; +Cc: peff
In-Reply-To: <1352815447-8824-1-git-send-email-kusmabite@gmail.com>
On Windows, the terminal cannot be opened in read-write mode, so
we need distinct pairs for reading and writing. Since this works
fine on other platforms as well, always open them in pairs.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
compat/terminal.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/compat/terminal.c b/compat/terminal.c
index 3217838..4a1fd3d 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -50,29 +50,36 @@ char *git_terminal_prompt(const char *prompt, int echo)
{
static struct strbuf buf = STRBUF_INIT;
int r;
- FILE *fh;
+ FILE *input_fh, *output_fh;
- fh = fopen("/dev/tty", "w+");
- if (!fh)
+ input_fh = fopen("/dev/tty", "r");
+ if (!input_fh)
return NULL;
+ output_fh = fopen("/dev/tty", "w");
+ if (!output_fh) {
+ fclose(input_fh);
+ return NULL;
+ }
+
if (!echo && disable_echo()) {
- fclose(fh);
+ fclose(input_fh);
+ fclose(output_fh);
return NULL;
}
- fputs(prompt, fh);
- fflush(fh);
+ fputs(prompt, output_fh);
+ fflush(output_fh);
- r = strbuf_getline(&buf, fh, '\n');
+ r = strbuf_getline(&buf, input_fh, '\n');
if (!echo) {
- fseek(fh, SEEK_CUR, 0);
- putc('\n', fh);
- fflush(fh);
+ putc('\n', output_fh);
+ fflush(output_fh);
}
restore_term();
- fclose(fh);
+ fclose(input_fh);
+ fclose(output_fh);
if (r == EOF)
return NULL;
--
1.8.0.7.gbeffeda
--
*** 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 related
* [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling
From: Erik Faye-Lund @ 2012-11-13 14:04 UTC (permalink / raw)
To: git, msysgit; +Cc: peff
In-Reply-To: <1352815447-8824-1-git-send-email-kusmabite@gmail.com>
By moving the echo-disabling code to a separate function, we can
implement OS-specific versions of it for non-POSIX platforms.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
compat/terminal.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/compat/terminal.c b/compat/terminal.c
index bbb038d..3217838 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -14,6 +14,7 @@ static void restore_term(void)
return;
tcsetattr(term_fd, TCSAFLUSH, &old_term);
+ close(term_fd);
term_fd = -1;
}
@@ -24,6 +25,27 @@ static void restore_term_on_signal(int sig)
raise(sig);
}
+static int disable_echo()
+{
+ struct termios t;
+
+ term_fd = open("/dev/tty", O_RDWR);
+ if (tcgetattr(term_fd, &t) < 0)
+ goto error;
+
+ old_term = t;
+ sigchain_push_common(restore_term_on_signal);
+
+ t.c_lflag &= ~ECHO;
+ if (!tcsetattr(term_fd, TCSAFLUSH, &t))
+ return 0;
+
+error:
+ close(term_fd);
+ term_fd = -1;
+ return -1;
+}
+
char *git_terminal_prompt(const char *prompt, int echo)
{
static struct strbuf buf = STRBUF_INIT;
@@ -34,24 +56,9 @@ char *git_terminal_prompt(const char *prompt, int echo)
if (!fh)
return NULL;
- if (!echo) {
- struct termios t;
-
- if (tcgetattr(fileno(fh), &t) < 0) {
- fclose(fh);
- return NULL;
- }
-
- old_term = t;
- term_fd = fileno(fh);
- sigchain_push_common(restore_term_on_signal);
-
- t.c_lflag &= ~ECHO;
- if (tcsetattr(fileno(fh), TCSAFLUSH, &t) < 0) {
- term_fd = -1;
- fclose(fh);
- return NULL;
- }
+ if (!echo && disable_echo()) {
+ fclose(fh);
+ return NULL;
}
fputs(prompt, fh);
--
1.8.0.7.gbeffeda
--
*** 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 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