@matheusbsbs / matheus-bagliano / commits / 198b808e20

packed_ref_store: handle a packed-refs file that is a symlink

One of the tricks that `contrib/workdir/git-new-workdir` plays is to making `packed-refs` in the new workdir a symlink to the `packed-refs` file in the original repository. Before 42dfa7ecef ("commit_packed_refs(): use a staging file separate from the lockfile", 2017-06-23), a lockfile was used as the staging file, and because the `LOCK_NO_DEREF` was not used, the pointed-to file was locked and modified. But after that commit, the staging file was created using a tempfile, with the end result that rewriting the `packed-refs` file in the workdir overwrote the symlink rather than the original `packed-refs` file. Change `commit_packed_refs()` to use `get_locked_file_path()` to find the path of the file that it should overwrite. Since that path was properly resolved when the lockfile was created, this restores the pre-42dfa7ecef behavior. Also add a test case to document this use case and prevent a regression like this from recurring. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Jul 26, 2017 at 16:39 UTC 198b808e207e35ba390abe362c75040500997cea

2 files changed +33 -6

refs/packed-backend.c

+18 -6

@@ -610,19 +610,27 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err)

610 struct packed_ref_cache *packed_ref_cache =

611 get_packed_ref_cache(refs);

612 int ok;

613 + int ret = -1;

614 struct strbuf sb = STRBUF_INIT;

615 FILE *out;

616 struct ref_iterator *iter;

617 + char *packed_refs_path;

618

619 if (!is_lock_file_locked(&refs->lock))

620 die("BUG: commit_packed_refs() called when unlocked");

621

620 - strbuf_addf(&sb, "%s.new", refs->path);

622 + /*

623 + * If packed-refs is a symlink, we want to overwrite the

624 + * symlinked-to file, not the symlink itself. Also, put the

625 + * staging file next to it:

626 + */

627 + packed_refs_path = get_locked_file_path(&refs->lock);

628 + strbuf_addf(&sb, "%s.new", packed_refs_path);

629 if (create_tempfile(&refs->tempfile, sb.buf) < 0) {

630 strbuf_addf(err, "unable to create file %s: %s",

631 sb.buf, strerror(errno));

632 strbuf_release(&sb);

625 - return -1;

633 + goto out;

634 }

635 strbuf_release(&sb);

636

@@ -660,17 +668,21 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err)

668 goto error;

669 }

670

663 - if (rename_tempfile(&refs->tempfile, refs->path)) {

671 + if (rename_tempfile(&refs->tempfile, packed_refs_path)) {

672 strbuf_addf(err, "error replacing %s: %s",

673 refs->path, strerror(errno));

666 - return -1;

674 + goto out;

675 }

676

669 - return 0;

677 + ret = 0;

678 + goto out;

679

680 error:

681 delete_tempfile(&refs->tempfile);

673 - return -1;

682 +

683 +out:

684 + free(packed_refs_path);

685 + return ret;

686 }

687

688 /*

t/t3210-pack-refs.sh

+15

@@ -238,4 +238,19 @@ test_expect_success 'retry acquiring packed-refs.lock' '

238 git -c core.packedrefstimeout=3000 pack-refs --all --prune

239 '

240

241 +test_expect_success SYMLINKS 'pack symlinked packed-refs' '

242 + # First make sure that symlinking works when reading:

243 + git update-ref refs/heads/loosy refs/heads/master &&

244 + git for-each-ref >all-refs-before &&

245 + mv .git/packed-refs .git/my-deviant-packed-refs &&

246 + ln -s my-deviant-packed-refs .git/packed-refs &&

247 + git for-each-ref >all-refs-linked &&

248 + test_cmp all-refs-before all-refs-linked &&

249 + git pack-refs --all --prune &&

250 + git for-each-ref >all-refs-packed &&

251 + test_cmp all-refs-before all-refs-packed &&

252 + test -h .git/packed-refs &&

253 + test "$(readlink .git/packed-refs)" = "my-deviant-packed-refs"

254 +'

255 +

256 test_done