All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Michael Haggerty" <mhagger@alum.mit.edu>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"David Turner" <novalis@novalis.org>,
	"Brandon Williams" <bmwill@google.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] packed_ref_store: handle a packed-refs file that is a symlink
Date: Thu, 27 Jul 2017 14:28:54 -0400	[thread overview]
Message-ID: <20170727182854.wxjvjs2x746n3x2t@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqq8tj9okzv.fsf@gitster.mtv.corp.google.com>

On Thu, Jul 27, 2017 at 10:19:48AM -0700, Junio C Hamano wrote:

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
> > 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.
> 
> Because when we take a lock hold_lock_file() eventually calls into
> lock_file() which by default takes the lock on the target of the
> symbolic link (which is the sensible default, which is triggered in
> this codepath), so this change to use the name of that file is all
> that is needed.
> 
> Makes sense.  Makes me wonder why we need a separate .new file
> (instead of writing into the .lock instead), but that is a different
> issue.

It comes from 42dfa7ece (commit_packed_refs(): use a staging file
separate from the lockfile, 2017-06-23). That commit explains that we
want to be able to put the new contents into service before we release
the lock. But it doesn't say why that's useful.

I recall from past discussions that this will help close some races,
and e5cc7d7d2 (repack_without_refs(): don't lock or unlock the packed
refs, 2017-07-01) alludes to this. I think the races in question have to
do with holding the packed-refs lock while pruning the just-packed
files, but I'm having trouble digging up specifics in the archive.

-Peff

  reply	other threads:[~2017-07-27 18:28 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-01 18:30 [PATCH v3 00/30] Create a reference backend for packed refs Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 01/30] t1408: add a test of stale packed refs covered by loose refs Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 02/30] add_packed_ref(): teach function to overwrite existing refs Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 03/30] packed_ref_store: new struct Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 04/30] packed_ref_store: move `packed_refs_path` here Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 05/30] packed_ref_store: move `packed_refs_lock` member here Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 06/30] clear_packed_ref_cache(): take a `packed_ref_store *` parameter Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 07/30] validate_packed_ref_cache(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 08/30] get_packed_ref_cache(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 09/30] get_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 10/30] add_packed_ref(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 11/30] lock_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 12/30] commit_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 13/30] rollback_packed_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 14/30] get_packed_ref(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 15/30] repack_without_refs(): " Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 16/30] packed_peel_ref(): new function, extracted from `files_peel_ref()` Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 17/30] packed_ref_store: support iteration Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 18/30] packed_read_raw_ref(): new function, replacing `resolve_packed_ref()` Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 19/30] packed-backend: new module for handling packed references Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 20/30] packed_ref_store: make class into a subclass of `ref_store` Michael Haggerty
2017-07-01 18:30 ` [PATCH v3 21/30] commit_packed_refs(): report errors rather than dying Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 22/30] commit_packed_refs(): use a staging file separate from the lockfile Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 23/30] packed_refs_lock(): function renamed from lock_packed_refs() Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 24/30] packed_refs_lock(): report errors via a `struct strbuf *err` Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 25/30] packed_refs_unlock(), packed_refs_is_locked(): new functions Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 26/30] clear_packed_ref_cache(): don't protest if the lock is held Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 27/30] commit_packed_refs(): remove call to `packed_refs_unlock()` Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 28/30] repack_without_refs(): don't lock or unlock the packed refs Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 29/30] t3210: add some tests of bogus packed-refs file contents Michael Haggerty
2017-07-01 18:31 ` [PATCH v3 30/30] read_packed_refs(): die if `packed-refs` contains bogus data Michael Haggerty
2017-07-05  9:12 ` [PATCH v3 00/30] Create a reference backend for packed refs Jeff King
2017-07-20 23:05   ` Stefan Beller
2017-07-20 23:20     ` Jonathan Nieder
2017-07-26 23:39       ` [PATCH] packed_ref_store: handle a packed-refs file that is a symlink Michael Haggerty
2017-07-27  0:15         ` Stefan Beller
2017-07-27  0:18         ` Jonathan Nieder
2017-07-27 11:12           ` Michael Haggerty
2017-07-27 17:19         ` Junio C Hamano
2017-07-27 18:28           ` Jeff King [this message]
2017-07-27 19:40             ` Junio C Hamano
2017-07-28  6:07               ` Michael Haggerty
2021-05-31 14:18         ` Ævar Arnfjörð Bjarmason
2021-06-03 19:39           ` Jeff King
2021-06-03 19:58             ` [PATCH] t: use portable wrapper for readlink(1) Jeff King
2021-06-04 21:09               ` Ævar Arnfjörð Bjarmason
2021-06-03 20:23             ` [PATCH] packed_ref_store: handle a packed-refs file that is a symlink Felipe Contreras
2021-06-03 21:08               ` Jeff King
2021-06-03 22:25                 ` Felipe Contreras
2021-06-04 21:37                 ` Ævar Arnfjörð Bjarmason
2021-06-05  1:07                   ` Felipe Contreras
2021-06-04 21:12             ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170727182854.wxjvjs2x746n3x2t@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=novalis@novalis.org \
    --cc=pclouds@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.