Git development
 help / color / mirror / Atom feed
* 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: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-12 20:24 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>

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.

  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>.

  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.

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.

-Peff

^ permalink raw reply

* Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Junio C Hamano @ 2012-11-12 20:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Ackermann, git
In-Reply-To: <20121112191953.GA4623@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Nov 12, 2012 at 09:45:31AM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > On Mon, Oct 29, 2012 at 07:33:47PM +0100, Thomas Ackermann wrote:
>> >
>> >> This patch addresses Junios comment in WC:
>> >> "Misapplication of a patch fixed; the ones near the tip needs to
>> >>  update the links to point at the html files, though."
>> >> 
>> >> See older mail in this thread:
>> >> [...]
>> >> That means that for the patch [6/8], which adds content-type to the
>> >> text files, to be complete, it needs to update Makefile to produce
>> >> html files from them.
>> >> [...]
>> >> So IMHO no open issues with this patch.
>> >
>> > OK, that explains the situation. Thanks, I'll merge it to master in the
>> > next iteration.
>> 
>> What happened to this topic?
>
> I mistyped in the above response; it should have been "next" not
> "master".

Ah, ok, I was just wondering why I didn't see it in your 'master'.
Thanks.

> But then I forgot to merge it to next in the last iteration; it is
> ta/doc-cleanup in pu, and is marked in "What's Cooking" to be merged to
> next.
>
> -Peff

^ permalink raw reply

* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Jeff King @ 2012-11-12 20:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <7vzk2mh9dm.fsf@alter.siamese.dyndns.org>

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. So you'd have to generate the pick
script on inclusion of git-sh-setup, whether the includer wants to call
the function or not.

I wonder if we should simply generate these at build time and store them
in the script.

I'm also not sure that saving one process invocation is worth spending a
lot of thought cycles on. Maybe somebody on Windows could run a
filter-branch with the patch I just sent and measure whether it even
makes a difference.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Junio C Hamano @ 2012-11-12 20:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121112194434.GB4623@sigill.intra.peff.net>

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.

> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 5314249..7a693ba 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -74,7 +74,7 @@ finish_ident() {
>  }
>  
>  set_ident () {
> -	parse_ident_from_commit author AUTHOR committer COMMITTER
> +	parse_ident_from_commit_via_script "$ident_script"
>  	finish_ident AUTHOR
>  	finish_ident COMMITTER
>  }
> @@ -93,6 +93,7 @@ if [ "$(is_bare_repository)" = false ]; then
>  	require_clean_work_tree 'rewrite branches'
>  fi
>  
> +ident_script=$(pick_ident_script author AUTHOR committer COMMITTER)
>  tempdir=.git-rewrite
>  filter_env=
>  filter_tree=
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 22f0aed..1e20e17 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -225,10 +225,17 @@ pick_ident_script () {
>  	echo '/^$/q'
>  }
>  
> +# Feed a pick_ident_script return value to sed. Use this instead of
> +# parse_ident_from_commit below if you are going to be parsing commits in a
> +# tight loop and want to save a process.
> +parse_ident_from_commit_via_script() {
> +	LANG=C LC_ALL=C sed -ne "$1"
> +}
> +
>  # Create a pick-script as above and feed it to sed. Stdout is suitable for
>  # feeding to eval.
>  parse_ident_from_commit () {
> -	LANG=C LC_ALL=C sed -ne "$(pick_ident_script "$@")"
> +	parse_ident_from_commit_via_script "$(pick_ident_script "$@")"
>  }
>  
>  # Parse the author from a commit given as an argument. Stdout is suitable for

^ permalink raw reply

* Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
From: Marc Khouzam @ 2012-11-12 20:07 UTC (permalink / raw)
  To: git
In-Reply-To: <CAFj1UpE6OtJEojaED1_DZJD0kU=nVsFE_w8xa0oJE-6auCU2rw@mail.gmail.com>

Hi,

this patch allows tcsh-users to get the benefits of the awesome
git-completion.bash script.  It could also help other shells do the same.

==

The current tcsh-completion support for Git, as can be found on the
internet, takes the approach of defining the possible completions
explicitly.  This has the obvious draw-back to require constant
updating as the Git code base evolves.

The approach taken by this commit is to to re-use the advanced bash
completion script and use its result for tcsh completion.  This is
achieved by executing (versus sourcing) the bash script and
outputting the completion result for tcsh consumption.

Three solutions were looked at to implement this approach with (A)
being retained:

  A) Modifications:
          git-completion.bash and new git-completion.tcsh

     Modify the existing git-completion.bash script to support
     being sourced using bash (as now), but also executed using bash.
     When being executed, the script will output the result of the
     computed completion to be re-used elsewhere (e.g., in tcsh).
     Pros:
       1- allows the git-completion.bash script to easily be re-used
       2- tcsh support is mostly isolated in git-completion.tcsh
     Cons (for tcsh users only):
       1- requires the user to copy both git-completion.tcsh and
          git-completion.bash to ${HOME}
       2- requires bash script to have a fixed name and location:
          ${HOME}/.git-completion.bash

  B) Modifications:
          git-completion.bash

     Modify the existing git-completion.bash script to support
     being sourced using bash (as now), but also executed using bash,
     and sourced using tcsh.
     Pros:
       1- only requires the user to deal with a single file
       2- maintenance more obvious for tcsh since it is entirely part
          of the same git-completion.bash script.
     Cons:
       1- tcsh support could affect bash support as they share the
          same script
       2- small tcsh section must use syntax suitable for both tcsh
          and bash and must be at the beginning of the script
       3- requires the user to explicitly make the script executable
          when using tcsh (for tcsh users only)
       4- requires script to have a fixed name and location:
          ${HOME}/.git-completion.sh (for tcsh users only)

  C) Modifications:
          New git-completion.tcsh

     Provide a short tcsh script that converts git-completion.bash
     into an executable script suitable to be used by tcsh.
     Pros:
       1- tcsh support is entirely isolated in git-completion.tcsh
       2- new tcsh script can be as complex as needed
     Cons (for tcsh users only):
       1- requires the user to copy both git-completion.tcsh and
          git-completion.bash to ${HOME}
       2- requires bash script to have a fixed name and location:
          ${HOME}/.git-completion.bash
       3- sourcing the new script will generate a third script

Approach (A) was selected to keep the tcsh completion support well
isolated without introducing excessive complexity.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>

==

With the changes applied, tcsh users should:

#    1) Copy both this file and the bash completion script to your
${HOME} directory
#       using the names ${HOME}/.git-completion.tcsh and
${HOME}/.git-completion.bash.
#    2) Add the following line to your .tcshrc/.cshrc:
#        source ${HOME}/.git-completion.tcsh

The code can be found on GitHub.
Option (A):
https://github.com/marckhouzam/git/commit/86d3a8e740ae85b4b4462c997a0fd969b1b2d24c

Option (B):
https://github.com/marckhouzam/git/commit/e64606541682eaf66c0a56aceff279ca6e1d06cd

Option (C):
https://github.com/marckhouzam/git/commit/59792455f1e6a98d3ffeb828f4cff1ded0e4ed37

Thanks

Marc

---
 contrib/completion/git-completion.bash |   53 +++++++++++++++++++++++++++++++-
 contrib/completion/git-completion.tcsh |   34 ++++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletions(-)
 create mode 100755 contrib/completion/git-completion.tcsh

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index be800e0..6d4b57a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,4 +1,6 @@
-#!bash
+#!/bin/bash
+# The above line is important as this script can be executed when used
+# with another shell such as tcsh
 #
 # bash/zsh completion support for core Git.
 #
@@ -2481,3 +2483,52 @@ __git_complete gitk __gitk_main
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
 __git_complete git.exe __git_main
 fi
+
+# Method that will output the result of the completion done by
+# the bash completion script, so that it can be re-used in another
+# context than the bash complete command.
+# It accepts 1 to 2 arguments:
+# 1: The command-line to complete
+# 2: The index of the word within argument #1 in which the cursor is
+#    located (optional). If parameter 2 is not provided, it will be
+#    determined as best possible using parameter 1.
+_git_complete_with_output ()
+{
+       # Set COMP_WORDS to the command-line as bash would.
+       COMP_WORDS=($1)
+
+       # Set COMP_CWORD to the cursor location as bash would.
+       if [ -n "$2" ]; then
+               COMP_CWORD=$2
+       else
+               # Assume the cursor is at the end of parameter #1.
+               # We must check for a space as the last character which will
+               # tell us that the previous word is complete and the cursor
+               # is on the next word.
+               if [ "${1: -1}" == " " ]; then
+                       # The last character is a space, so our
location is at the end
+                       # of the command-line array
+                       COMP_CWORD=${#COMP_WORDS[@]}
+               else
+                       # The last character is not a space, so our
location is on the
+                       # last word of the command-line array, so we
must decrement the
+                       # count by 1
+                       COMP_CWORD=$((${#COMP_WORDS[@]}-1))
+               fi
+       fi
+
+       # Call _git() or _gitk() of the bash script, based on the first
+       # element of the command-line
+       _${COMP_WORDS[0]}
+
+       # Print the result that is stored in the bash variable ${COMPREPLY}
+       for i in ${COMPREPLY[@]}; do
+               echo "$i"
+       done
+}
+
+if [ -n "$1" ] ; then
+  # If there is an argument, we know the script is being executed
+  # so go ahead and run the _git_complete_with_output function
+  _git_complete_with_output "$1" "$2"
+fi
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
new file mode 100755
index 0000000..7b7baea
--- /dev/null
+++ b/contrib/completion/git-completion.tcsh
@@ -0,0 +1,34 @@
+#!tcsh
+#
+# tcsh completion support for core Git.
+#
+# Copyright (C) 2012 Marc Khouzam <marc.khouzam@gmail.com>
+# Distributed under the GNU General Public License, version 2.0.
+#
+# This script makes use of the git-completion.bash script to
+# determine the proper completion for git commands under tcsh.
+#
+# To use this completion script:
+#
+#    1) Copy both this file and the bash completion script to your
${HOME} directory
+#       using the names ${HOME}/.git-completion.tcsh and
${HOME}/.git-completion.bash.
+#    2) Add the following line to your .tcshrc/.cshrc:
+#        source ${HOME}/.git-completion.tcsh
+
+# One can change the below line to use a different location
+set __git_tcsh_completion_script = ${HOME}/.git-completion.bash
+
+# Check that the user put the script in the right place
+if ( ! -e ${__git_tcsh_completion_script} ) then
+       echo "ERROR in git-completion.tcsh script.  Cannot find:
${__git_tcsh_completion_script}.  Git completion will not work."
+       exit
+endif
+
+# Make the script executable if it is not
+if ( ! -x ${__git_tcsh_completion_script} ) then
+       chmod u+x ${__git_tcsh_completion_script}
+endif
+
+complete git  'p/*/`${__git_tcsh_completion_script} "${COMMAND_LINE}"
| sort | uniq`/'
+complete gitk 'p/*/`${__git_tcsh_completion_script} "${COMMAND_LINE}"
| sort | uniq`/'
+
--
1.7.0.4

^ permalink raw reply related

* Re: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
From: Jeff King @ 2012-11-12 19:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul Fox, Kalle Olavi Niemitalo, git
In-Reply-To: <7v4nkuk966.fsf@alter.siamese.dyndns.org>

On Mon, Nov 12, 2012 at 09:44:49AM -0800, Junio C Hamano wrote:

> How did this message happen?
> 
>     Subject: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
>     To: Kalle Olavi Niemitalo <kon@iki.fi>
>     Cc: Paul Fox <pgf@foxharp.boston.ma.us>, git@vger.kernel.org
>     Date: Sun, 11 Nov 2012 11:55:11 -0500
>     Message-ID: <20121111165510.GB19850@sigill.intra.peff.net>
>     References: <20121111163100.GB13188@sigill.intra.peff.net>
> 
>     The user's editor likely catches SIGINT (ctrl-C).  but if
>     the user spawns a command from the editor and uses ctrl-C to
>     kill that command, the SIGINT will likely also kill git
>     itself (depending on the editor, this can leave the terminal
>     in an unusable state).
> 
>     Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>
>     Signed-off-by: Jeff King <peff@peff.net>
>     ---
> 
> Judging from S-o-b, message-id and EHLO, I think this was sent by
> Peff, but came without Sender: or anything.
> 
> Just being curious.

I screwed up when sending out the series and did not properly move
Paul's "From" address from the email header down to the body. It is not
a git or send-email screw-up; I load format-patch output directly into
mutt as a template. Since I do not often send out other people's
patches, I never bothered to write a script to migrate the "from" into
the body automatically.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Jeff King @ 2012-11-12 19:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <7vpq3ik97i.fsf@alter.siamese.dyndns.org>

On Mon, Nov 12, 2012 at 09:44:01AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > The only ident-parsing function we currently provide is
> > get_author_ident_from_commit. This is not very
> > flexible for two reasons:
> >
> >   1. It takes a commit as an argument, and can't read from
> >      commit headers saved on disk.
> >
> >   2. It will only parse authors, not committers.
> >
> > This patch provides a more flexible interface which will
> > parse multiple idents from a commit provide on stdin. We can
> > easily use it as a building block for the current function
> > to retain compatibility.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> > Since we are counting processes in this series, I should note that this
> > actually adds a subshell invocation for each call, since it went from:
> >
> >   script='...'
> >   sed $script
> >
> > to:
> >
> >   sed "$(make_script)"
> >
> > For filter-branch, which is really the only high-performance caller we
> > have, this is negated by the fact that it will do author and committer
> > at the same time, saving us an extra subshell (in addition to an extra
> > sed invocation).
> 
> Given that pick-ident-script is a const function, a caller that
> repeatedly call is could call it once and use it in a variable, no?

The problem is that it is a helper called from parse_ident_from_commit.
And that function just passes along its arguments, so it does not know
that it is being called repeatedly with the same arguments. So you'd
have to either change the interface or memoize internally.

I don't think memoization is a good option for two reasons:

  1. Storing the arguments to compare to later is complex. You don't
     want to just store "$*" from the last run and see if we got the
     same arguments. You'd have to quote your delimiter (e.g., you would
     not want to confuse ("foo", "bar") with ("foo bar"). Though in this
     instance, we know that our args do not have spaces, so we could get
     away with that.

  2. If you are in a subshell or even a while loop, your memoized
     variable will not be retained.

So unless somebody has some clever scheme for memoizing shell functions
without any process overhead, it is probably not worth it.

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:

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5314249..7a693ba 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -74,7 +74,7 @@ finish_ident() {
 }
 
 set_ident () {
-	parse_ident_from_commit author AUTHOR committer COMMITTER
+	parse_ident_from_commit_via_script "$ident_script"
 	finish_ident AUTHOR
 	finish_ident COMMITTER
 }
@@ -93,6 +93,7 @@ if [ "$(is_bare_repository)" = false ]; then
 	require_clean_work_tree 'rewrite branches'
 fi
 
+ident_script=$(pick_ident_script author AUTHOR committer COMMITTER)
 tempdir=.git-rewrite
 filter_env=
 filter_tree=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 22f0aed..1e20e17 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -225,10 +225,17 @@ pick_ident_script () {
 	echo '/^$/q'
 }
 
+# Feed a pick_ident_script return value to sed. Use this instead of
+# parse_ident_from_commit below if you are going to be parsing commits in a
+# tight loop and want to save a process.
+parse_ident_from_commit_via_script() {
+	LANG=C LC_ALL=C sed -ne "$1"
+}
+
 # Create a pick-script as above and feed it to sed. Stdout is suitable for
 # feeding to eval.
 parse_ident_from_commit () {
-	LANG=C LC_ALL=C sed -ne "$(pick_ident_script "$@")"
+	parse_ident_from_commit_via_script "$(pick_ident_script "$@")"
 }
 
 # Parse the author from a commit given as an argument. Stdout is suitable for

^ permalink raw reply related

* Re: [regression] Newer gits cannot clone any remote repos
From: Douglas Mencken @ 2012-11-12 19:35 UTC (permalink / raw)
  To: Kevin; +Cc: git
In-Reply-To: <CAO54GHC9ibTwxqLnk1vGSo9R011HE05JOL1mgpTGzeWX7cCcwQ@mail.gmail.com>

> On Mon, Nov 12, 2012 at 12:12 PM, Kevin <ikke@ikke.info> wrote:
> > Maybe handy to say that you're on a Powerpc platform.

Oh, and yes, I'm on 2 x 2-core ("4-core") machine.

^ permalink raw reply

* Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Jeff King @ 2012-11-12 19:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Ackermann, git
In-Reply-To: <7vpq3iiukk.fsf@alter.siamese.dyndns.org>

On Mon, Nov 12, 2012 at 09:45:31AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Mon, Oct 29, 2012 at 07:33:47PM +0100, Thomas Ackermann wrote:
> >
> >> This patch addresses Junios comment in WC:
> >> "Misapplication of a patch fixed; the ones near the tip needs to
> >>  update the links to point at the html files, though."
> >> 
> >> See older mail in this thread:
> >> [...]
> >> That means that for the patch [6/8], which adds content-type to the
> >> text files, to be complete, it needs to update Makefile to produce
> >> html files from them.
> >> [...]
> >> So IMHO no open issues with this patch.
> >
> > OK, that explains the situation. Thanks, I'll merge it to master in the
> > next iteration.
> 
> What happened to this topic?

I mistyped in the above response; it should have been "next" not
"master".

But then I forgot to merge it to next in the last iteration; it is
ta/doc-cleanup in pu, and is marked in "What's Cooking" to be merged to
next.

-Peff

^ permalink raw reply

* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Drew Northup @ 2012-11-12 18:55 UTC (permalink / raw)
  To: glpk xypron; +Cc: git, jnareb, Junio C Hamano
In-Reply-To: <20121111232820.284510@gmx.net>

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.
>
> Example
> http://server/?p=project.git&a=rss&f=</title><script>alert(document.cookie)</script><title>
>
> The generated XML contains
> <script>alert(document.cookie)</script>
>
> Depending on the system used to render the XML this might lead
> to the execution of javascript in the security context of the
> gitweb server pages.
>
> Please, escape all URL parameters.
>
> Version tested:
> gitweb v.1.8.0.dirty with git 1.7.2.5
>
> Best regards
>> Heinrich Schuchardt

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;
 }


(I am not a perl god, so this was the lowest hanging fruit.)

If desired I'll fashion this up into a proper patch.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply related

* Re: [PATCHv3 1/4] Refactor print_state into get_state
From: Ramkumar Ramachandra @ 2012-11-12 18:29 UTC (permalink / raw)
  To: Phil Hord
  Cc: git, phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
	Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <1352487385-5929-2-git-send-email-hordp@cisco.com>

Phil Hord wrote:
> Recently git-status learned to display the state of the git
> sequencer in long form to help the user remember an interrupted
> command.  This information is useful to other callers who do
> not want it printed in the same way.

Nit: Please mention that "Recently" refers to 83c750ac to save future
readers the trouble of running blame.

Thanks.

Ram

^ permalink raw reply

* Re: splitting off shell test framework
From: Drew Northup @ 2012-11-12 18:18 UTC (permalink / raw)
  To: Adam Spiers; +Cc: Git Mailing List, felipe.contreras, Jason J Pyeron CTR (US)
In-Reply-To: <CAOkDyE8KxFvM4CJhC4U=Jb95D6HQ-4qQBtKAgBMyHH15UOhvqg@mail.gmail.com>

On Mon, Nov 12, 2012 at 11:37 AM, Adam Spiers <git@adamspiers.org> wrote:
> Hi all,
>
> I've been pretty impressed with git's test framework, and I'm not
> aware of many other (decent) shell-based test frameworks out there.
> (One that springs to mind is the one used by rvm, but last time I
> looked - admittedly a while ago now - it had limitations).
>
> Recently a situation arose where I craved the ability to test
> something via shell.  I did a quick proof of concept and successfully
> extracted out the non-git-specific bits of git's test framework to be
> used to test something entirely unrelated to git:
>
>     https://github.com/aspiers/shell-env/tree/master/t
>
> 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

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Phil Hord @ 2012-11-12 18:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <7vip9aiuk8.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Phil Hord <hordp@cisco.com> writes:
>
>> State token strings which may be emitted and their meanings:
>>     merge              a merge is in progress
>>     am                 an am is in progress
>>     am-is-empty        the am patch is empty
>>     rebase             a rebase is in progress
>>     rebase-interactive an interactive rebase is in progress
>>     cherry-pick        a cherry-pick is in progress
>>     bisect             a bisect is in progress
>>     conflicted         there are unresolved conflicts
>>     commit-pending     a commit operation is waiting to be completed
>>     splitting          interactive rebase, commit is being split
>>
>> I also considered adding these tokens, but I decided it was not
>> appropriate since these changes are not sequencer-related.  But
>> it is possible I am being too short-sighted or have chosen the
>> switch name poorly.
>>     changed-index  Changes exist in the index
>>     changed-files  Changes exist in the working directory
>>     untracked      New files exist in the working directory
> I tend to agree; unlike all the normal output from "status -s" that
> are per-file, the above are the overall states of the working tree.
>
> It is just that most of the "overall states" look as if they are
> dominated by "sequencer states", but that is only because you chose
> to call states related to things like "am" and "bisect" that are not
> sequencer states as such.
>
> It probably should be called the tree state, working tree state, or
> somesuch.

I think you are agreeing that I chose the switch name poorly, right?

Do you think '--tree-state' is an acceptable switch or do you have other
suggestions?

Phil

^ permalink raw reply

* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Junio C Hamano @ 2012-11-12 17:47 UTC (permalink / raw)
  To: Michael Haggerty, David Aguilar
  Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
	Lars R. Damerow, Jeff King, Marc Jordan
In-Reply-To: <508E0FAC.5050600@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> The log message of the original commit (0454dd93bf) described the
> following scenario: a /home partition under which user home directories
> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid
> hitting /home/.git, /home/.git/objects, and /home/objects (which would
> attempt to automount those directories).  I believe that this scenario
> would not be slowed down by my patches.
>
> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
> slowdown?

Yeah, I was also wondering about that.

David?

>> Is there another way to accomplish this without the performance hit?
>> Maybe something that can be solved with configuration?
>
> Without doing the symlink expansion there is no way for git to detect
> that GIT_CEILING_DIRECTORIES contains symlinks and is therefore
> ineffective.  So the user has no warning about the misconfiguration
> (except that git runs slowly).
>
> On 10/29/2012 02:42 AM, Junio C Hamano wrote:
>> Perhaps not canonicalize elements on the CEILING list ourselves? If
>> we make it a user error to put symlinked alias in the variable, and
>> document it clearly, wouldn't it suffice?
>
> There may be no other choice.  (That, and fix the test suite in another
> way to tolerate a $PWD that involves symlinks.)

^ permalink raw reply

* Re: Strange behaviour of git diff branch1 ... branch2
From: Junio C Hamano @ 2012-11-12 17:46 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Aaron Schrab
In-Reply-To: <CACsJy8BkKRxp9AQW0M0vDSL8Mb16VGSrZ1OnQpwCrERWKVLSyA@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Sat, Oct 27, 2012 at 7:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>>>
>>>Notice the --cc in the first line, which is combined diff. Usually
>>>combined-diff is between two points and one parent. Though somehow git
>>>passes 4 parents down combined-diff.c:show_combined_header, as you can
>>>see in the "index" line. I think we should fix rev parsing code as it
>>>does not make sense to pass 4 identical parents this way.
>>
>> The two heads home from HEAD...HEAD the user has on the command line.
>>
>> The user is getting exactly what she asked; there is nothing to fix.
>
> Is there any use case where HEAD...HEAD (or "..." alone) is actually useful?

That is an invalid question, implying a proposal that will introduce
more confusion to the users while hurting the code.

There is no use case where "cat A >>A" is actually useful, either,
but it does not mean "cat A >>A" should be special cased to do
something different from what it happens to do in line with the case
where "cat A >>B" does.  I think "diff ..." or "diff HEAD...HEAD"
are the same as that example.

^ permalink raw reply

* Re: [PATCHv3 3/4] git-status: show short sequencer state
From: Junio C Hamano @ 2012-11-12 17:45 UTC (permalink / raw)
  To: Phil Hord
  Cc: git, phil.hord, Jeff King, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <1352487385-5929-4-git-send-email-hordp@cisco.com>

Phil Hord <hordp@cisco.com> writes:

> State token strings which may be emitted and their meanings:
>     merge              a merge is in progress
>     am                 an am is in progress
>     am-is-empty        the am patch is empty
>     rebase             a rebase is in progress
>     rebase-interactive an interactive rebase is in progress
>     cherry-pick        a cherry-pick is in progress
>     bisect             a bisect is in progress
>     conflicted         there are unresolved conflicts
>     commit-pending     a commit operation is waiting to be completed
>     splitting          interactive rebase, commit is being split
>
> I also considered adding these tokens, but I decided it was not
> appropriate since these changes are not sequencer-related.  But
> it is possible I am being too short-sighted or have chosen the
> switch name poorly.
>     changed-index  Changes exist in the index
>     changed-files  Changes exist in the working directory
>     untracked      New files exist in the working directory

I tend to agree; unlike all the normal output from "status -s" that
are per-file, the above are the overall states of the working tree.

It is just that most of the "overall states" look as if they are
dominated by "sequencer states", but that is only because you chose
to call states related to things like "am" and "bisect" that are not
sequencer states as such.

It probably should be called the tree state, working tree state, or
somesuch.

^ permalink raw reply

* Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Junio C Hamano @ 2012-11-12 17:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Ackermann, git
In-Reply-To: <20121029215739.GG20513@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Oct 29, 2012 at 07:33:47PM +0100, Thomas Ackermann wrote:
>
>> This patch addresses Junios comment in WC:
>> "Misapplication of a patch fixed; the ones near the tip needs to
>>  update the links to point at the html files, though."
>> 
>> See older mail in this thread:
>> [...]
>> That means that for the patch [6/8], which adds content-type to the
>> text files, to be complete, it needs to update Makefile to produce
>> html files from them.
>> [...]
>> So IMHO no open issues with this patch.
>
> OK, that explains the situation. Thanks, I'll merge it to master in the
> next iteration.

What happened to this topic?

^ permalink raw reply

* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Junio C Hamano @ 2012-11-12 17:45 UTC (permalink / raw)
  To: gitzilla; +Cc: Felipe Contreras, Michael J Gruber, Git Mailing List, Jeff King
In-Reply-To: <509FD9BC.7050204@gmail.com>

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?

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).

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).

I think (c) implements worst of both choices. It cannot exploit
knowledge specific to the original SCM like (a) would, and while it
can enforce consistency the same way as (b) would, it would be a
separate program, unlike (b).

So...

^ permalink raw reply

* Re: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
From: Junio C Hamano @ 2012-11-12 17:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Paul Fox, Kalle Olavi Niemitalo, git
In-Reply-To: <20121111165510.GB19850@sigill.intra.peff.net>

How did this message happen?

    Subject: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control
    To: Kalle Olavi Niemitalo <kon@iki.fi>
    Cc: Paul Fox <pgf@foxharp.boston.ma.us>, git@vger.kernel.org
    Date: Sun, 11 Nov 2012 11:55:11 -0500
    Message-ID: <20121111165510.GB19850@sigill.intra.peff.net>
    References: <20121111163100.GB13188@sigill.intra.peff.net>

    The user's editor likely catches SIGINT (ctrl-C).  but if
    the user spawns a command from the editor and uses ctrl-C to
    kill that command, the SIGINT will likely also kill git
    itself (depending on the editor, this can leave the terminal
    in an unusable state).

    Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>
    Signed-off-by: Jeff King <peff@peff.net>
    ---

Judging from S-o-b, message-id and EHLO, I think this was sent by
Peff, but came without Sender: or anything.

Just being curious.

^ permalink raw reply

* Re: [PATCH v5 01/15] fast-export: avoid importing blob marks
From: Junio C Hamano @ 2012-11-12 17:44 UTC (permalink / raw)
  To: Jeff King
  Cc: Torsten Bögershausen, Felipe Contreras, git,
	Johannes Schindelin, Max Horn, Sverre Rabbelier, Brandon Casey,
	Brandon Casey, Jonathan Nieder, Ilari Liusvaara, Pete Wyckoff,
	Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <20121111163827.GA11408@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> Major issue:  "echo -n" is still not portable.
>> 
>> Could we simply use
>> 
>> touch  marks-cur  &&
>> touch marks-new
>
> Yes, "echo -n" is definitely not portable.  Our preferred way of
> creating an empty file is just ">file".

Yes.

And it is misleading to use "touch" in this case; unless you are
updating the timestamp of an existing file while preserving the
contents of it, please don't use the command.

Thanks.

^ permalink raw reply

* Re: [PATCH] Re:gitweb: add readme to overview page
From: Junio C Hamano @ 2012-11-12 17:44 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: git
In-Reply-To: <1352652039-31453-1-git-send-email-xypron.glpk@gmx.de>

Heinrich Schuchardt <xypron.glpk@gmx.de> writes:

> In this version of the patch the formatting has been corrected.
>
> Warnings for double / in filenames are avoided.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> ---

The above is *not* a proper log message.

Those of us who are reading the messages on the list would know that
this is a replacement for your earlier

    Message-ID: <1352647962-21910-1-git-send-email-xypron.glpk@gmx.de>

where it has more proper description of the change, but because
nobody will be applying that earlier one to the history (instead,
you would want this version to be applied), the description is lost
in the history.

>  gitweb/gitweb.perl |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..699ffac 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6369,6 +6369,18 @@ sub git_project_list {
>  	}
>  
>  	git_project_search_form($searchtext, $search_use_regexp);
> +	# If XSS prevention is on, we don't include README.html.
> +	# TODO: Allow a readme in some safe format.
> +	my $path = "";
> +	if (defined $project_filter) {
> +		$path = "/$project_filter";
> +	}
> +	if (!$prevent_xss && -s "$projectroot$path/README.html") {
> +		print "<div class=\"title\">readme</div>\n" .
> +			"<div class=\"readme\">\n";
> +		insert_file("$projectroot$path/README.html");
> +		print "\n</div>\n"; # class="readme"
> +	}
>  	git_project_list_body(\@list, $order);
>  	git_footer_html();
>  }

^ permalink raw reply

* Re: [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Junio C Hamano @ 2012-11-12 17:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121018072522.GA9999@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The only ident-parsing function we currently provide is
> get_author_ident_from_commit. This is not very
> flexible for two reasons:
>
>   1. It takes a commit as an argument, and can't read from
>      commit headers saved on disk.
>
>   2. It will only parse authors, not committers.
>
> This patch provides a more flexible interface which will
> parse multiple idents from a commit provide on stdin. We can
> easily use it as a building block for the current function
> to retain compatibility.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Since we are counting processes in this series, I should note that this
> actually adds a subshell invocation for each call, since it went from:
>
>   script='...'
>   sed $script
>
> to:
>
>   sed "$(make_script)"
>
> For filter-branch, which is really the only high-performance caller we
> have, this is negated by the fact that it will do author and committer
> at the same time, saving us an extra subshell (in addition to an extra
> sed invocation).

Given that pick-ident-script is a const function, a caller that
repeatedly call is could call it once and use it in a variable, no?

>
>  git-sh-setup.sh | 62 +++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 43 insertions(+), 19 deletions(-)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index ee0e0bc..22f0aed 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -191,28 +191,52 @@ get_author_ident_from_commit () {
>  	fi
>  }
>  
> +# Generate a sed script to parse identities from a commit.
> +#
> +# Reads the commit from stdin, which should be in raw format (e.g., from
> +# cat-file or "--pretty=raw").
> +#
> +# The first argument specifies the ident line to parse (e.g., "author"), and
> +# the second specifies the environment variable to put it in (e.g., "AUTHOR"
> +# for "GIT_AUTHOR_*"). Multiple pairs can be given to parse author and
> +# committer.
> +pick_ident_script () {
> +	while test $# -gt 0
> +	do
> +		lid=$1; shift
> +		uid=$1; shift
> +		printf '%s' "
> +		/^$lid /{
> +			s/'/'\\\\''/g
> +			h
> +			s/^$lid "'\([^<]*\) <[^>]*> .*$/\1/'"
> +			s/.*/GIT_${uid}_NAME='&'/p
> +
> +			g
> +			s/^$lid "'[^<]* <\([^>]*\)> .*$/\1/'"
> +			s/.*/GIT_${uid}_EMAIL='&'/p
> +
> +			g
> +			s/^$lid "'[^<]* <[^>]*> \(.*\)$/@\1/'"
> +			s/.*/GIT_${uid}_DATE='&'/p
> +		}
> +		"
> +	done
> +	echo '/^$/q'
> +}
> +
> +# Create a pick-script as above and feed it to sed. Stdout is suitable for
> +# feeding to eval.
> +parse_ident_from_commit () {
> +	LANG=C LC_ALL=C sed -ne "$(pick_ident_script "$@")"
> +}
> +
> +# Parse the author from a commit given as an argument. Stdout is suitable for
> +# feeding to eval to set the usual GIT_* ident variables.
>  get_author_ident_from_commit () {
> -	pick_author_script='
> -	/^author /{
> -		s/'\''/'\''\\'\'\''/g
> -		h
> -		s/^author \([^<]*\) <[^>]*> .*$/\1/
> -		s/.*/GIT_AUTHOR_NAME='\''&'\''/p
> -
> -		g
> -		s/^author [^<]* <\([^>]*\)> .*$/\1/
> -		s/.*/GIT_AUTHOR_EMAIL='\''&'\''/p
> -
> -		g
> -		s/^author [^<]* <[^>]*> \(.*\)$/@\1/
> -		s/.*/GIT_AUTHOR_DATE='\''&'\''/p
> -
> -		q
> -	}
> -	'
>  	encoding=$(git config i18n.commitencoding || echo UTF-8)
>  	git show -s --pretty=raw --encoding="$encoding" "$1" -- |
> -	LANG=C LC_ALL=C sed -ne "$pick_author_script"
> +	parse_ident_from_commit author AUTHOR
>  }
>  
>  # Clear repo-local GIT_* environment variables. Useful when switching to

^ permalink raw reply

* Re: [PATCH] Documentation/log: fix description of format.pretty
From: Junio C Hamano @ 2012-11-12 17:42 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Git List
In-Reply-To: <20121112153855.GA3546@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Ramkumar Ramachandra wrote:
>
>> Oops, I read about `--pretty` in pretty-formats.txt and didn't realize
>> that `--format` existed.  However, your patch is still wrong because
>> there seems to be a subtle (and confusing) difference between
>> `--pretty` and `--format`.  In the latter, you can't omit the format,
>> and expect it to be picked up from format.pretty:
>>
>>   $ git log --format
>>   fatal: unrecognized argument: --format

We probably should say "--format needs an argument" here.

> ... It is based on the
> following text from Documentation/config.txt:
>
> 	format.pretty::
> 		The default pretty format for log/show/whatchanged command,
> 		See linkgit:git-log[1], linkgit:git-show[1],
> 		linkgit:git-whatchanged[1].
>
> I do imagine it can be made clearer.  s/--format/--pretty/ does not go
> far enough --- it only replaces one confusing explanation with
> another.

The --format and --pretty are more or less the same thing since
3a4c1a5 (Add --format that is a synonym to --pretty, 2009-02-24).
The --format option was added as a synonym because majority of new
users who have never heard of --pretty found it more natural; in
that sense, Ram's patch goes backwards.

The entry in config.txt came from 94c22a5 (log/show/whatchanged:
introduce format.pretty configuration, 2008-03-02) that predates the
synonym, and originally it was to allow

    [format] pretty = fuller

to make "git log" by default use "git log --pretty=fuller" in the
configuration; back then there wasn't a support for custom formats
like

    [format] pretty = "%h %s"

until 3640754 (Give short-hands to --pretty=tformat:%formatstring,
2009-02-24) introduced it.

"The default pretty format" in the description was written to refer
to the original "raw/medium/short/email/full/fuller" set, but these
days, we accept anything that you can write after "--pretty=" (or
its synonym "--format=") on the command line.

This is one of the reasons why I do not want to see abbreviated
descriptions for configuration variables duplicated in the manual
pages of individual commands, as the practice tends to lead to this
kind of confusion.

^ permalink raw reply

* [PATCH 4/4] remote-hg: avoid bad refs
From: Felipe Contreras @ 2012-11-12 17:41 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1352742068-15346-1-git-send-email-felipe.contreras@gmail.com>

Turns out fast-export throws bad 'reset' commands because of a behavior
in transport-helper that is not even needed.

Either way, better to ignore them, otherwise the user will get warnings
when we OK them.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 3cdc1e2..48f8f5d 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -704,6 +704,9 @@ def do_export(parser):
         elif ref.startswith('refs/tags/'):
             tag = ref[len('refs/tags/'):]
             parser.repo.tag([tag], node, None, True, None, {})
+        else:
+            # transport-helper/fast-export bugs
+            continue
         print "ok %s" % ref
 
     print
-- 
1.8.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox