git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] sane rebase/stash support with submodules
@ 2008-05-14 17:03 Johannes Schindelin
  2008-05-14 17:03 ` [PATCH 1/3] diff options: Introduce --ignore-submodules Johannes Schindelin
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-14 17:03 UTC (permalink / raw)
  To: git, gitster


Submodules are special in that they cannot be blindly updated with
a checkout.  Indeed, submodules do not even have to be checked out (and
in that case, must not be updated).  It is the user's responsibility to
care about the working tree state of submodules.

Therefore, it does not make sense in stash/rebase to test for dirty
submodules: they would not be updated anyway.

This patch series does exactly that: working tree states of submodules
are ignored with stash and rebase, by calling update-index and
the diff programs with the freshly introduced --ignore-submodules option.

Johannes Schindelin (3):
  diff options: Introduce --ignore-submodules
  Teach update-index about --ignore-submodules
  Ignore dirty submodule states during rebase and stash

 Documentation/diff-options.txt     |    3 +
 Documentation/git-update-index.txt |    5 ++
 builtin-update-index.c             |    4 ++
 cache.h                            |    1 +
 diff.c                             |    9 ++++
 diff.h                             |    1 +
 git-rebase--interactive.sh         |   11 ++--
 git-rebase.sh                      |    8 ++--
 git-stash.sh                       |    6 +-
 read-cache.c                       |    4 ++
 t/t7402-submodule-rebase.sh        |   92 ++++++++++++++++++++++++++++++++++++
 11 files changed, 132 insertions(+), 12 deletions(-)
 create mode 100755 t/t7402-submodule-rebase.sh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 17:03 [PATCH 0/3] sane rebase/stash support with submodules Johannes Schindelin
@ 2008-05-14 17:03 ` Johannes Schindelin
  2008-05-14 18:28   ` Junio C Hamano
  2008-05-14 17:03 ` [PATCH 2/3] Teach update-index about --ignore-submodules Johannes Schindelin
  2008-05-14 17:03 ` [PATCH 3/3] Ignore dirty submodule states during rebase and stash Johannes Schindelin
  2 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-14 17:03 UTC (permalink / raw)
  To: git, gitster


The new option --ignore-submodules can now be used to ignore changes in
submodules.

Why?  Sometimes it is not interesting when a submodule changed.

For example, when reordering some commits in the superproject, a dirty
submodule is usually totally uninteresting.  So we will use this option
in git-rebase to test for a dirty working tree.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/diff-options.txt |    3 +++
 diff.c                         |    9 +++++++++
 diff.h                         |    1 +
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 13234fa..859d679 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -228,6 +228,9 @@ endif::git-format-patch[]
 --no-ext-diff::
 	Disallow external diff drivers.
 
+--ignore-submodules::
+	Ignore changes to submodules in the diff generation.
+
 --src-prefix=<prefix>::
 	Show the given source prefix instead of "a/".
 
diff --git a/diff.c b/diff.c
index 439d474..d57bc29 100644
--- a/diff.c
+++ b/diff.c
@@ -2496,6 +2496,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
 		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
+	else if (!strcmp(arg, "--ignore-submodules"))
+		DIFF_OPT_SET(options, IGNORE_SUBMODULES);
 
 	/* misc options */
 	else if (!strcmp(arg, "-z"))
@@ -3355,6 +3357,9 @@ void diff_addremove(struct diff_options *options,
 	char concatpath[PATH_MAX];
 	struct diff_filespec *one, *two;
 
+	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
+		return;
+
 	/* This may look odd, but it is a preparation for
 	 * feeding "there are unchanged files which should
 	 * not produce diffs, but when you are doing copy
@@ -3399,6 +3404,10 @@ void diff_change(struct diff_options *options,
 	char concatpath[PATH_MAX];
 	struct diff_filespec *one, *two;
 
+	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
+			&& S_ISGITLINK(new_mode))
+		return;
+
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
 		unsigned tmp;
 		const unsigned char *tmp_c;
diff --git a/diff.h b/diff.h
index 3a02d38..1dfe1f9 100644
--- a/diff.h
+++ b/diff.h
@@ -63,6 +63,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_REVERSE_DIFF        (1 << 15)
 #define DIFF_OPT_CHECK_FAILED        (1 << 16)
 #define DIFF_OPT_RELATIVE_NAME       (1 << 17)
+#define DIFF_OPT_IGNORE_SUBMODULES   (1 << 18)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
-- 
1.5.5.1.375.g1becb

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] Teach update-index about --ignore-submodules
  2008-05-14 17:03 [PATCH 0/3] sane rebase/stash support with submodules Johannes Schindelin
  2008-05-14 17:03 ` [PATCH 1/3] diff options: Introduce --ignore-submodules Johannes Schindelin
@ 2008-05-14 17:03 ` Johannes Schindelin
  2008-05-14 17:03 ` [PATCH 3/3] Ignore dirty submodule states during rebase and stash Johannes Schindelin
  2 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-14 17:03 UTC (permalink / raw)
  To: git, gitster


Like with the diff machinery, update-index should sometimes just
ignore submodules (e.g. to determine a clean state before a rebase).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-update-index.txt |    5 +++++
 builtin-update-index.c             |    4 ++++
 cache.h                            |    1 +
 read-cache.c                       |    4 ++++
 4 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 66be18e..0664060 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	     [--cacheinfo <mode> <object> <file>]\*
 	     [--chmod=(+|-)x]
 	     [--assume-unchanged | --no-assume-unchanged]
+	     [--ignore-submodules]
 	     [--really-refresh] [--unresolve] [--again | -g]
 	     [--info-only] [--index-info]
 	     [-z] [--stdin]
@@ -54,6 +55,10 @@ OPTIONS
         default behavior is to error out.  This option makes
         git-update-index continue anyway.
 
+--ignore-submodules:
+	Do not try to update submodules.  This option is only respected
+	when passed before --refresh.
+
 --unmerged::
         If --refresh finds unmerged changes in the index, the default
         behavior is to error out.  This option makes git-update-index
diff --git a/builtin-update-index.c b/builtin-update-index.c
index a8795d3..d4c85c0 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -593,6 +593,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 				refresh_flags |= REFRESH_QUIET;
 				continue;
 			}
+			if (!strcmp(path, "--ignore-submodules")) {
+				refresh_flags |= REFRESH_IGNORE_SUBMODULES;
+				continue;
+			}
 			if (!strcmp(path, "--add")) {
 				allow_add = 1;
 				continue;
diff --git a/cache.h b/cache.h
index c761915..3d4e8e7 100644
--- a/cache.h
+++ b/cache.h
@@ -393,6 +393,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
 #define REFRESH_UNMERGED	0x0002	/* allow unmerged */
 #define REFRESH_QUIET		0x0004	/* be quiet about it */
 #define REFRESH_IGNORE_MISSING	0x0008	/* ignore non-existent */
+#define REFRESH_IGNORE_SUBMODULES	0x0008	/* ignore submodules */
 extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen);
 
 struct lock_file {
diff --git a/read-cache.c b/read-cache.c
index 8b467f8..bc03981 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -942,6 +942,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 	int allow_unmerged = (flags & REFRESH_UNMERGED) != 0;
 	int quiet = (flags & REFRESH_QUIET) != 0;
 	int not_new = (flags & REFRESH_IGNORE_MISSING) != 0;
+	int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;
 	unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
 
 	for (i = 0; i < istate->cache_nr; i++) {
@@ -949,6 +950,9 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 		int cache_errno = 0;
 
 		ce = istate->cache[i];
+		if (ignore_submodules && S_ISGITLINK(ce->ce_mode))
+			continue;
+
 		if (ce_stage(ce)) {
 			while ((i < istate->cache_nr) &&
 			       ! strcmp(istate->cache[i]->name, ce->name))
-- 
1.5.5.1.375.g1becb

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] Ignore dirty submodule states during rebase and stash
  2008-05-14 17:03 [PATCH 0/3] sane rebase/stash support with submodules Johannes Schindelin
  2008-05-14 17:03 ` [PATCH 1/3] diff options: Introduce --ignore-submodules Johannes Schindelin
  2008-05-14 17:03 ` [PATCH 2/3] Teach update-index about --ignore-submodules Johannes Schindelin
@ 2008-05-14 17:03 ` Johannes Schindelin
  2 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-14 17:03 UTC (permalink / raw)
  To: git, gitster


When rebasing or stashing, chances are that you do not care about
dirty submodules, since they are not updated by those actions anyway.
So ignore the submodules' states.

Note: the submodule states -- as committed in the superproject --
will still be stashed and rebased, it is _just_ the state of the
submodule in the working tree which is ignored.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-rebase--interactive.sh  |   11 +++--
 git-rebase.sh               |    8 ++--
 git-stash.sh                |    6 +-
 t/t7402-submodule-rebase.sh |   92 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 12 deletions(-)
 create mode 100755 t/t7402-submodule-rebase.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2a01182..a9daa1b 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -56,9 +56,9 @@ output () {
 require_clean_work_tree () {
 	# test if working tree is dirty
 	git rev-parse --verify HEAD > /dev/null &&
-	git update-index --refresh &&
-	git diff-files --quiet &&
-	git diff-index --cached --quiet HEAD -- ||
+	git update-index --ignore-submodules --refresh &&
+	git diff-files --quiet --ignore-submodules &&
+	git diff-index --cached --quiet HEAD --ignore-submodules -- ||
 	die "Working tree is dirty"
 }
 
@@ -526,11 +526,12 @@ do
 		# Sanity check
 		git rev-parse --verify HEAD >/dev/null ||
 			die "Cannot read HEAD"
-		git update-index --refresh && git diff-files --quiet ||
+		git update-index --ignore-submodules --refresh &&
+			git diff-files --quiet --ignore-submodules ||
 			die "Working tree is dirty"
 
 		# do we have anything to commit?
-		if git diff-index --cached --quiet HEAD --
+		if git diff-index --cached --quiet --ignore-submodules HEAD --
 		then
 			: Nothing to commit -- skip this
 		else
diff --git a/git-rebase.sh b/git-rebase.sh
index 68855c1..dd7dfe1 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -60,7 +60,7 @@ continue_merge () {
 	fi
 
 	cmt=`cat "$dotest/current"`
-	if ! git diff-index --quiet HEAD --
+	if ! git diff-index --quiet --ignore-submodules HEAD --
 	then
 		if ! git commit --no-verify -C "$cmt"
 		then
@@ -150,7 +150,7 @@ while test $# != 0
 do
 	case "$1" in
 	--continue)
-		git diff-files --quiet || {
+		git diff-files --quiet --ignore-submodules || {
 			echo "You must edit all merge conflicts and then"
 			echo "mark them as resolved using git add"
 			exit 1
@@ -282,8 +282,8 @@ else
 fi
 
 # The tree must be really really clean.
-git update-index --refresh || exit
-diff=$(git diff-index --cached --name-status -r HEAD --)
+git update-index --ignore-submodules --refresh || exit
+diff=$(git diff-index --cached --name-status -r --ignore-submodules HEAD --)
 case "$diff" in
 ?*)	echo "cannot rebase: your index is not up-to-date"
 	echo "$diff"
diff --git a/git-stash.sh b/git-stash.sh
index c2b6820..4938ade 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -15,8 +15,8 @@ trap 'rm -f "$TMP-*"' 0
 ref_stash=refs/stash
 
 no_changes () {
-	git diff-index --quiet --cached HEAD -- &&
-	git diff-files --quiet
+	git diff-index --quiet --cached HEAD --ignore-submodules -- &&
+	git diff-files --quiet --ignore-submodules
 }
 
 clear_stash () {
@@ -130,7 +130,7 @@ show_stash () {
 }
 
 apply_stash () {
-	git diff-files --quiet ||
+	git diff-files --quiet --ignore-submodules ||
 		die 'Cannot restore on top of a dirty state'
 
 	unstash_index=
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
new file mode 100755
index 0000000..5becb3e
--- /dev/null
+++ b/t/t7402-submodule-rebase.sh
@@ -0,0 +1,92 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Johannes Schindelin
+#
+
+test_description='Test rebasing and stashing with dirty submodules'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	echo file > file &&
+	git add file &&
+	test_tick &&
+	git commit -m initial &&
+	git clone . submodule &&
+	git add submodule &&
+	test_tick &&
+	git commit -m submodule &&
+	echo second line >> file &&
+	(cd submodule && git pull) &&
+	test_tick &&
+	git commit -m file-and-submodule -a
+
+'
+
+test_expect_success 'rebase with a dirty submodule' '
+
+	(cd submodule &&
+	 echo 3rd line >> file &&
+	 test_tick &&
+	 git commit -m fork -a) &&
+	echo unrelated >> file2 &&
+	git add file2 &&
+	test_tick &&
+	git commit -m unrelated file2 &&
+	echo other line >> file &&
+	test_tick &&
+	git commit -m update file &&
+	CURRENT=$(cd submodule && git rev-parse HEAD) &&
+	EXPECTED=$(git rev-parse HEAD~2:submodule) &&
+	GIT_TRACE=1 git rebase --onto HEAD~2 HEAD^ &&
+	STORED=$(git rev-parse HEAD:submodule) &&
+	test $EXPECTED = $STORED &&
+	test $CURRENT = $(cd submodule && git rev-parse HEAD)
+
+'
+
+cat > fake-editor.sh << \EOF
+#!/bin/sh
+echo $EDITOR_TEXT
+EOF
+chmod a+x fake-editor.sh
+
+test_expect_success 'interactive rebase with a dirty submodule' '
+
+	test submodule = $(git diff --name-only) &&
+	HEAD=$(git rev-parse HEAD) &&
+	GIT_EDITOR="\"$(pwd)/fake-editor.sh\"" EDITOR_TEXT="pick $HEAD" \
+		git rebase -i HEAD^ &&
+	test submodule = $(git diff --name-only)
+
+'
+
+test_expect_success 'rebase with dirty file and submodule fails' '
+
+	echo yet another line >> file &&
+	test_tick &&
+	git commit -m next file &&
+	echo rewrite > file &&
+	test_tick &&
+	git commit -m rewrite file &&
+	echo dirty > file &&
+	! git rebase --onto HEAD~2 HEAD^
+
+'
+
+test_expect_success 'stash with a dirty submodule' '
+
+	echo new > file &&
+	CURRENT=$(cd submodule && git rev-parse HEAD) &&
+	git stash &&
+	test new != $(cat file) &&
+	test submodule = $(git diff --name-only) &&
+	test $CURRENT = $(cd submodule && git rev-parse HEAD) &&
+	git stash apply &&
+	test new = $(cat file) &&
+	test $CURRENT = $(cd submodule && git rev-parse HEAD)
+
+'
+
+test_done
-- 
1.5.5.1.375.g1becb

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 17:03 ` [PATCH 1/3] diff options: Introduce --ignore-submodules Johannes Schindelin
@ 2008-05-14 18:28   ` Junio C Hamano
  2008-05-14 18:42     ` Johannes Schindelin
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-05-14 18:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> @@ -3355,6 +3357,9 @@ void diff_addremove(struct diff_options *options,
>  	char concatpath[PATH_MAX];
>  	struct diff_filespec *one, *two;
>  
> +	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
> +		return;
> +
>  	/* This may look odd, but it is a preparation for
>  	 * feeding "there are unchanged files which should
>  	 * not produce diffs, but when you are doing copy

So both removal and addition of a submodule is an uninteresting event.

> @@ -3399,6 +3404,10 @@ void diff_change(struct diff_options *options,
>  	char concatpath[PATH_MAX];
>  	struct diff_filespec *one, *two;
>  
> +	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
> +			&& S_ISGITLINK(new_mode))
> +		return;

And a submodule changing from revision A to B is uninteresting.

Is it interesting if something that used to be a blob turns into a
submodule, or vice versa?  The code says it is, but I think it would be
more convenient to treat it as a removal of blob and addition of a
submodule.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 18:28   ` Junio C Hamano
@ 2008-05-14 18:42     ` Johannes Schindelin
  2008-05-14 19:17       ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-14 18:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Wed, 14 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > @@ -3355,6 +3357,9 @@ void diff_addremove(struct diff_options *options,
> >  	char concatpath[PATH_MAX];
> >  	struct diff_filespec *one, *two;
> >  
> > +	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
> > +		return;
> > +
> >  	/* This may look odd, but it is a preparation for
> >  	 * feeding "there are unchanged files which should
> >  	 * not produce diffs, but when you are doing copy
> 
> So both removal and addition of a submodule is an uninteresting event.

Yes.

> > @@ -3399,6 +3404,10 @@ void diff_change(struct diff_options *options,
> >  	char concatpath[PATH_MAX];
> >  	struct diff_filespec *one, *two;
> >  
> > +	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
> > +			&& S_ISGITLINK(new_mode))
> > +		return;
> 
> And a submodule changing from revision A to B is uninteresting.
> 
> Is it interesting if something that used to be a blob turns into a
> submodule, or vice versa?  The code says it is, but I think it would be
> more convenient to treat it as a removal of blob and addition of a
> submodule.

The point is: for the sake of a script (which wants to ignore 
submodules) asking if there is a diff, I think it makes sense to not 
ignore those changes.  IOW I think my patch is enough for the purpose of 
getting stash/rebase to behave.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 18:42     ` Johannes Schindelin
@ 2008-05-14 19:17       ` Junio C Hamano
  2008-05-14 22:09         ` Johannes Schindelin
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-05-14 19:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The point is: for the sake of a script (which wants to ignore 
> submodules) asking if there is a diff, I think it makes sense to not 
> ignore those changes.  IOW I think my patch is enough for the purpose of 
> getting stash/rebase to behave.

But the patch is not about stash/rebase but affects all diff users,
doesn't it?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 19:17       ` Junio C Hamano
@ 2008-05-14 22:09         ` Johannes Schindelin
  2008-05-14 22:12           ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-14 22:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Wed, 14 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The point is: for the sake of a script (which wants to ignore 
> > submodules) asking if there is a diff, I think it makes sense to not 
> > ignore those changes.  IOW I think my patch is enough for the purpose 
> > of getting stash/rebase to behave.
> 
> But the patch is not about stash/rebase but affects all diff users, 
> doesn't it?

Does it?  I thought I hid all that special handling behind the 
--ignore-submodules options.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 22:09         ` Johannes Schindelin
@ 2008-05-14 22:12           ` Junio C Hamano
  2008-05-15  1:09             ` Johannes Schindelin
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-05-14 22:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Wed, 14 May 2008, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > The point is: for the sake of a script (which wants to ignore 
>> > submodules) asking if there is a diff, I think it makes sense to not 
>> > ignore those changes.  IOW I think my patch is enough for the purpose 
>> > of getting stash/rebase to behave.
>> 
>> But the patch is not about stash/rebase but affects all diff users, 
>> doesn't it?
>
> Does it?  I thought I hid all that special handling behind the 
> --ignore-submodules options.

That's exactly the point.  The option reads "ignore submodules", not
"special option for use only by stash and rebase".

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-14 22:12           ` Junio C Hamano
@ 2008-05-15  1:09             ` Johannes Schindelin
  2008-05-15  1:50               ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-15  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Wed, 14 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Wed, 14 May 2008, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> > The point is: for the sake of a script (which wants to ignore 
> >> > submodules) asking if there is a diff, I think it makes sense to 
> >> > not ignore those changes.  IOW I think my patch is enough for the 
> >> > purpose of getting stash/rebase to behave.
> >> 
> >> But the patch is not about stash/rebase but affects all diff users, 
> >> doesn't it?
> >
> > Does it?  I thought I hid all that special handling behind the 
> > --ignore-submodules options.
> 
> That's exactly the point.  The option reads "ignore submodules", not 
> "special option for use only by stash and rebase".

But it also does not read "ignore submodules and those blobs/trees that 
happen to replace (or be replaced by) submodules".

I think it is not a bad thing to call it "ignore submodules", but show the 
submodules when something else than a submodule is involved, too.

I see where you are heading with the "split diff into removal of submodule 
/ addition of a blob", but I think that would be overly clever, and not 
helpful.  I know that I would want to have that marked as a problem when 
rebasing.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-15  1:09             ` Johannes Schindelin
@ 2008-05-15  1:50               ` Junio C Hamano
  2008-05-15 13:58                 ` Johannes Schindelin
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-05-15  1:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> That's exactly the point.  The option reads "ignore submodules", not 
>> "special option for use only by stash and rebase".
>
> But it also does not read "ignore submodules and those blobs/trees that 
> happen to replace (or be replaced by) submodules".

I think "ignore submodules" option, if exists, would mean "ignore changes
that involve submodules", and I think everybody would agree with that.

Because we are talking about an option to "diff", changes involve two
sides (preimage and postimage).  Logically, you can define the class of
changes that involve submodules in two ways:

 * both sides of the change must be a submodule; otherwise the change does
   not involve a submodule.

 * either side of the change is a submodule; such a change involve a
   submodule.

I am saying that the latter makes much more sense (worse yet, what you did
for creation and deletion is inconsistent --- they are void vs submodule
and you treat them as "change that involve submodule").

Didn't you make the option introduction to read "sometimes you are not
interested in submodules..." or something?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] diff options: Introduce --ignore-submodules
  2008-05-15  1:50               ` Junio C Hamano
@ 2008-05-15 13:58                 ` Johannes Schindelin
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2008-05-15 13:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Wed, 14 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> That's exactly the point.  The option reads "ignore submodules", not 
> >> "special option for use only by stash and rebase".
> >
> > But it also does not read "ignore submodules and those blobs/trees 
> > that happen to replace (or be replaced by) submodules".
> 
> I think "ignore submodules" option, if exists, would mean "ignore 
> changes that involve submodules", and I think everybody would agree with 
> that.
> 
> Because we are talking about an option to "diff", changes involve two
> sides (preimage and postimage).  Logically, you can define the class of
> changes that involve submodules in two ways:
> 
>  * both sides of the change must be a submodule; otherwise the change does
>    not involve a submodule.
> 
>  * either side of the change is a submodule; such a change involve a
>    submodule.
> 
> I am saying that the latter makes much more sense (worse yet, what you 
> did for creation and deletion is inconsistent --- they are void vs 
> submodule and you treat them as "change that involve submodule").

Okay, maybe the introduction, and the intuitive understanding of "ignore 
submodules" is the latter.

But I maintain that there is no sensible operation you would need it 
for.

So, how about --only-non-submodules?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-05-15 13:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-14 17:03 [PATCH 0/3] sane rebase/stash support with submodules Johannes Schindelin
2008-05-14 17:03 ` [PATCH 1/3] diff options: Introduce --ignore-submodules Johannes Schindelin
2008-05-14 18:28   ` Junio C Hamano
2008-05-14 18:42     ` Johannes Schindelin
2008-05-14 19:17       ` Junio C Hamano
2008-05-14 22:09         ` Johannes Schindelin
2008-05-14 22:12           ` Junio C Hamano
2008-05-15  1:09             ` Johannes Schindelin
2008-05-15  1:50               ` Junio C Hamano
2008-05-15 13:58                 ` Johannes Schindelin
2008-05-14 17:03 ` [PATCH 2/3] Teach update-index about --ignore-submodules Johannes Schindelin
2008-05-14 17:03 ` [PATCH 3/3] Ignore dirty submodule states during rebase and stash Johannes Schindelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).