> This is a bit difficult to read and there is no reason why we would need
> to read the entire upload_pack_config to determine if we need to filter
> things (we will need to read the config if cmd "fetch" is requested
> though).  Instead it may be better to do the following:
> 
>   if (value) {
>     strbuf_addstr(value, "shallow");
>     if (repo_config_get(r, "uplaodpack.filter"))
>       strbuf_addstr(value, " filter");
>   }
> 
> This way its easier to read and you only are reading the required value
> from the config.

Thanks, Brandon. I went ahead and used repo_config_get_bool(), and
indeed it works.

Removing the call to git_config() from there exposed another issue in
that configs were not read if upload-pack was used with protocol v2, so
I inserted a patch in the middle addressing that. While writing that
patch, I noticed that uploadpack.packobjectshook couldn't take filenames
with spaces, which I think is due to prepare_shell_cmd() in
run-command.c not quoting properly. Adding single quotes around "%s"
worked, but made other tests fail. Instead of continuing down that
rabbit hole, I just made the uploadpack.packobjectshook not have any
spaces, just like in t5544.

Jonathan Tan (3):
  upload-pack: fix error message typo
  upload-pack: read config when serving protocol v2
  {fetch,upload}-pack: support filter in protocol v2

 Documentation/technical/protocol-v2.txt |   9 ++
 fetch-pack.c                            |  23 ++++-
 t/t5701-git-serve.sh                    |  14 +++
 t/t5702-protocol-v2.sh                  | 112 ++++++++++++++++++++++++
 upload-pack.c                           |  19 +++-
 5 files changed, 171 insertions(+), 6 deletions(-)

-- 
2.17.0.441.gb46fe60e1d-goog

Reply via email to