Merge branch 'jk/close-duped-fd-before-unlock-for-bundle'
authorJunio C Hamano <gitster@pobox.com>
Sun, 18 Nov 2018 09:23:59 +0000 (18:23 +0900)
committerJunio C Hamano <gitster@pobox.com>
Sun, 18 Nov 2018 09:23:59 +0000 (18:23 +0900)
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

1  2 
bundle.c
diff --combined bundle.c
index 1ef584b93b87491dca873429f1fbbbadf19622d4,0ae8c2d796b284125189c3a809faabfa62fac50e..88c3e16d86162518ddde771703a3d2b0fc622f3b
+++ 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;
        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))
                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))
        }
        return 0;
  err:
-       if (!bundle_to_stdout) {
-               if (0 <= bundle_fd)
-                       close(bundle_fd);
-               rollback_lock_file(&lock);
-       }
+       rollback_lock_file(&lock);
        return -1;
  }