Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Make git-clean a builtin
From: Johannes Sixt @ 2007-10-08  6:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Linus Torvalds, Shawn Bohrer, git, gitster
In-Reply-To: <20071008022205.GA21277@coredump.intra.peff.net>

Jeff King schrieb:
> On Sun, Oct 07, 2007 at 07:17:50PM -0700, Linus Torvalds wrote:
> 
>> fchdir() is not portable.
> 
> I was using the "even Solaris has it!" test, but yes, it's not POSIX. I
> don't know how common it actually is (for curiosity's sake, do you know
> of a common platform that lacks it?).

Windows. ;)

-- Hannes

^ permalink raw reply

* Re: Trying to use git-filter-branch to compress history by removing large, obsolete binary files
From: Johannes Sixt @ 2007-10-08  6:22 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: J. Bruce Fields, Elijah Newren, Frank Lichtenheld, git
In-Reply-To: <Pine.LNX.4.64.0710080204140.4174@racer.site>

Johannes Schindelin schrieb:
> The rationale was this: filter-branch recently learnt how to rewrite many 
> branches, and it might be tedious to find out which ones.  But then, there 
> is git log --no-walk --all, so maybe I really should get rid of 
> refs/original/*?
> 
> I'd like to have some comments from the heavier filter-branch users on 
> that...

IMHO, a backup of the original refs is needed. However, it may be wise to 
store them in the refs/heads namespace so that 'git branch -d' can delete 
them and 'git branch -m' can move them back if something went wrong.

-- Hannes

^ permalink raw reply

* Re: Trying to use git-filter-branch to compress history by removing large, obsolete binary files
From: Alex Riesen @ 2007-10-08  6:15 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Frank Lichtenheld, git
In-Reply-To: <51419b2c0710071709s2f797df0u986447f5455f306d@mail.gmail.com>

Elijah Newren, Mon, Oct 08, 2007 02:09:50 +0200:
> On 10/7/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> > you missed something. Your example compresses to about 124k.
> 
> What version of git are you running?  I reran all the steps to which

git version 1.5.3.4.225.g31b973 (irrelevant custom modifications)

> you responded (repeated below for clarity) with git-1.5.3.3 and still
> get 11MB.  Also, you must have different filesystem extents than me
> since an empty git repo takes 196k here[1], so I don't think any repo
> is going to get down to 124k.

it is ext3. I do not install the hooks (~8k apparent, ~32k fs blocks)
and never activate logs by default.

> # Use vi to remove the line referring to refs/original...
> git reflog expire --all

another part of the suggestion re reflogs was to look into the logs,
to check if expire actually removed anything. It seems to have been
the culprit.

^ permalink raw reply

* Re: [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.
From: Johannes Schindelin @ 2007-10-08  5:36 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git
In-Reply-To: <200710080734.23878.chriscool@tuxfamily.org>

Hi,

On Mon, 8 Oct 2007, Christian Couder wrote:

> Le lundi 8 octobre 2007, Johannes Schindelin a ?crit :
>
> > On Mon, 8 Oct 2007, Christian Couder wrote:
> > > diff --git a/git-bisect.sh b/git-bisect.sh
> > > index 388887a..c556318 100755
> > > --- a/git-bisect.sh
> > > +++ b/git-bisect.sh
> > > @@ -143,7 +145,7 @@ bisect_write_bad() {
> > >
> > >  bisect_good() {
> > >  	bisect_autostart
> > > -        case "$#" in
> > > +	case "$#" in
> >
> > White space breakage.
> 
> The patch tries to fix some white space breakages.
> 
> > > @@ -153,7 +155,6 @@ bisect_good() {
> > >  		rev=$(git rev-parse --verify "$rev^{commit}") || exit
> > >  		bisect_write_good "$rev"
> > >  		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
> > > -
> >
> > ?
> 
> It also removes this unneeded blank line.

Both laudable changes; alas, they distracted me.

> > > @@ -164,6 +165,28 @@ bisect_write_good() {
> > >  	echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
> > >  }
> > >
> > > +bisect_dunno() {
> > > +	bisect_autostart
> > > +	case "$#" in
> > > +	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
> > > +	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
> > > +		test '' != "$revs" || die "Bad rev input: $@" ;;
> > > +	esac
> > > +	for rev in $revs
> > > +	do
> > > +		rev=$(git rev-parse --verify "$rev^{commit}") || exit
> > > +		bisect_write_dunno "$rev"
> > > +		echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"
> >
> > Should the last line not be put into bisect_write_dunno?  OTOH this is
> > the only call site of that function, so I strongly doubt that the
> > function (consisting of 3 lines, where the first is 'rev="$1"') is
> > necessary at all.
> 
> Well, there are "bisect_write_bad" and "bisect_write_good" that already 
> do the same thing as "bisect_write_dunno". In fact I thought that it was 
> better to just copy "bisect_dunno" from "bisect_good" and 
> "bisect_write_dunno" from "bisect_write_good".

If they also are called by just one site, and also do not do the complete 
printing to the log in the function (but also in the caller), I think they 
are not really worth it, either.

> If needed I can send another patch to factorise these functions.

That's not up to me to decide.  I'm just saying what I dislike.

Please do not take my criticism as a sign of a personal attack; if I did 
not find your patch worthwhile, I would not bother to respond.  So in a 
way, it is my way to show my appreciation for your work that I review and 
criticize it; for efficiency, I do not mention what I like ;-)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.
From: Christian Couder @ 2007-10-08  5:34 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio Hamano, git
In-Reply-To: <Pine.LNX.4.64.0710080444290.4174@racer.site>

Le lundi 8 octobre 2007, Johannes Schindelin a écrit :
> Hi,
>
> On Mon, 8 Oct 2007, Christian Couder wrote:
> > diff --git a/git-bisect.sh b/git-bisect.sh
> > index 388887a..c556318 100755
> > --- a/git-bisect.sh
> > +++ b/git-bisect.sh
> > @@ -143,7 +145,7 @@ bisect_write_bad() {
> >
> >  bisect_good() {
> >  	bisect_autostart
> > -        case "$#" in
> > +	case "$#" in
>
> White space breakage.

The patch tries to fix some white space breakages.

> > @@ -153,7 +155,6 @@ bisect_good() {
> >  		rev=$(git rev-parse --verify "$rev^{commit}") || exit
> >  		bisect_write_good "$rev"
> >  		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
> > -
>
> ?

It also removes this unneeded blank line.

> > @@ -164,6 +165,28 @@ bisect_write_good() {
> >  	echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
> >  }
> >
> > +bisect_dunno() {
> > +	bisect_autostart
> > +	case "$#" in
> > +	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
> > +	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
> > +		test '' != "$revs" || die "Bad rev input: $@" ;;
> > +	esac
> > +	for rev in $revs
> > +	do
> > +		rev=$(git rev-parse --verify "$rev^{commit}") || exit
> > +		bisect_write_dunno "$rev"
> > +		echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"
>
> Should the last line not be put into bisect_write_dunno?  OTOH this is
> the only call site of that function, so I strongly doubt that the
> function (consisting of 3 lines, where the first is 'rev="$1"') is
> necessary at all.

Well, there are "bisect_write_bad" and "bisect_write_good" that already do 
the same thing as "bisect_write_dunno". In fact I thought that it was 
better to just copy "bisect_dunno" from "bisect_good" 
and "bisect_write_dunno" from "bisect_write_good".

If needed I can send another patch to factorise these functions.

> > @@ -206,17 +229,104 @@ bisect_auto_next() {
> >  	bisect_next_check && bisect_next || :
> >  }
> >
> > +search_dunno() {
> > +	_hash="$1"
> > +	_dunno="$2"
> > +
> > +	for _val in $_dunno ; do
> > +		case $_hash in $_val) return 1 ;; esac
> > +	done
>
> This would be faster as
>
> 	case " $1" in " $2") return 1 ;; esac
>
> I guess.

I will try your suggestion and send an updated patch. Thanks.

> But as I said in the other reply, I think this logic belongs into the C
> core, instead of generating mostly useless information, passing it down
> to the script, and filtering it out again.

Yeah, it's not efficient.

Best regards,
Christian.

^ permalink raw reply

* Re: [PATCH 1/2] rev-list: implement --bisect-all
From: Johannes Schindelin @ 2007-10-08  5:08 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git
In-Reply-To: <200710080708.46579.chriscool@tuxfamily.org>

Hi,

On Mon, 8 Oct 2007, Christian Couder wrote:

> > IMHO such a logic does not belong into a shell script, but into the C 
> > core.
> 
> There is a lot of the bisect logic implemented in git-bisect.sh already. 
> The long term plan is to rewrite it in C,

Oh?  Did I miss something?

> but I am not sure that the "dunno" logic should be the first part of it 
> to be done in C.

The thing is, git-bisect is porcelain-ish.  And by having a lot of the 
functionality there, which is not really porcelain, but plumbing, you 
prevent other porcelains, such as git-gui or qgit, from using that 
function.

Which is bad.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-10-07 23:57 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, frank, gitster, Shawn Bohrer

This replaces git-clean.sh with builtin-clean.c, and moves git-clean.sh to the
examples.

Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---

Rewritten to use remove_directory_recursively() per Dscho's suggestion.  Patch
is now based ontop of 'next'. 

 Makefile                                      |    3 +-
 builtin-clean.c                               |  161 +++++++++++++++++++++++++
 builtin.h                                     |    1 +
 git-clean.sh => contrib/examples/git-clean.sh |    0 
 git.c                                         |    1 +
 5 files changed, 165 insertions(+), 1 deletions(-)
 create mode 100644 builtin-clean.c
 rename git-clean.sh => contrib/examples/git-clean.sh (100%)

diff --git a/Makefile b/Makefile
index 62bdac6..bed4c78 100644
--- a/Makefile
+++ b/Makefile
@@ -206,7 +206,7 @@ BASIC_LDFLAGS =
 
 SCRIPT_SH = \
 	git-bisect.sh git-checkout.sh \
-	git-clean.sh git-clone.sh git-commit.sh \
+	git-clone.sh git-commit.sh \
 	git-ls-remote.sh \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
@@ -324,6 +324,7 @@ BUILTIN_OBJS = \
 	builtin-check-attr.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
+	builtin-clean.o \
 	builtin-commit-tree.o \
 	builtin-count-objects.o \
 	builtin-describe.o \
diff --git a/builtin-clean.c b/builtin-clean.c
new file mode 100644
index 0000000..af61de0
--- /dev/null
+++ b/builtin-clean.c
@@ -0,0 +1,161 @@
+/*
+ * "git clean" builtin command
+ *
+ * Copyright (C) 2007 Shawn Bohrer
+ *
+ * Based on git-clean.sh by Pavel Roskin
+ */
+
+#include "builtin.h"
+#include "cache.h"
+#include "run-command.h"
+#include "dir.h"
+
+static int disabled = 0;
+static int show_only = 0;
+static int remove_directories = 0;
+static int quiet = 0;
+static int ignored = 0;
+static int ignored_only = 0;
+
+static const char builtin_clean_usage[] =
+"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] <paths>...";
+
+static int git_clean_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "clean.requireforce")) {
+		disabled = git_config_bool(var, value);
+	}
+	return 0;
+}
+
+int cmd_clean(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	int j;
+	struct child_process cmd;
+	const char **argv_ls_files;
+	char *buf = NULL;
+	char path[1024];
+	FILE *cmd_fout;
+	struct strbuf dir;
+
+	git_config(git_clean_config);
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (arg[0] != '-')
+			break;
+		if (!strcmp(arg, "--")) {
+			i++;
+			break;
+		}
+		if (!strcmp(arg, "-n")) {
+			show_only = 1;
+			disabled = 0;
+			continue;
+		}
+		if (!strcmp(arg, "-f")) {
+			disabled = 0;
+			continue;
+		}
+		if (!strcmp(arg, "-d")) {
+			remove_directories = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-q")) {
+			quiet = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-x")) {
+			ignored = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-X")) {
+			ignored_only = 1;
+			continue;
+		}
+		usage(builtin_clean_usage);
+	}
+
+	if (ignored && ignored_only)
+		usage(builtin_clean_usage);
+
+	if (disabled) {
+		die("clean.requireForce set and -n or -f not given; refusing to clean");
+	}
+
+	/* Paths (argc - i) + 8 (Possible arguments)*/
+	argv_ls_files = xmalloc((argc - i + 8) * sizeof(const char *));
+	argv_ls_files[0] = "ls-files";
+	argv_ls_files[1] = "--others";
+	argv_ls_files[2] = "--directory";
+	j = 3;
+	if (!ignored) {
+		argv_ls_files[j++] = "--exclude-per-directory=.gitignore";
+		if (ignored_only)
+			argv_ls_files[j++] = "--ignored";
+		if (!access(git_path("info/exclude"), F_OK)) {
+			char *exclude_path = git_path("info/exclude");
+			int len = strlen(exclude_path);
+			buf = (char*)malloc(len+16);
+			sprintf(buf, "--exclude-from=%s", exclude_path);
+			argv_ls_files[j++] = buf;
+		}
+	}
+	argv_ls_files[j++] = "--";
+	/* Add remaining paths passed in as arguments */
+	if (argc - i)
+		memcpy(argv_ls_files + j++, argv + i, (argc - i) * sizeof(const char *));
+	argv_ls_files[j + argc - i] = NULL;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.argv = argv_ls_files;
+	cmd.git_cmd = 1;
+	cmd.out = -1;
+	if (start_command(&cmd))
+		die("Could not run sub-command: git ls-files");
+
+	strbuf_init(&dir, 0);
+	cmd_fout = fdopen(cmd.out, "r");
+	while (fgets(path, sizeof(path), cmd_fout) != NULL) {
+		struct stat st;
+		char *p;
+		p = strrchr(path, '\n');
+		if ( p != NULL )
+			*p = '\0';
+		if (!lstat(path, &st) && (S_ISDIR(st.st_mode))) {
+			strbuf_addstr(&dir, path);
+			if (show_only && remove_directories) {
+				printf("Would remove %s\n", dir.buf);
+			} else if (quiet && remove_directories) {
+				remove_dir_recursively(&dir, 0);
+			} else if (remove_directories) {
+				printf("Removing %s\n", path);
+				remove_dir_recursively(&dir, 0);
+			} else if (show_only) {
+				printf("Would not remove %s\n", dir.buf);
+			} else {
+				printf("Not removing %s\n", dir.buf);
+			}
+			strbuf_reset(&dir);
+		} else {
+			if (show_only) {
+				printf("Would remove %s\n", path);
+				continue;
+			} else if (!quiet) {
+				printf("Removing %s\n", path);
+			}
+			unlink(path);
+		}
+	}
+
+	strbuf_release(&dir);
+	fclose(cmd_fout);
+	finish_command(&cmd);
+	if (buf != NULL)
+		free(buf);
+	free(argv_ls_files);
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 65cc0fb..cdefdc0 100644
--- a/builtin.h
+++ b/builtin.h
@@ -23,6 +23,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
+extern int cmd_clean(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
diff --git a/git-clean.sh b/contrib/examples/git-clean.sh
similarity index 100%
rename from git-clean.sh
rename to contrib/examples/git-clean.sh
diff --git a/git.c b/git.c
index d7c6bca..4e39169 100644
--- a/git.c
+++ b/git.c
@@ -320,6 +320,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: [PATCH 1/2] rev-list: implement --bisect-all
From: Christian Couder @ 2007-10-08  5:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio Hamano, git
In-Reply-To: <Pine.LNX.4.64.0710080442420.4174@racer.site>

Hi Dscho,

Le lundi 8 octobre 2007, Johannes Schindelin a écrit :
> Hi,
>
> On Mon, 8 Oct 2007, Christian Couder wrote:
> > This option makes it possible to see all the potential bisection
> > points. The best ones are displayed first.
>
> Would it not be better to pass --bisect-vars a list of commits that we do
> not want to see?  It could then mark them as "DUNNO", and still just
> output a single commit.

The problem is that after --bisect-vars we already pass some "good" and then 
a bad commit. So we have to use another flag like --bisect-dunno and put 
the dunno commits after this one.

Then there is the problem that revision.c reorders arguments and doesn't 
know about "--bisect-*" flags. It is also used by a lot of other commands.

After struggling with revision.c for some time, I thought it would be 
simpler and safer to come up first with something in shell.

Also please note that in some cases we cannot just output a single commit, 
because we just dunno which commit is the first bad one.

> IMHO such a logic does not belong into a shell script, but into the C
> core.

There is a lot of the bisect logic implemented in git-bisect.sh already. The 
long term plan is to rewrite it in C, but I am not sure that the "dunno" 
logic should be the first part of it to be done in C.

Also I thought it was still fine to prototype new features in shell.

Best regards,
Christian.

^ permalink raw reply

* Re: [PATCH] git-shell and git-cvsserver
From: Johannes Schindelin @ 2007-10-08  4:51 UTC (permalink / raw)
  To: Jan Wielemaker; +Cc: Git Mailing List
In-Reply-To: <200710051453.47622.wielemak@science.uva.nl>

Hi,

On Fri, 5 Oct 2007, Jan Wielemaker wrote:

> Hi,
> 
> I know, I shouldn't be using git-cvsserver :-( Anyway, I patched
> git-shell to start git-cvsserver if it is started interactively and the
> one and only line given to it is "cvs server".
> 
> The patch to shell.c is below. The trick with the EXEC_PATH is needed
> because git-cvsserver doesn't appear to be working if you do not include
> the git bindir in $PATH. I think that should be fixed in git-cvsserver
> and otherwise we should at least make the value come from the prefix
> make variable.  With this patch I was able to use both Unix and Windows
> cvs clients using git-shell as login shell.
> 
> Note that you must provide ~/.gitconfig with user and email in the
> restricted environment.
> 
> 	Enjoy --- Jan

I think this is a valuable contribution.  That's why I comment...

Please put a useful commit message (less like an email, more like 
something you want to read in git-log) at the beginning of the email, then 
a line containing _just_ "---", and after that some comments that are not 
meant to be stored in the history, like (I know this does not belong 
to...)

After that, there should be a diffstat, and then the patch.

The easiest to have this layout is to do a proper commit in git, use "git 
format-patch" to produce the patch, and then insert what you want to say 
in addition to the commit message between the "---" marker and the 
diffstat.

I strongly disagree (as you yourself, probably) with the notion that this 
does not belong into git-shell.


> +#define EXEC_PATH "/usr/local/bin"

This is definitely wrong.  Use git_exec_path() instead.

> +static int do_cvs_cmd(const char *me, char *arg)
> +{
> +	const char *my_argv[4];

Maybe rename this to cvsserver_args?

> +	const char *oldpath;
> +
> +	if ( !arg )
> +		die("no argument");
> +	if ( strcmp(arg, "server") )
> +		die("only allows git-cvsserver server: %s", arg);
> +
> +	my_argv[0] = "cvsserver";
> +	my_argv[1] = "server";
> +	my_argv[2] = NULL;
> +
> +	if ( (oldpath=getenv("PATH")) ) {

Please lose the spaces after the opening and before the closing brackets.  
And put spaces around the "=" sign.

It is really distracting to read different styles of code in the same 
project, and that's why we're pretty anal about coding styles.  Just have 
a look (in the same file) how we write things, and imitate it as closely 
as possible.

> +		char *newpath = malloc(strlen(oldpath)+strlen(EXEC_PATH)+5+1+1); > +		
> +		sprintf(newpath, "PATH=%s:%s", EXEC_PATH, oldpath);
> +		putenv(newpath);
> +	} else {
> +		char *newpath = malloc(strlen(EXEC_PATH)+5+1);
> +		
> +		sprintf(newpath, "PATH=%s", EXEC_PATH);
> +		putenv(newpath);
> +	}

You have redundant "putenv(newpath);" in both clauses.  AFAICT putenv() is 
deprecated, too, and we use setenv() elsewhere.

In addition, I strongly suggest using strbuf:

	struct strbuf newpath = STRBUF_INIT;

	strbuf_addstr(&newpath, git_exec_path());
	if ((oldpath = getenv("PATH"))) {
		strbuf_addch(&newpath, ':');
		strbuf_addstr(&newpath, oldpath);
	}

	setenv("PATH", strbuf_detach(&newpath, NULL), 1);

> +	return execv_git_cmd(my_argv);

... and then you call execv_git_cmd(), which already does all the details 
of setting up the exec dir correctly AFAIR.

>  int main(int argc, char **argv)
>  {
>  	char *prog;
> +	char buf[256];
>  	struct commands *cmd;
>  
>  	/* We want to see "-c cmd args", and nothing else */
> -	if (argc != 3 || strcmp(argv[1], "-c"))
> -		die("What do you think I am? A shell?");
> +	if (argc == 1) {
> +		if (fgets(buf, sizeof(buf)-1, stdin)) {
> +			char *end;
> +
> +			if ( (end=strchr(buf, '\n')) )
> +			{	while(end>buf && end[-1] <= ' ')
> +					end--;
> +				*end = '\0';
> +			} else {
> +				die("Bad command");
> +			}
> +
> +			prog = buf;
> +		} else {
> +			die("No command");
> +		}
> +	} else {
> +		if (argc != 3 || strcmp(argv[1], "-c"))
> +			die("What do you think I am? A shell?");
> +
> +		prog = argv[2];
> +		argv += 2;
> +		argc -= 2;
> +	}

And this is ugly.  If you want to support "cvs server", then just check 
for that string, and if it matches, return execl_git_cmd("cvsserver");

Otherwise proceed as in the original code.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Remove duplicate ref matches in fetch
From: Daniel Barkalow @ 2007-10-08  4:25 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git, Junio C Hamano, Johannes Schindelin

If multiple refspecs matched the same ref, the update would be
processed multiple times. Now having the same destination for the same
source has no additional effect, and having the same destination for
different sources is an error.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
I tested this with the usual tests, but (a) they don't include this case 
(yet), and (b) they passed before I fixed a bug. So this should get some 
good testing, and probably by somebody with valgrind --leak-check to make 
sure I'm deallocating things correctly.

 builtin-fetch.c |    1 +
 remote.c        |   27 +++++++++++++++++++++++++++
 remote.h        |    5 +++++
 3 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index cf7498b..caaca63 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -112,6 +112,7 @@ static struct ref *get_ref_map(struct transport *transport,
 			ref_map->merge = 1;
 		}
 	}
+	ref_remove_duplicates(ref_map);
 
 	return ref_map;
 }
diff --git a/remote.c b/remote.c
index e7d735b..e2ca4d3 100644
--- a/remote.c
+++ b/remote.c
@@ -380,6 +380,33 @@ int for_each_remote(each_remote_fn fn, void *priv)
 	return result;
 }
 
+void ref_remove_duplicates(struct ref *ref_map)
+{
+	struct ref **posn;
+	struct ref *next;
+	for (; ref_map; ref_map = ref_map->next) {
+		if (!ref_map->peer_ref)
+			continue;
+		posn = &ref_map->next;
+		while (*posn) {
+			if ((*posn)->peer_ref &&
+			    !strcmp((*posn)->peer_ref->name,
+				    ref_map->peer_ref->name)) {
+				if (strcmp((*posn)->name, ref_map->name))
+					die("%s tracks both %s and %s",
+					    ref_map->peer_ref->name,
+					    (*posn)->name, ref_map->name);
+				next = (*posn)->next;
+				free((*posn)->peer_ref);
+				free(*posn);
+				*posn = next;
+			} else {
+				posn = &(*posn)->next;
+			}
+		}
+	}
+}
+
 int remote_has_url(struct remote *remote, const char *url)
 {
 	int i;
diff --git a/remote.h b/remote.h
index 05add06..c62636d 100644
--- a/remote.h
+++ b/remote.h
@@ -49,6 +49,11 @@ struct ref *alloc_ref(unsigned namelen);
  */
 void free_refs(struct ref *ref);
 
+/*
+ * Removes and frees any duplicate refs in the map.
+ */
+void ref_remove_duplicates(struct ref *ref_map);
+
 struct refspec *parse_ref_spec(int nr_refspec, const char **refspec);
 
 int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
-- 
1.5.3.4.1155.gfe96ee

^ permalink raw reply related

* Re: [PATCH] Make git-clean a builtin
From: Johannes Schindelin @ 2007-10-08  3:57 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: git, frank, gitster
In-Reply-To: <11918014664038-git-send-email-shawn.bohrer@gmail.com>

Hi,

On Sun, 7 Oct 2007, Shawn Bohrer wrote:

> +	if (ignored && ignored_only)
> +		usage(builtin_clean_usage);

Maybe a helpful message in that case, too?  (It is not apparent from the 
usage what the user has done wrong here.)

> +	if (disabled) {
> +		die("clean.requireForce set and -n or -f not given; refusing to clean");
> +	}

Please lose the curly brackets here.  They are absolutely useless, and the 
rest of the git source code avoids unnecessary curly brackets.

> +	/* Paths (argc - i) + 8 (Possible arguments)*/
> +	argv_ls_files = xmalloc((argc - i + 8) * sizeof(const char *));
> +	argv_ls_files[0] = "ls-files";
> +	argv_ls_files[1] = "--others";
> +	argv_ls_files[2] = "--directory";
> [...]

As Linus already noted, it is probably easy, and much more efficient, to 
call read_directory() here.  The best example how to use 
read_directory() is... builtin-ls-files.c.

> diff --git a/git.c b/git.c
> index d7c6bca..4e39169 100644
> --- a/git.c
> +++ b/git.c
> @@ -320,6 +320,7 @@ static void handle_internal_command(int argc, const char **argv)
>  		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
>  		{ "cherry", cmd_cherry, RUN_SETUP },
>  		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
> +		{ "clean", cmd_clean, RUN_SETUP },

You definitely want to have NEED_WORK_TREE here, too.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.
From: Johannes Schindelin @ 2007-10-08  3:49 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git
In-Reply-To: <20071008053450.a52d7c5e.chriscool@tuxfamily.org>

Hi,

On Mon, 8 Oct 2007, Christian Couder wrote:

> diff --git a/git-bisect.sh b/git-bisect.sh
> index 388887a..c556318 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -143,7 +145,7 @@ bisect_write_bad() {
>  
>  bisect_good() {
>  	bisect_autostart
> -        case "$#" in
> +	case "$#" in

White space breakage.

> @@ -153,7 +155,6 @@ bisect_good() {
>  		rev=$(git rev-parse --verify "$rev^{commit}") || exit
>  		bisect_write_good "$rev"
>  		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
> -

?

> @@ -164,6 +165,28 @@ bisect_write_good() {
>  	echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
>  }
>  
> +bisect_dunno() {
> +	bisect_autostart
> +	case "$#" in
> +	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
> +	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
> +		test '' != "$revs" || die "Bad rev input: $@" ;;
> +	esac
> +	for rev in $revs
> +	do
> +		rev=$(git rev-parse --verify "$rev^{commit}") || exit
> +		bisect_write_dunno "$rev"
> +		echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"

Should the last line not be put into bisect_write_dunno?  OTOH this is the 
only call site of that function, so I strongly doubt that the function 
(consisting of 3 lines, where the first is 'rev="$1"') is necessary at 
all.

> @@ -206,17 +229,104 @@ bisect_auto_next() {
>  	bisect_next_check && bisect_next || :
>  }
>  
> +search_dunno() {
> +	_hash="$1"
> +	_dunno="$2"
> +
> +	for _val in $_dunno ; do
> +		case $_hash in $_val) return 1 ;; esac
> +	done

This would be faster as

	case " $1" in " $2") return 1 ;; esac

I guess.

But as I said in the other reply, I think this logic belongs into the C 
core, instead of generating mostly useless information, passing it down to 
the script, and filtering it out again.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] rev-list: implement --bisect-all
From: Johannes Schindelin @ 2007-10-08  3:44 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio Hamano, git
In-Reply-To: <20071008053438.6a829b91.chriscool@tuxfamily.org>

Hi,

On Mon, 8 Oct 2007, Christian Couder wrote:

> This option makes it possible to see all the potential bisection points. 
> The best ones are displayed first.

Would it not be better to pass --bisect-vars a list of commits that we do 
not want to see?  It could then mark them as "DUNNO", and still just 
output a single commit.

IMHO such a logic does not belong into a shell script, but into the C 
core.

Ciao,
Dscho

^ permalink raw reply

* [PATCH 2/2] Bisect: implement "bisect dunno" to mark untestable revisions.
From: Christian Couder @ 2007-10-08  3:34 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git

When there are some dunno revisions, we add the '--bisect-all'
option to "git rev-list --bisect-vars". Then we filter out the
dunno revisions from the result of the rev-list command, and we
modify the "bisect_rev" var accordingly.

We don't always use "--bisect-all" because it is slower
than "--bisect-vars" or "--bisect".

When we cannot find for sure the first bad commit because of
dunno commits, we print the hash of each possible first bad
commit and then we exit with code 2.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh               |  127 +++++++++++++++++++++++++++++++++++++++++--
 t/t6030-bisect-porcelain.sh |   71 ++++++++++++++++++++++++
 2 files changed, 193 insertions(+), 5 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 388887a..c556318 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -17,6 +17,8 @@ git bisect replay <logfile>
         replay bisection log.
 git bisect log
         show bisect log.
+git bisect dunno [<rev>...]
+        mark <rev>... untestable revisions.
 git bisect run <cmd>...
         use <cmd>... to automatically bisect.'
 
@@ -143,7 +145,7 @@ bisect_write_bad() {
 
 bisect_good() {
 	bisect_autostart
-        case "$#" in
+	case "$#" in
 	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
 	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
 		test '' != "$revs" || die "Bad rev input: $@" ;;
@@ -153,7 +155,6 @@ bisect_good() {
 		rev=$(git rev-parse --verify "$rev^{commit}") || exit
 		bisect_write_good "$rev"
 		echo "git-bisect good $rev" >>"$GIT_DIR/BISECT_LOG"
-
 	done
 	bisect_auto_next
 }
@@ -164,6 +165,28 @@ bisect_write_good() {
 	echo "# good: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
 }
 
+bisect_dunno() {
+	bisect_autostart
+	case "$#" in
+	0)    revs=$(git rev-parse --verify HEAD) || exit ;;
+	*)    revs=$(git rev-parse --revs-only --no-flags "$@") &&
+		test '' != "$revs" || die "Bad rev input: $@" ;;
+	esac
+	for rev in $revs
+	do
+		rev=$(git rev-parse --verify "$rev^{commit}") || exit
+		bisect_write_dunno "$rev"
+		echo "git-bisect dunno $rev" >>"$GIT_DIR/BISECT_LOG"
+	done
+	bisect_auto_next
+}
+
+bisect_write_dunno() {
+	rev="$1"
+	echo "$rev" >"$GIT_DIR/refs/bisect/dunno-$rev"
+	echo "# dunno: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
+}
+
 bisect_next_check() {
 	missing_good= missing_bad=
 	git show-ref -q --verify refs/bisect/bad || missing_bad=t
@@ -206,17 +229,104 @@ bisect_auto_next() {
 	bisect_next_check && bisect_next || :
 }
 
+search_dunno() {
+	_hash="$1"
+	_dunno="$2"
+
+	for _val in $_dunno ; do
+		case $_hash in $_val) return 1 ;; esac
+	done
+	return 0
+}
+
+filter_dunno() {
+	_eval="$1"
+	_dunno="$2"
+
+	if [ -z "$_dunno" ]; then
+		eval $_eval
+		return
+	fi
+
+	# Let's parse the output of:
+	# "git rev-list --bisect-vars --bisect-all ..."
+	eval $_eval | while read hash line
+	do
+		case "$VARS,$FOUND,$TRIED,$hash" in
+			# We display some vars.
+			1,*,*,*) echo "$hash $line" ;;
+
+			# Split line.
+			,*,*,---*) ;;
+
+			# We had nothing to search.
+			,,,bisect_rev*)
+				echo "bisect_rev="
+				VARS=1
+				;;
+
+			# We did not find a good bisect rev.
+			# This should happen only if the "bad"
+			# commit is also a "dunno" commit.
+			,,*,bisect_rev*)
+				echo "bisect_rev=$TRIED"
+				VARS=1
+				;;
+
+			# We are searching.
+			,,*,*)
+				TRIED="${TRIED:+$TRIED|}$hash"
+				if search_dunno "$hash" "$_dunno" ; then
+					echo "bisect_rev=$hash"
+					echo "bisect_tried=\"$TRIED\""
+					FOUND=1
+				fi
+				;;
+
+			# We have already found a rev to be tested.
+			,1,*,bisect_rev*) VARS=1 ;;
+			,1,*,*) ;;
+
+			# ???
+			*) die "filter_dunno error " \
+			    "VARS: '$VARS' " \
+			    "FOUND: '$FOUND' " \
+			    "TRIED: '$TRIED' " \
+			    "hash: '$hash' " \
+			    "line: '$line'"
+			;;
+		esac
+	done
+}
+
+exit_if_dunno_commits () {
+	_tried=$1
+	if expr "$_tried" : ".*[|].*" > /dev/null ; then
+		echo "There are only 'dunno' commit left to test."
+		echo "The first bad commit could be any of:"
+		echo "$_tried" | sed -e 's/[|]/\n/g'
+		echo "We cannot bisect more!"
+		exit 2
+	fi
+}
+
 bisect_next() {
-        case "$#" in 0) ;; *) usage ;; esac
+	case "$#" in 0) ;; *) usage ;; esac
 	bisect_autostart
 	bisect_next_check good
 
+	dunno=$(git for-each-ref --format='%(objectname)' \
+		"refs/bisect/dunno-*" | tr '[\012]' ' ') || exit
+
+	BISECT_OPT=''
+	test -n "$dunno" && BISECT_OPT='--bisect-all'
+
 	bad=$(git rev-parse --verify refs/bisect/bad) &&
 	good=$(git for-each-ref --format='^%(objectname)' \
 		"refs/bisect/good-*" | tr '[\012]' ' ') &&
-	eval="git rev-list --bisect-vars $good $bad --" &&
+	eval="git rev-list --bisect-vars $BISECT_OPT $good $bad --" &&
 	eval="$eval $(cat "$GIT_DIR/BISECT_NAMES")" &&
-	eval=$(eval "$eval") &&
+	eval=$(filter_dunno "$eval" "$dunno") &&
 	eval "$eval" || exit
 
 	if [ -z "$bisect_rev" ]; then
@@ -224,11 +334,16 @@ bisect_next() {
 		exit 1
 	fi
 	if [ "$bisect_rev" = "$bad" ]; then
+		exit_if_dunno_commits "$bisect_tried"
 		echo "$bisect_rev is first bad commit"
 		git diff-tree --pretty $bisect_rev
 		exit 0
 	fi
 
+	# We should exit here only if the "bad"
+	# commit is also a "dunno" commit (see above).
+	exit_if_dunno_commits "$bisect_rev"
+
 	echo "Bisecting: $bisect_nr revisions left to test after this"
 	echo "$bisect_rev" >"$GIT_DIR/refs/heads/new-bisect"
 	git checkout -q new-bisect || exit
@@ -363,6 +478,8 @@ case "$#" in
         bisect_bad "$@" ;;
     good)
         bisect_good "$@" ;;
+    dunno)
+        bisect_dunno "$@" ;;
     next)
         # Not sure we want "next" at the UI level anymore.
         bisect_next "$@" ;;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 03cdba5..7f41a46 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -71,6 +71,63 @@ test_expect_success 'bisect start with one bad and good' '
 	git bisect next
 '
 
+# $HASH1 is good, $HASH4 is bad, we dunno about $HASH3
+# but $HASH2 is bad,
+# so we should find $HASH2 as the first bad commit
+test_expect_success 'bisect dunno: successfull result' '
+	git bisect reset &&
+	git bisect start $HASH4 $HASH1 &&
+	git bisect dunno &&
+	git bisect bad > my_bisect_log.txt &&
+	grep "$HASH2 is first bad commit" my_bisect_log.txt &&
+	git bisect reset
+'
+
+# $HASH1 is good, $HASH4 is bad, we dunno about $HASH3 and $HASH2
+# so we should not be able to tell the first bad commit
+# among $HASH2, $HASH3 and $HASH4
+test_expect_success 'bisect dunno: cannot tell between 3 commits' '
+	git bisect start $HASH4 $HASH1 &&
+	git bisect dunno || return 1
+
+	if git bisect dunno > my_bisect_log.txt
+	then
+		echo Oops, should have failed.
+		false
+	else
+		test $? -eq 2 &&
+		grep "first bad commit could be any of" my_bisect_log.txt &&
+		! grep $HASH1 my_bisect_log.txt &&
+		grep $HASH2 my_bisect_log.txt &&
+		grep $HASH3 my_bisect_log.txt &&
+		grep $HASH4 my_bisect_log.txt &&
+		git bisect reset
+	fi
+'
+
+# $HASH1 is good, $HASH4 is bad, we dunno about $HASH3
+# but $HASH2 is good,
+# so we should not be able to tell the first bad commit
+# among $HASH3 and $HASH4
+test_expect_success 'bisect dunno: cannot tell between 2 commits' '
+	git bisect start $HASH4 $HASH1 &&
+	git bisect dunno || return 1
+
+	if git bisect good > my_bisect_log.txt
+	then
+		echo Oops, should have failed.
+		false
+	else
+		test $? -eq 2 &&
+		grep "first bad commit could be any of" my_bisect_log.txt &&
+		! grep $HASH1 my_bisect_log.txt &&
+		! grep $HASH2 my_bisect_log.txt &&
+		grep $HASH3 my_bisect_log.txt &&
+		grep $HASH4 my_bisect_log.txt &&
+		git bisect reset
+	fi
+'
+
 # We want to automatically find the commit that
 # introduced "Another" into hello.
 test_expect_success \
@@ -99,6 +156,20 @@ test_expect_success \
      grep "$HASH4 is first bad commit" my_bisect_log.txt &&
      git bisect reset'
 
+# $HASH1 is good, $HASH5 is bad, we dunno about $HASH3
+# but $HASH4 is good,
+# so we should find $HASH5 as the first bad commit
+HASH5=
+test_expect_success 'bisect dunno: add line and then a new test' '
+	add_line_into_file "5: Another new line." hello &&
+	HASH5=$(git rev-parse --verify HEAD) &&
+	git bisect start $HASH5 $HASH1 &&
+	git bisect dunno &&
+	git bisect good > my_bisect_log.txt &&
+	grep "$HASH5 is first bad commit" my_bisect_log.txt &&
+	git bisect reset
+'
+
 #
 #
 test_done
-- 
1.5.3.4.208.g996ad

^ permalink raw reply related

* [PATCH 1/2] rev-list: implement --bisect-all
From: Christian Couder @ 2007-10-08  3:34 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git

This is Junio's patch with some stuff to make --bisect-all
compatible with --bisect-vars.

This option makes it possible to see all the potential
bisection points. The best ones are displayed first.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-rev-list.c |   98 +++++++++++++++++++++++++++++++++++++++++++++-------
 log-tree.c         |    2 +-
 log-tree.h         |    1 +
 3 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 33726b8..0943b76 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -9,6 +9,7 @@
 #include "revision.h"
 #include "list-objects.h"
 #include "builtin.h"
+#include "log-tree.h"
 
 /* bits #0-15 in revision.h */
 
@@ -39,6 +40,7 @@ static const char rev_list_usage[] =
 "  special purpose:\n"
 "    --bisect\n"
 "    --bisect-vars"
+"    --bisect-all"
 ;
 
 static struct rev_info revs;
@@ -74,6 +76,7 @@ static void show_commit(struct commit *commit)
 			parents = parents->next;
 		}
 	}
+	show_decorations(commit);
 	if (revs.commit_format == CMIT_FMT_ONELINE)
 		putchar(' ');
 	else
@@ -278,6 +281,57 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)
 	return best;
 }
 
+struct commit_dist {
+	struct commit *commit;
+	int distance;
+};
+
+static int compare_commit_dist(const void *a_, const void *b_)
+{
+	struct commit_dist *a, *b;
+
+	a = (struct commit_dist *)a_;
+	b = (struct commit_dist *)b_;
+	if (a->distance != b->distance)
+		return b->distance - a->distance; /* desc sort */
+	return hashcmp(a->commit->object.sha1, b->commit->object.sha1);
+}
+
+static struct commit_list *best_bisection_sorted(struct commit_list *list, int nr)
+{
+	struct commit_list *p;
+	struct commit_dist *array = xcalloc(nr, sizeof(*array));
+	int cnt, i;
+
+	for (p = list, cnt = 0; p; p = p->next) {
+		int distance;
+		unsigned flags = p->item->object.flags;
+
+		if (revs.prune_fn && !(flags & TREECHANGE))
+			continue;
+		distance = weight(p);
+		if (nr - distance < distance)
+			distance = nr - distance;
+		array[cnt].commit = p->item;
+		array[cnt].distance = distance;
+		cnt++;
+	}
+	qsort(array, cnt, sizeof(*array), compare_commit_dist);
+	for (p = list, i = 0; i < cnt; i++) {
+		struct name_decoration *r = xmalloc(sizeof(*r) + 100);
+		struct object *obj = &(array[i].commit->object);
+
+		sprintf(r->name, "dist=%d", array[i].distance);
+		r->next = add_decoration(&name_decoration, obj, r);
+		p->item = array[i].commit;
+		p = p->next;
+	}
+	if (p)
+		p->next = NULL;
+	free(array);
+	return list;
+}
+
 /*
  * zero or positive weight is the number of interesting commits it can
  * reach, including itself.  Especially, weight = 0 means it does not
@@ -292,7 +346,8 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)
  * or positive distance.
  */
 static struct commit_list *do_find_bisection(struct commit_list *list,
-					     int nr, int *weights)
+					     int nr, int *weights,
+					     int find_all)
 {
 	int n, counted;
 	struct commit_list *p;
@@ -351,7 +406,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 		clear_distance(list);
 
 		/* Does it happen to be at exactly half-way? */
-		if (halfway(p, nr))
+		if (!find_all && halfway(p, nr))
 			return p;
 		counted++;
 	}
@@ -389,19 +444,22 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 				weight_set(p, weight(q));
 
 			/* Does it happen to be at exactly half-way? */
-			if (halfway(p, nr))
+			if (!find_all && halfway(p, nr))
 				return p;
 		}
 	}
 
 	show_list("bisection 2 counted all", counted, nr, list);
 
-	/* Then find the best one */
-	return best_bisection(list, nr);
+	if (!find_all)
+		return best_bisection(list, nr);
+	else
+		return best_bisection_sorted(list, nr);
 }
 
 static struct commit_list *find_bisection(struct commit_list *list,
-					  int *reaches, int *all)
+					  int *reaches, int *all,
+					  int find_all)
 {
 	int nr, on_list;
 	struct commit_list *p, *best, *next, *last;
@@ -434,14 +492,13 @@ static struct commit_list *find_bisection(struct commit_list *list,
 	weights = xcalloc(on_list, sizeof(*weights));
 
 	/* Do the real work of finding bisection commit. */
-	best = do_find_bisection(list, nr, weights);
-
+	best = do_find_bisection(list, nr, weights, find_all);
 	if (best) {
-		best->next = NULL;
+		if (!find_all)
+			best->next = NULL;
 		*reaches = weight(best);
 	}
 	free(weights);
-
 	return best;
 }
 
@@ -468,6 +525,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int read_from_stdin = 0;
 	int bisect_show_vars = 0;
+	int bisect_find_all = 0;
 
 	git_config(git_default_config);
 	init_revisions(&revs, prefix);
@@ -490,6 +548,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--bisect-all")) {
+			bisect_list = 1;
+			bisect_find_all = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--bisect-vars")) {
 			bisect_list = 1;
 			bisect_show_vars = 1;
@@ -536,9 +599,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list) {
 		int reaches = reaches, all = all;
 
-		revs.commits = find_bisection(revs.commits, &reaches, &all);
+		revs.commits = find_bisection(revs.commits, &reaches, &all,
+					      bisect_find_all);
 		if (bisect_show_vars) {
 			int cnt;
+			char hex[41];
 			if (!revs.commits)
 				return 1;
 			/*
@@ -550,15 +615,22 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			 * A bisect set of size N has (N-1) commits further
 			 * to test, as we already know one bad one.
 			 */
-			cnt = all-reaches;
+			cnt = all - reaches;
 			if (cnt < reaches)
 				cnt = reaches;
+			strcpy(hex, sha1_to_hex(revs.commits->item->object.sha1));
+
+			if (bisect_find_all) {
+				traverse_commit_list(&revs, show_commit, show_object);
+				printf("------\n");
+			}
+
 			printf("bisect_rev=%s\n"
 			       "bisect_nr=%d\n"
 			       "bisect_good=%d\n"
 			       "bisect_bad=%d\n"
 			       "bisect_all=%d\n",
-			       sha1_to_hex(revs.commits->item->object.sha1),
+			       hex,
 			       cnt - 1,
 			       all - reaches - 1,
 			       reaches - 1,
diff --git a/log-tree.c b/log-tree.c
index 2319154..f766758 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -15,7 +15,7 @@ static void show_parents(struct commit *commit, int abbrev)
 	}
 }
 
-static void show_decorations(struct commit *commit)
+void show_decorations(struct commit *commit)
 {
 	const char *prefix;
 	struct name_decoration *decoration;
diff --git a/log-tree.h b/log-tree.h
index e82b56a..b33f7cd 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -12,5 +12,6 @@ int log_tree_diff_flush(struct rev_info *);
 int log_tree_commit(struct rev_info *, struct commit *);
 int log_tree_opt_parse(struct rev_info *, const char **, int);
 void show_log(struct rev_info *opt, const char *sep);
+void show_decorations(struct commit *commit);
 
 #endif
-- 
1.5.3.4.208.g996ad

^ permalink raw reply related

* Re: Trying to use git-filter-branch to compress history by removing large, obsolete binary files
From: Sam Vilain @ 2007-10-08  2:28 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Johannes Schindelin, Frank Lichtenheld, git
In-Reply-To: <51419b2c0710071747w14d0c265x2de42fca50552394@mail.gmail.com>

Elijah Newren wrote:
> On 10/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> It should be as easy as git filter-branch and git clone.
> 
> Yes, a git filter-branch, git clone, AND git gc in the clone avoids
> all those funny ref editing commands.  However, cloning a 5.6GB repo
> (the size of one of the real repos I'm dealing with) will likely take
> a long time (and may push me past the limits of disk space), so using
> other steps to avoid the need to clone actually seems nicer.

You can just delete the logs and references that you don't want and run
git gc --prune.

However.

git gc creates a new pack before deleting the old one.  Garbage
collection usually does this; make a copy of everything to a new place
and then free all of the old space.  If *that* is a problem, ie you
don't have enough space for two copies of the repository and the junk,
you'll have to do a partial import, leave the junk you don't want
unpacked, cleanup and prune, then finish the import.  Which sounds like
a lot of hassle when you should really just find a place with more space
to work with!

Sam.

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Jeff King @ 2007-10-08  2:22 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Shawn Bohrer, git, gitster
In-Reply-To: <alpine.LFD.0.999.0710071916510.23684@woody.linux-foundation.org>

On Sun, Oct 07, 2007 at 07:17:50PM -0700, Linus Torvalds wrote:

> fchdir() is not portable.
> 
> I think it would be better to not chdir() at all. Yes, that means having 
> to prepend the prefix to the names, but that is what git generally does 
> (for that - and other - reasons).

I was using the "even Solaris has it!" test, but yes, it's not POSIX. I
don't know how common it actually is (for curiosity's sake, do you know
of a common platform that lacks it?). But I do agree that just building
up the path and avoiding the chdir at all is simple and portable.

-Peff

^ permalink raw reply

* Re: [PATCH] Make strbuf_cmp inline, constify its arguments and optimize it a bit
From: Jeff King @ 2007-10-08  2:19 UTC (permalink / raw)
  To: Alex Riesen; +Cc: David Kastrup, git
In-Reply-To: <20071007215749.GD2765@steel.home>

On Sun, Oct 07, 2007 at 11:57:49PM +0200, Alex Riesen wrote:

> > > Can't the result of the expression be reused in compiled?
> > > Isn't it a common expression?
> > 
> > No, since the call to memcmp might change a->len or b->len.  A
> 
> Huh?! How's that? It is not even given them!

But they are non-local variables (they are part of structs passed in as
pointers), so that translation unit has no idea how they are allocated.
They could be globals that memcmp mucks with as a side effect.

That being said, standards-conforming compilers _can_ realize that
memcmp is a special, standards-defined function with no side effects and
act accordingly. gcc provides the 'pure' function attribute for this
purpose, which is used by glibc.

-Peff

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Linus Torvalds @ 2007-10-08  2:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn Bohrer, git, gitster
In-Reply-To: <20071008020435.GA20050@coredump.intra.peff.net>



On Sun, 7 Oct 2007, Jeff King wrote:
> 
>   fd = open(".", O_RDONLY);
>   chdir(path);
>   ...
>   fchdir(fd);

fchdir() is not portable.

I think it would be better to not chdir() at all. Yes, that means having 
to prepend the prefix to the names, but that is what git generally does 
(for that - and other - reasons).

		Linus

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Jeff King @ 2007-10-08  2:08 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: git, gitster
In-Reply-To: <20071008020435.GA20050@coredump.intra.peff.net>

On Sun, Oct 07, 2007 at 10:04:35PM -0400, Jeff King wrote:

> This doesn't always put you back where you started, due to symlinks. For
> example:
> [and other complaints]

Oops, didn't see that there was another thread for the reworked patch.
My comments still stand, but it looks like others have weighed in as
well. Sorry.

-Peff

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Jeff King @ 2007-10-08  2:04 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: git, gitster
In-Reply-To: <11917040461528-git-send-email-shawn.bohrer@gmail.com>

On Sat, Oct 06, 2007 at 03:54:06PM -0500, Shawn Bohrer wrote:

> +static int remove_directory(const char *path)
> +{
> [...]
> +		chdir(path);
> [...]
> +	chdir("..");

This doesn't always put you back where you started, due to symlinks. For
example:

cat >foo.c <<'EOF'
int main(int argc, char **argv) {
  chdir(argv[1]);
  chdir("..");
  execlp("ls", 0);
}
EOF
gcc -o foo foo.c
ln -s /tmp sub
./foo sub

will show that you end up in the root directory.  Something like this is
more robust:

  fd = open(".", O_RDONLY);
  chdir(path);
  ...
  fchdir(fd);

In general, you shouldn't end up there because you don't actually
recurse for symlinks, but there is a race condition (and losing it means
you start recursively removing unintended directories -- oops).

On top of which, this line from the same function isn't very portable:

+                       if (dir->d_type == DT_DIR)

since POSIX specifies nothing but dir->d_name (Solaris, for example,
doesn't define d_type). You need to stat the file.

All of that being said, I think a lot of this is already done in
dir.[ch]. At the very least, you should be able to use
remove_dir_recursively, and for bonus points you can get rid of the
start_command call to ls-files by just walking the dir tree yourself.
I don't know if the latter is required, but it's nice when the
C-ification actually cleans up a bit and uses the internal C interfaces
(which are more efficient and often more clear to read) rather than just
converting shell to C.

-Peff

^ permalink raw reply

* Re: [PATCH] Make strbuf_cmp inline, constify its arguments and  optimize it a bit
From: Miles Bader @ 2007-10-08  1:45 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Wincent Colaiuta, David Kastrup, Pierre Habouzit, Timo Hirvonen,
	git, Junio C Hamano
In-Reply-To: <20071007223140.GG2765@steel.home>

Alex Riesen <raa.lkml@gmail.com> writes:
> int strbuf_cmp2(struct strbuf *a, struct strbuf *b)
> {
> 	int len = a->len < b->len ? a->len: b->len;
> 	int cmp = memcmp(a->buf, b->buf, len);
> 	if (cmp)
> 		return cmp;
> 	return a->len < b->len ? -1: a->len != b->len;
> }

BTW, why are you making such effort to return only -1, 0, or 1 in the
last line?  memcmp/strcmp make no such guarantee; e.g. glibc says:

     The `strcmp' function compares the string S1 against S2, returning
     a value that has the same sign as the difference between the first
     differing pair of characters (interpreted as `unsigned char'
     objects, then promoted to `int').

     If the two strings are equal, `strcmp' returns `0'.

     A consequence of the ordering used by `strcmp' is that if S1 is an
     initial substring of S2, then S1 is considered to be "less than"
     S2.

So I think the last line can just be:

   return a->len - b->len;

-miles

-- 
Suburbia: where they tear out the trees and then name streets after them.

^ permalink raw reply

* Re: Trying to use git-filter-branch to compress history by removing large, obsolete binary files
From: Dmitry Potapov @ 2007-10-08  1:06 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Alex Riesen, Frank Lichtenheld, git
In-Reply-To: <51419b2c0710071722k576c06d9i2f4dce730eae2059@mail.gmail.com>

On Sun, Oct 07, 2007 at 06:22:28PM -0600, Elijah Newren wrote:
> 
> git-filter-branch --tree-filter 'rm -f testme.txt' HEAD
> git reset --hard
> rm -rf .git/refs/original/
> vi .git/packed-refs
> # Use vi to remove the line referring to refs/original...
> git reflog expire --all --expire-unreachable=0
> git gc --prune
> 
> Seems like a wrapper is needed.  :-)

Actually, I would rather not, because you rarely need to remove anything
immediately, and 30 days delay is reasonable time to give you a chance
to recover that you removed accidentally. You can reduce it by setting
appropriate value for gc.reflogExpireUnreachable in your configuration.
The only thing you need to do is to remove .git/refs/original/heads/something
after you are sure that git-filter-branch did exactly what you wanted.

> 
> > Warning: all unreachable references will be removed!
> 
> What other scenarios could lead to unreachable references?

Any re-writing of history leads to that.

> I don't
> know how to determine whether this is safe or not (except that these
> were test repositories anyway, so I don't care what happens to them).

Git logs all your action, so even re-writing history would not be
so disastrous if you suddenly realized that you did something wrong.
The history is stored for 30 days by default. Usually, you do not
need to mess with Git internals like you did above. Your useless
files still will disappear after being unreachable for 30 days.

OTOH, if you want to have a clean repository immediately, I believe
'git clone' is a better option. After you made a local clone using
it, 'git gc' should remove old garbage.

Dmitry

^ permalink raw reply

* Re: Trying to use git-filter-branch to compress history by removing large, obsolete binary files
From: Johannes Schindelin @ 2007-10-08  1:06 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Elijah Newren, Frank Lichtenheld, git
In-Reply-To: <20071008010033.GA25654@fieldses.org>

Hi,

On Sun, 7 Oct 2007, J. Bruce Fields wrote:

> On Mon, Oct 08, 2007 at 01:34:07AM +0100, Johannes Schindelin wrote:
> > It does what it should do.  It is _your_ task to look at refs/original/* 
> > if everything went alright.  Then you just delete the checked refs.
> 
> It seems odd to me, by the way, that filter-branch has its own 
> home-grown backup mechanism.  Lots of other commands can "lose" commits, 
> but none of them keep an extra backup like this.

The rationale was this: filter-branch recently learnt how to rewrite many 
branches, and it might be tedious to find out which ones.  But then, there 
is git log --no-walk --all, so maybe I really should get rid of 
refs/original/*?

I'd like to have some comments from the heavier filter-branch users on 
that...

Ciao,
Dscho

^ permalink raw reply

* Re: Trying to use git-filter-branch to compress history by removing large, obsolete binary files
From: J. Bruce Fields @ 2007-10-08  1:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Elijah Newren, Frank Lichtenheld, git
In-Reply-To: <Pine.LNX.4.64.0710080129480.4174@racer.site>

On Mon, Oct 08, 2007 at 01:34:07AM +0100, Johannes Schindelin wrote:
> It does what it should do.  It is _your_ task to look at refs/original/* 
> if everything went alright.  Then you just delete the checked refs.

It seems odd to me, by the way, that filter-branch has its own
home-grown backup mechanism.  Lots of other commands can "lose" commits,
but none of them keep an extra backup like this.

And I find it tedious for quicker jobs which it might otherwise be
useful for (e.g. rewrites of commits in my tree not yet in upstream),
unless I wrap it in a script that cleans up after itself.

--b.

^ 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