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

Re: [PATCH v2 2/2] submodule: port submodule subcommand 'deinit' from shell to C

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 11, 2018, 20:48 UTC
Message-ID
<xmqqh8rsxgtw.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20180111201721.25930-3-pc44800@gmail.com>
Prathamesh Chavan <pc44800@gmail.com> writes:
Show 44 quoted lines
> +	/* remove the submodule work tree (unless the user already did it) */
> +	if (is_directory(path)) {
> +		struct strbuf sb_rm = STRBUF_INIT;
> +		const char *format;
> +
> +		/*
> +		 * protect submodules containing a .git directory
> +		 * NEEDSWORK: instead of dying, automatically call
> +		 * absorbgitdirs and (possibly) warn.
> +		 */
> +		if (is_directory(sub_git_dir))
> +			die(_("Submodule work tree '%s' contains a .git "
> +			      "directory (use 'rm -rf' if you really want "
> +			      "to remove it including all of its history)"),
> +			    displaypath);
> +
> +		if (!(flags & OPT_FORCE)) {
> +			struct child_process cp_rm = CHILD_PROCESS_INIT;
> +			cp_rm.git_cmd = 1;
> +			argv_array_pushl(&cp_rm.args, "rm", "-qn",
> +					 path, NULL);
> +
> +			if (run_command(&cp_rm))
> +				die(_("Submodule work tree '%s' contains local "
> +				      "modifications; use '-f' to discard them"),
> +				      displaypath);
> +		}
> +
> +		strbuf_addstr(&sb_rm, path);
> +
> +		if (!remove_dir_recursively(&sb_rm, 0))
> +			format = _("Cleared directory '%s'\n");
> +		else
> +			format = _("Could not remove submodule work tree '%s'\n");
> +
> +		if (!(flags & OPT_QUIET))
> +			printf(format, displaypath);
> +
> +		strbuf_release(&sb_rm);
> +	}
> +
> +	if (mkdir(path, 0777))
> +		die_errno(_("could not create empty submodule directory %s"),
> +		      displaypath);

If path was a directory (which presumably is the normal case) and recursive removal fails (i.e. when the code says "Could not remove"), this mkdir() would also fail with EEXIST.

In such a case, the original code did not die and instead continued to remove the entries for the submodule from the configuration. This "rewritten" version dies, leaving the stale configuration for the submodule we failed to get rid of from the working tree.

I offhand do not know which one of these error case behaviours is more useful; the user needs to do something (e.g. loosening the perm in some paths in the submodule that prevented "rm -rf" from working with "chmod u+w sub/some/path" and removing it manually) to recover in either case, and cleaning as much as possible by removing the configuration entries even when this mkdir() fails would probably be a better behaviour, as long as the command as a whole exits with non zero status to signal an error.

Previous: Prathamesh ChavanNext: Prathamesh Chavan
Message 12 of 19 in “Incremental rewrite of git-submodules”
  1. 0/2 Incremental rewrite of git-submodulesPrathamesh Chavan, Jan 9, 2018
  2. 1/2 submodule: port submodule subcommand 'sync' from shell to CPrathamesh Chavan, Jan 9, 2018
  3. Junio C HamanoJan 9, 2018
  4. 2/2 submodule: port submodule subcommand 'deinit' from shell to CPrathamesh Chavan, Jan 9, 2018
  5. Junio C HamanoJan 9, 2018
  6. Prathamesh ChavanJan 10, 2018
  7. Junio C HamanoJan 10, 2018
  8. Stefan BellerJan 9, 2018
  9. Brandon WilliamsJan 9, 2018
  10. 0/2 Incremental rewrite of git-submodulesPrathamesh Chavan, Jan 11, 2018
  11. 2/2 submodule: port submodule subcommand 'deinit' from shell to CPrathamesh Chavan, Jan 11, 2018
  12. Junio C HamanoJan 11, 2018
  13. 1/2 submodule: port submodule subcommand 'sync' from shell to CPrathamesh Chavan, Jan 11, 2018
  14. Junio C HamanoJan 11, 2018
  15. Junio C HamanoJan 11, 2018
  16. 0/2 Incremental rewrite of git-submodulesPrathamesh Chavan, Jan 14, 2018
  17. 1/2 submodule: port submodule subcommand 'sync' from shell to CPrathamesh Chavan, Jan 14, 2018
  18. 2/2 submodule: port submodule subcommand 'deinit' from shell to CPrathamesh Chavan, Jan 14, 2018
  19. Junio C HamanoJan 16, 2018

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.