Re: [PATCH] builtin-bundle create - use lock_file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Mark Levedahl <mdl123@xxxxxxxxxxx> writes:

> git bundle create would leave an invalid, partially written bundle if
> an error occured during creation. Fix that using lock_file.
>
> Signed-off-by: Mark Levedahl <mdl123@xxxxxxxxxxx>
> ---
>  struct lock_file is now static.
>  *caller* closes the lock file before commiting / rolling back.

I think you broke the support to write a bundle out to the
standard output by giving "-" as the path.

Also, the whole point of the "NEEDSWORK" suggestion to use
lockfile was because you do not have to do lwrite_or_die() in
the first place ;-).  When you exit before committing lockfiles
you hold, atexit handler rolls back and unlink them for you.

The following might be a better replacement.

By the way, by using lockfile's "create temporary, generate into
it and then rename to final", we are allowing an overwrite of an
existing bundle, which we did not allow earlier.  Is this check
something we would want to preserve?

---

 builtin-bundle.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index f4b4f03..1b65006 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -189,7 +189,9 @@ static int list_heads(struct bundle_header *header, int argc, const char **argv)
 static int create_bundle(struct bundle_header *header, const char *path,
 		int argc, const char **argv)
 {
+	static struct lock_file lock;
 	int bundle_fd = -1;
+	int bundle_to_stdout;
 	const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
 	const char **argv_pack = xmalloc(5 * sizeof(const char *));
 	int i, ref_count = 0;
@@ -198,14 +200,11 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	struct child_process rls;
 	FILE *rls_fout;
 
-	/*
-	 * NEEDSWORK: this should use something like lock-file
-	 * to create temporary that is cleaned up upon error.
-	 */
-	bundle_fd = (!strcmp(path, "-") ? 1 :
-			open(path, O_CREAT | O_EXCL | O_WRONLY, 0666));
-	if (bundle_fd < 0)
-		return error("Could not create '%s': %s", path, strerror(errno));
+	bundle_to_stdout = !strcmp(path, "-");
+	if (bundle_to_stdout)
+		bundle_fd = 1;
+	else
+		bundle_fd = hold_lock_file_for_update(&lock, path, 1);
 
 	/* write signature */
 	write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
@@ -341,6 +340,9 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	}
 	if (finish_command(&rls))
 		return error ("pack-objects died");
+	close(bundle_fd);
+	if (!bundle_to_stdout)
+		commit_lock_file(&lock);
 	return 0;
 }
 

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux