Re: [PATCH v3 6/6] cat-file: add remote-object-info to batch-command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 26, 2024 at 3:39 AM Eric Ju <eric.peijian@xxxxxxxxx> wrote:

> And finally for --buffer mode `remote-object-info`:
>  - Receive and parse input from user
>  - Store respective function attached to command in a queue
>  - After flush, loop through commands in queue:
>     If command is `remote-object-info`:
>         - Get object info from remote
>         - Loop through and print each object info
>     Else:
>         - Call respective function attached to command
>         - Get object info, print object info
>
> To summarize, `remote-object-info` gets object info from the remote and
> then loop through the object info passed in, print the info.

Maybe: s/print the info/printing the info/

> In order for remote-object-info to avoid remote communication overhead
> in the non-buffer mode, the objects are passed in as such:
>
> remote-object-info <remote> <oid> <oid> ... <oid>
>
> rather than
>
> remote-object-info <remote> <oid>
> remote-object-info <remote> <oid>
> ...
> remote-object-info <remote> <oid>

[...]

>  If no format is specified, the default format is `%(objectname)
> -%(objecttype) %(objectsize)`.
> +%(objecttype) %(objectsize)`, except for `remote-object-info` commands which use
> +`%(objectname) %(objectsize)` for now because "%(objecttype)" is not supported yet.
> +When "%(objecttype)" is supported, default format should be unified.

I think we should warn more clearly and strongly that users should
take into account that the default format will change. So they should
better not rely on the current format in their code.

Maybe something like:

`%(objectname) %(objectsize)` for now because "%(objecttype)" is not
supported yet.
WARNING: When "%(objecttype)" is supported, default format WILL be unified, so
DO NOT RELY on the current default format to stay the same!!!

>  If `--batch` is specified, or if `--batch-command` is used with the `contents`
>  command, the object information is followed by the object contents (consisting

[...]

> diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
> new file mode 100755
> index 0000000000..6826ff7a59
> --- /dev/null
> +++ b/t/t1017-cat-file-remote-object-info.sh
> @@ -0,0 +1,750 @@
> +#!/bin/sh
> +
> +test_description='git cat-file --batch-command with remote-object-info command'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +echo_without_newline () {
> +    printf '%s' "$*"
> +}
> +
> +echo_without_newline_nul () {
> +    echo_without_newline "$@" | tr '\n' '\0'
> +}
> +
> +strlen () {
> +    echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
> +}

The above functions have been copied verbatim from t1006-cat-file.sh.
I think this is worth a comment or a TODO before these functions
saying that common code might want to be unified in the future.

Maybe something like:

# TODO: refactor these functions which were copied from
t1006-cat-file.sh into a new common file, maybe "lib-cat-file.sh"

Except the above nits and another one I found in patch 4/6, the rest
of this patch series looks good to me.

Thanks!





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux