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

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

 





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
       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.

     - 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.

     - 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@xxxxxxxxxxxxx/
https://public-inbox.org/git/20160628181933.24620-1-chriscool@xxxxxxxxxxxxx/
https://public-inbox.org/git/20161130210420.15982-1-chriscool@xxxxxxxxxxxxx/
https://public-inbox.org/git/20170620075523.26961-1-chriscool@xxxxxxxxxxxxx/
https://public-inbox.org/git/20170803091926.1755-1-chriscool@xxxxxxxxxxxxx/

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@xxxxxxxxxxxxx/
https://public-inbox.org/git/20170322165220.5660-1-benpeart@xxxxxxxxxxxxx/
https://public-inbox.org/git/20170714132651.170708-1-benpeart@xxxxxxxxxxxxx/

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




[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