* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-12 20:27 UTC (permalink / raw)
To: Drew Northup; +Cc: glpk xypron, git, jnareb, Junio C Hamano
In-Reply-To: <20121112202413.GD4623@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 03:24:13PM -0500, Jeff King wrote:
> I think the right answer is going to be a well-placed call to esc_html.
I'm guessing the right answer is this:
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..a51a8ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -8055,6 +8055,7 @@ sub git_feed {
$feed_type = 'history';
}
$title .= " $feed_type";
+ $title = esc_html($title);
my $descr = git_get_project_description($project);
if (defined $descr) {
$descr = esc_html($descr);
but I did not test it (and I am not that familiar with gitweb, so it is
a slight guess from spending 5 minutes grepping and reading).
-Peff
^ permalink raw reply related
* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Jeff King @ 2012-11-12 20:32 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <1352742068-15346-2-git-send-email-felipe.contreras@gmail.com>
On Mon, Nov 12, 2012 at 06:41:05PM +0100, Felipe Contreras wrote:
> From: Ramkumar Ramachandra <artagnon@gmail.com>
>
> 'hg commit' fails otherwise in some versiosn of mercurial because of
s/versiosn/versions/
> +setup () {
> + (
> + echo "[ui]"
> + echo "username = A U Thor <author@example.com>"
> + ) >> "$HOME"/.hgrc
> +}
This makes sense, but I wonder if we should use something different from
the git author ident set up by the test scripts, just to double check
that we do not have any bugs in confusing the two during the import.
Something like "H G Wells <wells@example.com>" would work, and satisfies
my deep-seated desire for bad puns.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Junio C Hamano @ 2012-11-12 20:32 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121112201235.GA7210@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 12, 2012 at 12:08:37PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > Changing the interface for get_author_ident_from_commit would be a pain,
>> > but if we just wanted to help filter-branch, we could do something like
>> > this:
>>
>> Yes, that is the direction I was alluding to.
>>
>> Callers of get_author_ident_from_commit can also do the same and
>> avoid rebuilding the same $pick_author_script over and over again,
>> or get_author_ident_from_commit can do so for its callers.
>
> I don't think get_author_ident_from_commit can do so on demand due to
> the subshell issue I mentioned.
OK, that is what I obviously missed.
> I wonder if we should simply generate these at build time and store them
> in the script.
Yeah, my knee-jerk reaction was that it was wasteful to run script
at runtime to build a constant string, but the engineering cost may
not be worth it.
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Junio C Hamano @ 2012-11-12 20:36 UTC (permalink / raw)
To: Jeff King; +Cc: Drew Northup, glpk xypron, git, jnareb
In-Reply-To: <20121112202701.GE4623@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 12, 2012 at 03:24:13PM -0500, Jeff King wrote:
>
>> I think the right answer is going to be a well-placed call to esc_html.
>
> I'm guessing the right answer is this:
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..a51a8ba 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -8055,6 +8055,7 @@ sub git_feed {
> $feed_type = 'history';
> }
> $title .= " $feed_type";
> + $title = esc_html($title);
> my $descr = git_get_project_description($project);
> if (defined $descr) {
> $descr = esc_html($descr);
>
> but I did not test it (and I am not that familiar with gitweb, so it is
> a slight guess from spending 5 minutes grepping and reading).
Yeah, that looks correct, given the way how the other variables
emitted with the same "print" like $descr and $owner are formed.
^ permalink raw reply
* Re: [PATCHv3] replace: parse revision argument for -d
From: Jeff King @ 2012-11-12 20:42 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <a35a8f44b908bded0b475b02e7917011fb3bf90b.1352728712.git.git@drmicha.warpmail.net>
On Mon, Nov 12, 2012 at 03:18:02PM +0100, Michael J Gruber wrote:
> '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
Thanks, I don't see any other functional problems.
> diff --git a/builtin/replace.c b/builtin/replace.c
> index e3aaf70..33e6ec3 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -46,24 +46,28 @@ typedef int (*each_replace_name_fn)(const char *name, const char *ref,
>
> static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
> {
> - const char **p;
> + const char **p, *q;
I find this readable today, but I wonder if in six months we will wonder
what in the world "q" means. Maybe "short_refname" or something would be
appropriate?
-Peff
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Felipe Contreras @ 2012-11-12 20:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: gitzilla, Michael J Gruber, Git Mailing List, Jeff King
In-Reply-To: <7vwqxqiul3.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
>
>> 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.
>
> More importantly, which do users prefer: quickly produce an
> incorrect result, or spend some more time to get it right?
Why not both?
If I do 'git clone hg::http://selenic.com/hg' I expect it to work, no
matter what. Then, if I care about getting it right, like for example
if the project is moving to git, then check
.git/hg/origin/bad-authors, and fill them with the right ones.
Of course, the current remote helper framework doesn't have the option
to map authors, but it could be added. That would be better than
letting every remote helper tool to have a custom way of mapping
authors, and also custom configuration for them.
> Because the exporting tool has a lot more intimate knowledge about
> how the names are represented in the history of the original SCM,
> canonicalization of the names, if done at that point, would likely
> to give us more useful results, than a canonicalization done at the
> beginning of the importer, which lacks SCM specific details. So in
> that sense, (a) is more preferrable than (b).
But it doesn't have more intimate knowledge. It has exactly the same
information as fast-import; nothing.
What intimate knowledge is a tool expected to get from this?
% hg commit -u 'Foo Bar<foo.bar@example.com> <none@none>' -m one
% hg --debug log
changeset: 0:5ef37a2c773f02d0e01f1ecdcc59149832d294e8
tag: tip
phase: draft
parent: -1:0000000000000000000000000000000000000000
parent: -1:0000000000000000000000000000000000000000
manifest: 0:c6d4cd25b9fc2f83b0dd51f4acbea9486fce54d7
user: Foo Bar<foo.bar@example.com> <none@none>
date: Sun Nov 11 18:33:00 2012 +0100
files+: file
extra: branch=default
description:
one
Some tools might, but if they did, then bad authors wouldn't be a problem.
> On the other hand, we would want consistency across the converted
> results no matter what SCM the history was originally in. E.g. a
> name without email that came from CVS or SVN would consistently want
> to become "name <noname@noname>" or "name <name>" or whatever, and
> letting exporting tools responsible for the canonicalization will
> lead them to create their own garbage. In that sense, (b) can be
> better than (a).
Or 'Unknown <unknown>' or '<none@none>' or '<>', or any of the forms
conversion tools have been doing for ages.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Update cygwin.c for new mingw-64 win32 api headers
From: Jeff King @ 2012-11-12 20:58 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <1352679440-4098-1-git-send-email-mlevedahl@gmail.com>
On Sun, Nov 11, 2012 at 07:17:20PM -0500, Mark Levedahl wrote:
> The cygwin project recently switched to a new implementation of the
> windows api, now using header files from the mingw-64 project. These
> new header files are incompatible with the way cygwin.c included the
> old headers: cygwin.c can be compiled using the new or the older (mingw)
> headers, but different files must be included in different order for each
> to work. The new headers are in use only for the current release series
> (based upon the v1.7.x dll version). The previous release series using
> the v1.5 dll is kept available but unmaintained for use on older versions
> of Windows. So, patch cygwin.c to use the new include ordering only if
> the dll version is 1.7 or higher.
I have very little knowledge of cygwin compatibility, so thanks for a
nice explanation. I'll queue it in 'pu' for now, and hopefully we can
get some test reports from other cygwin folks (on new and old cygwin).
> diff --git a/Makefile b/Makefile
> index f69979e..1cc5d96 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1082,6 +1082,7 @@ ifeq ($(uname_O),Cygwin)
> NO_SYMLINK_HEAD = YesPlease
> NO_IPV6 = YesPlease
> OLD_ICONV = UnfortunatelyYes
> + V15_MINGW_HEADERS = YesPlease
> endif
The "if" part of the conditional that did not make it into the context
above is an expr match for "1.6.*" From the name, I would think that we
would want to use these headers on cygwin 1.5.* , too. Is v1.5 too old
to care about now?
-Peff
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jakub Narębski @ 2012-11-12 21:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Drew Northup, glpk xypron, git
In-Reply-To: <7vmwymh83r.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 9:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>> On Mon, Nov 12, 2012 at 03:24:13PM -0500, Jeff King wrote:
>>
>>> I think the right answer is going to be a well-placed call to esc_html.
>>
>> I'm guessing the right answer is this:
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 10ed9e5..a51a8ba 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -8055,6 +8055,7 @@ sub git_feed {
>> $feed_type = 'history';
>> }
>> $title .= " $feed_type";
>> + $title = esc_html($title);
>> my $descr = git_get_project_description($project);
>> if (defined $descr) {
>> $descr = esc_html($descr);
>>
>> but I did not test it (and I am not that familiar with gitweb, so it is
>> a slight guess from spending 5 minutes grepping and reading).
>
> Yeah, that looks correct, given the way how the other variables
> emitted with the same "print" like $descr and $owner are formed.
It looks like good solution to me too.
Nb. the problems with feed are mainly because it is generated
by hand even more than HTML (which uses CGI.pm).
--
Jakub Narębski
--
Jakub Narebski
^ permalink raw reply
* Re: [RFC PATCH 1/1] status: Allow for short-form via config option
From: Jeff King @ 2012-11-12 21:17 UTC (permalink / raw)
To: Thomas Adam; +Cc: git
In-Reply-To: <1352674383-23654-2-git-send-email-thomas@xteddy.org>
On Sun, Nov 11, 2012 at 10:53:03PM +0000, Thomas Adam wrote:
> It is currently not possible to use the short-form output of git status
> without declaring an alias to do so.
>
> This isn't always desirable therfore, define a git config option which can
> be set to display the short-form: status.shortwithbranch
We usually try to avoid booleans for selection among options, even if
there is currently only one useful option. That way, it makes it easier
to extend later when another option presents itself. So having
"status.format" that you could set to "short", "long", or "porcelain"
would make more sense (although "short" is likely to be the only useful
one currently).
And then we can have a separate boolean like "status.branch" to show the
branch by default when showing short format. That would would naturally
extend to more booleans as other options are added (Phil Hord's recent
"show tree state" patches come to mind).
We also need to consider the impact of options on scripts. I think
status.format should be OK, since any script calling "git status" would
have to explicitly pass "--porcelain" already, which would override this
option. But we would want to make sure that "status.branch" would not
affect the porcelain.
> ---
> builtin/commit.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
It would need documentation, and tests would be good both to make sure
the feature works, but also to demonstrate that it does not break
--porcelain.
> diff --git a/builtin/commit.c b/builtin/commit.c
> index a17a5df..552a9f1 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1142,6 +1142,18 @@ static int git_status_config(const char *k, const char *v, void *cb)
> return error(_("Invalid untracked files mode '%s'"), v);
> return 0;
> }
> +
> + if (!strcmp(k, "status.shortwithbranch")) {
> + if (git_config_bool(k, v)) {
> + if (!v)
> + return config_error_nonbool(k);
> + else if(!strcmp(v, "true")) {
> + status_format = STATUS_FORMAT_SHORT;
> + s->show_branch = 1;
> + }
> + return 0;
> + }
> + }
I'm not sure what is going on with the extra nonbool and "true" check.
Shouldn't it just be:
if (git_config_bool(k, v)) {
status_format = STATUS_FORMAT_SHORT;
s->show_branch = 1;
}
else {
/* what do we do when it is false? */
}
return 0;
If we follow my suggestions above, then it would be more like:
if (!strcmp(k, "status.format")) {
if (!v)
return config_error_nonbool(k);
return parse_status_format(v, &status_format);
}
if (!strcmp(k, "status.branch")) {
s->show_branch = git_config_bool(k, v);
return 0;
}
but that would still have to resolve the setting of show_branch when
--porcelain is in use. I think you would need to store the
config-derived value separately, and then only fill it into
s->show_branch if no value was given on the command-line _and_ we are
not showing the porcelain format.
-Peff
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-12 21:34 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Junio C Hamano, Drew Northup, glpk xypron, git
In-Reply-To: <CANQwDwdRTeaVS5cMic5gv9SP1A8Z1vruOsZBFfMDQDTZHBAtvQ@mail.gmail.com>
On Mon, Nov 12, 2012 at 10:13:27PM +0100, Jakub Narębski wrote:
> > Yeah, that looks correct, given the way how the other variables
> > emitted with the same "print" like $descr and $owner are formed.
>
> It looks like good solution to me too.
>
> Nb. the problems with feed are mainly because it is generated
> by hand even more than HTML (which uses CGI.pm).
Yeah, I noticed that. Here it is in patch form with a test. It would be
nice if people interested in gitweb would add more entries to the XSS
test below (I put in the one that fails, along with an obvious variation
that is actually OK).
I didn't look carefully through the rest of gitweb for more XSS
instances. From a glance, it looks like we mostly use the safe CGI
methods, but probably it could use a full audit (which again, I would be
happy if people who care more about gitweb would do).
-- >8 --
Subject: [PATCH] gitweb: escape html in rss title
The title of an RSS feed is generated from many components,
including the filename provided as a query parameter, but we
failed to quote it. Besides showing the wrong output, this
is a vector for XSS attacks.
Signed-off-by: Jeff King <peff@peff.net>
---
gitweb/gitweb.perl | 1 +
t/t9502-gitweb-standalone-parse-output.sh | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..a51a8ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -8055,6 +8055,7 @@ sub git_feed {
$feed_type = 'history';
}
$title .= " $feed_type";
+ $title = esc_html($title);
my $descr = git_get_project_description($project);
if (defined $descr) {
$descr = esc_html($descr);
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 731e64c..3a8e7d3 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -185,5 +185,20 @@ test_expect_success 'forks: project_index lists all projects (incl. forks)' '
test_cmp expected actual
'
+xss() {
+ echo >&2 "Checking $1..." &&
+ gitweb_run "$1" &&
+ if grep "$TAG" gitweb.body; then
+ echo >&2 "xss: $TAG should have been quoted in output"
+ return 1
+ fi
+ return 0
+}
+
+test_expect_success 'xss checks' '
+ TAG="<magic-xss-tag>" &&
+ xss "a=rss&p=$TAG" &&
+ xss "a=rss&p=foo.git&f=$TAG"
+'
test_done
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* Re: [PATCH] Update cygwin.c for new mingw-64 win32 api headers
From: Mark Levedahl @ 2012-11-12 21:39 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121112205832.GI4623@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 3:58 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 11, 2012 at 07:17:20PM -0500, Mark Levedahl wrote:
>
>> The cygwin project recently switched to a new implementation of the
>> windows api, now using header files from the mingw-64 project. These
>> new header files are incompatible with the way cygwin.c included the
>> old headers: cygwin.c can be compiled using the new or the older (mingw)
>> headers, but different files must be included in different order for each
>> to work. The new headers are in use only for the current release series
>> (based upon the v1.7.x dll version). The previous release series using
>> the v1.5 dll is kept available but unmaintained for use on older versions
>> of Windows. So, patch cygwin.c to use the new include ordering only if
>> the dll version is 1.7 or higher.
>
> I have very little knowledge of cygwin compatibility, so thanks for a
> nice explanation. I'll queue it in 'pu' for now, and hopefully we can
> get some test reports from other cygwin folks (on new and old cygwin).
>
>> diff --git a/Makefile b/Makefile
>> index f69979e..1cc5d96 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1082,6 +1082,7 @@ ifeq ($(uname_O),Cygwin)
>> NO_SYMLINK_HEAD = YesPlease
>> NO_IPV6 = YesPlease
>> OLD_ICONV = UnfortunatelyYes
>> + V15_MINGW_HEADERS = YesPlease
>> endif
>
> The "if" part of the conditional that did not make it into the context
> above is an expr match for "1.6.*" From the name, I would think that we
> would want to use these headers on cygwin 1.5.* , too. Is v1.5 too old
> to care about now?
>
> -Peff
The opening if expression is:
ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
which I believe matches any version below 1.7. 1.5 is what is released
opensource, 1.6 is (or was) available as a supported product from
redhat. So, I think the stanza is ok as is.
Mark
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Jeff King @ 2012-11-12 21:41 UTC (permalink / raw)
To: Felipe Contreras; +Cc: A Large Angry SCM, Michael J Gruber, Git Mailing List
In-Reply-To: <CAMP44s1m8sAD9D0F-6b=+dm_AvLb_4_f7h=3A_VMYMDUEcTW7g@mail.gmail.com>
On Sun, Nov 11, 2012 at 07:48:14PM +0100, Felipe Contreras wrote:
> > 3. Exporters should not use it if they have any broken-down
> > representation at all. Even knowing that the first half is a human
> > name and the second half is something else would give it a better
> > shot at cleaning than fast-import would get.
>
> I'm not sure what you mean by this. If they have name and email, then
> sure, it's easy.
But not as easy as just printing it. What if you have this:
name="Peff <angle brackets> King"
email="<peff@peff.net>"
Concatenating them does not produce a valid git author name. Sending the
concatenation through fast-import's cleanup function would lose
information (namely, the location of the boundary between name and
email).
Similarly, one might have other structured data (e.g., CVS username)
where the structure is a useful hint, but some conversion to name+email
is still necessary.
-Peff
^ permalink raw reply
* Re: [PATCH] Update cygwin.c for new mingw-64 win32 api headers
From: Jeff King @ 2012-11-12 21:43 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <CAK2bgnXLnKmnVwzE5U_1qZueUdj-QaT23f-iFmhJyRto3tWnhA@mail.gmail.com>
On Mon, Nov 12, 2012 at 04:39:39PM -0500, Mark Levedahl wrote:
> >> diff --git a/Makefile b/Makefile
> >> index f69979e..1cc5d96 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -1082,6 +1082,7 @@ ifeq ($(uname_O),Cygwin)
> >> NO_SYMLINK_HEAD = YesPlease
> >> NO_IPV6 = YesPlease
> >> OLD_ICONV = UnfortunatelyYes
> >> + V15_MINGW_HEADERS = YesPlease
> >> endif
> >
> > The "if" part of the conditional that did not make it into the context
> > above is an expr match for "1.6.*" From the name, I would think that we
> > would want to use these headers on cygwin 1.5.* , too. Is v1.5 too old
> > to care about now?
>
> The opening if expression is:
> ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
> which I believe matches any version below 1.7. 1.5 is what is released
> opensource, 1.6 is (or was) available as a supported product from
> redhat. So, I think the stanza is ok as is.
Oh, right, I'm an idiot. I read the patch, thought "gee, I wonder what
that conditional just outside the context is", looked it up, then
promptly misremembered exactly what it said when writing the rest of my
email.
Yes, what you have is absolutely right. Sorry for the noise.
-Peff
^ permalink raw reply
* Candidates for sequencer?
From: Ramkumar Ramachandra @ 2012-11-12 21:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
Hi,
I'd like to get more commands to call into sequencer.c, so that we can
(finally) implement a sensible `git continue` and `git abort`.
Currently, am and rebase come to mind, but they are shell scripts and
I don't see how we can make them call into sequencer.c. Is there any
way to make sequencer.c more useful? Should we implement a `git
continue` and `git reset` anyway, lifting code from 83c750ac to check
the tree state?
Ram
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Ramkumar Ramachandra @ 2012-11-12 21:51 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Joe Perches, Jonathan Nieder, Pascal Obry
In-Reply-To: <1352653463-1923-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> cc-cmd is only per-file, and many times receipients get lost without
> seing the full patch series.
s/seing/seeing
> [...]
Looks good otherwise.
Ram
^ permalink raw reply
* Re: Candidates for sequencer?
From: Jeff King @ 2012-11-12 22:16 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0n0tTZ3EgzeesOr2B1LjeEUjTnWQh5dDfW28jA1ia0-gA@mail.gmail.com>
On Tue, Nov 13, 2012 at 03:16:51AM +0530, Ramkumar Ramachandra wrote:
> I'd like to get more commands to call into sequencer.c, so that we can
> (finally) implement a sensible `git continue` and `git abort`.
> Currently, am and rebase come to mind, but they are shell scripts and
> I don't see how we can make them call into sequencer.c. Is there any
> way to make sequencer.c more useful? Should we implement a `git
> continue` and `git reset` anyway, lifting code from 83c750ac to check
> the tree state?
I'd love it if ther was a mechanism for interruptible commands to notify
the rest of git that they are in progress. This would not only make
cross-language commands like "rebase" and "am" work easily, but it would
mean that we can automatically support third-party commands that we
don't even know about.
I am already doing something like this for some personal scripts which
do sequences of merges and rebases, and my "git continue" looks like:
if test -f "$GIT_DIR/continue"; then
eval "$(cat "$GIT_DIR/continue")"
elif test -d "$GIT_DIR/rebase-merge"; then
git rebase --continue
elif test -d "$GIT_DIR/rebase-apply"; then
if test -f "$GIT_DIR/rebase-apply/applying"; then
git am --continue
else
git rebase --continue
fi
fi
[... etc ...]
It works pretty well in practice, but I'd have a few suggestions on top:
1. Even if "am" and "rebase" learn to use $GIT_DIR/continue, we should
probably continue to support the fallback state-examination in case
people use mixed versions of git. And clearly the code should be
shared with what's in wt-status.c.
2. The "continue" file should probably be called "in-progress", and
should have more structured data, like the name of the operation.
3. The list of operations in progress needs to be a stack. If I am
rebasing and start a cherry pick, what would "git continue" do? It
probably makes sense to continue the most recently started
operation first.
4. Rebase already detects an in-progress rebase. It should probably
also detect an "am" or "cherry-pick" in progress. But it's not
commutative; you should be able to start a "cherry-pick" in the
middle of a "rebase". This is really independent of the storage
format (we could do it right now if the state logic from wt-status
is made available in a machine-readable format).
We _might_ want to put information in the in-progress file that
helps programs reach that decision. I'm not sure of a sane way of
doing it, though. You could say "don't rebase while I'm in
progress", but that depends on the writer knowing the universe of
all programs that might be run. It might be possible to come up
with some taxonomy of operations like "a history rewriting
operation is in progress" which would cause other history rewriting
operations to choose not to start. But coming up with the right set
of attributes might be impossible.
-Peff
^ permalink raw reply
* Re: [PATCH] custom log formats for "diff --submodule=log"
From: Jeff King @ 2012-11-12 22:19 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Jeffrey S. Haemer
In-Reply-To: <CALkWK0nT8RXXUUkLuS8zWVMG=wsm0b740q-qYb3iN3cO7Tb=0A@mail.gmail.com>
On Sun, Nov 11, 2012 at 06:35:16PM +0530, Ramkumar Ramachandra wrote:
> Ramkumar Ramachandra wrote:
> > Don't you mean `git diff` in place of `git log -p`
> > though? I don't think `git log --submodule` does anything differently
> > from `git log`.
>
> Sorry for the nonsense. I just realized that it affects the diffs
> shown by `git log -p`.
Right. I don't know if that is a common workflow (I do not really use
submodules), but it was the first thing I tried when I heard about
--submodule (and it does make sense to me that it would work).
But the logic could apply equally well to:
git diff --submodule=log:%an
-Peff
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jean-Jacques Lafay @ 2012-11-12 22:27 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, msysgit, Git List, Philip Oakley
In-Reply-To: <20121111165431.GA25884@sigill.intra.peff.net>
2012/11/11 Jeff King <peff@peff.net>:
> On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
>
> 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).
I guess the optimal algorithm may also depend on the commit graph
general shape, but intuitively, I'd say that the critical factor is
the number and distribution of tags. As soon as you have a significant
number of tags (let's say 1% of the commits are tagged, evenly
distributed), you'll quickly end up with every commit marked as
containing or not the target commit, so that each additional tag check
is cheap.
This suggests a complexity of O(number of commits) more often then
not, however you choose to traverse the graph.
At least on my almost-real-life repo*, with ~140,000 commits and
~2,000 tags, tag --contains takes a few seconds, of course more than
the worst-case test on a 40,000 commit / 1 tag repo, but still in the
same order of magnitude.
* : meaning we're in the process of migrating from clearcase (deep
sighs allowed !), and for now I kept almost everything in a single git
repo, which may not be the eventual choice
Jean-Jacques.
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Felipe Contreras @ 2012-11-12 22:47 UTC (permalink / raw)
To: Jeff King; +Cc: A Large Angry SCM, Michael J Gruber, Git Mailing List
In-Reply-To: <20121112214127.GA10531@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 10:41 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 11, 2012 at 07:48:14PM +0100, Felipe Contreras wrote:
>
>> > 3. Exporters should not use it if they have any broken-down
>> > representation at all. Even knowing that the first half is a human
>> > name and the second half is something else would give it a better
>> > shot at cleaning than fast-import would get.
>>
>> I'm not sure what you mean by this. If they have name and email, then
>> sure, it's easy.
>
> But not as easy as just printing it. What if you have this:
>
> name="Peff <angle brackets> King"
> email="<peff@peff.net>"
>
> Concatenating them does not produce a valid git author name. Sending the
> concatenation through fast-import's cleanup function would lose
> information (namely, the location of the boundary between name and
> email).
Right. Unfortunately I'm not aware of any DSCM that does that.
> Similarly, one might have other structured data (e.g., CVS username)
> where the structure is a useful hint, but some conversion to name+email
> is still necessary.
CVS might be the only one that has such structured data. I think in
subversion the username has no meaning. A 'felipec' subversion
username is as bad as a mercurial 'felipec' username.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Joe Perches @ 2012-11-12 22:52 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Felipe Contreras, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <CALkWK0mRKznNN7750h=k6aE3OJ7hBLVC-G51gEYi2+NuYjPWrQ@mail.gmail.com>
On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
> > cc-cmd is only per-file, and many times receipients get lost without
> > seing the full patch series.
>
> s/seing/seeing
>
> > [...]
>
> Looks good otherwise.
s/receipients/recipients/ too
Practically this is ok but I think it's unnecessary.
Output from git format-patch is always in a single
directory.
My work flow is to use a script for --to and --cc
lines that can be set to emit the same addresses for
all files in a patch series or generate different
addresses per patch file.
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-12 23:03 UTC (permalink / raw)
To: Joe Perches
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <1352760759.18715.7.camel@joe-AO722>
On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> Felipe Contreras wrote:
>> > cc-cmd is only per-file, and many times receipients get lost without
>> > seing the full patch series.
>>
>> s/seing/seeing
>>
>> > [...]
>>
>> Looks good otherwise.
>
> s/receipients/recipients/ too
>
> Practically this is ok but I think it's unnecessary.
>
> Output from git format-patch is always in a single
> directory.
A temporary directory.
> My work flow is to use a script for --to and --cc
> lines that can be set to emit the same addresses for
> all files in a patch series or generate different
> addresses per patch file.
For --to-cmd and --cc-cmd? So basically you check the dirname of the
argument passed?
While that works, it means you have to run the same command multiple
times, one for each mail.
If the command is using something expensive such as 'git blame' and
you have many patches, this is particularly bad. Also, it's not
elegant :)
--
Felipe Contreras
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Andreas Schwab @ 2012-11-12 23:09 UTC (permalink / raw)
To: Drew Northup; +Cc: glpk xypron, git, jnareb, Junio C Hamano
In-Reply-To: <CAM9Z-n=6xsC7yiKJ+NU-CxNPxEXWmJzvXLUocgZgWPQnuK6G4Q@mail.gmail.com>
Drew Northup <n1xim.email@gmail.com> writes:
> Something like this may be useful to defuse the "file" parameter, but
> I presume a more definitive fix is in order...
A proper fix will have to add esc_html to the feed generation, something
like this (untested):
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..a51a8ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -8055,6 +8055,7 @@ sub git_feed {
$feed_type = 'history';
}
$title .= " $feed_type";
+ $title = esc_html($title);
my $descr = git_get_project_description($project);
if (defined $descr) {
$descr = esc_html($descr);
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: splitting off shell test framework
From: Adam Spiers @ 2012-11-12 23:09 UTC (permalink / raw)
To: Drew Northup; +Cc: Git Mailing List, felipe.contreras, Jason J Pyeron CTR (US)
In-Reply-To: <CAM9Z-n=ypt_fyFcPjYD28NNoxnJ2ZSovd1TbERdA8RJY1Va=kA@mail.gmail.com>
On Mon, Nov 12, 2012 at 6:18 PM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Mon, Nov 12, 2012 at 11:37 AM, Adam Spiers <git@adamspiers.org> wrote:
>> As it turned out to be fairly easy, I was wondering if there would be
>> any interest in doing this more formally, i.e. splitting off the
>> framework so that it could be used and improved outside the scope of
>> git development? Of course this would pose the question how git would
>> consume this new project without any risk of destabilisation. I'm
>> guessing that simply using a git submodule would solve the problem,
>> but ICBW ...
>>
>> Just an idea. Interesting, or terrible? :)
>
> Done at least once already:
>
> http://comments.gmane.org/gmane.comp.version-control.git/201591
Nice! So hopefully someone will submit patches to build a two-way bridge
via git subtree. Having them diverge would be sad.
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Joe Perches @ 2012-11-12 23:13 UTC (permalink / raw)
To: Felipe Contreras
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <CAMP44s0B42yt8eugYSjYweoX8WOXXF1f-0N-5FuUyrY5rgCiow@mail.gmail.com>
On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
> On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
> >> Felipe Contreras wrote:
> >> > cc-cmd is only per-file, and many times receipients get lost without
> >> > seing the full patch series.
> >>
> >> s/seing/seeing
> >>
> >> > [...]
> >>
> >> Looks good otherwise.
> >
> > s/receipients/recipients/ too
> >
> > Practically this is ok but I think it's unnecessary.
> >
> > Output from git format-patch is always in a single
> > directory.
>
> A temporary directory.
>
> > My work flow is to use a script for --to and --cc
> > lines that can be set to emit the same addresses for
> > all files in a patch series or generate different
> > addresses per patch file.
>
> For --to-cmd and --cc-cmd? So basically you check the dirname of the
> argument passed?
yes. basename and dirname
> While that works, it means you have to run the same command multiple
> times, one for each mail.
Shrug. it's not a generally significant cost.
The script could also output the addresses to yet another file.
> If the command is using something expensive such as 'git blame' and
> you have many patches, this is particularly bad. Also, it's not
> elegant :)
Elegant is a beholder viewpoint.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-12 23:14 UTC (permalink / raw)
To: Jean-Jacques Lafay; +Cc: René Scharfe, msysgit, Git List, Philip Oakley
In-Reply-To: <CA+TMuX2p4ck0qXijH+OOcBoveBC42U8PqnXdisau57RXwt1isg@mail.gmail.com>
On Mon, Nov 12, 2012 at 11:27:14PM +0100, Jean-Jacques Lafay wrote:
> 2012/11/11 Jeff King <peff@peff.net>:
> > On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
> >
> > 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).
>
> I guess the optimal algorithm may also depend on the commit graph
> general shape, but intuitively, I'd say that the critical factor is
> the number and distribution of tags. As soon as you have a significant
> number of tags (let's say 1% of the commits are tagged, evenly
> distributed), you'll quickly end up with every commit marked as
> containing or not the target commit, so that each additional tag check
> is cheap.
>
> This suggests a complexity of O(number of commits) more often then
> not, however you choose to traverse the graph.
We can do much better than O(number of commits), though, if we stop
traversing down a path when its timestamp shows that it is too old to
contain the commits we are searching for. The problem is that the
timestamps cannot always be trusted, because they are generated on
machines with wrong clocks, or by buggy software. This could be solved
by calculating and caching a "generation" number, but last time it was
discussed there was a lot of arguing and nothing got done.
Another approach, used by the merge-base calculation, is to treat
parents in a breadth-first way, but sort them by timestamp, and walk
backwards to find the merge-base (and if you get to a merge-base, you
can stop). In that case, bad timestamps may cause you to look at extra
commits (because you process a commit prematurely and end up going
deeper than the merge base), but it can never give you a wrong answer.
Thinking on it more, though, the merge-base style of computation would
mean you always have to walk backwards to the oldest tag. Which is in
the same order of magnitude as the number of commits, assuming you have
tags near the start of history. So I think we will always want to keep a
cutoff, anyway, and there is not much point in switching off of a
depth-first approach (but of course it does make sense to use iteration
instead of recursion to do so).
> At least on my almost-real-life repo*, with ~140,000 commits and
> ~2,000 tags, tag --contains takes a few seconds, of course more than
> the worst-case test on a 40,000 commit / 1 tag repo, but still in the
> same order of magnitude.
Try "git rev-list --count --all" to get a sense of how long O(# of
commits) takes. Before the depth-first implementation, "tag --contains"
was O(commits * tags) in the worst case. With depth first, it's
O(commits), and then with the timestamp cutoff, it's really O(commits
since needle), where "needle" is the oldest thing you're looking for.
Here are those numbers on linux-2.6 (with linux-stable tags):
[to get a sense of the repo size]
$ git for-each-ref refs/tags | wc -l
909
$ git rev-list --count --all
363413
[this is O(commits)]
$ time git rev-list --count --all
real 0m4.034s
user 0m3.960s
sys 0m0.056s
[before depth-first, ffc4b80^; you can see that it is much worse than
O(commits), though not as bad as the worst case (because finding the
merge bases for recent tags is not O(commits)]
$ time git tag --contains HEAD~200
real 0m42.838s
user 0m42.527s
sys 0m0.156s
[after depth-first, ffc4b80; you can see that this is O(commits),
because we will go depth-first down to the roots, but do only a
single traversal]
$ time git tag --contains HEAD~200
real 0m3.939s
user 0m3.784s
sys 0m0.140s
[with my generation patches; much faster]
$ time git tag --contains HEAD~200
real 0m0.037s
user 0m0.020s
sys 0m0.012s
I was thinking we had merged the timestamp cutoff (which has the same
performance characteristics as generations, just with the skew issue) to
master, but it looks like we didn't.
-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
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