Merge pull request libgit2#2204 from libgit2/rb/submodule-reference-counting

Make submodules externally refcounted

Author: Vicent Marti

examples/status.c

@@ -363,18 +363,21 @@ static void print_short(git_repository *repo, git_status_list *status)
 			unsigned int smstatus = 0;
 
 			if (!git_submodule_lookup(
-					&sm, repo, s->index_to_workdir->new_file.path) &&
-				!git_submodule_status(&smstatus, sm))
-			{
-				if (smstatus & GIT_SUBMODULE_STATUS_WD_MODIFIED)
-					extra = " (new commits)";
-				else if (smstatus & GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED)
-					extra = " (modified content)";
-				else if (smstatus & GIT_SUBMODULE_STATUS_WD_WD_MODIFIED)
-					extra = " (modified content)";
-				else if (smstatus & GIT_SUBMODULE_STATUS_WD_UNTRACKED)
-					extra = " (untracked content)";
+					&sm, repo, s->index_to_workdir->new_file.path)) {
+
+				if (!git_submodule_status(&smstatus, sm)) {
+					if (smstatus & GIT_SUBMODULE_STATUS_WD_MODIFIED)
+						extra = " (new commits)";
+					else if (smstatus & GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED)
+						extra = " (modified content)";
+					else if (smstatus & GIT_SUBMODULE_STATUS_WD_WD_MODIFIED)
+						extra = " (modified content)";
+					else if (smstatus & GIT_SUBMODULE_STATUS_WD_UNTRACKED)
+						extra = " (untracked content)";
+				}
 			}
+
+			git_submodule_free(sm);
 		}
 
 		/**

include/git2/submodule.h

@@ -123,21 +123,27 @@ typedef enum {
  *   There may or may not be anything else at that path, but nothing that
  *   looks like a submodule.  In this case, this returns GIT_ENOTFOUND.
  *
- * The submodule object is owned by the containing repo and will be freed
- * when the repo is freed.  The caller need not free the submodule.
+ * You must call `git_submodule_free` when done with the submodule.
  *
- * @param submodule Pointer to submodule description object pointer..
- * @param repo The repository.
- * @param name The name of the submodule.  Trailing slashes will be ignored.
+ * @param out Output ptr to submodule; pass NULL to just get return code
+ * @param repo The parent repository
+ * @param name The name of or path to the submodule; trailing slashes okay
  * @return 0 on success, GIT_ENOTFOUND if submodule does not exist,
- *         GIT_EEXISTS if submodule exists in working directory only, -1 on
- *         other errors.
+ *         GIT_EEXISTS if submodule exists in working directory only,
+ *         -1 on other errors.
  */
 GIT_EXTERN(int) git_submodule_lookup(
-	git_submodule **submodule,
+	git_submodule **out,
 	git_repository *repo,
 	const char *name);
 
+/**
+ * Release a submodule
+ *
+ * @param submodule Submodule object
+ */
+GIT_EXTERN(void) git_submodule_free(git_submodule *submodule);
+
 /**
  * Iterate over all tracked submodules of a repository.
  *
@@ -175,17 +181,19 @@ GIT_EXTERN(int) git_submodule_foreach(
  * `git_submodule_add_finalize()` to wrap up adding the new submodule and
  * .gitmodules to the index to be ready to commit.
  *
- * @param submodule The newly created submodule ready to open for clone
- * @param repo Superproject repository to contain the new submodule
- * @param url URL for the submodules remote
+ * You must call `git_submodule_free` on the submodule object when done.
+ *
+ * @param out The newly created submodule ready to open for clone
+ * @param repo The repository in which you want to create the submodule
+ * @param url URL for the submodule's remote
  * @param path Path at which the submodule should be created
  * @param use_gitlink Should workdir contain a gitlink to the repo in
  *        .git/modules vs. repo directly in workdir.
  * @return 0 on success, GIT_EEXISTS if submodule already exists,
  *         -1 on other errors.
  */
 GIT_EXTERN(int) git_submodule_add_setup(
-	git_submodule **submodule,
+	git_submodule **out,
 	git_repository *repo,
 	const char *url,
 	const char *path,
@@ -493,15 +501,23 @@ GIT_EXTERN(int) git_submodule_open(
  *
  * Call this to reread cached submodule information for this submodule if
  * you have reason to believe that it has changed.
+ *
+ * @param submodule The submodule to reload
+ * @param force Force reload even if the data doesn't seem out of date
+ * @return 0 on success, <0 on error
  */
-GIT_EXTERN(int) git_submodule_reload(git_submodule *submodule);
+GIT_EXTERN(int) git_submodule_reload(git_submodule *submodule, int force);
 
 /**
  * Reread all submodule info.
  *
  * Call this to reload all cached submodule information for the repo.
+ *
+ * @param repo The repository to reload submodule data for
+ * @param force Force full reload even if the data doesn't seem out of date
+ * @return 0 on success, <0 on error
  */
-GIT_EXTERN(int) git_submodule_reload_all(git_repository *repo);
+GIT_EXTERN(int) git_submodule_reload_all(git_repository *repo, int force);
 
 /**
  * Get the status for a submodule.

src/checkout.c

@@ -147,19 +147,23 @@ static bool checkout_is_workdir_modified(
 		git_submodule *sm;
 		unsigned int sm_status = 0;
 		const git_oid *sm_oid = NULL;
+		bool rval = false;
 
-		if (git_submodule_lookup(&sm, data->repo, wditem->path) < 0 ||
-			git_submodule_status(&sm_status, sm) < 0)
-			return true;
-
-		if (GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status))
+		if (git_submodule_lookup(&sm, data->repo, wditem->path) < 0) {
+			giterr_clear();
 			return true;
+		}
 
-		sm_oid = git_submodule_wd_id(sm);
-		if (!sm_oid)
-			return false;
+		if (git_submodule_status(&sm_status, sm) < 0 ||
+			GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status))
+			rval = true;
+		else if ((sm_oid = git_submodule_wd_id(sm)) == NULL)
+			rval = false;
+		else
+			rval = (git_oid__cmp(&baseitem->id, sm_oid) != 0);
 
-		return (git_oid__cmp(&baseitem->id, sm_oid) != 0);
+		git_submodule_free(sm);
+		return rval;
 	}
 
 	/* Look at the cache to decide if the workdir is modified.  If not,
@@ -324,12 +328,17 @@ static bool submodule_is_config_only(
 {
 	git_submodule *sm = NULL;
 	unsigned int sm_loc = 0;
+	bool rval = false;
 
-	if (git_submodule_lookup(&sm, data->repo, path) < 0 ||
-		git_submodule_location(&sm_loc, sm) < 0 ||
-		sm_loc == GIT_SUBMODULE_STATUS_IN_CONFIG)
+	if (git_submodule_lookup(&sm, data->repo, path) < 0)
 		return true;
 
+	if (git_submodule_location(&sm_loc, sm) < 0 ||
+		sm_loc == GIT_SUBMODULE_STATUS_IN_CONFIG)
+		rval = true;
+
+	git_submodule_free(sm);
+
 	return false;
 }
 
@@ -1258,7 +1267,6 @@ static int checkout_submodule(
 	const git_diff_file *file)
 {
 	int error = 0;
-	git_submodule *sm;
 
 	/* Until submodules are supported, UPDATE_ONLY means do nothing here */
 	if ((data->strategy & GIT_CHECKOUT_UPDATE_ONLY) != 0)
@@ -1269,7 +1277,7 @@ static int checkout_submodule(
 			data->opts.dir_mode, GIT_MKDIR_PATH)) < 0)
 		return error;
 
-	if ((error = git_submodule_lookup(&sm, data->repo, file->path)) < 0) {
+	if ((error = git_submodule_lookup(NULL, data->repo, file->path)) < 0) {
 		/* I've observed repos with submodules in the tree that do not
 		 * have a .gitmodules - core Git just makes an empty directory
 		 */
@@ -1510,7 +1518,7 @@ static int checkout_create_submodules(
 
 	/* initial reload of submodules if .gitmodules was changed */
 	if (data->reload_submodules &&
-		(error = git_submodule_reload_all(data->repo)) < 0)
+		(error = git_submodule_reload_all(data->repo, 1)) < 0)
 		return error;
 
 	git_vector_foreach(&data->diff->deltas, i, delta) {
@@ -1534,7 +1542,7 @@ static int checkout_create_submodules(
 	}
 
 	/* final reload once submodules have been updated */
-	return git_submodule_reload_all(data->repo);
+	return git_submodule_reload_all(data->repo, 1);
 }
 
 static int checkout_lookup_head_tree(git_tree **out, git_repository *repo)

src/diff.c

@@ -528,12 +528,15 @@ int git_diff__oid_for_file(
 	/* calculate OID for file if possible */
 	if (S_ISGITLINK(mode)) {
 		git_submodule *sm;
-		const git_oid *sm_oid;
 
-		if (!git_submodule_lookup(&sm, repo, path) &&
-			(sm_oid = git_submodule_wd_id(sm)) != NULL)
-			git_oid_cpy(oid, sm_oid);
-		else {
+		memset(oid, 0, sizeof(*oid));
+
+		if (!git_submodule_lookup(&sm, repo, path)) {
+			const git_oid *sm_oid = git_submodule_wd_id(sm);
+			if (sm_oid)
+				git_oid_cpy(oid, sm_oid);
+			git_submodule_free(sm);
+		} else {
 			/* if submodule lookup failed probably just in an intermediate
 			 * state where some init hasn't happened, so ignore the error
 			 */
@@ -615,24 +618,24 @@ static int maybe_modified_submodule(
 	}
 
 	if (ign <= 0 && git_submodule_ignore(sub) == GIT_SUBMODULE_IGNORE_ALL)
-		return 0;
-
-	if ((error = git_submodule__status(
+		/* ignore it */;
+	else if ((error = git_submodule__status(
 			&sm_status, NULL, NULL, found_oid, sub, ign)) < 0)
-		return error;
+		/* return error below */;
 
 	/* check IS_WD_UNMODIFIED because this case is only used
 	 * when the new side of the diff is the working directory
 	 */
-	if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status))
+	else if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status))
 		*status = GIT_DELTA_MODIFIED;
 
 	/* now that we have a HEAD OID, check if HEAD moved */
-	if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 &&
+	else if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 &&
 		!git_oid_equal(&info->oitem->id, found_oid))
 		*status = GIT_DELTA_MODIFIED;
 
-	return 0;
+	git_submodule_free(sub);
+	return error;
 }
 
 static int maybe_modified(
@@ -960,10 +963,8 @@ static int handle_unmatched_new_item(
 		delta_type = GIT_DELTA_ADDED;
 
 	else if (nitem->mode == GIT_FILEMODE_COMMIT) {
-		git_submodule *sm;
-
 		/* ignore things that are not actual submodules */
-		if (git_submodule_lookup(&sm, info->repo, nitem->path) != 0) {
+		if (git_submodule_lookup(NULL, info->repo, nitem->path) != 0) {
 			giterr_clear();
 			delta_type = GIT_DELTA_IGNORED;
 		}

src/diff_file.c

@@ -177,11 +177,17 @@ static int diff_file_content_commit_to_str(
 		unsigned int sm_status = 0;
 		const git_oid *sm_head;
 
-		if ((error = git_submodule_lookup(&sm, fc->repo, fc->file->path)) < 0 ||
-			(error = git_submodule_status(&sm_status, sm)) < 0) {
+		if ((error = git_submodule_lookup(&sm, fc->repo, fc->file->path)) < 0) {
 			/* GIT_EEXISTS means a "submodule" that has not been git added */
-			if (error == GIT_EEXISTS)
+			if (error == GIT_EEXISTS) {
+				giterr_clear();
 				error = 0;
+			}
+			return error;
+		}
+
+		if ((error = git_submodule_status(&sm_status, sm)) < 0) {
+			git_submodule_free(sm);
 			return error;
 		}
 
@@ -196,6 +202,8 @@ static int diff_file_content_commit_to_str(
 
 		if (GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status))
 			status = "-dirty";
+
+		git_submodule_free(sm);
 	}
 
 	git_oid_tostr(oid, sizeof(oid), &fc->file->id);
@@ -312,7 +320,8 @@ static int diff_file_content_load_workdir_file(
 
 		error = git_filter_list_apply_to_data(&out, fl, &raw);
 
-		git_buf_free(&raw);
+		if (out.ptr != raw.ptr)
+			git_buf_free(&raw);
 
 		if (!error) {
 			fc->map.len  = out.size;