All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Calvin Wan <calvinwan@google.com>
To: ZheNing Hu <adlternative@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Philip Oakley <philipoakley@iee.email>,
	johncai86@gmail.com, Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH v4 8/8] cat-file: add --batch-command remote-object-info command
Date: Mon, 8 Aug 2022 10:43:23 -0700	[thread overview]
Message-ID: <CAFySSZDMwMa-RFLVeAONnySrmsg53G21mXd-BDjyZEpyAVbMbg@mail.gmail.com> (raw)
In-Reply-To: <CAOLTT8RyDCokvJBaC1CCoT2+3AE1kOn7evjkTMK_V6KWoaH30g@mail.gmail.com>

> >  `rest`::
> >         If this atom is used in the output string, input lines are split
>
> Why not forbidden %(rest) here too?

Good catch. This should definitely be forbidden. I thought at first
this was inconsequential, but since I also have remote as part of
the input, this would no longer hold true.

> > +               if (strstr(format, "%(objectsize:disk)"))
> > +                       die(_("objectsize:disk is currently not supported with remote-object-info"));
> > +               if (strstr(format, "%(deltabase)"))
> > +                       die(_("deltabase is currently not supported with remote-object-info"));
>
> %(rest) too?

ditto

> Maybe such code style will be better?
>
>         if (!gtransport->smart_options) {
>                return -1;
>         }
>         ...
>         return transport_fetch_refs(gtransport, NULL);

That does look better!

> > -                       cmd->fn(opt, p, output, data);
> > +                       if (!strcmp(cmd->name, "remote-object-info")) {
> > +                               char *line = xstrdup_or_null(p);
> > +                               parse_remote_info(opt, line, output, data, cmd, NULL);
>
> memory leak: "line".

ack

> > +test_expect_success 'remote-object-info fails on unspported filter option (deltabase)' '
> > +       (
> > +               cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
> > +
> > +               test_must_fail git cat-file --batch-command="%(deltabase)" 2>err <<-EOF &&
> > +               remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1
> > +               EOF
> > +               test_i18ngrep "deltabase is currently not supported with remote-object-info" err
> > +       )
> > +'
> > +
>
> %(rest) too?

ditto

> > +test_expect_success 'batch-command remote-object-info file://' '
> > +       (
> > +               cd server &&
> > +
> > +               echo "$hello_sha1 $hello_size" >expect &&
> > +               echo "$tree_sha1 $tree_size" >>expect &&
> > +               echo "$commit_sha1 $commit_size" >>expect &&
> > +               echo "$tag_sha1 $tag_size" >>expect &&
> > +               git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
> > +               remote-object-info "file://$(pwd)" $hello_sha1
> > +               remote-object-info "file://$(pwd)" $tree_sha1
> > +               remote-object-info "file://$(pwd)" $commit_sha1
> > +               remote-object-info "file://$(pwd)" $tag_sha1
> > +               EOF
> > +               test_cmp expect actual
> > +       )
>
> Can we support <rev> instead of only <oid> here?

Not at the current moment. The server is unable to handle
anything besides oids.

>
> $ git cat-file --batch-check
> HEAD
> 28583b8d8ca72730d7c9e0ea50861ad431a6dea4 commit 3038
> master
> ab336e8f1c8009c8b1aab8deb592148e69217085 commit 281
> origin/master
> 23b219f8e3f2adfb0441e135f0a880e6124f766c commit 282
> origin/master:git.c
> e5d62fa5a92e95e1ede041ebf913d841744c31f8 blob 28398
>
> So cat-file --batch-check can support it.
>
> $git cat-file --batch-commands
> remote-object-info "file://$(pwd)" master:git.c
>
> I guess it cannot work now, right?

Correct.

  reply	other threads:[~2022-08-08 17:44 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-08 23:19 [PATCH] fetch —object-info-format: client option for object-info Calvin Wan
2022-02-08 23:56 ` [PATCH v2] " Calvin Wan
2022-02-09 12:48   ` Philip Oakley
2022-02-10 22:32     ` Calvin Wan
2022-02-09 20:41   ` [PATCH v2] fetch object-info-format: " Jonathan Tan
2022-02-10 22:58     ` Calvin Wan
2022-03-28 19:11   ` [PATCH v3 0/3] object-info: add option for retrieving object info Calvin Wan
2022-03-28 19:11     ` [PATCH v3 1/3] fetch-pack: refactor packet writing and fetch options Calvin Wan
2022-03-29 22:54       ` Junio C Hamano
2022-03-29 23:01       ` Taylor Blau
2022-03-30 21:55       ` Jonathan Tan
2022-03-28 19:11     ` [PATCH v3 2/3] transfer.advertiseObjectInfo: add object-info config Calvin Wan
2022-03-29 22:34       ` Junio C Hamano
2022-03-29 22:48         ` Calvin Wan
2022-03-29 23:09           ` Taylor Blau
2022-03-28 19:11     ` [PATCH v3 3/3] object-info: add option for retrieving object info Calvin Wan
2022-03-29 19:57       ` Junio C Hamano
2022-03-29 22:54       ` Junio C Hamano
2022-03-30  0:49         ` Junio C Hamano
2022-03-30 22:31           ` Calvin Wan
2022-03-30 22:43         ` Jonathan Tan
2022-03-30 23:42           ` Junio C Hamano
2022-03-29 23:19       ` Taylor Blau
2022-03-30 22:47         ` Calvin Wan
2022-03-30 22:06       ` John Cai
2022-03-31 19:56         ` Calvin Wan
2022-04-01 16:16           ` Junio C Hamano
2022-03-30 22:07       ` Jonathan Tan
2022-03-30 22:12       ` Josh Steadmon
2022-03-30 22:46         ` Calvin Wan
2022-03-29 20:35     ` [PATCH v3 0/3] " Junio C Hamano
2022-03-29 22:40       ` Calvin Wan
2022-03-31  1:50     ` Junio C Hamano
2022-05-02 17:08     ` [PATCH v4 0/8] cat-file: add --batch-command remote-object-info command Calvin Wan
2022-05-02 17:08       ` [PATCH v4 1/8] fetch-pack: refactor packet writing Calvin Wan
2022-05-02 17:08       ` [PATCH v4 2/8] fetch-pack: move fetch default settings Calvin Wan
2022-05-02 22:58         ` Junio C Hamano
2022-05-03 23:06           ` Jonathan Tan
2022-05-05 18:13             ` Calvin Wan
2022-05-02 17:08       ` [PATCH v4 3/8] object-store: add function to free object_info contents Calvin Wan
2022-05-02 23:23         ` Junio C Hamano
2022-05-04 19:09           ` Junio C Hamano
2022-05-05  0:15           ` Junio C Hamano
2022-05-05 16:47             ` Calvin Wan
2022-05-05 17:01               ` Junio C Hamano
2022-05-02 17:09       ` [PATCH v4 4/8] object-info: send attribute packet regardless of object ids Calvin Wan
2022-05-03  0:05         ` Junio C Hamano
2022-05-03 19:21           ` Calvin Wan
2022-05-03 23:11         ` Jonathan Tan
2022-05-02 17:09       ` [PATCH v4 5/8] transport: add client side capability to request object-info Calvin Wan
2022-05-03  0:54         ` Junio C Hamano
2022-05-03 18:58           ` Calvin Wan
2022-05-04 15:42             ` Junio C Hamano
2022-05-03 23:15         ` Jonathan Tan
2022-05-04 15:50           ` Junio C Hamano
2022-05-02 17:09       ` [PATCH v4 6/8] transport: add object-info fallback to fetch Calvin Wan
2022-05-03 23:27         ` Jonathan Tan
2022-05-02 17:09       ` [PATCH v4 7/8] cat-file: move parse_cmd and DEFAULT_FORMAT up Calvin Wan
2022-05-02 17:09       ` [PATCH v4 8/8] cat-file: add --batch-command remote-object-info command Calvin Wan
2022-05-04 21:27         ` Jonathan Tan
2022-05-05 18:13           ` Calvin Wan
2022-05-05 18:44             ` Junio C Hamano
2022-05-05 19:09               ` Junio C Hamano
2022-05-05 19:19                 ` Calvin Wan
2022-07-31 15:24         ` ZheNing Hu
2022-08-08 17:43           ` Calvin Wan [this message]
2022-07-28 23:02       ` [PATCH v5 0/6] " Calvin Wan
2022-07-28 23:56         ` Junio C Hamano
2022-07-29  0:02           ` Junio C Hamano
2022-07-31  8:41         ` Phillip Wood
2022-08-04 22:57           ` Calvin Wan
2022-09-30 23:23         ` Junio C Hamano
2022-07-28 23:02       ` [PATCH v5 1/6] fetch-pack: refactor packet writing Calvin Wan
2022-07-28 23:02       ` [PATCH v5 2/6] fetch-pack: move fetch initialization Calvin Wan
2022-07-28 23:02       ` [PATCH v5 3/6] protocol-caps: initialization bug fix Calvin Wan
2022-07-29 17:51         ` Junio C Hamano
2022-07-28 23:02       ` [PATCH v5 4/6] serve: advertise object-info feature Calvin Wan
2022-07-29 17:57         ` Junio C Hamano
2022-08-01 18:28           ` Calvin Wan
2022-08-01 18:44             ` Ævar Arnfjörð Bjarmason
2022-08-01 18:47             ` Junio C Hamano
2022-08-01 18:58               ` Calvin Wan
2022-07-28 23:02       ` [PATCH v5 5/6] transport: add client support for object-info Calvin Wan
2022-07-29 18:06         ` Junio C Hamano
2022-08-04 20:28           ` Calvin Wan
2022-08-01 13:30         ` Phillip Wood
2022-08-04 22:20           ` Calvin Wan
2022-08-08 10:07             ` Phillip Wood
2022-08-01 14:26         ` Phillip Wood
2022-08-08  9:16         ` Phillip Wood
2022-07-28 23:02       ` [PATCH v5 6/6] cat-file: add remote-object-info to batch-command Calvin Wan
2022-07-29  6:25         ` Ævar Arnfjörð Bjarmason
2022-08-07  5:50           ` ZheNing Hu
2022-08-08 18:07           ` Calvin Wan
2022-08-11 10:58             ` Ævar Arnfjörð Bjarmason
2022-07-29 18:21         ` Junio C Hamano
2022-08-08 18:37           ` Calvin Wan
2022-09-28 13:12         ` Ævar Arnfjörð Bjarmason
2022-07-31 15:02       ` [PATCH v4 0/8] cat-file: add --batch-command remote-object-info command ZheNing Hu
2022-08-08 17:32         ` Calvin Wan
2022-08-13 22:17       ` Junio C Hamano
2022-02-09 19:09 ` [PATCH] fetch —object-info-format: client option for object-info John Cai
2022-02-10 22:49   ` Calvin Wan

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=CAFySSZDMwMa-RFLVeAONnySrmsg53G21mXd-BDjyZEpyAVbMbg@mail.gmail.com \
    --to=calvinwan@google.com \
    --cc=adlternative@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=me@ttaylorr.com \
    --cc=philipoakley@iee.email \
    /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.