Git development
 help / color / mirror / Atom feed
* [PATCH 3/6] builtin-help: make it possible to exclude some commands in list_commands()
From: Miklos Vajna @ 2008-07-28 23:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr69ex00x.fsf@gitster.siamese.dyndns.org>

The supposed method is to build a list of commands to be excluded using
add_cmdname(), then pass the list as the new exclude parameter. If no
exclude is needed, NULL should be used.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Sun, Jul 27, 2008 at 06:36:30PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > +   struct cmdname {
> > +           size_t len;
> > +           char name[1];
> > +   } **names;
> > +};
>
> I thought we do this kind of thing using FLEX_ARRAY macro.  Is there
> any
> reason its use is not appropriate here?

Now I'm using it. :-)

Note that FLEX_ARRAY would be a drop-in replacement only in case it
would be name[0], so I needed

-       struct cmdname *ent = xmalloc(sizeof(*ent) + len);
+       struct cmdname *ent = xmalloc(sizeof(*ent) + len + 1);

later in add_cmdname().

 help.c |   26 +++++++++++---------------
 help.h |   14 +++++++++++++-
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/help.c b/help.c
index 7a42517..858f76a 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,7 @@
 #include "common-cmds.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "help.h"
 
 static struct man_viewer_list {
 	struct man_viewer_list *next;
@@ -300,18 +301,11 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdnames {
-	int alloc;
-	int cnt;
-	struct cmdname {
-		size_t len;
-		char name[1];
-	} **names;
-} main_cmds, other_cmds;
+struct cmdnames main_cmds, other_cmds;
 
-static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
+void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
-	struct cmdname *ent = xmalloc(sizeof(*ent) + len);
+	struct cmdname *ent = xmalloc(sizeof(*ent) + len + 1);
 
 	ent->len = len;
 	memcpy(ent->name, name, len);
@@ -463,7 +457,7 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,
 	return longest;
 }
 
-static unsigned int load_command_list(const char *prefix)
+static unsigned int load_command_list(const char *prefix, struct cmdnames *exclude)
 {
 	unsigned int longest = 0;
 	unsigned int len;
@@ -502,13 +496,15 @@ static unsigned int load_command_list(const char *prefix)
 	      sizeof(*other_cmds.names), cmdname_compare);
 	uniq(&other_cmds);
 	exclude_cmds(&other_cmds, &main_cmds);
+	if (exclude)
+		exclude_cmds(&main_cmds, exclude);
 
 	return longest;
 }
 
-void list_commands(const char *prefix, const char *title)
+void list_commands(const char *prefix, const char *title, struct cmdnames *exclude)
 {
-	unsigned int longest = load_command_list(prefix);
+	unsigned int longest = load_command_list(prefix, exclude);
 	const char *exec_path = git_exec_path();
 
 	if (main_cmds.cnt) {
@@ -558,7 +554,7 @@ static int is_in_cmdlist(struct cmdnames *c, const char *s)
 
 int is_git_command(const char *s, const char *prefix)
 {
-	load_command_list(prefix);
+	load_command_list(prefix, NULL);
 	return is_in_cmdlist(&main_cmds, s) ||
 		is_in_cmdlist(&other_cmds, s);
 }
@@ -704,7 +700,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	if (show_all) {
 		printf("usage: %s\n\n", git_usage_string);
-		list_commands("git-", "git commands");
+		list_commands("git-", "git commands", NULL);
 		printf("%s\n", git_more_info_string);
 		return 0;
 	}
diff --git a/help.h b/help.h
index 0741662..3eb8cfb 100644
--- a/help.h
+++ b/help.h
@@ -1,7 +1,19 @@
 #ifndef HELP_H
 #define HELP_H
 
+struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[FLEX_ARRAY];
+	} **names;
+};
+
 int is_git_command(const char *s, const char *prefix);
-void list_commands(const char *prefix, const char *title);
+void list_commands(const char *prefix, const char *title, struct cmdnames *exclude);
+void add_cmdname(struct cmdnames *cmds, const char *name, int len);
+
+extern struct cmdnames main_cmds, other_cmds;
 
 #endif /* HELP_H */
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
From: Johannes Schindelin @ 2008-07-28 23:27 UTC (permalink / raw)
  To: Jeff King; +Cc: sverre, Git Mailinglist, Miklos Vajna
In-Reply-To: <20080728192651.GA26677@sigill.intra.peff.net>

Hi,

On Mon, 28 Jul 2008, Jeff King wrote:

> On Mon, Jul 28, 2008 at 08:09:55PM +0100, Johannes Schindelin wrote:
> 
> > Well, I have to say that the workflow is a bit backwards if the person 
> > who _publishes_ the thing is the one saying "Ooops, my version no 
> > goodie, other version please, but so that pull still works".
> > 
> > I would have expected the one who has the good version to make the 
> > choice.
> 
> My situation was two long-running branches, "stable" and "devel", both 
> of which were worked on by many developers. One person was in charge of 
> integration and branch management. They wanted "stable" to get the 
> contents of "devel" (which were now ready for release), ignoring any 
> small fixes that had been done on "stable" (since they had all been 
> moved over to "devel" previously, but in subtly different ways that 
> would create conflicts). And "git reset" was not an option, because they 
> wanted to keep the history of "stable" in case those fixes needed to be 
> looked at later.
> 
> So the logical sequence was:
> 
>   git checkout production
>   git merge -s theirs master

To me, this suggests that they were too married to 'production' being the 
"dominant" branch.

Thing is: they had two branches.  They should be merged, but one should 
prevail: 'master'.

So if I have two branches, say "x" and "y", and I want to merge them, but 
really throw away the tree of "x", I would check out 'y', naturally.  Then 
'git merge -s ours x'.

If the result should become the state of 'x', too, I would then just 
'git push origin y:x'.

Maybe I am "Git-braindead" by now, so that you can make fun of me like I 
used to make fun of CVSers and SVNers...

Ciao,
Dscho

^ permalink raw reply

* [PATCH 6/6] Add a new test for using a custom merge strategy
From: Miklos Vajna @ 2008-07-28 23:39 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807281506510.2725@eeepc-johanness>

Testing is done by creating a simple git-merge-theirs strategy which
just picks the upstream tree. (In other words, this is not the opposite
of -s ours.)

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Mon, Jul 28, 2008 at 03:12:59PM +0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Note that what was asked for, and what Junio implemented before
> deciding
> that it would do more harm than good in git.git, is not the same as
> what
> you provide.

Thanks, now I see the difference. The updated commit message is
hopefully better.

Also I added a check to make sure the upstream and the result tree is
the same as well.

 t/t7606-merge-custom.sh |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100755 t/t7606-merge-custom.sh

diff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh
new file mode 100755
index 0000000..13e8ff5
--- /dev/null
+++ b/t/t7606-merge-custom.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Testing a custom strategy.'
+
+. ./test-lib.sh
+
+cat > git-merge-theirs << EOF
+#!/bin/sh
+eval git read-tree --reset -u \\\$\$#
+EOF
+chmod +x git-merge-theirs
+PATH=.:$PATH
+export PATH
+
+test_expect_success 'setup' '
+	echo c0 > c0.c &&
+	git add c0.c &&
+	git commit -m c0 &&
+	git tag c0 &&
+	echo c1 > c1.c &&
+	git add c1.c &&
+	git commit -m c1 &&
+	git tag c1 &&
+	git reset --hard c0 &&
+	echo c2 > c2.c &&
+	git add c2.c &&
+	git commit -m c2 &&
+	git tag c2
+'
+
+test_expect_success 'merge c2 with a custom strategy' '
+	git reset --hard c1 &&
+	git merge -s theirs c2 &&
+	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
+	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
+	test "$(git rev-parse c2^{tree})" = "$(git rev-parse HEAD^{tree})" &&
+	git diff --exit-code &&
+	test -f c0.c &&
+	test ! -f c1.c &&
+	test -f c2.c
+'
+
+test_done
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* Re: [PATCHv2] git-mv: Keep moved index entries inact
From: Johannes Schindelin @ 2008-07-28 23:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, Petr Baudis, git
In-Reply-To: <7vwsj5rf48.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1943 bytes --]

Hi,

On Mon, 28 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Mon, 28 Jul 2008, SZEDER Gábor wrote:
> >
> >> there is a race somewhere in these 'git-mv: Keep moved index entries
> >> inact' changes.
> >> 
> >> The test cases 'git mv should overwrite symlink to a file' or 'git mv
> >> should overwrite file with a symlink' fail occasionaly.  It's quite
> >> non-deterministic:  I have run t7001-mv.sh in a loop (see below) and
> >> one or the other usually fails around 50 runs (but sometimes only
> >> after 150).  Adding some tracing echos to the tests shows that both
> >> tests fail when running 'git diff-files' at the end.
> >
> > To make it more convenient to test: with this patch it fails all the time:
> 
> It's because we rename(2) but do not read back ctime, and reuse the cached
> data from the old path that was renamed.  After the failed test that moves
> a regular file "move" to "symlink":
> 
> $ stat symlink
>   File: `symlink'
>   Size: 2               Blocks: 8          IO Block: 4096   regular file
> Device: 30ah/778d       Inode: 18104337    Links: 1
> Access: (0664/-rw-rw-r--)  Uid: ( 1012/   junio)   Gid: (   40/     src)
> Access: 2008-07-28 11:49:55.000000000 -0700
> Modify: 2008-07-28 11:48:41.000000000 -0700
> Change: 2008-07-28 11:48:42.000000000 -0700
> 
> But the cached stat information looks like this:
> 
> $ ../../git-ls-files --stat
> ctime=1217270921, mtime=1217270921, ino=18104337, mode=100644, uid=1012, gid=40symlink
> 
> We need to refresh the entry to pick up potential ctime changes.

Yep.

Tested-by: me

BTW I have no idea how we could test for this, short of introducing the 
"sleep 1" I did earlier.  Maybe guard it with a TEST_EXPENSIVE_CTIME 
variable or something similar.  Dunno.

And my suggestion to use test-chmtime: please just forget about it, and 
just assume that I had some very good wiid(1) in my pipe(2).

Ciao,
Dscho

^ permalink raw reply

* [PATCH] builtin-merge: allow using a custom strategy
From: Miklos Vajna @ 2008-07-28 23:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807281505010.2725@eeepc-johanness>

Allow using a custom strategy, as long as it's named git-merge-foo. The
error handling is now done using is_git_command(). The list of available
strategies is now shown by list_commands().

If an invalid strategy is supplied, like -s foobar, then git-merge would
list all git-merge-* commands. This is not perfect, since for example
git-merge-index is not a valid strategy.

These are removed from the output by scanning the list of main commands;
if the git-merge-foo command is listed in the all_strategy list, then
it's shown, otherwise excluded. This does not exclude commands somewhere
else in the PATH, where custom strategies are expected.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Mon, Jul 28, 2008 at 03:06:09PM +0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> The change in the signature of list_commands() is not part of this
> patch.  So at least one of your commits should result in an
> uncompileable revision...

Right. I just squashed patch 4 and 5, and this solves the problem: patch
3 changes the signature and fixes all usage (in help.c, it's not used
outside help.c), then the squashed patch introduces the usage of the new
list_commands() in builtin-merge.c.

(Also in the 'merge-custom' branch of vmiklos.git on repo.or.cz.)

 builtin-merge.c |   32 +++++++++++++++++++++++++-------
 1 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index e78fa18..29826b1 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -22,6 +22,7 @@
 #include "log-tree.h"
 #include "color.h"
 #include "rerere.h"
+#include "help.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -77,7 +78,7 @@ static int option_parse_message(const struct option *opt,
 static struct strategy *get_strategy(const char *name)
 {
 	int i;
-	struct strbuf err;
+	struct strategy *ret;
 
 	if (!name)
 		return NULL;
@@ -86,12 +87,29 @@ static struct strategy *get_strategy(const char *name)
 		if (!strcmp(name, all_strategy[i].name))
 			return &all_strategy[i];
 
-	strbuf_init(&err, 0);
-	for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
-		strbuf_addf(&err, " %s", all_strategy[i].name);
-	fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
-	fprintf(stderr, "Available strategies are:%s.\n", err.buf);
-	exit(1);
+	if (!is_git_command(name, "git-merge-")) {
+		struct cmdnames not_strategies;
+
+		memset(&not_strategies, 0, sizeof(struct cmdnames));
+		for (i = 0; i < main_cmds.cnt; i++) {
+			int j, found = 0;
+			struct cmdname *ent = main_cmds.names[i];
+			for (j = 0; j < ARRAY_SIZE(all_strategy); j++)
+				if (!strncmp(ent->name, all_strategy[j].name, ent->len)
+						&& !all_strategy[j].name[ent->len])
+					found = 1;
+			if (!found)
+				add_cmdname(&not_strategies, ent->name, ent->len);
+		}
+		fprintf(stderr, "Could not find merge strategy '%s'.\n\n", name);
+		list_commands("git-merge-", "strategies", &not_strategies);
+		exit(1);
+	}
+
+	ret = xmalloc(sizeof(struct strategy));
+	memset(ret, 0, sizeof(struct strategy));
+	ret->name = xstrdup(name);
+	return ret;
 }
 
 static void append_strategy(struct strategy *s)
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* Re: [PATCH 6/6] Add a new test for using a custom merge strategy
From: Johannes Schindelin @ 2008-07-28 23:54 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <f3af7df2dda2dcb7801314cc993470264259f970.1217288180.git.vmiklos@frugalware.org>

Hi,

On Tue, 29 Jul 2008, Miklos Vajna wrote:

> Testing is done by creating a simple git-merge-theirs strategy which
> just picks the upstream tree. (In other words, this is not the opposite
> of -s ours.)

Actually, this _is_ the opposite of -s ours, no?  -s ours just takes our 
tree, your -s theirs just takes their tree.

Sorry for the confusion I caused,
Dscho

^ permalink raw reply

* Re: Showing changes about to be commited via git svn dcommit
From: Miklos Vajna @ 2008-07-28 23:53 UTC (permalink / raw)
  To: Lee Marlow; +Cc: git
In-Reply-To: <7968d7490807281554u3954cf51qe2cd87b94284c77f@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

On Mon, Jul 28, 2008 at 04:54:19PM -0600, Lee Marlow <lee.marlow@gmail.com> wrote:
> $> git svn dcommit --dry-run | grep -E '^diff-tree ' | cut -b 11- |
> git diff-tree --stdin -p -v
> 
> Is this the real way to do it?  Do others do something similar?

Depends on how did you created your git-svn repo. If you have only one
branch with no prefix, then I would try:

        git log git-svn..master

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCHv2] git-mv: Keep moved index entries inact
From: Johannes Schindelin @ 2008-07-28 23:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, Petr Baudis, git
In-Reply-To: <alpine.DEB.1.00.0807290137370.2725@eeepc-johanness>

Hi,

On Tue, 29 Jul 2008, Johannes Schindelin wrote:

> BTW I have no idea how we could test for this, short of introducing the 
> "sleep 1" I did earlier.  Maybe guard it with a TEST_EXPENSIVE_CTIME 
> variable or something similar.  Dunno.

IOW something like this squashed into your patch:

-- snipsnap --

 t/t7001-mv.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index b0fa407..c749059 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -179,6 +179,10 @@ test_expect_success 'git mv should overwrite symlink to a file' '
 	git init &&
 	echo 1 >moved &&
 	ln -s moved symlink &&
+	if test ! -z "$TEST_EXPENSIVE_CTIME"
+	then
+		sleep 1
+	fi &&
 	git add moved symlink &&
 	test_must_fail git mv moved symlink &&
 	git mv -f moved symlink &&

^ permalink raw reply related

* Re: [PATCH] builtin-merge: allow using a custom strategy
From: Johannes Schindelin @ 2008-07-28 23:59 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1217288908-21124-1-git-send-email-vmiklos@frugalware.org>

Hi,

On Tue, 29 Jul 2008, Miklos Vajna wrote:

> On Mon, Jul 28, 2008 at 03:06:09PM +0200, Johannes Schindelin 
> <Johannes.Schindelin@gmx.de> wrote:
>
> > The change in the signature of list_commands() is not part of this 
> > patch.  So at least one of your commits should result in an 
> > uncompileable revision...
> 
> Right. I just squashed patch 4 and 5, and this solves the problem: patch 
> 3 changes the signature and fixes all usage (in help.c, it's not used 
> outside help.c), then the squashed patch introduces the usage of the new 
> list_commands() in builtin-merge.c.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH 6/6] Add a new test for using a custom merge strategy
From: Miklos Vajna @ 2008-07-28 23:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807290153300.2725@eeepc-johanness>

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

On Tue, Jul 29, 2008 at 01:54:17AM +0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Actually, this _is_ the opposite of -s ours, no?  -s ours just takes our 
> tree, your -s theirs just takes their tree.
> 
> Sorry for the confusion I caused,

Aah. :-)

I did not read the source of git-merge-ours and based on your
description I thought my knowledge / the doc about -s ours was not
correct.

So I guess my original patch was right, then.

Note to self: "take 2" gets messy, time to send a "take 3" soon. ;-)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Hackontest ideas?
From: Petr Baudis @ 2008-07-29  0:01 UTC (permalink / raw)
  To: git

  Hi,

  participating in this might be fun, even if there is not much time
left to sign up:

	http://www.hackontest.org/index.php?action=Root-projectDetail(120)

(What feature in Git or a Git-related tool would you implement, given 24
hours staight and unlimited pizza supply?)

  P.S.: Disclaimer - yes, if someone suggests something cool enough to
do with Git, I might apply. ;-)

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH 6/6] Add a new test for using a custom merge strategy
From: Miklos Vajna @ 2008-07-29  0:01 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <20080728235847.GR32057@genesis.frugalware.org>

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

On Tue, Jul 29, 2008 at 01:58:47AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> So I guess my original patch was right, then.

s/patch/description/. The check for the same trees are still valid.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
From: Sverre Rabbelier @ 2008-07-29  0:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, Git Mailinglist, Miklos Vajna
In-Reply-To: <alpine.DEB.1.00.0807290123300.2725@eeepc-johanness>

On Tue, Jul 29, 2008 at 01:27, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> To me, this suggests that they were too married to 'production' being the
> "dominant" branch.

<snip>

> If the result should become the state of 'x', too, I would then just
> 'git push origin y:x'.

But this means that everybody doing a 'git pull' on that repo will get
complaints when pulling, right? Should they just send out a message to
all their users that they'll need to rebase all their changes now?
(Not being sarcastic, am trying to work out what the recommended
workflow is here.)

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Hackontest ideas?
From: Miklos Vajna @ 2008-07-29  0:10 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20080729000103.GH32184@machine.or.cz>

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Tue, Jul 29, 2008 at 02:01:03AM +0200, Petr Baudis <pasky@ucw.cz> wrote:
>   participating in this might be fun, even if there is not much time
> left to sign up:
> 
> 	http://www.hackontest.org/index.php?action=Root-projectDetail(120)
> 
> (What feature in Git or a Git-related tool would you implement, given 24
> hours staight and unlimited pizza supply?)
> 
>   P.S.: Disclaimer - yes, if someone suggests something cool enough to
> do with Git, I might apply. ;-)

Restartable git-clone? :-)

It was a GSoC idea this year, but in the end nobody started working on
it.

(I was about to work on it, but finally my 'builtin-merge' application
was accepted.)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCHv2] git-mv: Keep moved index entries inact
From: Petr Baudis @ 2008-07-29  0:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, SZEDER Gábor, git
In-Reply-To: <7vwsj5rf48.fsf@gitster.siamese.dyndns.org>

On Mon, Jul 28, 2008 at 12:19:19PM -0700, Junio C Hamano wrote:
> We need to refresh the entry to pick up potential ctime changes.
> 
>  read-cache.c       |    7 ++++++-
>  builtin-ls-files.c |   21 +++++++++++++++------
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/read-cache.c b/read-cache.c
> index 1cae361..834096f 100644

Oops, silly me. Sorry for being slower than you at fixing this. ;-)

> diff --git a/builtin-ls-files.c b/builtin-ls-files.c
> index e8d568e..a6b30c8 100644

If you are going to apply this, you might be interested in squashing
this:

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index f43af41..2ead7af 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -53,7 +53,14 @@ OPTIONS
 
 -s::
 --stage::
-	Show stage files in the output
+	Show cached files in an extended format also listing the entry
+	mode, sha1 and stage number; see below for details.
+
+-S::
+--stat::
+	Show cached files in a special format listing stat information
+	stored in the index; this is useful probably only for Git
+	debugging purposes.
 
 --directory::
 	If a whole directory is classified as "other", show just its

(and even if you aren't, maybe the first part is useful, though it's
minor.)

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply related

* Re: Showing changes about to be commited via git svn dcommit
From: Lee Marlow @ 2008-07-29  0:32 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <20080728235326.GQ32057@genesis.frugalware.org>

Works perfectly and makes sense to boot!  Thanks, Miklos

-Lee

On Mon, Jul 28, 2008 at 5:53 PM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Mon, Jul 28, 2008 at 04:54:19PM -0600, Lee Marlow <lee.marlow@gmail.com> wrote:
>> $> git svn dcommit --dry-run | grep -E '^diff-tree ' | cut -b 11- |
>> git diff-tree --stdin -p -v
>>
>> Is this the real way to do it?  Do others do something similar?
>
> Depends on how did you created your git-svn repo. If you have only one
> branch with no prefix, then I would try:
>
>        git log git-svn..master
>

^ permalink raw reply

* Re: Hackontest ideas?
From: Tarmigan @ 2008-07-29  0:34 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20080729000103.GH32184@machine.or.cz>

On Mon, Jul 28, 2008 at 5:01 PM, Petr Baudis <pasky@ucw.cz> wrote:
>  participating in this might be fun, even if there is not much time
> left to sign up:
>
>        http://www.hackontest.org/index.php?action=Root-projectDetail(120)
>
> (What feature in Git or a Git-related tool would you implement, given 24
> hours staight and unlimited pizza supply?)
>
>  P.S.: Disclaimer - yes, if someone suggests something cool enough to
> do with Git, I might apply. ;-)

It might be cool if git-daemon supported
avahi/zeroconf/bonjour/rendezvous as a server and maybe git-status(?
or maybe a new command) had a flag that could make it an avahi client
and list repositories on the local network being advertised over
avahi.

It looks like bzr has an avahi plugin.  Not sure whether it would be a
useful feature for people.  What do other folks think?

As a project, it seems fairly self-contained and well defined, and
might be doable for a small team in 24 hours.

-Tarmigan

^ permalink raw reply

* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
From: Junio C Hamano @ 2008-07-29  0:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, sverre, Git Mailinglist, Miklos Vajna
In-Reply-To: <20080728192651.GA26677@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> My situation was two long-running branches, "stable" and "devel",
> both of which were worked on by many developers. One person was in
> charge of integration and branch management. They wanted "stable" to
> get the contents of "devel" (which were now ready for release), ignoring
> any small fixes that had been done on "stable" (since they had all been
> moved over to "devel" previously, but in subtly different ways that
> would create conflicts). And "git reset" was not an option, because they
> wanted to keep the history of "stable" in case those fixes needed to be
> looked at later.

I sense a slightly broken workflow here, whether the "-s theirs" strategy
is used or the merge is done in the other direction using "-s ours"
strategy.

Remember, when you create a merge commit between one history and another,
you are making this statement:

    I have looked at the tree state and the development history behind
    both of these commits, and came up with this tree, which I believe
    suits the purpose of _my_ history better than either of them.

That is why, after making such a merge with "git merge other", you won't
see any output from "git log ..other", which asks "what do I have yet to
merge?"  Everything that was included in other is now in your history and
there is nothing you have to worry about having left out anymore.

So if you suspect that the sutuation "in case those fixes needed to be
looked at later" ever arises, such a merge should *not* be recorded as a
proper merge on the 'stable' branch, because at that point when you are
doing that "-s theirs" merge (and this applies equally to the case where
you make "-s ours" merge as well), you actually have not looked at "those
fixes" closely enough to make the above statement with confidence.

Having said that, that "looking back in history" can easily be done if you
mark such a "Use '-s theirs' for expediency" merge as potentially an iffy
one in its commit log message somewhere.  Later if you actually hit
issues, you can locate such a merge commit, and inspect the output from
"git log $commit^2..$commit^1".  You would see those fixes the "devel"
history did not have in the "stable" branch when such a merge was made.

So the above is not a fundamental objection to the approach, and that is
why I said "slightly broken".  With a proper explanation between the right
use case (I think what you outlined is an example of good practice) and
the wrong use case (for example, the one described in $gmane/89024, the
whole thing after 'I think "-s theirs" is even worse.', not just the part
that was quoted in $gmane/89178), I think it is Ok to have "-s theirs"
strategy in our toolset.

Even though having said all of the above, I would actually prefer such a
"pull all of the devel down to stable" be done with this workflow instead:

 (1) go to 'devel';
 (2) merge all of 'stable';
 (3) look at the result and prove it is perfect;
 (4) go to 'stable';
 (5) merge 'devel'.

The last step would be a fast-forward, and you do not need "-s theirs"
anywhere in this procedure.  Step (2) can be helped with "-s ours" (which
have the same issue I discussed above), but the result is checked before
it hits the 'stable' (presumably more precious branch), which is
conceptually a big difference.  This is where the existing asymmetry
between theirs and ours comes from.

Incidentally, this is how 'maint' skips to tip of 'master' after a new
major version is released, but 'maint' is merged up into 'master' often
enough that we rarely need to even use "ours" strategy.

^ permalink raw reply

* Re: [PATCHv2] git-mv: Keep moved index entries inact
From: Junio C Hamano @ 2008-07-29  0:46 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, SZEDER Gábor, git
In-Reply-To: <20080729001751.GH10151@machine.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> On Mon, Jul 28, 2008 at 12:19:19PM -0700, Junio C Hamano wrote:
>> We need to refresh the entry to pick up potential ctime changes.
>> 
>>  read-cache.c       |    7 ++++++-
>>  builtin-ls-files.c |   21 +++++++++++++++------
>>  2 files changed, 21 insertions(+), 7 deletions(-)
>> 
>> diff --git a/read-cache.c b/read-cache.c
>> index 1cae361..834096f 100644
>
> Oops, silly me. Sorry for being slower than you at fixing this. ;-)

I did think about ctime issues when I applied the patch.

rename(2) is hardlink to new name followed by unlink of old name, so
internally link count changes twice (and the first "link to new" can
exceed max links and is even allowed to make the whole thing fail); but I
do not think of any other reason for this change in ctime.

^ permalink raw reply

* Re: Hackontest ideas?
From: Junio C Hamano @ 2008-07-29  0:55 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20080729000103.GH32184@machine.or.cz>

Petr Baudis <pasky@ucw.cz> writes:

> (What feature in Git or a Git-related tool would you implement, given 24
> hours staight and unlimited pizza supply?)

"Use 'assume unchanged' bit to implement narrow checkout".

^ permalink raw reply

* Re: Hackontest ideas?
From: Junio C Hamano @ 2008-07-29  1:05 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20080729000103.GH32184@machine.or.cz>

Petr Baudis <pasky@ucw.cz> writes:

> (What feature in Git or a Git-related tool would you implement, given 24
> hours staight and unlimited pizza supply?)

"git-merge-blame"
(http://git.or.cz/gitwiki/SoC2007Ideas#head-cfde15f16950c2579a89cc109762e911546e6fe3).

^ permalink raw reply

* Re: Hackontest ideas?
From: Petr Baudis @ 2008-07-29  1:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5f5o6em.fsf@gitster.siamese.dyndns.org>

On Mon, Jul 28, 2008 at 05:55:45PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@ucw.cz> writes:
> 
> > (What feature in Git or a Git-related tool would you implement, given 24
> > hours staight and unlimited pizza supply?)
> 
> "Use 'assume unchanged' bit to implement narrow checkout".

I think Nguyen Thai Ngoc Duy is already working on this? (Though I think
he does not use the assume unchanged bit; but this will be likely done
before the end of September.)

(This is a bit annoying, by the way - the deadline is way too early...)

				Petr "Pasky" Baudis

^ permalink raw reply

* Re: [PATCH] Make use of stat.ctime configurable
From: Junio C Hamano @ 2008-07-29  1:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Brown, Alex Riesen, Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <alpine.LFD.1.10.0807280906530.3486@nehalem.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Mon, 28 Jul 2008, David Brown wrote:
>
>> On Mon, Jul 28, 2008 at 08:31:28AM +0200, Alex Riesen wrote:
>> 
>> > because there are situations where it produces too much false
>> > positives. Like when file system crawlers keep changing it when
>> > scanning and using the ctime for marking scanned files.
>> 
>> That's interesting, since most backup software uses the ctime to determine
>> file changes.
>
> It really is just Beagle that is (was? I can dream) a piece of 
> unbelievable crap.
>
> Anybody who uses extended attributes as part of a indexing scheme is just 
> insane. Modifying the file you are indexing is not just fundamentally 
> wrong to begin with, but it will then also be incredibly inefficient to 
> read those entries one at a time.

It's a typo and you are saying it _is_ fundamentally wrong, aren't you?

If you are prepared to pick up new files, you need to crawl everywhere
anyway, so if the xattr is used to leave a mark "The last time I looked at
this file was this" in the file itself, it does not sound too bad to me.
It would be irritating that it touches ctime, though, but I do not use it
so it is not my problem ;-)

http://beagle-project.org/FAQ "Do I really need extended attributes?"
talks about BEAGLE_DISABLE_XATTR environment variable and interestingly
it says disabling use of xattr would slow you down.

^ permalink raw reply

* Re: [PATCH] Make use of stat.ctime configurable
From: Linus Torvalds @ 2008-07-29  1:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: David Brown, Alex Riesen, Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <7vbq0ho5g7.fsf@gitster.siamese.dyndns.org>



On Mon, 28 Jul 2008, Junio C Hamano wrote:
> >
> > Anybody who uses extended attributes as part of a indexing scheme is just 
> > insane. Modifying the file you are indexing is not just fundamentally 
> > wrong to begin with, but it will then also be incredibly inefficient to 
> > read those entries one at a time.
> 
> It's a typo and you are saying it _is_ fundamentally wrong, aren't you?

Not a typo, and I'm sayin that "it's not _just_ fundamentally wrong"

So yes, it's fundamentally wrong, but it's worse than that. It's 
fundamentally wrong _and_ it's inefficient as hell.

> If you are prepared to pick up new files, you need to crawl everywhere
> anyway, so if the xattr is used to leave a mark "The last time I looked at
> this file was this" in the file itself, it does not sound too bad to me.

It's absolutely horrible. 

It means that you have another extra indirection and accompanying disk 
seek to check the thing. It's a total performance nightmare. Trust me, 
anybody who uses extended attributes like this simply does not know what 
he is doing.

> It would be irritating that it touches ctime, though, but I do not use it
> http://beagle-project.org/FAQ "Do I really need extended attributes?"
> talks about BEAGLE_DISABLE_XATTR environment variable and interestingly
> it says disabling use of xattr would slow you down.

They don't have a clue. They say that, but it's simply not true. 

Of course, the fact that they think it is probably implies that they did 
something EVEN MORE STUPID for the non-xattr case. That wouldn't surprise 
me at all. If I had to guess, I'd guess that they used an SQL database and 
query language, and did all their tests with hot caches too.

The kernel does caching really well, and the kernel is fast as hell, so 
_of_course_ when you benchmark, using kernel data structures looks good, 
especially if you benchmark against code that isn't well written for the 
particular usage case.

			Linus

^ permalink raw reply

* Re: [PATCH] Make use of stat.ctime configurable
From: Junio C Hamano @ 2008-07-29  1:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Brown, Alex Riesen, Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <alpine.LFD.1.10.0807281817230.3486@nehalem.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> The kernel does caching really well, and the kernel is fast as hell, so 
> _of_course_ when you benchmark, using kernel data structures looks good, 
> especially if you benchmark against code that isn't well written for the 
> particular usage case.

Ok.  While I have your attention on st_ctime, let me ask you a stupid
question.  Why does "rename(old, new)" change st_ctime when you move a
regular file?

^ 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