From 198b808e207e35ba390abe362c75040500997cea Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 26 Jul 2017 16:39:42 -0700 Subject: 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 Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a28befbfa3..59e7d1a509 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -610,19 +610,27 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); int ok; + int ret = -1; struct strbuf sb = STRBUF_INIT; FILE *out; struct ref_iterator *iter; + char *packed_refs_path; if (!is_lock_file_locked(&refs->lock)) die("BUG: commit_packed_refs() called when unlocked"); - strbuf_addf(&sb, "%s.new", refs->path); + /* + * If packed-refs is a symlink, we want to overwrite the + * symlinked-to file, not the symlink itself. Also, put the + * staging file next to it: + */ + packed_refs_path = get_locked_file_path(&refs->lock); + strbuf_addf(&sb, "%s.new", packed_refs_path); if (create_tempfile(&refs->tempfile, sb.buf) < 0) { strbuf_addf(err, "unable to create file %s: %s", sb.buf, strerror(errno)); strbuf_release(&sb); - return -1; + goto out; } strbuf_release(&sb); @@ -660,17 +668,21 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) goto error; } - if (rename_tempfile(&refs->tempfile, refs->path)) { + if (rename_tempfile(&refs->tempfile, packed_refs_path)) { strbuf_addf(err, "error replacing %s: %s", refs->path, strerror(errno)); - return -1; + goto out; } - return 0; + ret = 0; + goto out; error: delete_tempfile(&refs->tempfile); - return -1; + +out: + free(packed_refs_path); + return ret; } /* -- cgit v1.2.1