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

Re: [PATCH v6 00/40] Add initial experimental external ODB support

From
Ben Peart <peartben@gmail.com>
Date
Oct 2, 2017, 14:18 UTC
Message-ID
<1d53569f-4bc8-0d8e-0182-eec0eb6634cd@gmail.com>
In-Reply-To
<20170916080731.13925-1-chriscool@tuxfamily.org>
On 9/16/2017 4:06 AM, Christian Couder wrote:
> Highlevel view of the patches in the series
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>

This is a massive patch series and IMO keeping it a single monolithic set of patches makes it difficult to review and unwieldy to make progress on as an "all or nothing" series.

I highly recommend breaking it up into multiple smaller patch series that can be reviewed and accepted individually. I have to admit I've skipped reviewing the last couple of iterations simply because of the time investment required and difficulty separating out the various pieces.

I think using your division of the patches below is a good place to start. Many of these would be good changes even if they weren't part of the larger external ODB effort.

       - Patch 1/40 is a small code cleanup that I already sent to the
Show 10 quoted lines
>        mailing list but may be removed in the end due to ongoing work
>        on "git clone".
> 
>      - Patches 02/40 to 07/40 create a "Git/Packet.pm" module by
>        refactoring "t0021/rot13-filter.pl". Functions from this new
>        module will be used later in test scripts. According to Junio's
>        suggestion compared to v5 we now first fully refactor
>        "t0021/rot13-filter.pl" before creating the "Git/Packet.pm"
>        module.
>

This seems like a very logical thing to do and should be split out so that progress can be made independently.

Show 7 quoted lines
>      - Patches 08/40 to 16/40 create the external ODB insfrastructure
>        in external-odb.{c,h} and odb-helper.{c,h} for the script mode.
>        The main changes compared to v5 are the following:
>          - we mark as "extern" functions in *.h files
> 	- we use sha1_pos() instead of sha1_entry_pos()
> 	- we check the size in the header when we 'get' a Git object
>

This is the heart of the ODB infrastructure series. I'll respond in more detail in the specific patches.

Show 193 quoted lines
>      - Patches 17/40 to 23/40 improve lib-http to make it possible to
>        use it as an external ODB to test storing blobs in an HTTP
>        server. The "upload.sh" and "list.sh" files are now properly
>        indented and they use %% instead of % in parameter
>        substitutions compared to v5.
> 
>      - Patches 24/40 to 32/40 improve the external ODB insfrastructure
>        to support sub-processes and make everything work using
>        them. The main changes compared to v5 are the following:
>          - we mark as "extern" functions in *.h files
> 	- we use the new subprocess_handshake() function
> 	- we check the size in the header when we 'get' a Git object
> 
>      - Patch 33/40 uses attributes to mark blobs that should be handled
>        by an external odb.
> 
>      - Patch 34/40 adds documentation about the external odb
>        mechanism. This patch has been much improved since v5.
> 
>      - Patches 35/40 to 39/40 add the --initial-refspec to git clone
>        along with tests.
> 
>      - Patch 40/40 adds documentation about transfering objects and
>        metadata when using the external odb mechanism. This patch is
>        new since v5.
> 
> Future work
> ~~~~~~~~~~~
> 
> There are still things that could be cleaned or improved. I think I
> may work on:
> 
>    - Integrate changes in recent "read-object-process" work by Ben Peart.
> 
>    - Better test all the combinations of the different modes with and
>      without "have" and "put_*" instructions.
> 
>    - Maybe implement the missing kinds of 'put' ('put_git_obj' and
>      'put_direct'), so that Git could pass either a git object a plain
>      object or ask the helper to retreive it directly from Git's object
>      database.
> 
>    - Add more long running tests and improve tests in general.
> 
> Previous work and discussions
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> (Sorry for the old Gmane links, I hope I will try to replace them with
> public-inbox.org at one point.)
> 
> Peff started to work on this and discuss this some years ago:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/206886/focus=207040
> http://thread.gmane.org/gmane.comp.version-control.git/247171
> http://thread.gmane.org/gmane.comp.version-control.git/202902/focus=203020
> 
> His work, which is not compile-tested any more, is still there:
> 
> https://github.com/peff/git/commits/jk/external-odb-wip
> 
> Initial discussions about this new series are there:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/288151/focus=295160
> 
> Version 1, 2, 3, 4 and 5 of this series are here:
> 
> https://public-inbox.org/git/20160613085546.11784-1-chriscool@tuxfamily.org/
> https://public-inbox.org/git/20160628181933.24620-1-chriscool@tuxfamily.org/
> https://public-inbox.org/git/20161130210420.15982-1-chriscool@tuxfamily.org/
> https://public-inbox.org/git/20170620075523.26961-1-chriscool@tuxfamily.org/
> https://public-inbox.org/git/20170803091926.1755-1-chriscool@tuxfamily.org/
> 
> Some of the discussions related to Ben Peart's work that is used by
> this series are here:
> 
> https://public-inbox.org/git/20170113155253.1644-1-benpeart@microsoft.com/
> https://public-inbox.org/git/20170322165220.5660-1-benpeart@microsoft.com/
> https://public-inbox.org/git/20170714132651.170708-1-benpeart@microsoft.com/
> 
> Links
> ~~~~~
> 
> This patch series is available here:
> 
> https://github.com/chriscool/git/commits/external-odb
> 
> Version 1, 2, 3, 4 and 5 are here:
> 
> https://github.com/chriscool/git/commits/gl-external-odb12
> https://github.com/chriscool/git/commits/gl-external-odb22
> https://github.com/chriscool/git/commits/gl-external-odb61
> https://github.com/chriscool/git/commits/gl-external-odb239
> https://github.com/chriscool/git/commits/gl-external-odb373
> 
> 
> Ben Peart (2):
>    odb-helper: add init_object_process()
>    Add t0450 to test 'get_direct' mechanism
> 
> Christian Couder (38):
>    builtin/clone: get rid of 'value' strbuf
>    t0021/rot13-filter: refactor packet reading functions
>    t0021/rot13-filter: improve 'if .. elsif .. else' style
>    t0021/rot13-filter: improve error message
>    t0021/rot13-filter: add packet_initialize()
>    t0021/rot13-filter: add capability functions
>    Add Git/Packet.pm from parts of t0021/rot13-filter.pl
>    sha1_file: prepare for external odbs
>    Add initial external odb support
>    odb-helper: add odb_helper_init() to send 'init' instruction
>    t0400: add 'put_raw_obj' instruction to odb-helper script
>    external odb: add 'put_raw_obj' support
>    external-odb: accept only blobs for now
>    t0400: add test for external odb write support
>    Add GIT_NO_EXTERNAL_ODB env variable
>    Add t0410 to test external ODB transfer
>    lib-httpd: pass config file to start_httpd()
>    lib-httpd: add upload.sh
>    lib-httpd: add list.sh
>    lib-httpd: add apache-e-odb.conf
>    odb-helper: add odb_helper_get_raw_object()
>    pack-objects: don't pack objects in external odbs
>    Add t0420 to test transfer to HTTP external odb
>    external-odb: add 'get_direct' support
>    odb-helper: add 'script_mode' to 'struct odb_helper'
>    Add t0460 to test passing git objects
>    odb-helper: add put_object_process()
>    Add t0470 to test passing raw objects
>    odb-helper: add have_object_process()
>    Add t0480 to test "have" capability and raw objects
>    external-odb: use 'odb=magic' attribute to mark odb blobs
>    Add Documentation/technical/external-odb.txt
>    clone: add 'initial' param to write_remote_refs()
>    clone: add --initial-refspec option
>    clone: disable external odb before initial clone
>    Add tests for 'clone --initial-refspec'
>    Add t0430 to test cloning using bundles
>    Doc/external-odb: explain transfering objects and metadata
> 
>   Documentation/technical/external-odb.txt |  447 +++++++++++++
>   Makefile                                 |    2 +
>   builtin/clone.c                          |   91 ++-
>   builtin/pack-objects.c                   |    4 +
>   cache.h                                  |   18 +
>   environment.c                            |    4 +
>   external-odb.c                           |  196 ++++++
>   external-odb.h                           |   12 +
>   odb-helper.c                             | 1076 ++++++++++++++++++++++++++++++
>   odb-helper.h                             |   45 ++
>   perl/Git/Packet.pm                       |  118 ++++
>   sha1_file.c                              |  155 +++--
>   t/lib-httpd.sh                           |    8 +-
>   t/lib-httpd/apache-e-odb.conf            |  214 ++++++
>   t/lib-httpd/list.sh                      |   41 ++
>   t/lib-httpd/upload.sh                    |   45 ++
>   t/t0021/rot13-filter.pl                  |  110 +--
>   t/t0400-external-odb.sh                  |   85 +++
>   t/t0410-transfer-e-odb.sh                |  147 ++++
>   t/t0420-transfer-http-e-odb.sh           |  152 +++++
>   t/t0430-clone-bundle-e-odb.sh            |   85 +++
>   t/t0450-read-object.sh                   |   28 +
>   t/t0450/read-object                      |   68 ++
>   t/t0460-read-object-git.sh               |   28 +
>   t/t0460/read-object-git                  |   78 +++
>   t/t0470-read-object-http-e-odb.sh        |  119 ++++
>   t/t0470/read-object-plain                |   83 +++
>   t/t0480-read-object-have-http-e-odb.sh   |  119 ++++
>   t/t0480/read-object-plain-have           |  103 +++
>   t/t5616-clone-initial-refspec.sh         |   48 ++
>   30 files changed, 3588 insertions(+), 141 deletions(-)
>   create mode 100644 Documentation/technical/external-odb.txt
>   create mode 100644 external-odb.c
>   create mode 100644 external-odb.h
>   create mode 100644 odb-helper.c
>   create mode 100644 odb-helper.h
>   create mode 100644 perl/Git/Packet.pm
>   create mode 100644 t/lib-httpd/apache-e-odb.conf
>   create mode 100644 t/lib-httpd/list.sh
>   create mode 100644 t/lib-httpd/upload.sh
>   create mode 100755 t/t0400-external-odb.sh
>   create mode 100755 t/t0410-transfer-e-odb.sh
>   create mode 100755 t/t0420-transfer-http-e-odb.sh
>   create mode 100755 t/t0430-clone-bundle-e-odb.sh
>   create mode 100755 t/t0450-read-object.sh
>   create mode 100755 t/t0450/read-object
>   create mode 100755 t/t0460-read-object-git.sh
>   create mode 100755 t/t0460/read-object-git
>   create mode 100755 t/t0470-read-object-http-e-odb.sh
>   create mode 100755 t/t0470/read-object-plain
>   create mode 100755 t/t0480-read-object-have-http-e-odb.sh
>   create mode 100755 t/t0480/read-object-plain-have
>   create mode 100755 t/t5616-clone-initial-refspec.sh
>
Previous: Christian CouderNext: Christian Couder
Message 48 of 49 in “Add initial experimental external ODB support”
  1. 00/40 Add initial experimental external ODB supportChristian Couder, Sep 16, 2017
  2. 01/40 builtin/clone: get rid of 'value' strbufChristian Couder, Sep 16, 2017
  3. 04/40 t0021/rot13-filter: improve error messageChristian Couder, Sep 16, 2017
  4. 05/40 t0021/rot13-filter: add packet_initialize()Christian Couder, Sep 16, 2017
  5. 07/40 Add Git/Packet.pm from parts of t0021/rot13-filter.plChristian Couder, Sep 16, 2017
  6. 11/40 t0400: add 'put_raw_obj' instruction to odb-helper scriptChristian Couder, Sep 16, 2017
  7. 10/40 odb-helper: add odb_helper_init() to send 'init' instructionChristian Couder, Sep 16, 2017
  8. 16/40 Add t0410 to test external ODB transferChristian Couder, Sep 16, 2017
  9. 14/40 t0400: add test for external odb write supportChristian Couder, Sep 16, 2017
  10. 19/40 lib-httpd: add list.shChristian Couder, Sep 16, 2017
  11. 22/40 pack-objects: don't pack objects in external odbsChristian Couder, Sep 16, 2017
  12. 26/40 odb-helper: add init_object_process()Christian Couder, Sep 16, 2017
  13. 29/40 odb-helper: add put_object_process()Christian Couder, Sep 16, 2017
  14. 30/40 Add t0470 to test passing raw objectsChristian Couder, Sep 16, 2017
  15. 31/40 odb-helper: add have_object_process()Christian Couder, Sep 16, 2017
  16. 32/40 Add t0480 to test "have" capability and raw objectsChristian Couder, Sep 16, 2017
  17. 33/40 external-odb: use 'odb=magic' attribute to mark odb blobsChristian Couder, Sep 16, 2017
  18. 38/40 Add tests for 'clone --initial-refspec'Christian Couder, Sep 16, 2017
  19. 39/40 Add t0430 to test cloning using bundlesChristian Couder, Sep 16, 2017
  20. 40/40 Doc/external-odb: explain transfering objects and metadataChristian Couder, Sep 16, 2017
  21. 37/40 clone: disable external odb before initial cloneChristian Couder, Sep 16, 2017
  22. 35/40 clone: add 'initial' param to write_remote_refs()Christian Couder, Sep 16, 2017
  23. 36/40 clone: add --initial-refspec optionChristian Couder, Sep 16, 2017
  24. 34/40 Add Documentation/technical/external-odb.txtChristian Couder, Sep 16, 2017
  25. 25/40 odb-helper: add 'script_mode' to 'struct odb_helper'Christian Couder, Sep 16, 2017
  26. 28/40 Add t0460 to test passing git objectsChristian Couder, Sep 16, 2017
  27. 23/40 Add t0420 to test transfer to HTTP external odbChristian Couder, Sep 16, 2017
  28. 21/40 odb-helper: add odb_helper_get_raw_object()Christian Couder, Sep 16, 2017
  29. 20/40 lib-httpd: add apache-e-odb.confChristian Couder, Sep 16, 2017
  30. 18/40 lib-httpd: add upload.shChristian Couder, Sep 16, 2017
  31. 24/40 external-odb: add 'get_direct' supportChristian Couder, Sep 16, 2017
  32. 27/40 Add t0450 to test 'get_direct' mechanismChristian Couder, Sep 16, 2017
  33. 15/40 Add GIT_NO_EXTERNAL_ODB env variableChristian Couder, Sep 16, 2017
  34. 13/40 external-odb: accept only blobs for nowChristian Couder, Sep 16, 2017
  35. 17/40 lib-httpd: pass config file to start_httpd()Christian Couder, Sep 16, 2017
  36. 12/40 external odb: add 'put_raw_obj' supportChristian Couder, Sep 16, 2017
  37. 06/40 t0021/rot13-filter: add capability functionsChristian Couder, Sep 16, 2017
  38. 09/40 Add initial external odb supportChristian Couder, Sep 16, 2017
  39. Jonathan TanSep 19, 2017
  40. Christian CouderSep 27, 2017
  41. Jonathan TanSep 29, 2017
  42. Ben PeartOct 2, 2017
  43. Christian CouderOct 3, 2017
  44. Jonathan TanOct 4, 2017
  45. 03/40 t0021/rot13-filter: improve 'if .. elsif .. else' styleChristian Couder, Sep 16, 2017
  46. 08/40 sha1_file: prepare for external odbsChristian Couder, Sep 16, 2017
  47. 02/40 t0021/rot13-filter: refactor packet reading functionsChristian Couder, Sep 16, 2017
  48. Ben PeartOct 2, 2017
  49. Christian CouderOct 3, 2017

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.