git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/5] fast-import: stop using lock_ref_sha1

From
Ronnie Sahlberg <sahlberg@google.com>
Date
Aug 25, 2014, 17:22 UTC
Message-ID
<CAL=YDW=k4QpejdqPgnVy+05hjsVHw35BBbKmsAEOP-9gBMjCJw@mail.gmail.com>
In-Reply-To
<20140823053322.GE18075@peff.net>

The next ref transaction series does a similar change and ends up removing the function lock_ref_sha1() completely. https://code-review.googlesource.com/#/c/1017/19/refs.c

So I think we can drop this patch.
ronnie sahlberg
On Fri, Aug 22, 2014 at 10:33 PM, Jeff King <peff@peff.net> wrote:
Show 83 quoted lines
> We can use lock_any_ref_for_update instead. Besides being
> more flexible, the only difference between the two is that
> lock_ref_sha1 does not allow "top-level" refs like
> "refs/foo" to be updated. However, we know that we do not
> have such a ref, because we explicitly add the "tags/"
> prefix ourselves.
>
> Note that we now must feed the whole name "refs/tags/X"
> instead of just "tags/X" to the function. As a result, our
> failure error message is uses the longer name. This is
> probably a good thing, though.
>
> As an interesting side note, if we forgot to switch this
> input to the function, the tests do not currently catch it.
> We import a tag "X" and then check that we can access it at
> "tags/X". If we accidentally created "tags/X" at the
> top-level $GIT_DIR instead of under "refs/", we would still
> find it due to our ref lookup procedure!
>
> We do not make such a mistake in this patch, of course, but
> while we're thinking about it, let's make the fast-import
> tests more robust by checking for fully qualified
> refnames.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> As I mentioned, I'd be OK with dropping this one in favor of just
> waiting for Ronnie's transaction patches to graduate.
>
>  fast-import.c          | 4 ++--
>  t/t9300-fast-import.sh | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index a1479e9..04a85a4 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1738,8 +1738,8 @@ static void dump_tags(void)
>
>         for (t = first_tag; t; t = t->next_tag) {
>                 strbuf_reset(&ref_name);
> -               strbuf_addf(&ref_name, "tags/%s", t->name);
> -               lock = lock_ref_sha1(ref_name.buf, NULL);
> +               strbuf_addf(&ref_name, "refs/tags/%s", t->name);
> +               lock = lock_any_ref_for_update(ref_name.buf, NULL, 0, NULL);
>                 if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0)
>                         failure |= error("Unable to update %s", ref_name.buf);
>         }
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 5fc9ef2..f4c6673 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -153,7 +153,7 @@ tag series-A
>  An annotated tag without a tagger
>  EOF
>  test_expect_success 'A: verify tag/series-A' '
> -       git cat-file tag tags/series-A >actual &&
> +       git cat-file tag refs/tags/series-A >actual &&
>         test_cmp expect actual
>  '
>
> @@ -165,7 +165,7 @@ tag series-A-blob
>  An annotated tag that annotates a blob.
>  EOF
>  test_expect_success 'A: verify tag/series-A-blob' '
> -       git cat-file tag tags/series-A-blob >actual &&
> +       git cat-file tag refs/tags/series-A-blob >actual &&
>         test_cmp expect actual
>  '
>
> @@ -232,8 +232,8 @@ EOF
>  test_expect_success \
>         'A: tag blob by sha1' \
>         'git fast-import <input &&
> -       git cat-file tag tags/series-A-blob-2 >actual &&
> -       git cat-file tag tags/series-A-blob-3 >>actual &&
> +       git cat-file tag refs/tags/series-A-blob-2 >actual &&
> +       git cat-file tag refs/tags/series-A-blob-3 >>actual &&
>         test_cmp expect actual'
>
>  test_tick
> --
> 2.1.0.346.ga0367b9
Previous: Jeff KingNext: Michael Haggerty
Message 10 of 14 in “fix pack-refs pruning of refs/foo”
  1. 0/5 fix pack-refs pruning of refs/fooJeff King, Aug 23, 2014
  2. 1/5 git-prompt: do not look for refs/stash in $GIT_DIRJeff King, Aug 23, 2014
  3. 2/5 pack-refs: prune top-level refs like "refs/foo"Jeff King, Aug 23, 2014
  4. 3/5 fast-import: clean up pack_data pointer in end_packfileJeff King, Aug 23, 2014
  5. Ronnie SahlbergAug 25, 2014
  6. Jeff KingAug 25, 2014
  7. 4/5 fast-import: fix buffer overflow in dump_tagsJeff King, Aug 23, 2014
  8. Ronnie SahlbergAug 25, 2014
  9. 5/5 fast-import: stop using lock_ref_sha1Jeff King, Aug 23, 2014
  10. Ronnie SahlbergAug 25, 2014
  11. Michael HaggertyAug 23, 2014
  12. Jeff KingAug 23, 2014
  13. Ronnie SahlbergAug 25, 2014
  14. Jeff KingAug 25, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.