Git development
 help / color / mirror / Atom feed
* Re: rebase -p confusion in 1.6.1
From: Michael J Gruber @ 2009-01-15 14:45 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <alpine.DEB.1.00.0901151518520.3586@pacific.mpi-cbg.de>

Johannes Schindelin venit, vidit, dixit 15.01.2009 15:25:
> Hi,
> 
> On Thu, 15 Jan 2009, Michael J Gruber wrote:
> 
>> Stephan Beyer venit, vidit, dixit 15.01.2009 14:55:
>>
>>>> First of all: git 1.6.0.6 gives you the unchanged graph after using
>>>> rebase -i -p.
>>> This is true and it is a far better behavior than now, but I think it's
>>> not the expected behavior. (I have written about the behavior I'd expect
>>> in another reply to the original mail.)
>> Yep, I think -p should preserve only merges in side branches
> 
> you mean everything in master..work?
> 
>> (and therefore produce what you suggest, and what you get without -p). 
>> If it preserves all merges then there is nothing to rewrite here.
> 
> The merge _is_ outside of master, so I do not understand what the heck you 
> are talking about.

Easy Dscho, easy ;)
[meaning "take it such..."]

I'm not sure what -p is supposed to do:

A) Should it preserve all merge commits which it would need to rewrite?
That is lot to ask. Previous behaviour (intended or not) seemed to be to
do nothing in this case where the merge connects master and work.

B) Should it preserve only merges in side branches? I seem to mean by
that branches where the parents are on work and other branches but not
on master.

So at least on my side there is confusion about the intention behind
'-p' (say design goal), and therefore about the expectation.

> The more I think about it, I think it's possible I broke it with the 
> introduction of the "noop".
> 
> However, there could be a _different_ test case where the current -p 
> handling shows the same error.  Dunno.

It certainly worked after the noop introduction before the r-i-p series,
but not any more after. "worked" meaning it at least didn't leave out
commits in this case (but reproduced the existing DAG). I'm getting the
impression you suggest R.I.P. for r-i-p series ;) Fine with me...

Cheers,
Michael

^ permalink raw reply

* Re: [ANNOUNCE] tig-0.13
From: bill lam @ 2009-01-15 14:50 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20090115130659.GA18081@diku.dk>

On Thu, 15 Jan 2009, Jonas Fonseca wrote:
> presence of a {/usr/incude/}ncursesw/ncurses.h header. Where are the
> unicode ncurses.h files found on your system?

on ubuntu,
/usr/incude/ncursesw/curses.h 
/usr/incude/ncursesw/ncurses.h  ( just a sym link to curses.h above ) 

-- 
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3
唐詩286 張祜  題金陵渡
    金陵津渡小山樓  一宿行人自可愁  潮落夜江斜月裡  兩三星火是瓜州

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 14:51 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.DEB.1.00.0901151429440.3586@pacific.mpi-cbg.de>

On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> I turned this into a proper test case (to show what would be most helpful 
> if you report bugs like this in the future).

Thanks.  I'll keep that in mind.

What is the significance of test_tick?  I can see what it is
doing, but am trying to understand why.

Regards,

Sitaram

^ permalink raw reply

* Re: [STGIT][PATCH] new: translate non word characters in patch name to '-'
From: David Kågedal @ 2009-01-15 14:53 UTC (permalink / raw)
  To: git
In-Reply-To: <154e089b0812291215h72dfe04aod080f665eb7f5592@mail.gmail.com>

"Hannes Eder" <hannes@hanneseder.net> writes:

> On 12/28/08, Karl Hasselström <kha@treskal.com> wrote:
>> On 2008-12-27 13:37:20 +0100, Hannes Eder wrote:
>>
>>  > This allows following usage:
>>  >
>>  > $ stg new full/path/file-fix-foobar
>>  > Now at patch "full-path-file-fix-foobar"
>>  >
>>  > Signed-off-by: Hannes Eder <hannes@hanneseder.net>
>>  > ---
>>  >
>>  > I ran into as a '/' in a patch messed up stgit.
>>  >
>>  > I find this useful as 'stg uncommit' does the same translation.
>>
>>
>> Clearly, bad path names shouldn't mess anything up -- see
>>
>>   https://gna.org/bugs/?10919
>>
>>  But I would prefer "stg new" to quit with an error message when given
>>  an illegal patch name, not silently mangle it. (Of course, the
>>  commands that generate patch names from commit messages -- such as
>>  "stg new" when not given an explicit patch name -- should mangle the
>>  commit message as much as necessary. But when the user gives us a
>>  patch name, we should either use that as is or fail with an
>>  informative message.)
>>
>>  I think the right thing to do would be to construct a function that
>>  validates patch names (I don't think we have one right now), and then
>>  call it whenever we need to check if a patch name is OK. Such as when
>>  the user gives us the name of a new patch. And when we've
>>  auto-generated a patch name from a commit message, we should probably
>>  assert that that the check function is OK with the name.
>
> What about instead of 'fail with an informative message', just issue a
> warning that
> the name has been mangled. I use pathnames in patch names frequently,
> so this would be very handy.

No, I agree with Karl. For example, a tool (such as the Emacs
frontend) might want to do a "stg new foobar" and then do something
with the patch it now knows is called "foobar". And if it was called
something else the tool will fail.

-- 
David Kågedal

^ permalink raw reply

* [PATCH 2/2] api-run-command.txt: talk about run_hook()
From: Stephan Beyer @ 2009-01-15 15:00 UTC (permalink / raw)
  To: git
  Cc: Paolo Bonzini, Miklos Vajna, Shawn O. Pearce, Daniel Barkalow,
	Christian Couder, gitster, Stephan Beyer
In-Reply-To: <1232031618-5243-1-git-send-email-s-beyer@gmx.net>

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---

	I noticed that the last patch is lacking some documentation.
	Here it is. Should perhaps be squashed.

 Documentation/technical/api-run-command.txt |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
index 82e9e83..1241f48 100644
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -52,6 +52,22 @@ Functions
 	Wait for the completion of an asynchronous function that was
 	started with start_async().
 
+`run_hook`::
+
+	Run a hook.
+	The first argument is a string to an index file or NULL
+	if the default index file or no index is used at all.
+	The second argument is the name of the hook.
+	The further variable number (up to 9) of arguments correspond
+	to the hook arguments.
+	The last argument has to be NULL to terminate the variable
+	arguments list.
+	If the hook is not executable, the return value is zero.
+	If it is executable, the hook will be executed and the exit
+	status of the hook is returned.
+	On execution, .stdout_to_stderr and .no_stdin will be set.
+	(See below.)
+
 
 Data structures
 ---------------
-- 
1.6.1.160.gecdb

^ permalink raw reply related

* [PATCH 1/2] Move run_hook() from builtin-commit.c into run-command.c (libgit)
From: Stephan Beyer @ 2009-01-15 15:00 UTC (permalink / raw)
  To: git
  Cc: Paolo Bonzini, Miklos Vajna, Shawn O. Pearce, Daniel Barkalow,
	Christian Couder, gitster, Stephan Beyer

A function that runs a hook is used in several Git commands.
builtin-commit.c has the one that is most general for cases without
piping. This patch moves it into libgit and lets the other builtins
use this libified run_hook().

Note: The run_hook() in receive-pack.c feeds the standard input of
the pre-receive or post-receive hook. So that function is renamed
to run_receive_hook().

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---

	This patch exists because I needed some run_hook() in sequencer
	and I noticed that slight variations are used across other
	builtins. :-)
	Stripping out a libified version seemed better to me than
	copy and paste.

 builtin-commit.c       |   34 ----------------------------------
 builtin-gc.c           |   30 +-----------------------------
 builtin-merge.c        |   31 +------------------------------
 builtin-receive-pack.c |    6 +++---
 run-command.c          |   35 +++++++++++++++++++++++++++++++++++
 run-command.h          |    2 ++
 6 files changed, 42 insertions(+), 96 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index e88b78f..6f8d9fb 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -361,40 +361,6 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	return s.commitable;
 }
 
-static int run_hook(const char *index_file, const char *name, ...)
-{
-	struct child_process hook;
-	const char *argv[10], *env[2];
-	char index[PATH_MAX];
-	va_list args;
-	int i;
-
-	va_start(args, name);
-	argv[0] = git_path("hooks/%s", name);
-	i = 0;
-	do {
-		if (++i >= ARRAY_SIZE(argv))
-			die ("run_hook(): too many arguments");
-		argv[i] = va_arg(args, const char *);
-	} while (argv[i]);
-	va_end(args);
-
-	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-	env[0] = index;
-	env[1] = NULL;
-
-	if (access(argv[0], X_OK) < 0)
-		return 0;
-
-	memset(&hook, 0, sizeof(hook));
-	hook.argv = argv;
-	hook.no_stdin = 1;
-	hook.stdout_to_stderr = 1;
-	hook.env = env;
-
-	return run_command(&hook);
-}
-
 static int is_a_merge(const unsigned char *sha1)
 {
 	struct commit *commit = lookup_commit(sha1);
diff --git a/builtin-gc.c b/builtin-gc.c
index f8eae4a..a201438 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -144,34 +144,6 @@ static int too_many_packs(void)
 	return gc_auto_pack_limit <= cnt;
 }
 
-static int run_hook(void)
-{
-	const char *argv[2];
-	struct child_process hook;
-	int ret;
-
-	argv[0] = git_path("hooks/pre-auto-gc");
-	argv[1] = NULL;
-
-	if (access(argv[0], X_OK) < 0)
-		return 0;
-
-	memset(&hook, 0, sizeof(hook));
-	hook.argv = argv;
-	hook.no_stdin = 1;
-	hook.stdout_to_stderr = 1;
-
-	ret = start_command(&hook);
-	if (ret) {
-		warning("Could not spawn %s", argv[0]);
-		return ret;
-	}
-	ret = finish_command(&hook);
-	if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
-		warning("%s exited due to uncaught signal", argv[0]);
-	return ret;
-}
-
 static int need_to_gc(void)
 {
 	/*
@@ -194,7 +166,7 @@ static int need_to_gc(void)
 	else if (!too_many_loose_objects())
 		return 0;
 
-	if (run_hook())
+	if (run_hook(NULL, "pre-auto-gc", NULL))
 		return 0;
 	return 1;
 }
diff --git a/builtin-merge.c b/builtin-merge.c
index cf86975..e4555b0 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -300,35 +300,6 @@ static void squash_message(void)
 	strbuf_release(&out);
 }
 
-static int run_hook(const char *name)
-{
-	struct child_process hook;
-	const char *argv[3], *env[2];
-	char index[PATH_MAX];
-
-	argv[0] = git_path("hooks/%s", name);
-	if (access(argv[0], X_OK) < 0)
-		return 0;
-
-	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", get_index_file());
-	env[0] = index;
-	env[1] = NULL;
-
-	if (squash)
-		argv[1] = "1";
-	else
-		argv[1] = "0";
-	argv[2] = NULL;
-
-	memset(&hook, 0, sizeof(hook));
-	hook.argv = argv;
-	hook.no_stdin = 1;
-	hook.stdout_to_stderr = 1;
-	hook.env = env;
-
-	return run_command(&hook);
-}
-
 static void finish(const unsigned char *new_head, const char *msg)
 {
 	struct strbuf reflog_message = STRBUF_INIT;
@@ -374,7 +345,7 @@ static void finish(const unsigned char *new_head, const char *msg)
 	}
 
 	/* Run a post-merge hook */
-	run_hook("post-merge");
+	run_hook(NULL, "post-merge", squash ? "1" : "0", NULL);
 
 	strbuf_release(&reflog_message);
 }
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index db67c31..6564a97 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -136,7 +136,7 @@ static int hook_status(int code, const char *hook_name)
 	}
 }
 
-static int run_hook(const char *hook_name)
+static int run_receive_hook(const char *hook_name)
 {
 	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
 	struct command *cmd;
@@ -358,7 +358,7 @@ static void execute_commands(const char *unpacker_error)
 		return;
 	}
 
-	if (run_hook(pre_receive_hook)) {
+	if (run_receive_hook(pre_receive_hook)) {
 		while (cmd) {
 			cmd->error_string = "pre-receive hook declined";
 			cmd = cmd->next;
@@ -627,7 +627,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 			unlink(pack_lockfile);
 		if (report_status)
 			report(unpack_status);
-		run_hook(post_receive_hook);
+		run_receive_hook(post_receive_hook);
 		run_update_post_hook(commands);
 	}
 	return 0;
diff --git a/run-command.c b/run-command.c
index c90cdc5..602fe85 100644
--- a/run-command.c
+++ b/run-command.c
@@ -342,3 +342,38 @@ int finish_async(struct async *async)
 #endif
 	return ret;
 }
+
+int run_hook(const char *index_file, const char *name, ...)
+{
+	struct child_process hook;
+	const char *argv[10], *env[2];
+	char index[PATH_MAX];
+	va_list args;
+	int i;
+
+	va_start(args, name);
+	argv[0] = git_path("hooks/%s", name);
+	i = 0;
+	do {
+		if (++i >= ARRAY_SIZE(argv))
+			die("run_hook(): too many arguments");
+		argv[i] = va_arg(args, const char *);
+	} while (argv[i]);
+	va_end(args);
+
+	if (access(argv[0], X_OK) < 0)
+		return 0;
+
+	memset(&hook, 0, sizeof(hook));
+	hook.argv = argv;
+	hook.no_stdin = 1;
+	hook.stdout_to_stderr = 1;
+	if (index_file) {
+		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+		env[0] = index;
+		env[1] = NULL;
+		hook.env = env;
+	}
+
+	return run_command(&hook);
+}
diff --git a/run-command.h b/run-command.h
index a8b0c20..0211e1d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -49,6 +49,8 @@ int start_command(struct child_process *);
 int finish_command(struct child_process *);
 int run_command(struct child_process *);
 
+extern int run_hook(const char *index_file, const char *name, ...);
+
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
-- 
1.6.1.160.gecdb

^ permalink raw reply related

* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 15:03 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.DEB.1.00.0901151448120.3586@pacific.mpi-cbg.de>

On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Remember: there is code that is so simple that it has no obvious flaws, 
> and there is code that is so complicated that it has no obvious flaws.
I've always heard the first part as "obviously no flaws"...

^ permalink raw reply

* Re: [ANNOUNCE] tig-0.13
From: Adeodato Simó @ 2009-01-15 15:05 UTC (permalink / raw)
  To: git
In-Reply-To: <20090115145003.GA6938@b2j>

* bill lam [Thu, 15 Jan 2009 22:50:03 +0800]:

> On Thu, 15 Jan 2009, Jonas Fonseca wrote:
> > presence of a {/usr/incude/}ncursesw/ncurses.h header. Where are the
> > unicode ncurses.h files found on your system?

> on ubuntu,
> /usr/incude/ncursesw/curses.h 
> /usr/incude/ncursesw/ncurses.h  ( just a sym link to curses.h above ) 

You should send the output of ./configure, and publish the resulting
config.log file somewhere.

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
Man is certainly stark mad; he cannot make a flea, yet he makes gods by the
dozens.
                -- Michel de Montaigne

^ permalink raw reply

* Re: [ANNOUNCE] tig-0.13
From: Jonas Fonseca @ 2009-01-15 15:08 UTC (permalink / raw)
  To: git
In-Reply-To: <20090115145003.GA6938@b2j>

bill lam <cbill.lam@gmail.com> wrote Thu, Jan 15, 2009:
> On Thu, 15 Jan 2009, Jonas Fonseca wrote:
> > presence of a {/usr/incude/}ncursesw/ncurses.h header. Where are the
> > unicode ncurses.h files found on your system?
> 
> on ubuntu,
> /usr/incude/ncursesw/curses.h 
> /usr/incude/ncursesw/ncurses.h  ( just a sym link to curses.h above ) 

Then I am puzzled why the configure script doesn't find it. Can you send
me your config.log and the output of running configure. Maybe off-list.

-- 
Jonas Fonseca

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Stephan Beyer @ 2009-01-15 15:09 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: git
In-Reply-To: <slrngmujc4.sf.sitaramc@sitaramc.homelinux.net>

Sitaram Chamarty wrote:
> On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > I turned this into a proper test case (to show what would be most helpful 
> > if you report bugs like this in the future).
> 
> Thanks.  I'll keep that in mind.
> 
> What is the significance of test_tick?  I can see what it is
> doing, but am trying to understand why.

When you run the test case a second, third, fourth time, the commit
times would be all different without test_tick. This is bad for
bugfixing when you need to inspect the test case repo a little
further. (When the commit times change, the commit ids change,
too.)
So setting the time and counting it artificially up is nice
to get the same SHAs over and over.

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 15:14 UTC (permalink / raw)
  To: git
In-Reply-To: <496F3C99.1040800@drmicha.warpmail.net>

On 2009-01-15, Michael J Gruber <git@drmicha.warpmail.net> wrote:
> Second, what result do you expect? If the merge is to be preserved then

I don't know.  I did this while trying to understand where
and how "-p" makes a difference.  If there's anything you
can point me to that explains rebase -p, especially from a
"here's where it comes useful" point of view, I'd appreciate
it.

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 15:21 UTC (permalink / raw)
  To: git
In-Reply-To: <20090115150913.GE10045@leksak.fem-net>

On 2009-01-15, Stephan Beyer <s-beyer@gmx.net> wrote:
> Sitaram Chamarty wrote:
>> What is the significance of test_tick?  I can see what it is
>> doing, but am trying to understand why.

> So setting the time and counting it artificially up is nice
> to get the same SHAs over and over.

I should have been clearer...

I was trying to understand why the "counting up" part is
needed.

Regards,

Sitaram

^ permalink raw reply

* Re: cygwin git diff crash
From: Johannes Schindelin @ 2009-01-15 15:34 UTC (permalink / raw)
  To: Jeremy Ramer; +Cc: Git Mailing List
In-Reply-To: <b9fd99020901150642i2e61bb78yf2dab2b8713592df@mail.gmail.com>

Hi,

On Thu, 15 Jan 2009, Jeremy Ramer wrote:

> On Wed, Jan 14, 2009 at 3:33 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Wed, 14 Jan 2009, Jeremy Ramer wrote:
> >
> >> Ok this is a weird issue and it's probably cygwin's fault, but I
> >> haven't found any way to fix it so I' thought I would throw it out
> >> here for comment.
> >>
> >> I am using git 1.6.0.4 on cygwin. I have a repo where if any file has
> >> changes and git detects as mode 100644 I get this error:
> >> $git diff
> >>       3 [main] git 2744 C:\cygwin\bin\git.exe: *** fatal error - could
> >> not load user32, Win32 error
> >>
> >> If I change the mode to 100755 git diff will work fine.
> >> $chmod a+x test.cpp
> >> $git diff
> >> diff --git a/test.cpp b/test.cpp
> >> old mode 100644
> >> new mode 100755
> >> index 7c0dfcd..20987a7
> >> --- a/test.cpp
> >> +++ b/test.cpp
> >> @@ -6,9 +6,11 @@ int main()
> >>
> >>  void func()
> >>  {
> >> +       int a;^M
> >>  }
> >>
> >>  void func2()
> >>  {
> >> +       int b;^M
> >>  }
> >>
> >> Anybody have a clue as to why this might occur?  I have seen this in
> >> many of the repo's I use, but it is not repeatable.  I tried making a
> >> test repo but could not reproduce.
> >
> > Wow, that _is_ weird.  Does your test suite pass?
> >
> > Ciao,
> > Dscho
> >
> 
> Forgive my ignorance, but I am not sure how to run the test suite.  I
> did a quick google search but didn't find anything.  I am using
> cygwin's packaged version of git and am not building it from source,
> so maybe the suite is not available.  If I get a chance I will try
> building from source.

It is really easy: just make sure that make, gcc, libiconv-dev, 
openssl-dev and libcurl-dev are installed (that's it AFAIR), then download 
a tarball, e.g.

	http://repo.or.cz/w/git.git?a=snapshot;h=next;sf=tgz

unpack it, cd to it and run "make".  (I would _not_ run configure...)

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH 1/3] bash-completion: Support running when set -u is enabled
From: Shawn O. Pearce @ 2009-01-15 15:34 UTC (permalink / raw)
  To: ted; +Cc: git, gitster
In-Reply-To: <1231963342-24461-1-git-send-email-ted@tedpavlic.com>

ted@tedpavlic.com wrote:
> From: Ted Pavlic <ted@tedpavlic.com>
> 
>   Under "set -u" semantics, it is an error to access undefined
>   variables. Some user environments may enable this setting in the
>   interactive shell.
> 
>   In any context where the completion functions access an undefined
>   variable, accessing a default empty string (aka "${1-}" instead of
>   "$1") is a reasonable way to code the function, as it silences
>   the undefined variable error while still supplying an empty string.
> 
>   In this patch, functions that should always take an argument still use
>   $1. Functions that have optional arguments use ${1-}.
> 
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>

Yup.  This patch isn't as bad as everyone made it out to be.
I think we should go ahead and include it.

But usually we don't indent the commit message like you did
above; instead the message should be aligned on the left margin at
column 0.  git log will indent the message during display, hence
any identation you add in the patch email just makes the message
that much harder to read from git log.

> ---
>  contrib/completion/git-completion.bash |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments
From: Shawn O. Pearce @ 2009-01-15 15:35 UTC (permalink / raw)
  To: ted; +Cc: git, gitster
In-Reply-To: <1231963342-24461-3-git-send-email-ted@tedpavlic.com>

ted@tedpavlic.com wrote:
> From: Ted Pavlic <ted@tedpavlic.com>
> 
>   Adds a few simple comments above commands that take arguments. These
>   comments are meant to remind developers of potential problems that can
>   occur when the script is sourced on systems with "set -u." Any
>   function which "requires" arguments really ought to be called with
>   explicit arguments given.
> 
>   Also adds a #!bash to the top of bash completions so that editing
>   software can always identify that the file is of sh type.
> 
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>

Yup, this and also 2/3 look fine.  But again, the message, it
shouldn't be indented.

> ---
>  contrib/completion/git-completion.bash |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)

-- 
Shawn.

^ permalink raw reply

* Re: [ANNOUNCE] tig-0.13
From: bill lam @ 2009-01-15 15:41 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git
In-Reply-To: <20090115150841.GA23045@diku.dk>

On Thu, 15 Jan 2009, Jonas Fonseca wrote:
> Then I am puzzled why the configure script doesn't find it. Can you send
> me your config.log and the output of running configure. Maybe off-list.

I just delete everything and do a git reset --hard, ./configure does
detect ncursesw and link to it.  I'm not sure my previous error was
resulted from config cache.  I ldd tig and confirm indeed it linked to
the unicode version.  

Sorry for the noise.  m(__)m

-- 
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3
唐詩019 孟浩然  夏日南亭懷辛大
    山光忽西落  池月漸東上  散髮乘夜涼  開軒臥閑敞  荷風送香氣  竹露滴清響
    欲取鳴琴彈  恨無知音賞  感此懷故人  中宵勞夢想

^ permalink raw reply

* Re: [PATCH 1/2] Move run_hook() from builtin-commit.c into run-command.c (libgit)
From: Johannes Schindelin @ 2009-01-15 15:46 UTC (permalink / raw)
  To: Stephan Beyer
  Cc: git, Paolo Bonzini, Miklos Vajna, Shawn O. Pearce,
	Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <1232031618-5243-1-git-send-email-s-beyer@gmx.net>

Hi,

On Thu, 15 Jan 2009, Stephan Beyer wrote:

> 	Stripping out a libified version seemed better to me than
> 	copy and paste.

Oh, definitely.

> -	ret = start_command(&hook);
> -	if (ret) {
> -		warning("Could not spawn %s", argv[0]);
> -		return ret;
> -	}
> -	ret = finish_command(&hook);
> -	if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
> -		warning("%s exited due to uncaught signal", argv[0]);

What are the side effects of replacing this with "ret = 
run_command(&hook);"?  This has to be discussed and defended in the commit 
message.

> diff --git a/run-command.c b/run-command.c
> index c90cdc5..602fe85 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -342,3 +342,38 @@ int finish_async(struct async *async)
>  #endif
>  	return ret;
>  }
> +
> +int run_hook(const char *index_file, const char *name, ...)
> +{
> +	struct child_process hook;
> +	const char *argv[10], *env[2];
> +	char index[PATH_MAX];
> +	va_list args;
> +	int i;
> +
> +	va_start(args, name);
> +	argv[0] = git_path("hooks/%s", name);
> +	i = 0;
> +	do {
> +		if (++i >= ARRAY_SIZE(argv))
> +			die("run_hook(): too many arguments");
> +		argv[i] = va_arg(args, const char *);
> +	} while (argv[i]);
> +	va_end(args);
> +
> +	if (access(argv[0], X_OK) < 0)
> +		return 0;
> +
> +	memset(&hook, 0, sizeof(hook));
> +	hook.argv = argv;
> +	hook.no_stdin = 1;
> +	hook.stdout_to_stderr = 1;
> +	if (index_file) {
> +		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> +		env[0] = index;
> +		env[1] = NULL;
> +		hook.env = env;
> +	}
> +
> +	return run_command(&hook);
> +}

Lots of improvements possible (I agree; _after_ this patch):

- deuglify the loop,

- use ALLOC_GROW instead of having a fixed size argv,

- use an strbuf for the index file

- checking executability of argv[0] before filling argv,

and possibly others, too.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments
From: Ted Pavlic @ 2009-01-15 15:47 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20090115153543.GF10179@spearce.org>

> Yup, this and also 2/3 look fine.  But again, the message, it
> shouldn't be indented.

I will re-submit with no indentation.

Sorry for all the fuss. I've learned a great deal.

Thanks --
Ted


-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

^ permalink raw reply

* Re: [PATCH 2/2] api-run-command.txt: talk about run_hook()
From: Jakub Narebski @ 2009-01-15 15:49 UTC (permalink / raw)
  To: Stephan Beyer
  Cc: git, Paolo Bonzini, Miklos Vajna, Shawn O. Pearce,
	Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <1232031618-5243-2-git-send-email-s-beyer@gmx.net>

Stephan Beyer <s-beyer@gmx.net> writes:

> +`run_hook`::
> +
> +	Run a hook.
> +	The first argument is a string to an index file or NULL
> +	if the default index file or no index is used at all.

Errr...

        The first argument is a filename path to an index file,
        or NULL if hook uses default index file or no index is
        used at all. 

> +	The second argument is the name of the hook.

O.K.

> +	The further variable number (up to 9) of arguments correspond
> +	to the hook arguments.
> +	The last argument has to be NULL to terminate the variable
> +	arguments list.

Why the limitation of maximum of 9 hook arguments?

> +	If the hook is not executable, the return value is zero.

Or the hook does not exist, I assume.

> +	If it is executable, the hook will be executed and the exit
> +	status of the hook is returned.
> +	On execution, .stdout_to_stderr and .no_stdin will be set.
> +	(See below.)
> +
>  
>  Data structures
>  ---------------
> -- 
> 1.6.1.160.gecdb
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: SZEDER Gábor @ 2009-01-15 15:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Melchiorsen, git, Johannes.Schindelin
In-Reply-To: <7vfxjlxuu5.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 14, 2009 at 04:43:14PM -0800, Junio C Hamano wrote:
> I've always had trouble with the instruction we give for splitting one
> commit into two using the interactive rebase in the documentation, as it
> always had a strong "Huh?" effect on me when it suddenly starts talking
> about doing a "git reset HEAD^"; I suspect your change may improve this
> situation quite a bit.

I think we might want do differentiate editing a commit (modifying
either the commit message or the patch or both) or splitting a commit.

The first is served well with the current 'edit' rebase command IMHO.
I don't really see the point of the additional 'git reset --soft
HEAD^'.

 * If you want to edit the commit message only, then you are
   better off with 'git commit --amend', because it preserves the
   previous commit message.  But with 'git reset --soft HEAD^' and
   'git commit' the commit message is "lost"; you have to use 'git
   commit -c ORIG_HEAD' instead, which is not that straightforward
   (and we don't have completion support for it).

 * If you want to modify the patch, too, then you would have to use
   'git add' anyway, regardless of whether there was a 'git reset
   --soft HEAD^', or not.  The only benefit of that 'reset' I'm seeing
   is that in that case 'git diff --cached' would show all the changes
   that would be committed; without the 'reset' 'git diff --cached
   HEAD^' is needed.

   But I'm not sure whether that benefit would offset the confusion of
   one more rebase command with just slightly different meaning.

For the second we could introduce a new rebase command like 'split',
which would do the same as 'edit' but would also perform that 'git
reset HEAD^' mentioned in the documentation automatically.  Or perhaps
it could be called 'divide', since the 's' abbreviation for 'split' is
already taken by 'squash'.  (Or maybe use capital 'S' for 'split'?
might be confusing...)


Regards,
Gábor

^ permalink raw reply

* 'mail' link on http://repo.or.cz/w/git.git no workee?
From: Johannes Schindelin @ 2009-01-15 15:53 UTC (permalink / raw)
  To: pasky, git

Hi Pasky,

sorry to bother you between two games of Go; Could you have a look at the 
'mail' link with the commit "Update 1.6.2 draft release notes"?  It is not 
working for me...

That is, it links to marc (not gmane?) but finds no matches...

Ciao,
Dscho

^ permalink raw reply

* [PATCH noindent 3/3] bash-completion: Added comments to remind about required arguments
From: ted @ 2009-01-15 16:02 UTC (permalink / raw)
  To: spearce; +Cc: git, gitster, Ted Pavlic
In-Reply-To: <1232035343-10544-2-git-send-email-ted@tedpavlic.com>

From: Ted Pavlic <ted@tedpavlic.com>

Adds a few simple comments above commands that take arguments. These
comments are meant to remind developers of potential problems that can
occur when the script is sourced on systems with "set -u." Any
function which "requires" arguments really ought to be called with
explicit arguments given.

Also adds a #!bash to the top of bash completions so that editing
software can always identify that the file is of sh type.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---

This patch series is identical to the last, but the commit message has
been stripped of its indentation (by request).

 contrib/completion/git-completion.bash |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 201f9a6..f8b845a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,3 +1,4 @@
+#!bash
 #
 # bash completion support for core Git.
 #
@@ -50,6 +51,8 @@ case "$COMP_WORDBREAKS" in
 *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
 esac
 
+# __gitdir accepts 0 or 1 arguments (i.e., location)
+# returns location of .git repo
 __gitdir ()
 {
 	if [ -z "${1-}" ]; then
@@ -67,6 +70,8 @@ __gitdir ()
 	fi
 }
 
+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
+# returns text to add to bash PS1 prompt (includes branch name)
 __git_ps1 ()
 {
 	local g="$(git rev-parse --git-dir 2>/dev/null)"
@@ -119,6 +124,7 @@ __git_ps1 ()
 	fi
 }
 
+# __gitcomp_1 requires 2 arguments
 __gitcomp_1 ()
 {
 	local c IFS=' '$'\t'$'\n'
@@ -131,6 +137,8 @@ __gitcomp_1 ()
 	done
 }
 
+# __gitcomp accepts 1, 2, 3, or 4 arguments
+# generates completion reply with compgen
 __gitcomp ()
 {
 	local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -150,6 +158,7 @@ __gitcomp ()
 	esac
 }
 
+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
 	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -168,6 +177,7 @@ __git_heads ()
 	done
 }
 
+# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
 __git_tags ()
 {
 	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -186,6 +196,7 @@ __git_tags ()
 	done
 }
 
+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
 __git_refs ()
 {
 	local i is_hash=y dir="$(__gitdir "${1-}")"
@@ -218,6 +229,7 @@ __git_refs ()
 	done
 }
 
+# __git_refs2 requires 1 argument (to pass to __git_refs)
 __git_refs2 ()
 {
 	local i
@@ -226,6 +238,7 @@ __git_refs2 ()
 	done
 }
 
+# __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
 	local cmd i is_hash=y
@@ -470,6 +483,7 @@ __git_aliases ()
 	done
 }
 
+# __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
 	local word cmdline=$(git --git-dir="$(__gitdir)" \
@@ -482,6 +496,7 @@ __git_aliased_command ()
 	done
 }
 
+# __git_find_subcommand requires 1 argument
 __git_find_subcommand ()
 {
 	local word subcommand c=1
-- 
1.6.1.87.g15624

^ permalink raw reply related

* [PATCH noindent 1/3] bash-completion: Support running when set -u is enabled
From: ted @ 2009-01-15 16:02 UTC (permalink / raw)
  To: spearce; +Cc: git, gitster, Ted Pavlic

From: Ted Pavlic <ted@tedpavlic.com>

Under "set -u" semantics, it is an error to access undefined variables.
Some user environments may enable this setting in the interactive shell.

In any context where the completion functions access an undefined
variable, accessing a default empty string (aka "${1-}" instead of "$1")
is a reasonable way to code the function, as it silences the undefined
variable error while still supplying an empty string.

In this patch, functions that should always take an argument still use
$1. Functions that have optional arguments use ${1-}.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---

This patch series is identical to the last, but the commit message has
been stripped of its indentation (by request).

 contrib/completion/git-completion.bash |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7b074d7..5d1515c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -52,7 +52,7 @@ esac
 
 __gitdir ()
 {
-	if [ -z "$1" ]; then
+	if [ -z "${1-}" ]; then
 		if [ -n "$__git_dir" ]; then
 			echo "$__git_dir"
 		elif [ -d .git ]; then
@@ -111,7 +111,7 @@ __git_ps1 ()
 			fi
 		fi
 
-		if [ -n "$1" ]; then
+		if [ -n "${1-}" ]; then
 			printf "$1" "${b##refs/heads/}$r"
 		else
 			printf " (%s)" "${b##refs/heads/}$r"
@@ -143,8 +143,8 @@ __gitcomp ()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "$2" \
-			-W "$(__gitcomp_1 "$1" "$4")" \
+		COMPREPLY=($(compgen -P "${2-}" \
+			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
 			-- "$cur"))
 		;;
 	esac
@@ -152,13 +152,13 @@ __gitcomp ()
 
 __git_heads ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "$1")"
+	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/heads
 		return
 	fi
-	for i in $(git ls-remote "$1" 2>/dev/null); do
+	for i in $(git ls-remote "${1-}" 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
@@ -170,13 +170,13 @@ __git_heads ()
 
 __git_tags ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "$1")"
+	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/tags
 		return
 	fi
-	for i in $(git ls-remote "$1" 2>/dev/null); do
+	for i in $(git ls-remote "${1-}" 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
@@ -188,7 +188,7 @@ __git_tags ()
 
 __git_refs ()
 {
-	local i is_hash=y dir="$(__gitdir "$1")"
+	local i is_hash=y dir="$(__gitdir "${1-}")"
 	local cur="${COMP_WORDS[COMP_CWORD]}" format refs
 	if [ -d "$dir" ]; then
 		case "$cur" in
-- 
1.6.1.87.g15624

^ permalink raw reply related

* [PATCH noindent 2/3] bash-completion: Try bash completions before simple filetype
From: ted @ 2009-01-15 16:02 UTC (permalink / raw)
  To: spearce; +Cc: git, gitster, Ted Pavlic
In-Reply-To: <1232035343-10544-1-git-send-email-ted@tedpavlic.com>

From: Ted Pavlic <ted@tedpavlic.com>

When a git completion is not found, a bash shell should try bash-type
completions first before going to standard filetype completions. This
patch /adds/ "-o bashdefault" to the completion line. If that option is
not available, it uses the old method.

This behavior was inspired by Mercurial's bash completion script.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---

This patch series is identical to the last, but the commit message has
been stripped of its indentation (by request).

 contrib/completion/git-completion.bash |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5d1515c..201f9a6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1766,13 +1766,16 @@ _gitk ()
 	__git_complete_revlist
 }
 
-complete -o default -o nospace -F _git git
-complete -o default -o nospace -F _gitk gitk
+complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
+	|| complete -o default -o nospace -F _git git
+complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
+	|| complete -o default -o nospace -F _gitk gitk
 
 # The following are necessary only for Cygwin, and only are needed
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o default -o nospace -F _git git.exe
+complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
+	|| complete -o default -o nospace -F _git git.exe
 fi
-- 
1.6.1.87.g15624

^ permalink raw reply related

* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:04 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <496F4BF0.6020805@drmicha.warpmail.net>

Hi,

On Thu, 15 Jan 2009, Michael J Gruber wrote:

> I'm not sure what -p is supposed to do:
> 
> A) Should it preserve all merge commits which it would need to rewrite?
> That is lot to ask. Previous behaviour (intended or not) seemed to be to
> do nothing in this case where the merge connects master and work.
> 
> B) Should it preserve only merges in side branches? I seem to mean by
> that branches where the parents are on work and other branches but not
> on master.

The intention was this:

	$ git rebase -p master

would need to rewrite _all_ commits that are in "master..".  All of them, 
including the merge commits.

The fact that I implemented it as "-i -p" is only due to technical 
reasons; I know (ahem, now I should put that into the past tense) the code 
base pretty well.

An additional shortcut was to avoid rewriting commits when they did not 
need rewriting.  IOW if the commit-to-pick has only parents that were 
_not_ rewritten, we can avoid cherry-picking or merging, and just reset 
--hard <original commit>.

There was a problem, though; for some reason, the code as I did it fscked 
up the order of the commits when -p was specified.  Therefore, rewritten 
commits had the wrong parents.

I thought it should be easy to fix, but then I got a job that I actually 
like, so my Git time budget was tremendously reduced.

> > The more I think about it, I think it's possible I broke it with the 
> > introduction of the "noop".
> 
> It certainly worked after the noop introduction before the r-i-p series, 
> but not any more after.

Umm... which rebase -i -p series do you mean?  "noop" was introduced 
pretty recently if my Alzheimered brain does not fool me.

Ciao,
Dscho

^ permalink raw reply


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