All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFC] git submodule: add submodules with git add -f <path>
@ 2010-07-02 19:22 Ævar Arnfjörð Bjarmason
  2010-07-05 17:33 ` [PATCH] " Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 10+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-02 19:22 UTC (permalink / raw)
  To: git; +Cc: Ævar Arnfjörð Bjarmason

Change `git submodule add' to add the new submodule <path> with `git
add --force'.

I keep my /etc in .git with a .gitignore that contains just
"*". I.e. `git status' will ignore everything that isn't in the tree
already. When I do:

    git submodule add <url> hlagh

git-submodule will get as far as checking out the remote repository
into hlagh, but it'll die right afterwards when it fails to add the
new path:

    The following paths are ignored by one of your .gitignore files:
    hlagh
    Use -f if you really want to add them.
    fatal: no files added
    Failed to add submodule 'hlagh'

Currently there's no way to add a submodule in this situation other
than to remove the ignored path from the .gitignore while I'm at it.

That's silly, when you run `git submodule add' you're explicitly
saying that you want to add something *new* to the repository. Instead
it should just add the path with `git add --force'.

Initially I implemented this by adding new -f and --force options to
`git submodule add'. But if the --force option isn't supplied it'll
get as far as cloning `hlagh', but won't add it.

So the first thing the user has to do is to remove `hlagh' and then
try again with the --force option.

That sucks, it should just add the path to begin with. I can't think
of any usecase where you've gone through the trouble of typing out
`git submodule add ..', but wish to be overriden by a `gitignore'. The
submodule semantics should be more like `git init', not `git add'.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-submodule.txt |    4 ++++
 git-submodule.sh                |    4 ++--
 t/t7400-submodule-basic.sh      |   24 +++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index cdabfd2..76a832a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -95,6 +95,10 @@ is the superproject and submodule repositories will be kept
 together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
++
+The submodule will be added with "git add --force <path>". I.e. git
+doesn't care if the new path is in a `gitignore`. Your invocation of
+"git submodule add" is considered enough to override it.
 
 status::
 	Show the status of the submodules. This will print the SHA-1 of the
diff --git a/git-submodule.sh b/git-submodule.sh
index d9950c2..ad2417d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -234,12 +234,12 @@ cmd_add()
 		) || die "Unable to checkout submodule '$path'"
 	fi
 
-	git add "$path" ||
+	git add --force "$path" ||
 	die "Failed to add submodule '$path'"
 
 	git config -f .gitmodules submodule."$path".path "$path" &&
 	git config -f .gitmodules submodule."$path".url "$repo" &&
-	git add .gitmodules ||
+	git add --force .gitmodules ||
 	die "Failed to register submodule '$path'"
 }
 
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 97ff074..d9f2785 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -42,7 +42,8 @@ test_expect_success 'setup - hide init subdirectory' '
 '
 
 test_expect_success 'setup - repository to add submodules to' '
-	git init addtest
+	git init addtest &&
+	git init addtest-ignore
 '
 
 # The 'submodule add' tests need some repository to add as a submodule.
@@ -85,6 +86,27 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'submodule add to .gitignored path' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
+	(
+		cd addtest-ignore &&
+		# Does not use test_commit due to the ignore
+		echo "*" > .gitignore &&
+		git add --force .gitignore &&
+		git commit -m"Ignore everything" &&
+		git submodule add "$submodurl" submod &&
+		git submodule init
+	) &&
+
+	rm -f heads head untracked &&
+	inspect addtest/submod ../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
+'
+
 test_expect_success 'submodule add --branch' '
 	echo "refs/heads/initial" >expect-head &&
 	cat <<-\EOF >expect-heads &&
-- 
1.7.2.rc1.192.g262ff.dirty

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

* [PATCH] git submodule: add submodules with git add -f <path>
  2010-07-02 19:22 [PATCH/RFC] git submodule: add submodules with git add -f <path> Ævar Arnfjörð Bjarmason
@ 2010-07-05 17:33 ` Ævar Arnfjörð Bjarmason
  2010-07-06  2:36   ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-05 17:33 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

Change `git submodule add' to add the new submodule <path> with `git
add --force'.

I keep my /etc in .git with a .gitignore that contains just
"*". I.e. `git status' will ignore everything that isn't in the tree
already. When I do:

    git submodule add <url> hlagh

git-submodule will get as far as checking out the remote repository
into hlagh, but it'll die right afterwards when it fails to add the
new path:

    The following paths are ignored by one of your .gitignore files:
    hlagh
    Use -f if you really want to add them.
    fatal: no files added
    Failed to add submodule 'hlagh'

Currently there's no way to add a submodule in this situation other
than to remove the ignored path from the .gitignore while I'm at it.

That's silly, when you run `git submodule add' you're explicitly
saying that you want to add something *new* to the repository. Instead
it should just add the path with `git add --force'.

Initially I implemented this by adding new -f and --force options to
`git submodule add'. But if the --force option isn't supplied it'll
get as far as cloning `hlagh', but won't add it.

So the first thing the user has to do is to remove `hlagh' and then
try again with the --force option.

That sucks, it should just add the path to begin with. I can't think
of any usecase where you've gone through the trouble of typing out
`git submodule add ..', but wish to be overriden by a `gitignore'. The
submodule semantics should be more like `git init', not `git add'.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Resubmitting this as a non-RFC. I think it's sane and ready for
inclusion.

 Documentation/git-submodule.txt |    4 ++++
 git-submodule.sh                |    4 ++--
 t/t7400-submodule-basic.sh      |   24 +++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index cdabfd2..76a832a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -95,6 +95,10 @@ is the superproject and submodule repositories will be kept
 together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
++
+The submodule will be added with "git add --force <path>". I.e. git
+doesn't care if the new path is in a `gitignore`. Your invocation of
+"git submodule add" is considered enough to override it.
 
 status::
 	Show the status of the submodules. This will print the SHA-1 of the
diff --git a/git-submodule.sh b/git-submodule.sh
index d9950c2..ad2417d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -234,12 +234,12 @@ cmd_add()
 		) || die "Unable to checkout submodule '$path'"
 	fi
 
-	git add "$path" ||
+	git add --force "$path" ||
 	die "Failed to add submodule '$path'"
 
 	git config -f .gitmodules submodule."$path".path "$path" &&
 	git config -f .gitmodules submodule."$path".url "$repo" &&
-	git add .gitmodules ||
+	git add --force .gitmodules ||
 	die "Failed to register submodule '$path'"
 }
 
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 97ff074..d9f2785 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -42,7 +42,8 @@ test_expect_success 'setup - hide init subdirectory' '
 '
 
 test_expect_success 'setup - repository to add submodules to' '
-	git init addtest
+	git init addtest &&
+	git init addtest-ignore
 '
 
 # The 'submodule add' tests need some repository to add as a submodule.
@@ -85,6 +86,27 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'submodule add to .gitignored path' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
+	(
+		cd addtest-ignore &&
+		# Does not use test_commit due to the ignore
+		echo "*" > .gitignore &&
+		git add --force .gitignore &&
+		git commit -m"Ignore everything" &&
+		git submodule add "$submodurl" submod &&
+		git submodule init
+	) &&
+
+	rm -f heads head untracked &&
+	inspect addtest/submod ../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
+'
+
 test_expect_success 'submodule add --branch' '
 	echo "refs/heads/initial" >expect-head &&
 	cat <<-\EOF >expect-heads &&
-- 
1.7.2.rc1.192.g262ff.dirty

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

* Re: [PATCH] git submodule: add submodules with git add -f <path>
  2010-07-05 17:33 ` [PATCH] " Ævar Arnfjörð Bjarmason
@ 2010-07-06  2:36   ` Junio C Hamano
  2010-07-06 21:51     ` Jens Lehmann
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-07-06  2:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Resubmitting this as a non-RFC.

I don't recall any negative nor supporting discussion on this one.  Will
queue in 'pu' to see if people who do care about submodules complain.

Thanks.

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

* Re: [PATCH] git submodule: add submodules with git add -f <path>
  2010-07-06  2:36   ` Junio C Hamano
@ 2010-07-06 21:51     ` Jens Lehmann
  2010-07-06 22:33       ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Lehmann @ 2010-07-06 21:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git

Am 06.07.2010 04:36, schrieb Junio C Hamano:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> 
>> Resubmitting this as a non-RFC.
> 
> I don't recall any negative nor supporting discussion on this one.  Will
> queue in 'pu' to see if people who do care about submodules complain.

First: I think this patch fixes a bug. Without it "git submodule add"
leaves the work tree in an inconsistent state when either .gitmodules
or the submodule path are ignored: the submodule is checked out and
populated but the path and/or a new .gitmodules file is not added to
the index. And even worse: The advice of the failed "git add" called
from the script that using "-f" might help is misleading here, as
"git submodule add" doesn't know this option.

But while I think adding the --force option to the "git add .gitmodules"
makes perfect sense (as the submodule can't be successfully added until
it is recorded in this file and there is really no point in ignoring
.gitmodules when you decide to use submodules), I'm not so sure about
what to do when the submodule path itself is ignored.

I see two possible behaviors here:

a) We just ignore .gitignore and add the submodule anyways (which is
   what this patch does)

b) We do the same a "git add <ignored file>" does: Print an error
   message, maybe even tell the user to use a - still to be added -
   "--force" (or "-f") option and exit. But without checking out the
   submodule first nor adding or changing .gitmodules.

IMHO b) is more consistent with the current behavior of "git add". And
when you later decide that the submodules files should live in the
superproject and you drop the submodule, the then probably still
present entry in .gitignore might really shoot you in the foot when
you add new files there and they won't show up because they are still
ignored.

What do others think?

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

* Re: [PATCH] git submodule: add submodules with git add -f <path>
  2010-07-06 21:51     ` Jens Lehmann
@ 2010-07-06 22:33       ` Ævar Arnfjörð Bjarmason
  2010-07-09 22:18         ` [PATCH] git add: Add the "--ignore-missing" option for the dry run Jens Lehmann
  0 siblings, 1 reply; 10+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-06 22:33 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Junio C Hamano, git

On Tue, Jul 6, 2010 at 21:51, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> [...]
> But while I think adding the --force option to the "git add .gitmodules"
> makes perfect sense (as the submodule can't be successfully added until
> it is recorded in this file and there is really no point in ignoring
> .gitmodules when you decide to use submodules), I'm not so sure about
> what to do when the submodule path itself is ignored.
>
> I see two possible behaviors here:
>
> a) We just ignore .gitignore and add the submodule anyways (which is
>   what this patch does)
>
> b) We do the same a "git add <ignored file>" does: Print an error
>   message, maybe even tell the user to use a - still to be added -
>   "--force" (or "-f") option and exit. But without checking out the
>   submodule first nor adding or changing .gitmodules.
>
> IMHO b) is more consistent with the current behavior of "git add". And
> when you later decide that the submodules files should live in the
> superproject and you drop the submodule, the then probably still
> present entry in .gitignore might really shoot you in the foot when
> you add new files there and they won't show up because they are still
> ignored.
>
> What do others think?

Option C would be treating it like git init as the current patch
does. But init isn't strictly comparable to git add or git submodule
add since it's not adding something to *another* repository.

I really don't care, B or C works for me, although C is of course
easier since I don't have to write another patch :)

Option b) is more consistent with git-add, but I can't find a way to
ask any git tool whether a non-existing path is ignored without
actually adding something. git add --dry-run will die on "pathspec
'foo' did not match any files" unless the file exists already.

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

* [PATCH] git add: Add the "--ignore-missing" option for the dry run
  2010-07-06 22:33       ` Ævar Arnfjörð Bjarmason
@ 2010-07-09 22:18         ` Jens Lehmann
  2010-07-12 22:14           ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Lehmann @ 2010-07-09 22:18 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git

Sometimes it is useful to know if a file or directory will be ignored
before it is added to the work tree. An example is "git submodule add",
where it would be really nice to be able to fail with an appropriate
error message before the submodule is cloned and checked out.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 07.07.2010 00:33, schrieb Ævar Arnfjörð Bjarmason:
> Option b) is more consistent with git-add, but I can't find a way to
> ask any git tool whether a non-existing path is ignored without
> actually adding something. git add --dry-run will die on "pathspec
> 'foo' did not match any files" unless the file exists already.

Right, and as IMHO creating a directory with the same name of the
submodule and then deleting it again after the "git add --dry-run"
is way too hackish, what about adding a "--ignore-missing" option to
"git add", which ignores any missing files when used with the option
"--dry-run"?

(And to add some bikeshedding-fodder: I also thought about naming
that option "--dry-run=ignore-missing" or "--only-ignored". I really
don't have any strong feelings about the particular naming, so if
anyone has a better idea, please speak up!)

With this patch it should be easy to have "git submodule add" return
an error /before/ adding a submodule path and its contents when it
is found in .gitignore.

Opinions?


 Documentation/git-add.txt |    9 ++++++++-
 builtin/add.c             |   16 ++++++++++++----
 dir.c                     |    2 +-
 dir.h                     |    1 +
 t/t3700-add.sh            |   25 +++++++++++++++++++++++++
 5 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 74741a4..bfea2c2 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -57,7 +57,8 @@ OPTIONS

 -n::
 --dry-run::
-        Don't actually add the file(s), just show if they exist.
+	Don't actually add the file(s), just show if they exist and/or will
+	be ignored.

 -v::
 --verbose::
@@ -131,6 +132,12 @@ subdirectories.
 	them, do not abort the operation, but continue adding the
 	others. The command shall still exit with non-zero status.

+--ignore-missing::
+	This option can only be used together with --dry-run. By using
+	this option the user can check if any of the given files would
+	be ignored, no matter if they are already present in the work
+	tree or not.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 17149cf..56a4e0a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -310,7 +310,7 @@ static const char ignore_error[] =
 "The following paths are ignored by one of your .gitignore files:\n";

 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
-static int ignore_add_errors, addremove, intent_to_add;
+static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;

 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only),
@@ -325,6 +325,7 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
+	OPT_BOOLEAN( 0 , "ignore-missing", &ignore_missing, "check if - even missing - files are ignored in dry run"),
 	OPT_END(),
 };

@@ -385,6 +386,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)

 	if (addremove && take_worktree_changes)
 		die("-A and -u are mutually incompatible");
+	if (!show_only && ignore_missing)
+		die("Option --ignore-missing can only be used together with --dry-run");
 	if ((addremove || take_worktree_changes) && !argc) {
 		static const char *here[2] = { ".", NULL };
 		argc = 1;
@@ -441,9 +444,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 			seen = find_used_pathspec(pathspec);
 		for (i = 0; pathspec[i]; i++) {
 			if (!seen[i] && pathspec[i][0]
-			    && !file_exists(pathspec[i]))
-				die("pathspec '%s' did not match any files",
-				    pathspec[i]);
+			    && !file_exists(pathspec[i])) {
+				if (ignore_missing) {
+					if (excluded(&dir, pathspec[i], DT_UNKNOWN))
+						dir_add_ignored(&dir, pathspec[i], strlen(pathspec[i]));
+				} else
+					die("pathspec '%s' did not match any files",
+					    pathspec[i]);
+			}
 		}
 		free(seen);
 	}
diff --git a/dir.c b/dir.c
index 151ea67..133f472 100644
--- a/dir.c
+++ b/dir.c
@@ -453,7 +453,7 @@ static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
 	return dir->entries[dir->nr++] = dir_entry_new(pathname, len);
 }

-static struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)
+struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)
 {
 	if (!cache_name_is_other(pathname, len))
 		return NULL;
diff --git a/dir.h b/dir.h
index 3bead5f..278d84c 100644
--- a/dir.h
+++ b/dir.h
@@ -72,6 +72,7 @@ extern int read_directory(struct dir_struct *, const char *path, int len, const
 extern int excluded_from_list(const char *pathname, int pathlen, const char *basename,
 			      int *dtype, struct exclude_list *el);
 extern int excluded(struct dir_struct *, const char *, int *);
+struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);
 extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,
 					  char **buf_p, struct exclude_list *which, int check_index);
 extern void add_excludes_from_file(struct dir_struct *, const char *fname);
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 6f031af..47fbf53 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -260,4 +260,29 @@ test_expect_success '"add non-existent" should fail' '
 	! (git ls-files | grep "non-existent")
 '

+test_expect_success 'git add --dry-run of existing changed file' "
+	echo new >>track-this &&
+	git add --dry-run track-this >actual 2>&1 &&
+	echo \"add 'track-this'\" | test_cmp - actual
+"
+
+test_expect_success 'git add --dry-run of non-existing file' "
+	echo ignored-file >>.gitignore &&
+	! (git add --dry-run track-this ignored-file >actual 2>&1) &&
+	echo \"fatal: pathspec 'ignored-file' did not match any files\" | test_cmp - actual
+"
+
+cat >expect <<EOF
+The following paths are ignored by one of your .gitignore files:
+ignored-file
+Use -f if you really want to add them.
+fatal: no files added
+add 'track-this'
+EOF
+
+test_expect_success 'git add --dry-run --ignore-missing of non-existing file' '
+	!(git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1) &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.2.rc1.218.gca56a.dirty

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

* Re: [PATCH] git add: Add the "--ignore-missing" option for the dry run
  2010-07-09 22:18         ` [PATCH] git add: Add the "--ignore-missing" option for the dry run Jens Lehmann
@ 2010-07-12 22:14           ` Junio C Hamano
  2010-07-17 15:11             ` [PATCH] git submodule add: Require the new --force option to add ignored paths Jens Lehmann
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-07-12 22:14 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Ævar Arnfjörð Bjarmason, git

Jens Lehmann <Jens.Lehmann@web.de> writes:

> With this patch it should be easy to have "git submodule add" return
> an error /before/ adding a submodule path and its contents when it
> is found in .gitignore.
>
> Opinions?

Sounds like a right thing to do.

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

* [PATCH] git submodule add: Require the new --force option to add ignored paths
  2010-07-12 22:14           ` Junio C Hamano
@ 2010-07-17 15:11             ` Jens Lehmann
  2010-07-17 15:53               ` [PATCH] git submodule add: Remove old docs about implicit -f Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Lehmann @ 2010-07-17 15:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git

To make the behavior of "git submodule add" more consistent with "git add"
ignored submodule paths should not be silently added when they match an
entry in a .gitignore file. To be able to override that default behavior
in the same way as we can do that for "git add", the new option "--force"
is introduced.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 13.07.2010 00:14, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> With this patch it should be easy to have "git submodule add" return
>> an error /before/ adding a submodule path and its contents when it
>> is found in .gitignore.
>>
>> Opinions?
> 
> Sounds like a right thing to do.

So here we go. This patch applies to pu as it needs the
'git add: Add the "--ignore-missing" option for the dry run'
patch to work.


 Documentation/git-submodule.txt |    7 ++++++-
 git-submodule.sh                |   16 ++++++++++++++--
 t/t7400-submodule-basic.sh      |   27 +++++++++++++++------------
 3 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 76a832a..617069f 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git submodule' [--quiet] add [-b branch]
+'git submodule' [--quiet] add [-b branch] [-f|--force]
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
@@ -187,6 +187,11 @@ OPTIONS
 --branch::
 	Branch of repository to add as submodule.

+-f::
+--force::
+	This option is only valid for the add command.
+	Allow adding an otherwise ignored submodule path.
+
 --cached::
 	This option is only valid for status and summary commands.  These
 	commands typically use the commit found in the submodule HEAD, but
diff --git a/git-submodule.sh b/git-submodule.sh
index ad2417d..170186f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli

 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b branch] [--reference <repository>] [--] <repository> [<path>]
+USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
@@ -19,6 +19,7 @@ require_work_tree

 command=
 branch=
+force=
 reference=
 cached=
 recursive=
@@ -133,6 +134,9 @@ cmd_add()
 			branch=$2
 			shift
 			;;
+		-f | --force)
+			force=$1
+			;;
 		-q|--quiet)
 			GIT_QUIET=1
 			;;
@@ -201,6 +205,14 @@ cmd_add()
 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"

+	if test -z "$force" && ! git add --dry-run --ignore-missing "$path" > /dev/null 2>&1
+	then
+		echo >&2 "The following path is ignored by one of your .gitignore files:" &&
+		echo >&2 $path &&
+		echo >&2 "Use -f if you really want to add it."
+		exit 1
+	fi
+
 	# perhaps the path exists and is already a git repo, else clone it
 	if test -e "$path"
 	then
@@ -234,7 +246,7 @@ cmd_add()
 		) || die "Unable to checkout submodule '$path'"
 	fi

-	git add --force "$path" ||
+	git add $force "$path" ||
 	die "Failed to add submodule '$path'"

 	git config -f .gitmodules submodule."$path".path "$path" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index d9f2785..9bda970 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -86,25 +86,28 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '

-test_expect_success 'submodule add to .gitignored path' '
-	echo "refs/heads/master" >expect &&
-	>empty &&
-
+test_expect_success 'submodule add to .gitignored path fails' '
 	(
 		cd addtest-ignore &&
+		cat <<-\EOF >expect &&
+		The following path is ignored by one of your .gitignore files:
+		submod
+		Use -f if you really want to add it.
+		EOF
 		# Does not use test_commit due to the ignore
 		echo "*" > .gitignore &&
 		git add --force .gitignore &&
 		git commit -m"Ignore everything" &&
-		git submodule add "$submodurl" submod &&
-		git submodule init
-	) &&
+		! git submodule add "$submodurl" submod >actual 2>&1 &&
+		test_cmp expect actual
+	)
+'

-	rm -f heads head untracked &&
-	inspect addtest/submod ../.. &&
-	test_cmp expect heads &&
-	test_cmp expect head &&
-	test_cmp empty untracked
+test_expect_success 'submodule add to .gitignored path with --force' '
+	(
+		cd addtest-ignore &&
+		git submodule add --force "$submodurl" submod
+	)
 '

 test_expect_success 'submodule add --branch' '
-- 
1.7.2.rc3.262.gcf61

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

* [PATCH] git submodule add: Remove old docs about implicit -f
  2010-07-17 15:11             ` [PATCH] git submodule add: Require the new --force option to add ignored paths Jens Lehmann
@ 2010-07-17 15:53               ` Ævar Arnfjörð Bjarmason
  2010-07-17 16:26                 ` Jens Lehmann
  0 siblings, 1 reply; 10+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-17 15:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jens Lehmann, Ævar Arnfjörð Bjarmason

git submodule add no longer implicitly adds with --force. Remove
references to the old functionality in the documentation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

> To make the behavior of "git submodule add" more consistent with "git add"
> ignored submodule paths should not be silently added when they match an
> entry in a .gitignore file. To be able to override that default behavior
> in the same way as we can do that for "git add", the new option "--force"
> is introduced.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>

You beat me to it. I like this better than the original functionality
that I implemented, and it's now possible with --ignore-missing.

So,:

    Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

This amended patch removes some obsolete documentation that you
missed.

> +               echo >&2 "The following path is ignored by one of your .gitignore files:" &&
> +               echo >&2 $path &&
> +               echo >&2 "Use -f if you really want to add it."

Is the "it" intentional? We currently say "them" in git add regardless
of how many things are being added, so perhaps we should say "it"
there too for singulars.

I'll jot this down as something to look at for plural support once
gettext gets merged.

 Documentation/git-submodule.txt |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 617069f..1ed331c 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -95,10 +95,6 @@ is the superproject and submodule repositories will be kept
 together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
-+
-The submodule will be added with "git add --force <path>". I.e. git
-doesn't care if the new path is in a `gitignore`. Your invocation of
-"git submodule add" is considered enough to override it.
 
 status::
 	Show the status of the submodules. This will print the SHA-1 of the
-- 
1.7.2.rc3.125.g94e09.dirty

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

* Re: [PATCH] git submodule add: Remove old docs about implicit -f
  2010-07-17 15:53               ` [PATCH] git submodule add: Remove old docs about implicit -f Ævar Arnfjörð Bjarmason
@ 2010-07-17 16:26                 ` Jens Lehmann
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Lehmann @ 2010-07-17 16:26 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano

Am 17.07.2010 17:53, schrieb Ævar Arnfjörð Bjarmason:
> git submodule add no longer implicitly adds with --force. Remove
> references to the old functionality in the documentation.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

> This amended patch removes some obsolete documentation that you
> missed.

Yeah, thanks!

    Acked-by: Jens Lehmann <Jens.Lehmann@web.de>


>> +               echo >&2 "The following path is ignored by one of your .gitignore files:" &&
>> +               echo >&2 $path &&
>> +               echo >&2 "Use -f if you really want to add it."
> 
> Is the "it" intentional? We currently say "them" in git add regardless
> of how many things are being added, so perhaps we should say "it"
> there too for singulars.

Yes, the "it" is intentional. While "git add" may be called with one
or more files, "git submodule add" can only be called with one path.
That's why I chose the singular form here.

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

end of thread, other threads:[~2010-07-17 16:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-02 19:22 [PATCH/RFC] git submodule: add submodules with git add -f <path> Ævar Arnfjörð Bjarmason
2010-07-05 17:33 ` [PATCH] " Ævar Arnfjörð Bjarmason
2010-07-06  2:36   ` Junio C Hamano
2010-07-06 21:51     ` Jens Lehmann
2010-07-06 22:33       ` Ævar Arnfjörð Bjarmason
2010-07-09 22:18         ` [PATCH] git add: Add the "--ignore-missing" option for the dry run Jens Lehmann
2010-07-12 22:14           ` Junio C Hamano
2010-07-17 15:11             ` [PATCH] git submodule add: Require the new --force option to add ignored paths Jens Lehmann
2010-07-17 15:53               ` [PATCH] git submodule add: Remove old docs about implicit -f Ævar Arnfjörð Bjarmason
2010-07-17 16:26                 ` Jens Lehmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.