mbox series

[v2,0/3] upload-pack: support a missing-action

Message ID 20240515132543.851987-1-christian.couder@gmail.com (mailing list archive)
Headers show
Series upload-pack: support a missing-action | expand

Message

Christian Couder May 15, 2024, 1:25 p.m. UTC
`git pack-objects` already supports a `--missing=<missing-action>`
option, so that it can avoid erroring out if some objects aren't
available.

It is interesting to have `git upload-pack` support a similar way to
avoid sending some objects in case they aren't available on the
server.

For example, in case both the server and the client are using a
separate promisor remote that contain some objects, it can be better
if the server doesn't try to send such objects back to the client, but
instead let the client get those objects separately from the promisor
remote. (The client needs to have the separate promisor remote
configured, for that to work.)

This could work better if there was something, like perhaps a
capability, for the client to tell the server something like:

  "I know how to fetch missing objects from this and that
  promisor remotes, so if you choose to, you may omit objects
  that you know are available from these promisor remotes when
  sending objects to me."

But that capability could be added later as other similar capabilities
in this area could be very useful. For example in case of a client
cloning, something like the following is currently needed:

  GIT_NO_LAZY_FETCH=0 git clone
      -c remote.my_promisor.promisor=true \
      -c remote.my_promisor.fetch="+refs/heads/*:refs/remotes/my_promisor/*" \
      -c remote.my_promisor.url=<MY_PROMISOR_URL> \
      --filter="blob:limit=5k" server

But it would be nice if there was a capability for the client to say
that it would like the server to give it information about the
promisor that it could use, so that the user doesn't have to pass all
the "remote.my_promisor.XXX" config options on the command like. (It
would then be a bit similar to the bundle-uri feature where all the
bundle related information comes from the server.)

Another example use of this feature could be a server where some
objects have been corrupted or deleted. It could still be useful for
clients who could get those objects from another source, like perhaps
a different client, to be able to fetch or clone from the server.

The fact that the new `uploadpack.missingAction` configuration
variable has to be set to a non default value on the server means that
regular client users cannot hurt themselves with this feature.

As `git rev-list` also supports a `--missing=<missing-action>` option,
the first 2 patches in this series are about refactoring related code
from both `git rev-list` and `git pack-objects` into new
"missing.{c,h}" files. Patch 3/3 then adds a new
`uploadpack.missingAction` configuration variable.

The changes since v1 are the following:

  - In patch 1/3 the refactored parse_missing_action_value() function
    doesn't change the fetch_if_missing global variable anymore. The
    clients of this function change it by themselves instead.

  - In patch 1/3 a minor typo was also fixed.

  - Old patch 2/4 that added a 'int print_ok' argument to
    parse_missing_action_value() was removed, and now callers deal
    with only the values they recognise.

Thanks to Junio for his review and for suggesting of the above
changes.

Christian Couder (3):
  rev-list: refactor --missing=<missing-action>
  pack-objects: use the missing action API
  upload-pack: allow configuring a missing-action

 Documentation/config/uploadpack.txt |   9 ++
 Makefile                            |   1 +
 builtin/pack-objects.c              |  48 ++++++-----
 builtin/rev-list.c                  |  43 ++--------
 missing.c                           |  36 ++++++++
 missing.h                           |  19 +++++
 t/t5706-upload-pack-missing.sh      | 125 ++++++++++++++++++++++++++++
 upload-pack.c                       |  19 +++++
 8 files changed, 242 insertions(+), 58 deletions(-)
 create mode 100644 missing.c
 create mode 100644 missing.h
 create mode 100755 t/t5706-upload-pack-missing.sh

Comments

Christian Couder May 15, 2024, 1:59 p.m. UTC | #1
Sorry everyone, it looks like I put the wrong message Id in the
--in-reply-to= option of my `git send-email` command and this appears
to be part of a different thread. The right message ID should have
been:

20240418184043.2900955-1-christian.couder@gmail.com

Sorry again.

On Wed, May 15, 2024 at 3:25 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> `git pack-objects` already supports a `--missing=<missing-action>`
> option, so that it can avoid erroring out if some objects aren't
> available.
>
> It is interesting to have `git upload-pack` support a similar way to
> avoid sending some objects in case they aren't available on the
> server.
>
> For example, in case both the server and the client are using a
> separate promisor remote that contain some objects, it can be better
> if the server doesn't try to send such objects back to the client, but
> instead let the client get those objects separately from the promisor
> remote. (The client needs to have the separate promisor remote
> configured, for that to work.)
>
> This could work better if there was something, like perhaps a
> capability, for the client to tell the server something like:
>
>   "I know how to fetch missing objects from this and that
>   promisor remotes, so if you choose to, you may omit objects
>   that you know are available from these promisor remotes when
>   sending objects to me."
>
> But that capability could be added later as other similar capabilities
> in this area could be very useful. For example in case of a client
> cloning, something like the following is currently needed:
>
>   GIT_NO_LAZY_FETCH=0 git clone
>       -c remote.my_promisor.promisor=true \
>       -c remote.my_promisor.fetch="+refs/heads/*:refs/remotes/my_promisor/*" \
>       -c remote.my_promisor.url=<MY_PROMISOR_URL> \
>       --filter="blob:limit=5k" server
>
> But it would be nice if there was a capability for the client to say
> that it would like the server to give it information about the
> promisor that it could use, so that the user doesn't have to pass all
> the "remote.my_promisor.XXX" config options on the command like. (It
> would then be a bit similar to the bundle-uri feature where all the
> bundle related information comes from the server.)
>
> Another example use of this feature could be a server where some
> objects have been corrupted or deleted. It could still be useful for
> clients who could get those objects from another source, like perhaps
> a different client, to be able to fetch or clone from the server.
>
> The fact that the new `uploadpack.missingAction` configuration
> variable has to be set to a non default value on the server means that
> regular client users cannot hurt themselves with this feature.
>
> As `git rev-list` also supports a `--missing=<missing-action>` option,
> the first 2 patches in this series are about refactoring related code
> from both `git rev-list` and `git pack-objects` into new
> "missing.{c,h}" files. Patch 3/3 then adds a new
> `uploadpack.missingAction` configuration variable.
>
> The changes since v1 are the following:
>
>   - In patch 1/3 the refactored parse_missing_action_value() function
>     doesn't change the fetch_if_missing global variable anymore. The
>     clients of this function change it by themselves instead.
>
>   - In patch 1/3 a minor typo was also fixed.
>
>   - Old patch 2/4 that added a 'int print_ok' argument to
>     parse_missing_action_value() was removed, and now callers deal
>     with only the values they recognise.
>
> Thanks to Junio for his review and for suggesting of the above
> changes.
>
> Christian Couder (3):
>   rev-list: refactor --missing=<missing-action>
>   pack-objects: use the missing action API
>   upload-pack: allow configuring a missing-action
>
>  Documentation/config/uploadpack.txt |   9 ++
>  Makefile                            |   1 +
>  builtin/pack-objects.c              |  48 ++++++-----
>  builtin/rev-list.c                  |  43 ++--------
>  missing.c                           |  36 ++++++++
>  missing.h                           |  19 +++++
>  t/t5706-upload-pack-missing.sh      | 125 ++++++++++++++++++++++++++++
>  upload-pack.c                       |  19 +++++
>  8 files changed, 242 insertions(+), 58 deletions(-)
>  create mode 100644 missing.c
>  create mode 100644 missing.h
>  create mode 100755 t/t5706-upload-pack-missing.sh
>
> --
> 2.45.1.148.g0f5efb064b
>