git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH/RFC] git add: do not add files from a submodule

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jul 21, 2008, 00:58 UTC
Message-ID
<alpine.DEB.1.00.0807210256510.3305@eeepc-johanness>
In-Reply-To
<alpine.DEB.1.00.0807201529150.3305@eeepc-johanness>

It comes quite as a surprise to an unsuspecting Git user that calling "git add submodule/file" (which is a mistake, alright) _removes_ the submodule in the index, and adds the file. Instead, complain loudly.

While at it, be nice when the user said "git add submodule/" which is most likely the consequence of tab-completion, and stage the submodule, instead of trying to add the contents of that directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	This would need a companion patch for "git diff submodule/",
	obviously.  However, revision.c does not need a valid cache,
	usually.  So I am hesitant.
	Oh, and I am sure somebody will come up with a more elegant 
	solution to this problem.  I sure do not, having smashed my head 
	against the wall for a few hours.
 builtin-add.c              |   42 +++++++++++++++++++++++++++++++++++++++---
 t/t7400-submodule-basic.sh |   18 ++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 6f5672a..9453557 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -50,6 +50,33 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
         free(seen);
 }
 
+static void treat_gitlinks(const char **pathspec, struct index_state *index)
+{
+	int i;
+
+	if (!pathspec || !*pathspec)
+		return;
+
+	for (i = 0; i < index->cache_nr; i++) {
+		struct cache_entry *ce = index->cache[i];
+		if (S_ISGITLINK(ce->ce_mode)) {
+			int len = ce_namelen(ce), j;
+			for (j = 0; pathspec[j]; j++) {
+				int len2 = strlen(pathspec[j]);
+				if (len2 <= len || pathspec[j][len] != '/' ||
+				    memcmp(ce->name, pathspec[j], len))
+					continue;
+				if (len2 == len + 1)
+					/* strip trailing slash */
+					pathspec[j] = xstrndup(ce->name, len);
+				else
+					die ("Path '%s' is in submodule '%.*s'",
+						pathspec[j], len, ce->name);
+			}
+		}
+	}
+}
+
 static void fill_directory(struct dir_struct *dir, const char **pathspec,
 		int ignored_too)
 {
@@ -245,6 +272,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int flags;
 	int add_new_files;
 	int require_pathspec;
+	struct index_state index;
 
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
@@ -283,12 +311,20 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	 * If we are adding new files, we need to scan the working
 	 * tree to find the ones that match pathspecs; this needs
 	 * to be done before we read the index.
+	 *
+	 * However, to avoid adding files from submodules, we have to
+	 * read the index first.  So read the index into a local
+	 * variable, and set the global index after fill_directory().
 	 */
-	if (add_new_files)
-		fill_directory(&dir, pathspec, ignored_too);
 
-	if (read_cache() < 0)
+	memset(&index, 0, sizeof(index));
+	if (read_index(&index) < 0)
 		die("index file corrupt");
+	treat_gitlinks(pathspec, &index);
+
+	if (add_new_files)
+		fill_directory(&dir, pathspec, ignored_too);
+	the_index = index;
 
 	if (refresh_only) {
 		refresh(verbose, pathspec);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index cbc0c34..6da2545 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -209,4 +209,22 @@ test_expect_success 'update --init' '
 
 '
 
+test_expect_success 'do not add files from a submodule' '
+
+	git reset --hard &&
+	test_must_fail git add init/a
+
+'
+
+test_expect_success 'gracefully add submodule with a trailing slash' '
+
+	commit=$(cd init &&
+	 echo b > a &&
+	 git commit -m update a >/dev/null &&
+	 git rev-parse HEAD) &&
+	git add init/ &&
+	test_must_fail git diff --exit-code --cached init
+
+'
+
 test_done
-- 
1.5.6.2.516.g22071
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 6 of 16 in “builtin-add.c: restructure the code for maintainability”
  1. 1/4 builtin-add.c: restructure the code for maintainabilityJunio C Hamano, Jul 20, 2008
  2. 2/4 git-add --all: add all filesJunio C Hamano, Jul 20, 2008
  3. 3/4 git-add --all: testsJunio C Hamano, Jul 20, 2008
  4. 4/4 git-add --all: documentationJunio C Hamano, Jul 20, 2008
  5. Johannes SchindelinJul 21, 2008
  6. git add: do not add files from a submoduleJohannes Schindelin, Jul 21, 2008
  7. Johannes SchindelinJul 22, 2008
  8. Junio C HamanoJul 23, 2008
  9. Pierre HabouzitJul 23, 2008
  10. Junio C HamanoJul 23, 2008
  11. Pierre HabouzitJul 23, 2008
  12. Johannes SchindelinJul 23, 2008
  13. Pierre HabouzitJul 23, 2008
  14. Junio C HamanoJul 21, 2008
  15. Junio C HamanoJul 21, 2008
  16. Junio C HamanoJul 21, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.