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

Re: [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents

From
Jens Lehmann <jens.lehmann@web.de>
Date
Dec 27, 2013, 13:58 UTC
Message-ID
<52BD8786.1080900@web.de>
In-Reply-To
<20131226161202.GN20443@google.com>
Am 26.12.2013 17:12, schrieb Jonathan Nieder:
Show 23 quoted lines
> From: Jens Lehmann <Jens.Lehmann@web.de>
> Date: Tue, 19 Jun 2012 20:55:45 +0200
> 
> Implement the functionality needed to enable work tree manipulating
> commands to that a deleted submodule should not only affect the index
> (leaving all the files of the submodule in the work tree) but also to
> remove the work tree of the superproject (including any untracked
> files).
> 
> That will only work properly when the submodule uses a gitfile instead of
> a .git directory and no untracked files are present. Otherwise the removal
> will fail with a warning (which is just what happened until now).
> 
> Extend rmdir_or_warn() to remove the directories of those submodules which
> are scheduled for removal. Also teach verify_clean_submodule() to check
> that a submodule configured to be removed is not modified before scheduling
> it for removal.
> 
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Should this share some code (or just the error message) with the 'git
> rm' code that checks whether a submodule is safe to remove?
Yes, that would make sense.
Show 6 quoted lines
> rmdir_or_warn is a pretty low-level function --- it feels odd to be
> relying on the git repository layout here.  On the other hand, that
> function only has two callers, so it is possible to check quickly
> whether it is safe.
> 
> I assume this is mostly for the sake of the caller in unpack-trees?
Yup.
> In builtin/apply.c, remove_file is used for deletion and rename
> patches.  Do we want this logic take effect there, too?

I think so. Recursive update should also affect apply and am when requested via command line or configuration. But the apply documentation states that it also handles files outside a git repository, so we would have to disable this logic in that case.

Show 117 quoted lines
>  submodule.c    | 37 +++++++++++++++++++++++++++++++++++++
>  submodule.h    |  1 +
>  unpack-trees.c |  6 +++---
>  wrapper.c      |  3 +++
>  4 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 3f18d4d..a25db46 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -412,6 +412,43 @@ int submodule_needs_update(const char *path)
>  	return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
>  }
>  
> +int depopulate_submodule(const char *path)
> +{
> +	struct strbuf dot_git = STRBUF_INIT;
> +	struct child_process cp;
> +	const char *argv[] = {"rm", "-rf", path, NULL};
> +
> +	/* Is it populated? */
> +	strbuf_addf(&dot_git, "%s/.git", path);
> +	if (!resolve_gitdir(dot_git.buf)) {
> +		strbuf_release(&dot_git);
> +		return 0;
> +	}
> +	strbuf_release(&dot_git);
> +
> +	/* Does it have a .git directory? */
> +	if (!submodule_uses_gitfile(path)) {
> +		warning(_("cannot remove submodule '%s' because it (or one of "
> +			  "its nested submodules) uses a .git directory"),
> +			  path);
> +		return -1;
> +	}
> +
> +	/* Remove the whole submodule directory */
> +	memset(&cp, 0, sizeof(cp));
> +	cp.argv = argv;
> +	cp.env = local_repo_env;
> +	cp.git_cmd = 0;
> +	cp.no_stdin = 1;
> +	if (run_command(&cp)) {
> +		warning("Could not remove submodule %s", path);
> +		strbuf_release(&dot_git);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  void show_submodule_summary(FILE *f, const char *path,
>  		const char *line_prefix,
>  		unsigned char one[20], unsigned char two[20],
> diff --git a/submodule.h b/submodule.h
> index 055918c..df291cf 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -24,6 +24,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
>  int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
>  int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
>  int submodule_needs_update(const char *path);
> +int depopulate_submodule(const char *path);
>  void show_submodule_summary(FILE *f, const char *path,
>  		const char *line_prefix,
>  		unsigned char one[20], unsigned char two[20],
> diff --git a/unpack-trees.c b/unpack-trees.c
> index ad3e9a0..89b506a 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -8,6 +8,7 @@
>  #include "progress.h"
>  #include "refs.h"
>  #include "attr.h"
> +#include "submodule.h"
>  
>  /*
>   * Error messages expected by scripts out of plumbing commands such as
> @@ -1263,14 +1264,13 @@ static void invalidate_ce_path(const struct cache_entry *ce,
>  /*
>   * Check that checking out ce->sha1 in subdir ce->name is not
>   * going to overwrite any working files.
> - *
> - * Currently, git does not checkout subprojects during a superproject
> - * checkout, so it is not going to overwrite anything.
>   */
>  static int verify_clean_submodule(const struct cache_entry *ce,
>  				  enum unpack_trees_error_types error_type,
>  				  struct unpack_trees_options *o)
>  {
> +	if (submodule_needs_update(ce->name) && is_submodule_modified(ce->name, 0))
> +		return 1;
>  	return 0;
>  }
>  
> diff --git a/wrapper.c b/wrapper.c
> index 0cc5636..425a3fd 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -2,6 +2,7 @@
>   * Various trivial helper wrappers around standard functions
>   */
>  #include "cache.h"
> +#include "submodule.h"
>  
>  static void do_nothing(size_t size)
>  {
> @@ -409,6 +410,8 @@ int unlink_or_warn(const char *file)
>  
>  int rmdir_or_warn(const char *file)
>  {
> +	if (submodule_needs_update(file) && depopulate_submodule(file))
> +		return -1;
>  	return warn_if_unremovable("rmdir", file, rmdir(file));
>  }
>  
> 
Previous: Jonathan NiederNext: Jonathan Nieder
Message 5 of 11 in “git checkout --recurse-submodules”
  1. 0/5 git checkout --recurse-submodulesJonathan Nieder, Dec 26, 2013
  2. 1/5 submodule: prepare for recursive checkout of submodulesJonathan Nieder, Dec 26, 2013
  3. Jens LehmannDec 27, 2013
  4. 2/5 submodule: teach unpack_trees() to remove submodule contentsJonathan Nieder, Dec 26, 2013
  5. Jens LehmannDec 27, 2013
  6. 3/5 submodule: teach unpack_trees() to repopulate submodulesJonathan Nieder, Dec 26, 2013
  7. 4/5 submodule: teach unpack_trees() to update submodulesJonathan Nieder, Dec 26, 2013
  8. 5/5 Teach checkout to recursively checkout submodulesJonathan Nieder, Dec 26, 2013
  9. Jens LehmannDec 27, 2013
  10. Junio C HamanoDec 26, 2013
  11. Jens LehmannDec 27, 2013

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.