Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add color to git-add--interactive diffs (Total different idea to solve the problem)
From: Peter Baumann @ 2007-10-23  5:34 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Tom Tobin, Dan Zwell, Jonathan del Strother, Shawn O. Pearce,
	Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0710230054130.25221@racer.site>

On Tue, Oct 23, 2007 at 12:55:44AM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 22 Oct 2007, Peter Baumann wrote:
> 
> > Wouldn't it make more sense to implement the diff coloring inside git 
> > apply so that you could use something like
> > 
> >         diff file1 file2|git apply --color
> > 
> > to make the generated diff with colors [1]? It already implements the
> > same semantic for generating a diffstat, using
> > 
> >         diff file1 file2|git apply --stat
> 
> No.  In both cases, "git diff" realises that the output is no terminal, 
> and switches off color generation.  (Just try with diff.color=true instead 
> of =auto.)
> 

I didn't mean git-diff here, instead I meant diff, so no coloring involved
on the diff side. The git-apply would be enhanced to do the coloring on
every diff it gets on its STDIN.

In the git-add -i case, the perl script whould do something along these
lines:

	foreach my $file (@files) {
		# read in the diff of a file *WITHOUT* using color
		@diff = `git-diff-files $file`;

		# ... store it away for later use in hunk selection ...


		# print out a nice colored diff for the user
		`echo @diff | git apply --color`
	}

Instead of handcoding the colorization in the git-add--interactive perl
script, just enhance git-apply to do the colorization *after the fact* for
you on _any_ patch you throw at it in its STDIN.

-Peter

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Theodore Tso @ 2007-10-23  5:30 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023050726.GD14735@spearce.org>

On Tue, Oct 23, 2007 at 01:07:26AM -0400, Shawn O. Pearce wrote:
> Junio has in the past proposed rewinding next, especially after a
> significant release (e.g. 1.5.3).  

Hmm, yes.  I think I'd want to rewind next after a while; the thought
of next drifting hundreds or thousands of commits away from master
just gives me the heebee-jeebies.  I'm sure it mostly works, but it
just feels wrong.  :-)

> A bunch of folks (myself included if I recall correctly) didn't want
> to do this, as we create topic branches locally from things in next
> and sometimes make commits over them to improve the topic further.

I guess I don't see why this would be a hardship; would a quick rebase
on the topic branches more or less take care of the problem?  

I guess that brings up another question; I've been regularly rebasing
the topics branches as master and next advances... probably more out
of superstition than anything else.  Is that a bad idea for any reason?


Hmm... I guess some of this would be really good to get into the Howto
section of the user guide when talking about git workflows!

	       	    	       	       	     	 - Ted

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Shawn O. Pearce @ 2007-10-23  5:07 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023045632.GD27132@thunk.org>

Theodore Tso <tytso@mit.edu> wrote:
> On Tue, Oct 23, 2007 at 12:46:57AM -0400, Shawn O. Pearce wrote:
> > By merging only individual topics forked from master into next you
> > can merge those individual topics into master at different points
> > in time.  For example db/fetch-pack has been in next for many weeks
> > and hasn't yet merged into master, yet jc/am-quiet was forked after
> > db/fetch-pack started and has already merged into master.
> > 
> > Your way would make jc/am-quiet wait until db/fetch-pack was ready.
> > That's a big risk in the sense that your tree is "blocked" and even
> > simple changes are held up by ones that suddenly became a lot more
> > complex then you originally thought they were going to be.
> 
> Yes, true.  Alternatively, what I've been doing is that if I wasn't
> sure that a particular topic was ready to go to 'master' very shortly
> after it went into 'next', I would never let it go into 'next', but
> rather keep it in 'pu' (which is OK, because pu is constantly getting
> rewound).  But I guess the downside of that is you might get fewer
> testers for the code, because fewer people are probably tracking and
> testing 'pu' as compared to 'next'.
> 
> Right?

Yes, that's a good point.

I think in Git part of the reason less people track pu is because
its very volatile.  Not because of the rewind policy, but becuase
sometimes the code there doesn't work properly so using it for real
"production" work is pretty risky.  On the other hand most of the
code that merges into next has been reasonbly well reviewed and
tested, so following it for "production" work is not as risky.

Junio has in the past proposed rewinding next, especially after a
significant release (e.g. 1.5.3).  A bunch of folks (myself included
if I recall correctly) didn't want to do this, as we create topic
branches locally from things in next and sometimes make commits
over them to improve the topic further.  But I also make topic
branches for things in pu, so I might as well just shut up and
not complain.  :-)

Of course another thought that just came to mind is it is very easy
for me to review next with a

	git log -p --reverse origin/next..build-next

just before merging it into my build branch and compiling it locally.
If next rewound frequently (as pu does) this would be more difficult.

-- 
Shawn.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Theodore Tso @ 2007-10-23  4:56 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023044657.GC14735@spearce.org>

On Tue, Oct 23, 2007 at 12:46:57AM -0400, Shawn O. Pearce wrote:
> By merging only individual topics forked from master into next you
> can merge those individual topics into master at different points
> in time.  For example db/fetch-pack has been in next for many weeks
> and hasn't yet merged into master, yet jc/am-quiet was forked after
> db/fetch-pack started and has already merged into master.
> 
> Your way would make jc/am-quiet wait until db/fetch-pack was ready.
> That's a big risk in the sense that your tree is "blocked" and even
> simple changes are held up by ones that suddenly became a lot more
> complex then you originally thought they were going to be.

Yes, true.  Alternatively, what I've been doing is that if I wasn't
sure that a particular topic was ready to go to 'master' very shortly
after it went into 'next', I would never let it go into 'next', but
rather keep it in 'pu' (which is OK, because pu is constantly getting
rewound).  But I guess the downside of that is you might get fewer
testers for the code, because fewer people are probably tracking and
testing 'pu' as compared to 'next'.

Right?

					- Ted

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Shawn O. Pearce @ 2007-10-23  4:46 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023043321.GC27132@thunk.org>

Theodore Tso <tytso@mit.edu> wrote:
> On Tue, Oct 23, 2007 at 12:05:22AM -0400, Shawn O. Pearce wrote:
> >
> > Yes.  Because 'next' always has commits in it that never appear in
> > 'master'.  So any topic forked from master must merge into next.
> > It can't be a fast-forward.  No forced merging required.
> 
> Why is it the case that 'next' always commits that never appear in
> 'master'.  So far in how I've been doing things that hasn't been the
> case.  When I do a "git checkout master; git merge next", it's always
> been a fast-forward merge. 
> 
> Oh, I see.  That's because if you put some trivial changes in
> 'master', and then pull those changes into next, there will be merge
> commits in 'next' that will never be in 'master.  Is that it?  

Exactly.  This of course means that next has been growing in distance
from master for quite some time.  It has well over 1000 commits now
in git.git that aren't in master.  Most of those will never merge
there either.
 
> I had been trying to avoid that case by always putting new commits,
> even trivial ones, into 'next', and then having them drop into
> 'master' at the next cycle; so 'master' was always trailing 'next',
> but they were always the same commit string (i.e., 'master' was always
> a subset of 'next').  
> 
> Aside from the WC script not working right, are there other
> disadvantages to my doing things that way as opposed to the way the
> Junio has been running the git repository?

The reason Junio does what he does is flexibility.

By merging only individual topics forked from master into next you
can merge those individual topics into master at different points
in time.  For example db/fetch-pack has been in next for many weeks
and hasn't yet merged into master, yet jc/am-quiet was forked after
db/fetch-pack started and has already merged into master.

Your way would make jc/am-quiet wait until db/fetch-pack was ready.
That's a big risk in the sense that your tree is "blocked" and even
simple changes are held up by ones that suddenly became a lot more
complex then you originally thought they were going to be.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Let git-add--interactive read "git colors" from git-config
From: Shawn O. Pearce @ 2007-10-23  4:40 UTC (permalink / raw)
  To: Jeff King
  Cc: Dan Zwell, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071023042956.GC28312@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> On Mon, Oct 22, 2007 at 09:19:58PM -0500, Dan Zwell wrote:
> 
> > This patch is againts Shawn Pearce's "pu" branch.
> 
> Don't do that. The code in 'pu' is a mess of half-working features. If
> your patch is accepted, then it has to be picked apart from those
> half-working features that aren't being accepted (which hopefully isn't
> hard if nobody has been working in the same area, but can be quite
> ugly).  Base your work on 'master' if possible, or 'next' if it relies
> on features only in next. If it relies on some topic branch that is
> _only_ in pu, then mention explicitly which topic.

And even when you base work on things in next, don't base it on the
tip of next.  Base it on a specific topic that is merged into next.
Next is also a mess of features, but they are more likely to be in
a working state than the features in pu.

Topics in next will merge to master at different times.  If your
changes depend on more than one topic that may make it more difficult
for the maintainer to merge your topic to master.

Fortunately In Dan's case the only topic in pu that impacted
git-add--interactive was his dz/color-addi topic, so this probably
applies to the tip of it just as well as it does to the tip of pu.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.
From: Shawn O. Pearce @ 2007-10-23  4:34 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Scott Parish, git
In-Reply-To: <Pine.LNX.4.64.0710221135100.25221@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Mon, 22 Oct 2007, Shawn O. Pearce wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > Earlier, we tried to find the git commands in several possible exec
> > > dirs.  Now, if all of these failed, try to find the git command in
> > > PATH.
> > ...
> > > diff --git a/exec_cmd.c b/exec_cmd.c
> > > index 9b74ed2..70b84b0 100644
> > > --- a/exec_cmd.c
> > > +++ b/exec_cmd.c
> > > @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)
> > >  	int i;
> > >  	const char *paths[] = { current_exec_path,
> > >  				getenv(EXEC_PATH_ENVIRONMENT),
> > > -				builtin_exec_path };
> > > +				builtin_exec_path,
> > > +				"" };
> > 
> > So if the user sets GIT_EXEC_PATH="" and exports it we'll search $PATH 
> > before the builtin exec path that Git was compiled with? Are we sure we 
> > want to do that?
> 
> I thought the proper way to unset EXEC_PATH was to "unset GIT_EXEC_PATH".  
> In that case, getenv(EXEC_PATH_ENVIRONMENT) returns NULL and we're fine, 
> no?

Sure.  But can't you also export an environment variable that is
set to the empty string?  At least on UNIX.  Windows thinks unset
and empty string are the same thing.

-- 
Shawn.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Theodore Tso @ 2007-10-23  4:33 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023040522.GX14735@spearce.org>

On Tue, Oct 23, 2007 at 12:05:22AM -0400, Shawn O. Pearce wrote:
>
> Yes.  Because 'next' always has commits in it that never appear in
> 'master'.  So any topic forked from master must merge into next.
> It can't be a fast-forward.  No forced merging required.

Why is it the case that 'next' always commits that never appear in
'master'.  So far in how I've been doing things that hasn't been the
case.  When I do a "git checkout master; git merge next", it's always
been a fast-forward merge. 

Oh, I see.  That's because if you put some trivial changes in
'master', and then pull those changes into next, there will be merge
commits in 'next' that will never be in 'master.  Is that it?  

I had been trying to avoid that case by always putting new commits,
even trivial ones, into 'next', and then having them drop into
'master' at the next cycle; so 'master' was always trailing 'next',
but they were always the same commit string (i.e., 'master' was always
a subset of 'next').  

Aside from the WC script not working right, are there other
disadvantages to my doing things that way as opposed to the way the
Junio has been running the git repository?

						- Ted

^ permalink raw reply

* Re: git.el fails on non-git managed files
From: Shawn O. Pearce @ 2007-10-23  4:31 UTC (permalink / raw)
  To: racin; +Cc: git
In-Reply-To: <1193077008.471ce910f15c5@imp.free.fr>

racin@free.fr wrote:
> I found the following on the development version of git.el: saving
> non-git-managed files in Emacs threw an error.
> 
> It is due to a simple error in the call to condition-case in a
> recently added function, git-update-save-file.
> 
> I attached the patch for your convenience.

I'm not sure what happened, but I didn't receive a patch with
your email.  Would you mind resending it to the list?  You may
also want to file a bug with your distribution to get the Emacs
bindings included.
 
-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] don't set-group-id on directories on apple
From: Scott Parish @ 2007-10-23  4:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <20071022142945.GO16291@srparish.net>

On Mon, Oct 22, 2007 at 07:29:45AM -0700, Scott Parish wrote:

> On Mon, Oct 22, 2007 at 03:16:01PM +0100, Johannes Schindelin wrote:
> 
> > On Mon, 22 Oct 2007, Scott R Parish wrote:
> > 
> > > "git init --shared=all" was failing because chmod was returning EPERM.
> > 
> > Not here. 
> > 
> > Is it possible that you have stricter permission settings?

I finally figured it out. I keep my home directory encrypted, but
its pretty slow (especially compiles) and i don't care who steals
open source code i'm playing with, so i keep that in /Users/Shared.
Since mkdir() on darwin keeps the parents group by default, the
group on my git clone was wheel, which i'm not a member of.

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: [PATCH] Let git-add--interactive read "git colors" from git-config
From: Jeff King @ 2007-10-23  4:29 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071022211958.045895ac@danzwell.com>

On Mon, Oct 22, 2007 at 09:19:58PM -0500, Dan Zwell wrote:

> This patch is againts Shawn Pearce's "pu" branch.

Don't do that. The code in 'pu' is a mess of half-working features. If
your patch is accepted, then it has to be picked apart from those
half-working features that aren't being accepted (which hopefully isn't
hard if nobody has been working in the same area, but can be quite
ugly).  Base your work on 'master' if possible, or 'next' if it relies
on features only in next. If it relies on some topic branch that is
_only_ in pu, then mention explicitly which topic.

-Peff

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Shawn O. Pearce @ 2007-10-23  4:27 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023012140.GC22997@thunk.org>

Theodore Tso <tytso@mit.edu> wrote:
> On Mon, Oct 22, 2007 at 06:15:09PM -0700, Junio C Hamano wrote:
> > 
> > Sorry, WI is for "what's in", WC is for "what's cooking".  I
> > should remove PU and RB from there.
> 
> I assume PU is what you used to build your proposed-update branch?

Actually I found PU of some use:

	git branch -f pu next
	git checkout pu
	./Meta/PU --continue

its a little sluggish to list, but made it pretty easy to pick
topics for merging into pu.  git-rerere really makes it easy to
recover conflicts in pu during future rebuilds of pu.

> based off of master, and then merged into next.  Or maybe I'm not
> understanding how to make the WC and git-topic.perl script work and
> sing for me perfectly?

The other tidbits I managed to learn by trial and error here was
to make sure I did the following:

	- make sure master is fully merged into next
	- make sure next is fully merged into pu
	- Run "Meta/git-topic.perl --base=master | less"

It wasn't uncommon for me to merge master->next, next->pu, run
git-topic, then reset both next and pu *back* to what I had last
published (remote tracking branches updated during push make this
easy) before moving on with my next and pu updating activities.

Of course take my notes above with a grain of salt; I only worked
this way for a week and it took me a couple of days to come up with
the above.  Junio may very well have it streamlined even more.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 2/2] Let git-add--interactive read colors from git-config
From: Jeff King @ 2007-10-23  4:27 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071022164048.71a3dceb@danzwell.com>

On Mon, Oct 22, 2007 at 04:40:48PM -0500, Dan Zwell wrote:

> Note: the code to parse git-style color strings to perl-style color
> strings should eventually be added to Git.pm so that other (perl)
> parts of git can be configured to read colors from .gitconfig in
> a nicer way. A git-style string is "ul red black", while perl 
> likes strings like "underline red on_black".

Why not do it as part of this patch, then?

> +	# Sane (visible) defaults:
> +	if (! @git_prompt_color) {
> +		@git_prompt_color = ("blue", "bold");
> +	}

I think it might be a bit more readable to keep the assignment and
defaults together:

  my @git_prompt_color = split /\s+/,
    qx(git config --get color.interactive.prompt) || 'blue bold';

Though I wonder why we are splitting here at all, since we just end up
converting the list into a scalar below. And if we just turned that into
a function, we could get a nice:

  my $prompt_color = git_color_to_ansicolor(
    qx(git config --get color.interactive.prompt) || 'blue bold');

-Peff

^ permalink raw reply

* [PATCH trailing ws fixed] use only the PATH for exec'ing git commands
From: Scott Parish @ 2007-10-23  4:08 UTC (permalink / raw)
  Cc: git
In-Reply-To: <20071022190102.GA23714@steel.home>

We need to correctly set up PATH for non-c based git commands. Since we
already do this, we can just use that PATH and execvp, instead of looping
over the paths with execve.

This patch adds a setup_path() function to exec_cmd.c, which sets
the PATH order correctly for our search order. execv_git_cmd() is
stripped down to setting up argv and calling execvp(). git.c's main()
only only needs to call setup_path().

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |  121 ++++++++++++++++++++++++++----------------------------------
 exec_cmd.h |    1 +
 git.c      |   43 +++------------------
 3 files changed, 60 insertions(+), 105 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 8b681d0..c228dbf 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -29,85 +29,68 @@ const char *git_exec_path(void)
 	return builtin_exec_path;
 }
 
+static void add_path(struct strbuf *out, const char *path)
+{
+	if (path && strlen(path)) {
+		if (is_absolute_path(path))
+			strbuf_addstr(out, path);
+		else
+			strbuf_addstr(out, make_absolute_path(path));
+
+		strbuf_addch(out, ':');
+	}
+}
+
+void setup_path(const char *cmd_path)
+{
+	const char *old_path = getenv("PATH");
+	struct strbuf new_path;
+
+	strbuf_init(&new_path, 0);
+
+	add_path(&new_path, argv_exec_path);
+	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
+	add_path(&new_path, builtin_exec_path);
+	add_path(&new_path, cmd_path);
+
+	if (old_path)
+		strbuf_addstr(&new_path, old_path);
+	else
+		strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin");
+
+	setenv("PATH", new_path.buf, 1);
+
+	strbuf_release(&new_path);
+}
 
 int execv_git_cmd(const char **argv)
 {
-	char git_command[PATH_MAX + 1];
-	int i;
-	const char *paths[] = { argv_exec_path,
-				getenv(EXEC_PATH_ENVIRONMENT),
-				builtin_exec_path };
-
-	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
-		size_t len;
-		int rc;
-		const char *exec_dir = paths[i];
-		const char *tmp;
-
-		if (!exec_dir || !*exec_dir) continue;
-
-		if (*exec_dir != '/') {
-			if (!getcwd(git_command, sizeof(git_command))) {
-				fprintf(stderr, "git: cannot determine "
-					"current directory: %s\n",
-					strerror(errno));
-				break;
-			}
-			len = strlen(git_command);
-
-			/* Trivial cleanup */
-			while (!prefixcmp(exec_dir, "./")) {
-				exec_dir += 2;
-				while (*exec_dir == '/')
-					exec_dir++;
-			}
-
-			rc = snprintf(git_command + len,
-				      sizeof(git_command) - len, "/%s",
-				      exec_dir);
-			if (rc < 0 || rc >= sizeof(git_command) - len) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-		} else {
-			if (strlen(exec_dir) + 1 > sizeof(git_command)) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-			strcpy(git_command, exec_dir);
-		}
-
-		len = strlen(git_command);
-		rc = snprintf(git_command + len, sizeof(git_command) - len,
-			      "/git-%s", argv[0]);
-		if (rc < 0 || rc >= sizeof(git_command) - len) {
-			fprintf(stderr,
-				"git: command name given is too long.\n");
-			break;
-		}
+	struct strbuf cmd;
+	const char *tmp;
 
-		/* argv[0] must be the git command, but the argv array
-		 * belongs to the caller, and my be reused in
-		 * subsequent loop iterations. Save argv[0] and
-		 * restore it on error.
-		 */
+	strbuf_init(&cmd, 0);
+	strbuf_addf(&cmd, "git-%s", argv[0]);
 
-		tmp = argv[0];
-		argv[0] = git_command;
+	/* argv[0] must be the git command, but the argv array
+	 * belongs to the caller, and my be reused in
+	 * subsequent loop iterations. Save argv[0] and
+	 * restore it on error.
+	 */
+	tmp = argv[0];
+	argv[0] = cmd.buf;
 
-		trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf(argv, -1, "trace: exec:");
 
-		/* execve() can only ever return if it fails */
-		execve(git_command, (char **)argv, environ);
+	/* execvp() can only ever return if it fails */
+	execvp(cmd.buf, (char **)argv);
 
-		trace_printf("trace: exec failed: %s\n", strerror(errno));
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-		argv[0] = tmp;
-	}
-	return -1;
+	argv[0] = tmp;
 
+	strbuf_release(&cmd);
+
+	return -1;
 }
 
 
diff --git a/exec_cmd.h b/exec_cmd.h
index da99287..a892355 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,6 +3,7 @@
 
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
+extern void setup_path(const char *);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index f659338..a639e42 100644
--- a/git.c
+++ b/git.c
@@ -6,28 +6,6 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
-static void prepend_to_path(const char *dir, int len)
-{
-	const char *old_path = getenv("PATH");
-	char *path;
-	int path_len = len;
-
-	if (!old_path)
-		old_path = "/usr/local/bin:/usr/bin:/bin";
-
-	path_len = len + strlen(old_path) + 1;
-
-	path = xmalloc(path_len + 1);
-
-	memcpy(path, dir, len);
-	path[len] = ':';
-	memcpy(path + len + 1, old_path, path_len - len);
-
-	setenv("PATH", path, 1);
-
-	free(path);
-}
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -403,7 +381,7 @@ int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
 	char *slash = strrchr(cmd, '/');
-	const char *exec_path = NULL;
+	const char *cmd_path = NULL;
 	int done_alias = 0;
 
 	/*
@@ -413,10 +391,7 @@ int main(int argc, const char **argv)
 	 */
 	if (slash) {
 		*slash++ = 0;
-		if (*cmd == '/')
-			exec_path = cmd;
-		else
-			exec_path = xstrdup(make_absolute_path(cmd));
+		cmd_path = cmd;
 		cmd = slash;
 	}
 
@@ -451,16 +426,12 @@ int main(int argc, const char **argv)
 	cmd = argv[0];
 
 	/*
-	 * We execute external git command via execv_git_cmd(),
-	 * which looks at "--exec-path" option, GIT_EXEC_PATH
-	 * environment, and $(gitexecdir) in Makefile while built,
-	 * in this order.  For scripted commands, we prepend
-	 * the value of the exec_path variable to the PATH.
+	 * We use PATH to find git commands, but we prepend some higher
+	 * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH
+	 * environment, and the $(gitexecdir) from the Makefile at build
+	 * time.
 	 */
-	if (exec_path)
-		prepend_to_path(exec_path, strlen(exec_path));
-	exec_path = git_exec_path();
-	prepend_to_path(exec_path, strlen(exec_path));
+	setup_path(cmd_path);
 
 	while (1) {
 		/* See if it's an internal command */
-- 
gitgui.0.8.4.11176.gd9205-dirty

^ permalink raw reply related

* Re: What's cooking in git/spearce.git (topics)
From: Shawn O. Pearce @ 2007-10-23  4:05 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20071023020044.GA27132@thunk.org>

Theodore Tso <tytso@mit.edu> wrote:
> On Mon, Oct 22, 2007 at 06:29:59PM -0700, Junio C Hamano wrote:
> > Well, the policy is never to commit directly on top of next
> > (iow, only merge other topics and nothing else).  Otherwise it
> > becomes hard to allow individual topics graduate to 'master'
> > independently.
> 
> I see.  So if it's non-trivial enough that you want it to "cook" in
> next for a cycle, you'll create a topic branch for it (based off of
> 'master'), and then force a merge into 'next'?

Yes.  Because 'next' always has commits in it that never appear in
'master'.  So any topic forked from master must merge into next.
It can't be a fast-forward.  No forced merging required.

Of course this isn't true for a new project.  That first topic
that forked from master to *create* next will be a fast-forward
as it creates next.  But that's no big deal.  The second topic will
merge into next, and that first topic can still be merged back into
master without merging next (or the second topic).

I was also doing the same thing Junio already explained to manage
next and pu while he was away.  Except I shortcut his:

	git checkout pu
	git reset --hard next

as:

	git branch -f pu next
	git checkout pu

as I'm was usually already sitting on next.  This saved my poor
little laptop from a second of IO chugging as it slewed around
between the two versions.  There were no files to update as it
switched from next to pu, and pu was already setup for merging
the proposed topics.  :-)

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 1/2] Added basic color support to git add --interactive
From: Jeff King @ 2007-10-23  4:03 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071022163244.4af72973@danzwell.com>

On Mon, Oct 22, 2007 at 04:32:44PM -0500, Dan Zwell wrote:

> +my ($use_color, $prompt_color, $header_color, $help_color);
> +my $color_config = qx(git config --get color.interactive);
> +if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
> +	$use_color = "true";
> +        # Sane (visible) defaults:
> +        $prompt_color = "blue bold";
> +        $header_color = "bold";
> +        $help_color = "red bold";

Bad indentation?

> +sub print_colored {
> +	my $color = shift;
> +	my @strings = @_;
> +
> +	if ($use_color) {
> +		print Term::ANSIColor::color($color);
> +		print(@strings);
> +		print Term::ANSIColor::color("reset");
> +	} else {
> +		print @strings;
> +	}
> +}

This does nothing for embedded newlines in the strings, which means that
you can end up with ${COLOR}text\n${RESET}, which fouls up changed
backgrounds. See commit 50f575fc. Since the strings you are printing are
small, I don't see any problem with making a copy, using a regex to
insert the color coding, and printing that (I think I even posted
example code in a previous thread on this subject).

-Peff

^ permalink raw reply

* Re: [PATCH] Add some fancy colors in the test library when terminal supports it.
From: Christian Couder @ 2007-10-23  4:08 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071022081341.GC32763@artemis.corp>

Hi Pierre,

Le lundi 22 octobre 2007, Pierre Habouzit a écrit :
> +
> +say_color () {
> +	[ "$nocolor" = 0 ] &&  [ "$1" != '-1' ] && tput setaf "$1"
> +	shift
> +	echo "* $*"
> +	tput op
> +}
> +
>  error () {
> -	echo "* error: $*"
> +	say_color 9 "* error: $*"

This will print something like "* * error: ..." instead of "* error: ..."

The following should work:

> +	say_color 9 "error: $*"

By the way, where do the 9 here and the 10 and the -1 below come from ?
"man 5 terminfo" says that only values form 0 to 7 are portably defined.
Maybe 9 is a bold red and 10 a bold green, or something like that, but it 
doesn't seem to work on my konsole.

Anyway, perhaps having:

_red=1
_green=2

and then using "say_color $_red stuff" might be easier to understand and 
change if needed.

Thanks for this good idea,
Christian.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Theodore Tso @ 2007-10-23  2:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Shawn O. Pearce, git
In-Reply-To: <7vtzoi8voo.fsf@gitster.siamese.dyndns.org>

On Mon, Oct 22, 2007 at 06:29:59PM -0700, Junio C Hamano wrote:
> Well, the policy is never to commit directly on top of next
> (iow, only merge other topics and nothing else).  Otherwise it
> becomes hard to allow individual topics graduate to 'master'
> independently.

I see.  So if it's non-trivial enough that you want it to "cook" in
next for a cycle, you'll create a topic branch for it (based off of
'master'), and then force a merge into 'next'?

					- Ted

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Jeff King @ 2007-10-23  3:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.999.0710221930330.30120@woody.linux-foundation.org>

On Mon, Oct 22, 2007 at 07:32:02PM -0700, Linus Torvalds wrote:

> Your patch is better than what used to be there, but
> 
> > -			/* Already picked as a destination? */
> > +			/* Already picked as a source? */
> >  			if (!p->src_dst)
> >  				continue;
> 
> the above is wrong, the whole thing should be dropped (we *want* to be 
> able to re-use sources).

Oops, you're right. I'm not sure what I was thinking.

> Anyway, the set of fixes I sent out earlier included fixing that stupid 
> loop as one of the things.

Looks like you have made some real progress. I'll try to review your
patch tonight, and hopefully make some progress on the inexact case.

-Peff

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Shawn O. Pearce @ 2007-10-23  3:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7du8vv7.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Thanks for keeping the git list running smoothly while I was away.

Funny thing.  There's this tool called "git" that makes it really
easy to fork a project, apply patches straight from email, and
republish it for others to read and work on top of.  You should
check it out sometime.  :-)
 
> I've pulled the four integration branches from you, and split
> out the topic branches out of next and pu so that I can take a
> look at them individually.  I haven't looked at the actual
> changes yet (but I do not have to, as long as I can trust
> capable others), and only skimmed the list messages (about 2200
> of them since I left).
> 
> git.git at k.org and alt-git.git at repo.or.cz should be in sync
> with you now.  I'll take a look at the recent changes after
> grabbing some sleep ;-)

We're glad to have you back.  Or should I say _I'm_ glad to have
you back.  Never underestimate a man until you've at least walked
a week in his shoes.  :-)

Most of the patches that happened while you were away were merged
or parked into my git/spearce.git work (in large part thanks
to Lars Hjemli's work during the first week you were offline).
So hopefully you can just pickup from "recent history" (e.g. today
forward) and if we missed anything really interesting authors can
repost once you've had a chance to get caught up.

-- 
Shawn.

^ permalink raw reply

* Re: What's cooking in git/spearce.git (topics)
From: Linus Torvalds @ 2007-10-23  2:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071022071644.GA7290@coredump.intra.peff.net>


Sorry, I missed this while being busy hacking and not reading email ;)

On Mon, 22 Oct 2007, Jeff King wrote:
> 
> Patch is below (please just squash with the one from Linus).

Your patch is better than what used to be there, but

> -			/* Already picked as a destination? */
> +			/* Already picked as a source? */
>  			if (!p->src_dst)
>  				continue;

the above is wrong, the whole thing should be dropped (we *want* to be 
able to re-use sources).

Anyway, the set of fixes I sent out earlier included fixing that stupid 
loop as one of the things.

		Linus

^ permalink raw reply

* [PATCH] Let git-add--interactive read "git colors" from git-config
From: Dan Zwell @ 2007-10-23  2:19 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Shawn O. Pearce, Jeff King, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071022163244.4af72973@danzwell.com>

Colors are specified in color.interactive.{prompt,header,help}.
They are specified as git color strings as described in the
documentation, then parsed into perl color strings (slightly
different). Ugly but visible defaults are still used.

Signed-off-by: Dan Zwell <dzwell@zwell.net>
---
This patch is againts Shawn Pearce's "pu" branch.
 Documentation/config.txt  |   17 ++-------
 git-add--interactive.perl |   78 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 99b3817..d06f55f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -390,19 +390,10 @@ color.interactive::
 
 color.interactive.<slot>::
 	Use customized color for `git add --interactive`
-	output. `<slot>` may be `prompt`, `header`, or `help`,
-	for three distinct types of common output from interactive
-	programs. The values may be a space-delimited combination
-	of up to three of the following:
-+
-(optional attribute, optional foreground color, and optional background)
-+
-dark, bold, underline, underscore, blink, reverse, concealed,
-black, red, green, yellow, blue, magenta, cyan, white, on_black,
-on_red, on_green, on_yellow, on_blue, on_magenta, on_cyan, on_white
-+
-Note these are not the same colors/attributes that the rest of
-git supports, but are specific to `git-add --interactive`.
+	output. `<slot>` may be `prompt`, `header`, or `help`, for
+	three distinct types of normal output from interactive
+	programs.  The values of these variables may be specified as
+	in color.branch.<slot>.
 
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 37be4b0..ca1ca28 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -6,12 +6,78 @@ my ($use_color, $prompt_color, $header_color, $help_color);
 my $color_config = qx(git config --get color.interactive);
 if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
 	$use_color = "true";
-	chomp( $prompt_color = qx(git config --get color.interactive.prompt) );
-	chomp( $header_color = qx(git config --get color.interactive.header) );
-	chomp( $help_color = qx(git config --get color.interactive.help) );
-	$prompt_color ||= "red bold";
-	$header_color ||= "bold";
-	$help_color ||= "blue bold";
+	# Grab the 3 main colors in git color string format:
+	my @git_prompt_color =
+		split(/\s+/, qx(git config --get color.interactive.prompt));
+	my @git_header_color =
+		split(/\s+/, qx(git config --get color.interactive.header));
+	my @git_help_color =
+		split(/\s+/, qx(git config --get color.interactive.help));
+
+	# Sane (visible) defaults:
+	if (! @git_prompt_color) {
+		@git_prompt_color = ("blue", "bold");
+	}
+	if (! @git_header_color) {
+		@git_header_color = ("bold");
+	}
+	if (! @git_help_color) {
+		@git_help_color = ("red", "bold");
+	}
+
+	# Parse the git colors into perl colors:
+	my %attrib_mappings = (
+		"bold"    => "bold",
+		"ul"      => "underline",
+		"blink"   => "blink",
+		# not supported:
+		#"dim"     => "",
+		"reverse" => "reverse"
+	);
+
+	my @tmp_perl_colors;
+	my $color_list;
+	# Loop over the array of (arrays of) git-style colors
+	foreach $color_list ([@git_prompt_color], [@git_header_color],
+	                     [@git_help_color]) {
+		my $fg_done;
+		my @perl_attribs;
+		my $word;
+		foreach $word (@{$color_list}) {
+			if ($word =~ /normal/) {
+				$fg_done = "true";
+			}
+			elsif ($word =~ /black|red|green|yellow/ ||
+			       $word =~ /blue|magenta|cyan|white/) {
+				# is a color.
+				if ($fg_done) {
+					# this is the background
+					push @perl_attribs, "on_" . $word;
+				}
+				else {
+					# this is foreground
+					$fg_done = "true";
+					push @perl_attribs, $word;
+				}
+			}
+			else {
+				# this is an attribute, not a color.
+				if ($attrib_mappings{$word}) {
+					push(@perl_attribs,
+						 $attrib_mappings{$word});
+				}
+			}
+		}
+		if (@perl_attribs) {
+			push @tmp_perl_colors, join(" ", @perl_attribs);
+		}
+		else {
+			#@perl_attribs is empty, need a placeholder
+			push @tmp_perl_colors, "reset";
+		}
+	}
+	($prompt_color, $header_color, $help_color) =
+		@tmp_perl_colors;
 
 	require Term::ANSIColor;
 }
-- 
1.5.3.4.207.gc0ee

^ permalink raw reply related

* Re: [PATCH] resend of git-add--interactive color patch against spearce/pu
From: Dan Zwell @ 2007-10-23  2:11 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Shawn O. Pearce, Jeff King, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071022163244.4af72973@danzwell.com>

I apparently missed the e-mail where Shawn Pearce explained where his
repository was. The following patch is my recent change(s), rebased
against that.

Dan

^ permalink raw reply

* [PATCH 1/2] gitweb: Refactor abbreviation-with-title-attribute code.
From: David Symonds @ 2007-10-23  1:31 UTC (permalink / raw)
  To: pasky, spearce; +Cc: git, David Symonds

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 gitweb/gitweb.perl |   45 +++++++++++++++++++++------------------------
 1 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ea84c75..a835bd1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -842,6 +842,23 @@ sub chop_str {
 	return "$body$tail";
 }
 
+# takes the same arguments as chop_str, but also wraps a <span> around the
+# result with a title attribute if it does get chopped. Additionally, the
+# string is HTML-escaped.
+sub chop_and_escape_str {
+	my $str = shift;
+	my $len = shift;
+	my $add_len = shift || 10;
+
+	my $chopped = chop_str($str, $len, $add_len);
+	if ($chopped eq $str) {
+		return esc_html($chopped);
+	} else {
+		return qq{<span title="} . esc_html($str) . qq{">} .
+			esc_html($chopped) . qq{</span>};
+	}
+}
+
 ## ----------------------------------------------------------------------
 ## functions returning short strings
 
@@ -3427,12 +3444,7 @@ sub git_shortlog_body {
 			print "<tr class=\"light\">\n";
 		}
 		$alternate ^= 1;
-		my $author = chop_str($co{'author_name'}, 10);
-		if ($author ne $co{'author_name'}) {
-			$author = "<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
-		} else {
-			$author = esc_html($author);
-		}
+		my $author = chop_and_escape_str($co{'author_name'}, 10);
 		# git_summary() used print "<td><i>$co{'age_string'}</i></td>\n" .
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
 		      "<td><i>" . $author . "</i></td>\n" .
@@ -3484,12 +3496,7 @@ sub git_history_body {
 		}
 		$alternate ^= 1;
 	# shortlog uses      chop_str($co{'author_name'}, 10)
-		my $author = chop_str($co{'author_name'}, 15, 3);
-		if ($author ne $co{'author_name'}) {
-			"<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
-		} else {
-			$author = esc_html($author);
-		}
+		my $author = chop_and_escape_str($co{'author_name'}, 15, 3);
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
 		      "<td><i>" . $author . "</i></td>\n" .
 		      "<td>";
@@ -3645,12 +3652,7 @@ sub git_search_grep_body {
 			print "<tr class=\"light\">\n";
 		}
 		$alternate ^= 1;
-		my $author = chop_str($co{'author_name'}, 15, 5);
-		if ($author ne $co{'author_name'}) {
-			$author = "<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
-		} else {
-			$author = esc_html($author);
-		}
+		my $author = chop_and_escape_str($co{'author_name'}, 15, 5);
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
 		      "<td><i>" . $author . "</i></td>\n" .
 		      "<td>" .
@@ -5165,12 +5167,7 @@ sub git_search {
 						print "<tr class=\"light\">\n";
 					}
 					$alternate ^= 1;
-					my $author = chop_str($co{'author_name'}, 15, 5);
-					if ($author ne $co{'author_name'}) {
-						$author = "<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
-					} else {
-						$author = esc_html($author);
-					}
+					my $author = chop_and_escape_str($co{'author_name'}, 15, 5);
 					print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
 					      "<td><i>" . $author . "</i></td>\n" .
 					      "<td>" .
-- 
1.5.3.1

^ permalink raw reply related

* [PATCH 2/2] gitweb: Use chop_and_escape_str in more places.
From: David Symonds @ 2007-10-23  1:31 UTC (permalink / raw)
  To: pasky, spearce; +Cc: git, David Symonds
In-Reply-To: <1193103083390-git-send-email-dsymonds@gmail.com>

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 gitweb/gitweb.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a835bd1..1b6c823 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3402,7 +3402,7 @@ sub git_project_list_body {
 		      "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
 		                        -class => "list", -title => $pr->{'descr_long'}},
 		                        esc_html($pr->{'descr'})) . "</td>\n" .
-		      "<td><i>" . esc_html(chop_str($pr->{'owner'}, 15)) . "</i></td>\n";
+		      "<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
 		print "<td class=\"". age_class($pr->{'age'}) . "\">" .
 		      (defined $pr->{'age_string'} ? $pr->{'age_string'} : "No commits") . "</td>\n" .
 		      "<td class=\"link\">" .
@@ -3657,7 +3657,7 @@ sub git_search_grep_body {
 		      "<td><i>" . $author . "</i></td>\n" .
 		      "<td>" .
 		      $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}), -class => "list subject"},
-			       esc_html(chop_str($co{'title'}, 50)) . "<br/>");
+			       chop_and_escape_str($co{'title'}, 50) . "<br/>");
 		my $comment = $co{'comment'};
 		foreach my $line (@$comment) {
 			if ($line =~ m/^(.*)($search_regexp)(.*)$/i) {
@@ -5173,7 +5173,7 @@ sub git_search {
 					      "<td>" .
 					      $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}),
 					              -class => "list subject"},
-					              esc_html(chop_str($co{'title'}, 50)) . "<br/>");
+					              chop_and_escape_str($co{'title'}, 50) . "<br/>");
 					while (my $setref = shift @files) {
 						my %set = %$setref;
 						print $cgi->a({-href => href(action=>"blob", hash_base=>$co{'id'},
-- 
1.5.3.1

^ 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