From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 18 Nov 2018 09:23:59 +0000 (+0900)
Subject: Merge branch 'jk/close-duped-fd-before-unlock-for-bundle'
X-Git-Tag: v2.20.0-rc0~1
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/9e3dc6bfb2756f9da7ddb64a1d9e41a22aa11e9b?hp=-c

Merge branch 'jk/close-duped-fd-before-unlock-for-bundle'

When "git bundle" aborts due to an empty commit ranges
(i.e. resulting in an empty pack), it left a file descriptor to an
lockfile open, which resulted in leftover lockfile on Windows where
you cannot remove a file with an open file descriptor.  This has
been corrected.

* jk/close-duped-fd-before-unlock-for-bundle:
bundle: dup() output descriptor closer to point-of-use
---

9e3dc6bfb2756f9da7ddb64a1d9e41a22aa11e9b
diff --combined bundle.c
index 1ef584b93b,0ae8c2d796..88c3e16d86
--- a/bundle.c
+++ b/bundle.c
@@@ -140,7 -140,7 +140,7 @@@ int verify_bundle(struct bundle_header 
  	int i, ret = 0, req_nr;
  	const char *message = _("Repository lacks these prerequisite commits:");
  
 -	init_revisions(&revs, NULL);
 +	repo_init_revisions(the_repository, &revs, NULL);
  	for (i = 0; i < p->nr; i++) {
  		struct ref_list_entry *e = p->list + i;
  		struct object *o = parse_object(the_repository, &e->oid);
@@@ -243,7 -243,7 +243,7 @@@ out
  }
  
  
- /* Write the pack data to bundle_fd, then close it if it is > 1. */
+ /* Write the pack data to bundle_fd */
  static int write_pack_data(int bundle_fd, struct rev_info *revs)
  {
  	struct child_process pack_objects = CHILD_PROCESS_INIT;
@@@ -256,6 -256,20 +256,20 @@@
  	pack_objects.in = -1;
  	pack_objects.out = bundle_fd;
  	pack_objects.git_cmd = 1;
+ 
+ 	/*
+ 	 * start_command() will close our descriptor if it's >1. Duplicate it
+ 	 * to avoid surprising the caller.
+ 	 */
+ 	if (pack_objects.out > 1) {
+ 		pack_objects.out = dup(pack_objects.out);
+ 		if (pack_objects.out < 0) {
+ 			error_errno(_("unable to dup bundle descriptor"));
+ 			child_process_clear(&pack_objects);
+ 			return -1;
+ 		}
+ 	}
+ 
  	if (start_command(&pack_objects))
  		return error(_("Could not spawn pack-objects"));
  
@@@ -369,7 -383,7 +383,7 @@@ static int write_bundle_refs(int bundle
  		 * commit that is referenced by the tag, and not the tag
  		 * itself.
  		 */
 -		if (oidcmp(&oid, &e->item->oid)) {
 +		if (!oideq(&oid, &e->item->oid)) {
  			/*
  			 * Is this the positive end of a range expressed
  			 * in terms of a tag (e.g. v2.0 from the range
@@@ -421,27 -435,16 +435,16 @@@ int create_bundle(struct bundle_header 
  	bundle_to_stdout = !strcmp(path, "-");
  	if (bundle_to_stdout)
  		bundle_fd = 1;
- 	else {
+ 	else
  		bundle_fd = hold_lock_file_for_update(&lock, path,
  						      LOCK_DIE_ON_ERROR);
  
- 		/*
- 		 * write_pack_data() will close the fd passed to it,
- 		 * but commit_lock_file() will also try to close the
- 		 * lockfile's fd. So make a copy of the file
- 		 * descriptor to avoid trying to close it twice.
- 		 */
- 		bundle_fd = dup(bundle_fd);
- 		if (bundle_fd < 0)
- 			die_errno("unable to dup file descriptor");
- 	}
- 
  	/* write signature */
  	write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
  
  	/* init revs to list objects for pack-objects later */
  	save_commit_buffer = 0;
 -	init_revisions(&revs, NULL);
 +	repo_init_revisions(the_repository, &revs, NULL);
  
  	/* write prerequisites */
  	if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
@@@ -463,10 -466,8 +466,8 @@@
  		goto err;
  
  	/* write pack */
- 	if (write_pack_data(bundle_fd, &revs)) {
- 		bundle_fd = -1; /* already closed by the above call */
+ 	if (write_pack_data(bundle_fd, &revs))
  		goto err;
- 	}
  
  	if (!bundle_to_stdout) {
  		if (commit_lock_file(&lock))
@@@ -474,11 -475,7 +475,7 @@@
  	}
  	return 0;
  err:
- 	if (!bundle_to_stdout) {
- 		if (0 <= bundle_fd)
- 			close(bundle_fd);
- 		rollback_lock_file(&lock);
- 	}
+ 	rollback_lock_file(&lock);
  	return -1;
  }