Git development
 help / color / mirror / Atom feed
* Re: gitk does not show path file list
From: Johannes Sixt @ 2009-11-24  7:57 UTC (permalink / raw)
  To: Mark Blakeney; +Cc: git
In-Reply-To: <loom.20091124T060334-836@post.gmane.org>

Mark Blakeney schrieb:
> This seems to me to be a straight out bug but given I've had no response here
> and  given this is such a simple issue then I guess it's not a bug and I'm just
> missing something? Please somebody, why does gitk (usually) not show the subset
> list of files affected when you give it a path?
> 
> E.g. If I am in a src dir then "gitk ." does not list files. Neither does "gitk
> $PWD" nor "gitk ../src". However "cd ..; git src" does list files!?

gitk doesn't list the files in your examples because the patterns you gave
are not initial substrings of any files in the list.

-- Hannes

^ permalink raw reply

* Re: [PATCH RFC] git-send-email --expand-aliases
From: Karl Wiberg @ 2009-11-24  7:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Chiang, Catalin Marinas, git
In-Reply-To: <7vfx84jsef.fsf@alter.siamese.dyndns.org>

On Tue, Nov 24, 2009 at 8:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karl Wiberg <kha@treskal.com> writes:
>
>> I think that sounds like a splendid idea. It would be interesting to
>> see just how thin a wrapper around git send-email (and format-patch)
>> stg mail could become, without sacrificing features anyone actually
>> uses. The main complication could be stg mail's templates.
>
> Why do you even need to run format-patch?  If stg mail supports a good
> templates to prepare message files, it would be natural to keep using that
> to prepare message files.

The only thing stg mail _really_ needs to do, strictly speaking, is to
be git send-email with an easy way to specify a patch, or a range of
patches, to send. Anything above and beyond that is functionality that
we have to write and maintain without the help of the larger git
community, and which won't be of use to said community for no good
reason. Take the template system for cover letters and patches, for
example: there's no reason why it couldn't be part of the git tools,
and if it had been, it would have had many more users and much more
developer love.

It's a question of deciding in which areas the benefits of doing it
ourselves are worth the cost, and where it's better to let git do the
job for us. And of recognizing that StGit is old enough that tradeoffs
that were worth it when git was not as mature and featureful as today
might be worth reconsidering from time to time.

(Alex: Sorry if I'm making a big deal out of this. Just because
rewriting stg mail entirely in terms of the git tools might be
_possible_ doesn't mean that just a few steps in that direction
wouldn't be worthwhile. But I thought I should raise the possibility.)

-- 
Karl Wiberg, kha@treskal.com
   subrabbit.wordpress.com
   www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCH 2/2] Enable support for IPv6 on MinGW
From: Johannes Sixt @ 2009-11-24  7:41 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git, gitster
In-Reply-To: <alpine.DEB.2.00.0911240055170.14228@cone.home.martin.st>

Martin Storsjö schrieb:
> The IPv6 support functions are loaded dynamically, to maintain backwards
> compatibility with versions of Windows prior to XP, and fallback wrappers
> are provided, implemented in terms of gethostbyname and gethostbyaddr.
> 
> Signed-off-by: Martin Storsjo <martin@martin.st>

Acked-by: Johannes Sixt <j6t@kdbg.org>

-- Hannes

^ permalink raw reply

* Re: [PATCH 1/2] Refactor winsock initialization into a separate  function
From: Johannes Sixt @ 2009-11-24  7:36 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git, gitster
In-Reply-To: <alpine.DEB.2.00.0911240054420.14228@cone.home.martin.st>

Martin Storsjö schrieb:
> Signed-off-by: Martin Storsjo <martin@martin.st>

I have used this series in my tree for 6 weeks without negative
sideffects. I haven't tested IPv6, though.

Here's an updated commit message with my ACK:

--- >8 ---
Refactor winsock initialization into a separate function

The winsock library must be initialized. Since gethostbyname() is the
first function that calls into winsock, it was overridden to do the
initialization. This refactoring helps the next patch, where other
functions can be called earlier.

Signed-off-by: Martin Storsjo <martin@martin.st>
Acked-by: Johannes Sixt <j6t@kdbg.org>
--- >8 ---


> ---
>  compat/mingw.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 15fe33e..f9d82ff 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -903,16 +903,25 @@ char **make_augmented_environ(const char *const *vars)
>  	return env;
>  }
>  
> -/* this is the first function to call into WS_32; initialize it */
> -#undef gethostbyname
> -struct hostent *mingw_gethostbyname(const char *host)
> +static void ensure_socket_initialization(void)
>  {
>  	WSADATA wsa;
> +	static int initialized = 0;
> +
> +	if (initialized)
> +		return;
>  
>  	if (WSAStartup(MAKEWORD(2,2), &wsa))
>  		die("unable to initialize winsock subsystem, error %d",
>  			WSAGetLastError());
>  	atexit((void(*)(void)) WSACleanup);
> +	initialized = 1;
> +}
> +
> +#undef gethostbyname
> +struct hostent *mingw_gethostbyname(const char *host)
> +{
> +	ensure_socket_initialization();
>  	return gethostbyname(host);
>  }
>  

^ permalink raw reply

* Re: [PATCH RFC] git-send-email --expand-aliases
From: Junio C Hamano @ 2009-11-24  7:25 UTC (permalink / raw)
  To: Karl Wiberg; +Cc: Alex Chiang, Catalin Marinas, git
In-Reply-To: <b8197bcb0911232312l251dfbc9va671388cfb7fe57b@mail.gmail.com>

Karl Wiberg <kha@treskal.com> writes:

> I think that sounds like a splendid idea. It would be interesting to
> see just how thin a wrapper around git send-email (and format-patch)
> stg mail could become, without sacrificing features anyone actually
> uses. The main complication could be stg mail's templates.

Why do you even need to run format-patch?  If stg mail supports a good
templates to prepare message files, it would be natural to keep using that
to prepare message files.

^ permalink raw reply

* Re: gitk does not show path file list
From: Junio C Hamano @ 2009-11-24  7:22 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Mark Blakeney
In-Reply-To: <loom.20091124T060334-836@post.gmane.org>

Mark Blakeney <markb@berlios.de> writes:

> ... why does gitk (usually) not show the subset list of files affected
> when you give it a path?
>
> E.g. If I am in a src dir then "gitk ." does not list files. Neither does "gitk
> $PWD" nor "gitk ../src". However "cd ..; git src" does list files!?

Paul, I do not read Tcl very well, but it appears that path_filter
procedure is at fault.

The call chain involved in this seems to be:

    gettreediffs
     - arranges gettreediffline to be fed output from "diff-tree $commit ."

 ->

    gettreediffline
     - finds the path (note that diff output is _always_ relative to the
       top of the work tree) from the patch;
     - calls path_filter with $vfilelimit($curview) and each filename
       In this case, the $vfilelimit($curview) is "." (dot)

 ->

    path_filter
     - compares strings in $filter and the $name; in this case, $filter is
       a dot "." and $name begins with "src/"

I see at least two problems in path_filter used this way:

 - A dot "." never would match anything from "diff-tree" (or any "diff"
   variant) after stripping a/ and b/ prefix.  gitk should prefix the
   current directory to each of the pathspec from command line (run
   "rev-parse --show-prefix" to learn where you are).

 - There is another callsite to path_filter for filtering output from
   "ls-files -u".  But the output from "ls-files" is relative to the cwd
   by default.  gitk should probably run it with --full-name option, so
   that it would get the same semantics as "diff" output.

It _might_ be the easiest to do an equivalent of (sorry, I do not talk Tcl
so this is in shell):

	prefix=$(git rev-parse --show-prefix)
	if test -n "$prefix"
        then
        	cd $(git rev-parse --show-cdup)
	fi

and then prepend prefix to all the pathspecs you would use from the
command line before doing anything else.  This "prepending" obviously need
to be aware of the ".", ".." and friends.

^ permalink raw reply

* Re: [PATCH RFC] git-send-email --expand-aliases
From: Karl Wiberg @ 2009-11-24  7:12 UTC (permalink / raw)
  To: Alex Chiang, Catalin Marinas; +Cc: Junio C Hamano, git
In-Reply-To: <20091124004554.GA27643@ldl.fc.hp.com>

On Tue, Nov 24, 2009 at 1:45 AM, Alex Chiang <achiang@hp.com> wrote:

> * Junio C Hamano <gitster@pobox.com>:
>
> > If you are changing StGit to call git-send-email anyway, why not
> > arrange stgit to call git-send-email to send the message out
> > instead, instead of sending messages on its own?
>
> Yeah, I thought about that as I was poking around further in StGit
> to figure out how it would be calling git-send-email. ;)
>
> > I imagine the internal implementation of stg mail would work
> > something like:
> >
> >     prepare messages to send out
> >     call git-send-email and have it send them
> >
> > What am I missing?
>
> My lack of familiarity with StGit internals. ;)
>
> Your suggestion is much better. I'll take a closer look at StGit and
> see how feasible it is.
>
> Unless Catalin has strong objections?

I think that sounds like a splendid idea. It would be interesting to
see just how thin a wrapper around git send-email (and format-patch)
stg mail could become, without sacrificing features anyone actually
uses. The main complication could be stg mail's templates.

Catalin, how wedded are you to those? ;-)

-- 
Karl Wiberg, kha@treskal.com
   subrabbit.wordpress.com
   www.treskal.com/kalle

^ permalink raw reply

* [PATCH] Documentation: update descriptions of revision options related to '--bisect'
From: Christian Couder @ 2009-11-24  6:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds

In commit ad3f9a7 (Add '--bisect' revision machinery argument) the
'--bisect' option was added to easily pass bisection refs to commands
using the revision machinery.

This patch updates the documentation of the related options to describe
the new behavior.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/rev-list-options.txt |   39 ++++++++++++++++++++++++-----------
 1 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b44fdd9..1f57aed 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -243,6 +243,15 @@ endif::git-rev-list[]
 	Pretend as if all the refs in `$GIT_DIR/refs/remotes` are listed
 	on the command line as '<commit>'.
 
+ifndef::git-rev-list[]
+--bisect::
+
+	Pretend as if the bad bisection ref `$GIT_DIR/refs/bisect/bad`
+	was listed and as if it was followed by `--not` and the good
+	bisection refs `$GIT_DIR/refs/bisect/good-*` on the command
+	line.
+endif::git-rev-list[]
+
 --stdin::
 
 	In addition to the '<commit>' listed on the command
@@ -538,7 +547,11 @@ Bisection Helpers
 --bisect::
 
 Limit output to the one commit object which is roughly halfway between
-the included and excluded commits. Thus, if
+included and excluded commits. Note that the bad bisection ref
+`$GIT_DIR/refs/bisect/bad` is added to the included commits (if it
+exists) and the good bisection refs `$GIT_DIR/refs/bisect/good-*` are
+added to the excluded commits (if they exist). Thus, supposing there
+are no refs in `$GIT_DIR/refs/bisect/`, if
 
 -----------------------------------------------------------------------
 	$ git rev-list --bisect foo ^bar ^baz
@@ -558,22 +571,24 @@ one.
 
 --bisect-vars::
 
-This calculates the same as `--bisect`, but outputs text ready
-to be eval'ed by the shell. These lines will assign the name of
-the midpoint revision to the variable `bisect_rev`, and the
-expected number of commits to be tested after `bisect_rev` is
-tested to `bisect_nr`, the expected number of commits to be
-tested if `bisect_rev` turns out to be good to `bisect_good`,
-the expected number of commits to be tested if `bisect_rev`
-turns out to be bad to `bisect_bad`, and the number of commits
-we are bisecting right now to `bisect_all`.
+This calculates the same as `--bisect`, except that refs in
+`$GIT_DIR/refs/bisect/` are not used, and except that this outputs
+text ready to be eval'ed by the shell. These lines will assign the
+name of the midpoint revision to the variable `bisect_rev`, and the
+expected number of commits to be tested after `bisect_rev` is tested
+to `bisect_nr`, the expected number of commits to be tested if
+`bisect_rev` turns out to be good to `bisect_good`, the expected
+number of commits to be tested if `bisect_rev` turns out to be bad to
+`bisect_bad`, and the number of commits we are bisecting right now to
+`bisect_all`.
 
 --bisect-all::
 
 This outputs all the commit objects between the included and excluded
 commits, ordered by their distance to the included and excluded
-commits. The farthest from them is displayed first. (This is the only
-one displayed by `--bisect`.)
+commits. Refs in `$GIT_DIR/refs/bisect/` are not used. The farthest
+from them is displayed first. (This is the only one displayed by
+`--bisect`.)
 +
 This is useful because it makes it easy to choose a good commit to
 test when you want to avoid to test some of them for some reason (they
-- 
1.6.5.1.gaf97d

^ permalink raw reply related

* Re: gitk does not show path file list
From: Mark Blakeney @ 2009-11-24  5:36 UTC (permalink / raw)
  To: git
In-Reply-To: <33e2b2760911170409q4cbdad8ay83ae5c941bc5ff95@mail.gmail.com>

This seems to me to be a straight out bug but given I've had no response here
and  given this is such a simple issue then I guess it's not a bug and I'm just
missing something? Please somebody, why does gitk (usually) not show the subset
list of files affected when you give it a path?

E.g. If I am in a src dir then "gitk ." does not list files. Neither does "gitk
$PWD" nor "gitk ../src". However "cd ..; git src" does list files!?

Is there a more appropriate forum/list for git newcomers?

^ permalink raw reply

* commit --quiet option
From: bill lam @ 2009-11-24  5:16 UTC (permalink / raw)
  To: git

In git version 1.6.6.rc0.15.g4fa8a0 using the --quiet option it still show some
output.  Is that intended?  Specifically I would like to exclude message about
the untracked files when using --quite option.

$ git commit -q -m commit
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       modified:   .git-completion.bash
#       modified:   bin/mirrorgit
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       a123
no changes added to commit (use "git add" and/or "git commit -a")

-- 
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3

^ permalink raw reply

* Re: Android needs repo, Chrome needs gclient. Neither work. What does  that say about git?
From: Adrian May @ 2009-11-24  3:48 UTC (permalink / raw)
  To: tytso; +Cc: git, chromium-discuss
In-Reply-To: <20091123135817.GB2532@thunk.org>

> If you don't have bolt-on scripts, and you move that into the the core
> SCM, then you force *all* projects to use whatever workflow was
> decided as being the One True Way of doing things as seen by the SCM
> designer.  So the question is whether the SCM *should* regiment all
> projects into following the SCM's designers idea of the One True
> Workflow.

Of course I'd want the workflow configurable by whoever controls the
main repository. I couldn't possibly suggest that all git projects
need the same workflow. The number of developers can vary by five
orders of magnitude - that calls for different workflow models.

> Git's approach is to say that it will be fairly flexible about
> dictating workflow --- who pushs to whom; whether there is a single
> "star" repository topology, or something that is more flexible, etc.
> You seem to hate this flexibility, because it makes life harder until
> you set up these bolt-on scripts.  But that's what many of us like
> about git; that it doesn't force us (the project lead) into a single
> way of doing things.

Leaving aside topology, I suppose we can agree that the subject
divides into two aspects: offering the developer some optional tools,
and asserting control over what gets commited to the official repo.
Perhaps we can also agree that the former belongs under the control of
the developer and the latter should be in the PM's hands. You seem to
have opinions about which of these two aspects is more or less
important, and to what extent git suffices, but I don't. I assume that
the project manager has his own opinions about both aspects and I'm
observing two big projects that independantly have augmented git's
features with their own scripts. Their docs talk about both aspects,
especially repo's, but they are entirely implemented in
developer-overridable scripts. So any repo functions to do with the
second aspect are either features that git needs to grow, or bits of
the git manual that the repo designer didn't read. I'd kinda like to
know which.

Returning to topology, I think that also divides up similarly. The PM
can't forbid you and me from casually swapping diffs back and forth,
but he can dictate who we are supposed to submit our final candidate
to for review. The very existence of a PM, who controls a subset of
the repositories in the world, already implies a star topology, and I
think pretty much everybody is using distributed source control in
this way, at least when it comes down to *controlling* anything.
People may also be causally bouncing diffs around, but that's not
control, it's communication. I've got a one-man project on github
which I edit from two locations, and even on that scale I find myself
working star-fashion because either computer might have junk
experiments in progress, but I only push to github if it's meaningful
and tidy.

That reminds me of a slightly different question: if a longer
experiment that I have committed several stages of turns out to be a
blind alley, I'd like to go back a few steps on main, declare
everything after that to be a side branch that I'll probably never use
again, and continue on main with my next attempt. Is that possible? I
know that I can just start a new branch from the before the bad
experiment, but if that happens a lot, the name of my current main
branch would be changing all the time, and I think that's bad. I
suspect what I want is possible, but I'm not sure how to do it.

> As far as my wanting to impose a particular regimen on my project's
> developers, I've never been a big fan of the Bondage and Discpline
> school of software engineering.  They can use whatever workflow they
> like; they just have to deliver patches that are clean.  If they are,
> I'll pull from their repository.  If they aren't, I won't.

Repo talks a lot about automating the workflow that leads to precisely
that decision. They evidently want something more evolved than
somebody simply having a look at the code. I'm not sure what they
want, but I'm pretty sure it's none of the developer's business.

Adrian.

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: Jeff King @ 2009-11-24  3:07 UTC (permalink / raw)
  To: bill lam; +Cc: Junio C Hamano, Nicolas Pitre, git
In-Reply-To: <20091124011339.GA18003@debian.b2j>

On Tue, Nov 24, 2009 at 09:13:39AM +0800, bill lam wrote:

> On Mon, 23 Nov 2009, Nicolas Pitre wrote:
> > Then, during the pack-objects process, there are 3 phases: counting 
> > objects, compressing objects, and writing objects.  However in the fetch 
> 
> during git-gc it shows yet another progress 
> 
> Removing duplicate objects: 100% (256/256), done.

Thanks, this doesn't seem to have been guarded at all (but since it is
on a 2-second delay, you have to have quite a lot of loose objects or a
slow disk to trigger it).

We should apply the patch below to keep things consistent.

I also checked every other call to start_progress; everything else seems
to be guarded. Most of them were easy to trace to an isatty check,
though the one in unpack-trees is influenced by o->verbose_update. That
in turn usually corresponds to a quiet option, though merge does seem to
use it unconditionally. Maybe that should be tweaked, too?

-- >8 --
Subject: [PATCH] prune-packed: only show progress when stderr is a tty

This matches the behavior of other git programs, and helps
keep cruft out of things like cron job output.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-prune-packed.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index be99eb0..f9463de 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -71,7 +71,7 @@ void prune_packed_objects(int opts)
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 {
-	int opts = VERBOSE;
+	int opts = isatty(2) ? VERBOSE : 0;
 	const struct option prune_packed_options[] = {
 		OPT_BIT('n', "dry-run", &opts, "dry run", DRY_RUN),
 		OPT_NEGBIT('q', "quiet", &opts, "be quiet", VERBOSE),
-- 
1.6.6.rc0.249.g9b4cf.dirty

^ permalink raw reply related

* [PATCH v3] remote-curl.c: fix rpc_out()
From: Tay Ray Chuan @ 2009-11-24  2:31 UTC (permalink / raw)
  To: git
  Cc: Shawn O. Pearce, Junio C Hamano, Sverre Rabbelier,
	Johannes Schindelin, paulfred
In-Reply-To: <20091124095508.d6312ab0.rctay89@gmail.com>

Remove the extraneous semicolon (';') at the end of the if statement
that allowed the code in its block to execute regardless of the
condition.

This fixes pushing to a smart http backend with chunked encoding.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

  Reworded the part on code execution, so that it doesn't say "the code
  doesn't execute", but rather "the code always executes".

  Thanks to Paul for spotting that.

 remote-curl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 69eaf58..a331bae 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -307,7 +307,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 		rpc->len = avail;
 	}

-	if (max < avail);
+	if (max < avail)
 		avail = max;
 	memcpy(ptr, rpc->buf + rpc->pos, avail);
 	rpc->pos += avail;
--
1.6.4.4

^ permalink raw reply related

* [PATCH v2 2/2] remote-curl.c: fix rpc_out()
From: Tay Ray Chuan @ 2009-11-24  1:55 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Junio C Hamano, Sverre Rabbelier,
	Johannes Schindelin
In-Reply-To: <20091124095021.93942a56.rctay89@gmail.com>

Remove the extraneous semicolon (';') at the end of the if statement,
that prevented code in its block from executing.

This fixes pushing to a smart http backend with chunked encoding.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 remote-curl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 69eaf58..a331bae 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -307,7 +307,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 		rpc->len = avail;
 	}

-	if (max < avail);
+	if (max < avail)
 		avail = max;
 	memcpy(ptr, rpc->buf + rpc->pos, avail);
 	rpc->pos += avail;
--
1.6.4.4

^ permalink raw reply related

* [PATCH v2 1/2] Do curl option disabling before enabling new options
From: Tay Ray Chuan @ 2009-11-24  1:50 UTC (permalink / raw)
  To: git; +Cc: Martin Storsjö
In-Reply-To: <20091124094810.797341d4.rctay89@gmail.com>

From: =?ISO-8859-15?Q?Martin_Storsj=F6?= <martin@martin.st>

This works around a bug in curl versions up to 7.19.4, where
disabling the CURLOPT_NOBODY option sets the internal state
incorrectly considering that CURLOPT_PUT was enabled earlier.

The bug is discussed at http://curl.haxx.se/bug/view.cgi?id=2727981
and is corrected in the latest version of curl in CVS.

This bug usually has no impact on git, but may surface if using
multi-pass authentication methods.

Signed-off-by: Martin Storsjo <martin@martin.st>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

  No changes from the previous version.

 http-push.c   |    2 +-
 remote-curl.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/http-push.c b/http-push.c
index 0e040f8..432b20f 100644
--- a/http-push.c
+++ b/http-push.c
@@ -408,10 +408,10 @@ static void start_put(struct transfer_request *request)
 	curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, &request->buffer);
 #endif
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);

 	if (start_active_slot(slot)) {
diff --git a/remote-curl.c b/remote-curl.c
index 4f28c22..69eaf58 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -356,8 +356,8 @@ static int post_rpc(struct rpc_state *rpc)
 	slot = get_active_slot();
 	slot->results = &results;

-	curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");

--
1.6.4.4

^ permalink raw reply related

* [PATCH v2 0/2] http fixes
From: Tay Ray Chuan @ 2009-11-24  1:48 UTC (permalink / raw)
  To: git; +Cc: Martin Storsjö, Shawn O. Pearce, Junio C Hamano
In-Reply-To: <20091123110347.d47728f8.rctay89@gmail.com>

This series is a re-roll and is based on 'master'.

Patch 1 ("Do curl option disabling before enabling new options"):
 No changes from the previous version.

Patch 2 ("remote-curl.c: fix rpc_out()"):
 Focus solely on the extraneous semicolon.

 http-push.c   |    2 +-
 remote-curl.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

--
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH 2/2] remote-curl.c: fix rpc_out()
From: Tay Ray Chuan @ 2009-11-24  1:35 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Junio C Hamano
In-Reply-To: <20091123210415.GH11919@spearce.org>

Hi,

On Tue, Nov 24, 2009 at 5:04 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> If I understand the change right, the only thing that really matters
> here is removing the extra ';' in the if (max < avail) condition.
>
> That bug was the only reason why rpc->len < rpc->pos, and is thus
> the only reason why avail would "go negative" (which it can't do
> since its unsigned, it just wrapped around to a massive value).
> [snip]
> Right.  This bug is really bad and should be fixed.  But the message
> above make this some little after-thought and doesn't explain what
> was wrong with this being here.

point noted.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: bill lam @ 2009-11-24  1:13 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.2.00.0911231043310.2059@xanadu.home>

On Mon, 23 Nov 2009, Nicolas Pitre wrote:
> Then, during the pack-objects process, there are 3 phases: counting 
> objects, compressing objects, and writing objects.  However in the fetch 

during git-gc it shows yet another progress 

Removing duplicate objects: 100% (256/256), done.

-- 
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3

^ permalink raw reply

* Re: 'error: unable to set permission to './objects/...'
From: Rafal Rusin @ 2009-11-24  0:59 UTC (permalink / raw)
  To: git
In-Reply-To: <7vhbsl5x5e.fsf@alter.siamese.dyndns.org>

OK, I solved problem by upgrading samba on server from 3.0.23c to
3.2.8 (it's old, because it's arm server). This newer version doesn't
have problem with chmod.
Junio, your idea to investigate why it fails was good :-)
Thanks for your time.

2009/11/23 Junio C Hamano <gitster@pobox.com>:
> Rafal Rusin <rafal.rusin@gmail.com> writes:
>
>> As for detecting this particular case, I think it's not possible.
>
> Why not?
>
>> I think the best solution is to add repository config switch like
>> 'usefilepermissions' true by default. If set to false, git would skip
>> chmod during push.
>> Does that make sense to you?
>
> Not at all.  That is like creating an "allow broken behaviour" option and
> hoping for the best.
>
> You shouldn't have to invent such a configuration variable to begin with,
> as "let umask set whatever permission and leave it be" should already be
> the case for !core.sharedrepository.  There is something wrong in your
> set up and you need to get to the bottom of it, instead of coming up with
> an even worse breakage as a unworkable workaround.
>
> There are some things I noticed while looking at the codepaths that are
> involved.
>
> move_temp_to_file() is designed to move a temporary file that was created
> by odb_mkstemp().  As the latter eventually uses mkstemp(), some
> implementations of which ignore umask and create a file that is readable
> only by the user, move_temp_to_file() must loosen the permission, even in
> a private repository that honors umask (i.e. not in a shared repository)..
>
>    Side note.  The creation of the temporary files in http.c that are
>    given to move_temp_to_file() is not quite correct; they are created by
>    fopen() without proper locking with mkstemp().  But that is a separate
>    issue.
>
> But the codepath tries to loosen it a bit too much.  Even if user's umask
> is 077, files created in this codepath end up with world-readable because
> we pretend that lstat() on the file returned 0444 (that is what a non-zero
> value given as the second parameter to set_shared_perm() means).  We
> should tighten it perhaps like the attached patch does.
>
> In case it is not obvious, this patch is _not_ meant to help you work
> around the chmod() failure you are seeing on your filesystem.  You need to
> first see _why_ it fails for you, in order to come closer to the real
> solution.
>
> -- >8 --
> Subject: [PATCH] move_temp_to_file(): don't loosen permission too much
>
> We always feed 0444 as the second parameter to set_shared_perm() when
> finalizing a temporary file we created using mkstemp(), as some versions
> of glibc creates a temporary file with too tight a permission, ignoring
> the user's umask.  As the second parameter tells set_shared_perm() to
> pretend that it is the permission bits the file currently has (i.e. what
> should have been set by the user's umask), we should make it just as
> restrictive as user's umask suggests.
>
> ---
>  sha1_file.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 63981fb..f0b8969 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2233,6 +2233,21 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len,
>        git_SHA1_Final(sha1, &c);
>  }
>
> +static int fix_tempfile_mode(const char *filename)
> +{
> +       static mode_t user_mode;
> +
> +       if (!user_mode) {
> +               user_mode = umask(0);
> +               umask(user_mode);
> +               user_mode = S_IFREG | ((user_mode ^ 0777) & 0666);
> +       }
> +
> +       if (!set_shared_perm(filename, user_mode))
> +               return 0;
> +       return error("unable to set permission to '%s'", filename);
> +}
> +
>  /*
>  * Move the just written object into its final resting place.
>  * NEEDSWORK: this should be renamed to finalize_temp_file() as
> @@ -2274,9 +2289,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
>        }
>
>  out:
> -       if (set_shared_perm(filename, (S_IFREG|0444)))
> -               return error("unable to set permission to '%s'", filename);
> -       return 0;
> +       return fix_tempfile_mode(filename);
>  }
>
>  static int write_buffer(int fd, const void *buf, size_t len)
>
>


Regards,
-- 
Rafał Rusin
http://rrusin.blogspot.com
http://www.touk.pl
http://top.touk.pl

^ permalink raw reply

* Re: [PATCH 0/2] Add support for IPv6 on MinGW
From: Junio C Hamano @ 2009-11-24  0:58 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git
In-Reply-To: <alpine.DEB.2.00.0911240052440.14228@cone.home.martin.st>

Martin Storsjö <martin@martin.st> writes:

> This is a short patch series that adds support for IPv6 on MinGW. These 
> patches have been in use in msysgit for a few months (but the code was 
> accidentally removed recently in a merge). For consistency, it would be 
> good to add them upstream, too.

I am aware of the exchange between you and J6t on msysgit@googlegroups
where he suggested you to send them here.  Giving better visibility to
these patches for public review is good.

But you didn't have to Cc: me; in Windows API issues I have no clue so I
won't be a good reviewer.  I do not even compile git on Windows myself,
let alone testing nor using.

As hinted by J6t, he will be saying Ack or something else, so I'll act on
these patches when it happens.

Preferrably, I'd like somebody reasonably high in msysgit foodchain to
volunteer to be a coordinator and send me a pull-request whenever a set of
changes that has been:

 (1) cooking in msysgit tree without issues; and

 (2) reviewed favorably here on this list

is ready for git.git.  That way, I do not even have to remember whose
patches in the msysgit area I should blindly trust and apply, or keep
track of which patches are still under discussion in general.

Thanks.

^ permalink raw reply

* Re: [PATCH RFC] git-send-email --expand-aliases
From: Alex Chiang @ 2009-11-24  0:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: catalin.marinas, git
In-Reply-To: <7v6390sqhz.fsf@alter.siamese.dyndns.org>

* Junio C Hamano <gitster@pobox.com>:
> Alex Chiang <achiang@hp.com> writes:
> 
> > I'm an StGit user, and while StGit has its own 'stg mail'
> > feature, it doesn't know how to expand email aliases (yet).
> >
> > Certainly, one way to solve that problem would be to hack stgit
> > so that it can parse alias files, but to me, that seems silly
> > when git-send-email can already do that.
> >
> > This patch teaches git-send-email to only expand email addresses
> > so that other git porcelains don't have to roll their own mail
> > alias parsers.
> 
> Certainly, one way to solve that would be to hack _both_ stgit and
> send-email so that the former runs the latter _only_ to ask for the
> expansion and then send the message out, but to me, that seems silly
> when git-send-email can already do both expanding aliases and sending
> the message ;-)
> 
> If you are changing StGit to call git-send-email anyway, why not arrange
> stgit to call git-send-email to send the message out instead, instead of
> sending messages on its own?

Yeah, I thought about that as I was poking around further in
StGit to figure out how it would be calling git-send-email. ;)

> > I imagine the internal implementation of stg mail to work
> > something like:
> >
> > 	call git-send-email --expand-aliases repeatedly, once for
> > 	all the combined --to= args, then for all the combined --cc= args,
> > 	and finally for all the combined --bcc= args (all passed
> > 	to stg mail), read from stdout until EOF
> 
> I imagine the internal implementation of stg mail would work something
> like:
> 
>     prepare messages to send out
>     call git-send-email and have it send them
> 
> What am I missing?

My lack of familiarity with StGit internals. ;)

Your suggestion is much better. I'll take a closer look at StGit
and see how feasible it is.

Unless Catalin has strong objections?

Thanks,
/ac

^ permalink raw reply

* Re: [PATCH RFC] git-send-email --expand-aliases
From: Junio C Hamano @ 2009-11-24  0:42 UTC (permalink / raw)
  To: Alex Chiang; +Cc: catalin.marinas, git
In-Reply-To: <20091123221628.GE26810@ldl.fc.hp.com>

Alex Chiang <achiang@hp.com> writes:

> I'm an StGit user, and while StGit has its own 'stg mail'
> feature, it doesn't know how to expand email aliases (yet).
>
> Certainly, one way to solve that problem would be to hack stgit
> so that it can parse alias files, but to me, that seems silly
> when git-send-email can already do that.
>
> This patch teaches git-send-email to only expand email addresses
> so that other git porcelains don't have to roll their own mail
> alias parsers.

Certainly, one way to solve that would be to hack _both_ stgit and
send-email so that the former runs the latter _only_ to ask for the
expansion and then send the message out, but to me, that seems silly
when git-send-email can already do both expanding aliases and sending
the message ;-)

If you are changing StGit to call git-send-email anyway, why not arrange
stgit to call git-send-email to send the message out instead, instead of
sending messages on its own?

> I imagine the internal implementation of stg mail to work
> something like:
>
> 	call git-send-email --expand-aliases repeatedly, once for
> 	all the combined --to= args, then for all the combined --cc= args,
> 	and finally for all the combined --bcc= args (all passed
> 	to stg mail), read from stdout until EOF

I imagine the internal implementation of stg mail would work something
like:

    prepare messages to send out
    call git-send-email and have it send them

What am I missing?

^ permalink raw reply

* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Junio C Hamano @ 2009-11-24  0:35 UTC (permalink / raw)
  To: René Scharfe
  Cc: Junio C Hamano, Carlo Marcelo Arenas Belon, Bert Wesarg, git
In-Reply-To: <4B0B21CF.5040504@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

>> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
>> index c41c2f7..2b2afa6 100755
>> --- a/git-sh-setup.sh
>> +++ b/git-sh-setup.sh
>> @@ -114,6 +114,20 @@ git_editor() {
>>  	eval "${GIT_EDITOR:=vi}" '"$@"'
>>  }
>>  
>> +sane_grep () {
>> +	GREP_OPTIONS= \
>> +	GREP_COLOR= \
>> +	GREP_COLORS= \
>> +	LC_ALL=C grep "$@"
>> +}
>> +
>> +sane_egrep () {
>> +	GREP_OPTIONS= \
>> +	GREP_COLOR= \
>> +	GREP_COLORS= \
>> +	LC_ALL=C egrep "$@"
>> +}
>> +
>
> Ah, yes, much nicer.

Actually I am having a second thought after spending some time trying to
come up with a commit log message.  This leaves the door open for user
scripts to honor the environment variables, but it also means that
everybody needs to be aware of the insanity.  It is really tempting to
treat these exactly like CDPATH and unconditionally unset it.

Oh, and unsetting GREP_COLORS/GREP_COLOR was a mistake, I think.  As long
as we do not pass --color (and unset GREP_OPTIONS to make sure it is not
given), their settings should not matter to us.

-- >8 --
Subject: [PATCH] Protect scripted Porcelains from GREP_OPTIONS insanity

If the user has exported the GREP_OPTIONS environment variable, the output
from "grep" and "egrep" in scripted Porcelains may be different from what
they expect.  For example, we may want to count number of matching lines,
by "grep" piped to "wc -l", and GREP_OPTIONS=-C3 will break such use.

The approach taken by this change to address this issue is to protect only
our own use of grep/egrep.  Because we do not unset it at the beginning of
our scripts, hook scripts run from the scripted Porcelains are exposed to
the same insanity this environment variable causes when grep/egrep is used
to implement logic (e.g. "grep | wc -l"), and it is entirely up to the
hook scripts to protect themselves.

On the other hand, applypatch-msg hook may want to show offending words in
the proposed commit log message using grep to the end user, and the user
might want to set GREP_OPTIONS=--color to paint the match more visibly.
The approach to protect only our own use without unsetting the environment
variable globally will allow this use case.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-sh-setup.sh            |    8 ++++++++
 git-am.sh                  |    4 ++--
 git-bisect.sh              |    4 ++--
 git-filter-branch.sh       |    2 +-
 git-instaweb.sh            |    8 ++++----
 git-rebase--interactive.sh |   10 +++++-----
 git-rebase.sh              |    2 +-
 git-submodule.sh           |    6 +++---
 8 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..aa07cc3 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -114,6 +114,14 @@ git_editor() {
 	eval "${GIT_EDITOR:=vi}" '"$@"'
 }
 
+sane_grep () {
+	GREP_OPTIONS= LC_ALL=C grep "$@"
+}
+
+sane_egrep () {
+	GREP_OPTIONS= LC_ALL=C egrep "$@"
+}
+
 is_bare_repository () {
 	git rev-parse --is-bare-repository
 }
diff --git a/git-am.sh b/git-am.sh
index c132f50..b49f26a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -205,7 +205,7 @@ check_patch_format () {
 			# and see if it looks like that they all begin with the
 			# header field names...
 			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
-			LC_ALL=C egrep -v '^[!-9;-~]+:' >/dev/null ||
+			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
 			patch_format=mbox
 		fi
 	} < "$1" || clean_abort
@@ -554,7 +554,7 @@ do
 			stop_here $this
 
 		# skip pine's internal folder data
-		grep '^Author: Mail System Internal Data$' \
+		sane_grep '^Author: Mail System Internal Data$' \
 			<"$dotest"/info >/dev/null &&
 			go_next && continue
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 6f6f039..0c422d5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -393,7 +393,7 @@ bisect_run () {
 
       cat "$GIT_DIR/BISECT_RUN"
 
-      if grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+      if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
 		> /dev/null; then
 	  echo >&2 "bisect run cannot continue any more"
 	  exit $res
@@ -405,7 +405,7 @@ bisect_run () {
 	  exit $res
       fi
 
-      if grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
+      if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
 	  echo "bisect run success"
 	  exit 0;
       fi
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..8ef1bde 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -457,7 +457,7 @@ if [ "$filter_tag_name" ]; then
 				git mktag) ||
 				die "Could not create new tag object for $ref"
 			if git cat-file tag "$ref" | \
-			   grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
+			   sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
 			then
 				warn "gpg signature stripped from tag object $sha1t"
 			fi
diff --git a/git-instaweb.sh b/git-instaweb.sh
index d96eddb..84805c6 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -41,7 +41,7 @@ resolve_full_httpd () {
 	case "$httpd" in
 	*apache2*|*lighttpd*)
 		# ensure that the apache2/lighttpd command ends with "-f"
-		if ! echo "$httpd" | grep -- '-f *$' >/dev/null 2>&1
+		if ! echo "$httpd" | sane_grep -- '-f *$' >/dev/null 2>&1
 		then
 			httpd="$httpd -f"
 		fi
@@ -297,8 +297,8 @@ EOF
 
 	# check to see if Dennis Stosberg's mod_perl compatibility patch
 	# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied
-	if test -f "$module_path/mod_perl.so" && grep 'MOD_PERL' \
-				"$GIT_DIR/gitweb/gitweb.cgi" >/dev/null
+	if test -f "$module_path/mod_perl.so" &&
+	   sane_grep 'MOD_PERL' "$GIT_DIR/gitweb/gitweb.cgi" >/dev/null
 	then
 		# favor mod_perl if available
 		cat >> "$conf" <<EOF
@@ -316,7 +316,7 @@ EOF
 		# plain-old CGI
 		resolve_full_httpd
 		list_mods=$(echo "$full_httpd" | sed "s/-f$/-l/")
-		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
+		$list_mods | sane_grep 'mod_cgi\.c' >/dev/null 2>&1 || \
 		echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
 		cat >> "$conf" <<EOF
 AddHandler cgi-script .cgi
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..6268e76 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -106,8 +106,8 @@ mark_action_done () {
 	sed -e 1q < "$TODO" >> "$DONE"
 	sed -e 1d < "$TODO" >> "$TODO".new
 	mv -f "$TODO".new "$TODO"
-	count=$(grep -c '^[^#]' < "$DONE")
-	total=$(($count+$(grep -c '^[^#]' < "$TODO")))
+	count=$(sane_grep -c '^[^#]' < "$DONE")
+	total=$(($count+$(sane_grep -c '^[^#]' < "$TODO")))
 	if test "$last_count" != "$count"
 	then
 		last_count=$count
@@ -147,7 +147,7 @@ die_abort () {
 }
 
 has_action () {
-	grep '^[^#]' "$1" >/dev/null
+	sane_grep '^[^#]' "$1" >/dev/null
 }
 
 pick_one () {
@@ -731,7 +731,7 @@ first and then run 'git rebase --continue' again."
 			git rev-list $REVISIONS |
 			while read rev
 			do
-				if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
+				if test -f "$REWRITTEN"/$rev -a "$(sane_grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
 				then
 					# Use -f2 because if rev-list is telling us this commit is
 					# not worthwhile, we don't want to track its multiple heads,
@@ -739,7 +739,7 @@ first and then run 'git rebase --continue' again."
 					# be rebasing on top of it
 					git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$DROPPED"/$rev
 					short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
-					grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
+					sane_grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
 					rm "$REWRITTEN"/$rev
 				fi
 			done
diff --git a/git-rebase.sh b/git-rebase.sh
index 6ec155c..0ec4355 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -467,7 +467,7 @@ orig_head=$branch
 mb=$(git merge-base "$onto" "$branch")
 if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 	# linear history?
-	! (git rev-list --parents "$onto".."$branch" | grep " .* ") > /dev/null
+	! (git rev-list --parents "$onto".."$branch" | sane_grep " .* ") > /dev/null
 then
 	if test -z "$force_rebase"
 	then
diff --git a/git-submodule.sh b/git-submodule.sh
index 0462e52..b7ccd12 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -57,7 +57,7 @@ resolve_relative_url ()
 #
 module_list()
 {
-	git ls-files --error-unmatch --stage -- "$@" | grep '^160000 '
+	git ls-files --error-unmatch --stage -- "$@" | sane_grep '^160000 '
 }
 
 #
@@ -567,7 +567,7 @@ cmd_summary() {
 	cd_to_toplevel
 	# Get modified modules cared by user
 	modules=$(git $diff_cmd $cached --raw $head -- "$@" |
-		egrep '^:([0-7]* )?160000' |
+		sane_egrep '^:([0-7]* )?160000' |
 		while read mod_src mod_dst sha1_src sha1_dst status name
 		do
 			# Always show modules deleted or type-changed (blob<->module)
@@ -581,7 +581,7 @@ cmd_summary() {
 	test -z "$modules" && return
 
 	git $diff_cmd $cached --raw $head -- $modules |
-	egrep '^:([0-7]* )?160000' |
+	sane_egrep '^:([0-7]* )?160000' |
 	cut -c2- |
 	while read mod_src mod_dst sha1_src sha1_dst status name
 	do
-- 
1.6.6.rc0.15.g4fa80.dirty

^ permalink raw reply related

* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: René Scharfe @ 2009-11-23 23:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Carlo Marcelo Arenas Belon, Bert Wesarg, git
In-Reply-To: <7v4ooku7cv.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Yes, but what about git commands that are implemented as shell scripts
>> and use grep?  Something like the following patch?
>>
>> We'd need to run this from time to time to make sure no new grep calls
>> creep in:
>>
>>    git grep -L "unset GREP_OPTIONS" -- $(git grep -l "grep" git-*.sh)
> 
> Hmm, but "bisect run" runs user's script and it may want to see
> GREP_OPTIONS from the environment, no?  Same for any of the hooks that am
> and rebase might want to run.
> 
> 
> 
>  git-sh-setup.sh            |   14 ++++++++++++++
>  git-am.sh                  |    4 ++--
>  git-bisect.sh              |    4 ++--
>  git-filter-branch.sh       |    2 +-
>  git-instaweb.sh            |    8 ++++----
>  git-rebase--interactive.sh |   10 +++++-----
>  git-rebase.sh              |    2 +-
>  git-submodule.sh           |    6 +++---
>  8 files changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index c41c2f7..2b2afa6 100755
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -114,6 +114,20 @@ git_editor() {
>  	eval "${GIT_EDITOR:=vi}" '"$@"'
>  }
>  
> +sane_grep () {
> +	GREP_OPTIONS= \
> +	GREP_COLOR= \
> +	GREP_COLORS= \
> +	LC_ALL=C grep "$@"
> +}
> +
> +sane_egrep () {
> +	GREP_OPTIONS= \
> +	GREP_COLOR= \
> +	GREP_COLORS= \
> +	LC_ALL=C egrep "$@"
> +}
> +

Ah, yes, much nicer.

René

^ permalink raw reply

* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Junio C Hamano @ 2009-11-23 23:52 UTC (permalink / raw)
  To: René Scharfe; +Cc: Carlo Marcelo Arenas Belon, Bert Wesarg, git
In-Reply-To: <4B0B185B.4090305@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Yes, but what about git commands that are implemented as shell scripts
> and use grep?  Something like the following patch?
>
> We'd need to run this from time to time to make sure no new grep calls
> creep in:
>
>    git grep -L "unset GREP_OPTIONS" -- $(git grep -l "grep" git-*.sh)

Hmm, but "bisect run" runs user's script and it may want to see
GREP_OPTIONS from the environment, no?  Same for any of the hooks that am
and rebase might want to run.



 git-sh-setup.sh            |   14 ++++++++++++++
 git-am.sh                  |    4 ++--
 git-bisect.sh              |    4 ++--
 git-filter-branch.sh       |    2 +-
 git-instaweb.sh            |    8 ++++----
 git-rebase--interactive.sh |   10 +++++-----
 git-rebase.sh              |    2 +-
 git-submodule.sh           |    6 +++---
 8 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..2b2afa6 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -114,6 +114,20 @@ git_editor() {
 	eval "${GIT_EDITOR:=vi}" '"$@"'
 }
 
+sane_grep () {
+	GREP_OPTIONS= \
+	GREP_COLOR= \
+	GREP_COLORS= \
+	LC_ALL=C grep "$@"
+}
+
+sane_egrep () {
+	GREP_OPTIONS= \
+	GREP_COLOR= \
+	GREP_COLORS= \
+	LC_ALL=C egrep "$@"
+}
+
 is_bare_repository () {
 	git rev-parse --is-bare-repository
 }
diff --git a/git-am.sh b/git-am.sh
index c132f50..b49f26a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -205,7 +205,7 @@ check_patch_format () {
 			# and see if it looks like that they all begin with the
 			# header field names...
 			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
-			LC_ALL=C egrep -v '^[!-9;-~]+:' >/dev/null ||
+			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
 			patch_format=mbox
 		fi
 	} < "$1" || clean_abort
@@ -554,7 +554,7 @@ do
 			stop_here $this
 
 		# skip pine's internal folder data
-		grep '^Author: Mail System Internal Data$' \
+		sane_grep '^Author: Mail System Internal Data$' \
 			<"$dotest"/info >/dev/null &&
 			go_next && continue
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 6f6f039..0c422d5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -393,7 +393,7 @@ bisect_run () {
 
       cat "$GIT_DIR/BISECT_RUN"
 
-      if grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+      if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
 		> /dev/null; then
 	  echo >&2 "bisect run cannot continue any more"
 	  exit $res
@@ -405,7 +405,7 @@ bisect_run () {
 	  exit $res
       fi
 
-      if grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
+      if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
 	  echo "bisect run success"
 	  exit 0;
       fi
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..8ef1bde 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -457,7 +457,7 @@ if [ "$filter_tag_name" ]; then
 				git mktag) ||
 				die "Could not create new tag object for $ref"
 			if git cat-file tag "$ref" | \
-			   grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
+			   sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
 			then
 				warn "gpg signature stripped from tag object $sha1t"
 			fi
diff --git a/git-instaweb.sh b/git-instaweb.sh
index d96eddb..84805c6 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -41,7 +41,7 @@ resolve_full_httpd () {
 	case "$httpd" in
 	*apache2*|*lighttpd*)
 		# ensure that the apache2/lighttpd command ends with "-f"
-		if ! echo "$httpd" | grep -- '-f *$' >/dev/null 2>&1
+		if ! echo "$httpd" | sane_grep -- '-f *$' >/dev/null 2>&1
 		then
 			httpd="$httpd -f"
 		fi
@@ -297,8 +297,8 @@ EOF
 
 	# check to see if Dennis Stosberg's mod_perl compatibility patch
 	# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied
-	if test -f "$module_path/mod_perl.so" && grep 'MOD_PERL' \
-				"$GIT_DIR/gitweb/gitweb.cgi" >/dev/null
+	if test -f "$module_path/mod_perl.so" &&
+	   sane_grep 'MOD_PERL' "$GIT_DIR/gitweb/gitweb.cgi" >/dev/null
 	then
 		# favor mod_perl if available
 		cat >> "$conf" <<EOF
@@ -316,7 +316,7 @@ EOF
 		# plain-old CGI
 		resolve_full_httpd
 		list_mods=$(echo "$full_httpd" | sed "s/-f$/-l/")
-		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
+		$list_mods | sane_grep 'mod_cgi\.c' >/dev/null 2>&1 || \
 		echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
 		cat >> "$conf" <<EOF
 AddHandler cgi-script .cgi
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..6268e76 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -106,8 +106,8 @@ mark_action_done () {
 	sed -e 1q < "$TODO" >> "$DONE"
 	sed -e 1d < "$TODO" >> "$TODO".new
 	mv -f "$TODO".new "$TODO"
-	count=$(grep -c '^[^#]' < "$DONE")
-	total=$(($count+$(grep -c '^[^#]' < "$TODO")))
+	count=$(sane_grep -c '^[^#]' < "$DONE")
+	total=$(($count+$(sane_grep -c '^[^#]' < "$TODO")))
 	if test "$last_count" != "$count"
 	then
 		last_count=$count
@@ -147,7 +147,7 @@ die_abort () {
 }
 
 has_action () {
-	grep '^[^#]' "$1" >/dev/null
+	sane_grep '^[^#]' "$1" >/dev/null
 }
 
 pick_one () {
@@ -731,7 +731,7 @@ first and then run 'git rebase --continue' again."
 			git rev-list $REVISIONS |
 			while read rev
 			do
-				if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
+				if test -f "$REWRITTEN"/$rev -a "$(sane_grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
 				then
 					# Use -f2 because if rev-list is telling us this commit is
 					# not worthwhile, we don't want to track its multiple heads,
@@ -739,7 +739,7 @@ first and then run 'git rebase --continue' again."
 					# be rebasing on top of it
 					git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$DROPPED"/$rev
 					short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
-					grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
+					sane_grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
 					rm "$REWRITTEN"/$rev
 				fi
 			done
diff --git a/git-rebase.sh b/git-rebase.sh
index 6ec155c..0ec4355 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -467,7 +467,7 @@ orig_head=$branch
 mb=$(git merge-base "$onto" "$branch")
 if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 	# linear history?
-	! (git rev-list --parents "$onto".."$branch" | grep " .* ") > /dev/null
+	! (git rev-list --parents "$onto".."$branch" | sane_grep " .* ") > /dev/null
 then
 	if test -z "$force_rebase"
 	then
diff --git a/git-submodule.sh b/git-submodule.sh
index 0462e52..b7ccd12 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -57,7 +57,7 @@ resolve_relative_url ()
 #
 module_list()
 {
-	git ls-files --error-unmatch --stage -- "$@" | grep '^160000 '
+	git ls-files --error-unmatch --stage -- "$@" | sane_grep '^160000 '
 }
 
 #
@@ -567,7 +567,7 @@ cmd_summary() {
 	cd_to_toplevel
 	# Get modified modules cared by user
 	modules=$(git $diff_cmd $cached --raw $head -- "$@" |
-		egrep '^:([0-7]* )?160000' |
+		sane_egrep '^:([0-7]* )?160000' |
 		while read mod_src mod_dst sha1_src sha1_dst status name
 		do
 			# Always show modules deleted or type-changed (blob<->module)
@@ -581,7 +581,7 @@ cmd_summary() {
 	test -z "$modules" && return
 
 	git $diff_cmd $cached --raw $head -- $modules |
-	egrep '^:([0-7]* )?160000' |
+	sane_egrep '^:([0-7]* )?160000' |
 	cut -c2- |
 	while read mod_src mod_dst sha1_src sha1_dst status name
 	do

^ 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