Re: [PATCHv3 0/9] object store: oid_object_info is the next contender

2018-04-26 Thread Brandon Williams 
On 04/25, Stefan Beller wrote:
> v3:
> * fixed and extended the commit message of last commit
> * fixed the last patch, as Jonathan Tan suggested, see interdiff:
> 
> $ git diff remotes/origin/sb/oid-object-info (which is v2)
> diff --git c/sha1_file.c w/sha1_file.c
> index 94123e0299..dcd6b879ac 100644
> --- c/sha1_file.c
> +++ w/sha1_file.c
> @@ -1289,14 +1289,13 @@ int oid_object_info_extended(struct repository 
> *r, const struct object_id *oid,
>  
> /* Check if it is a missing object */
> if (fetch_if_missing && repository_format_partial_clone &&
> -   !already_retried) {
> +   !already_retried && r == the_repository) {
> /*
>  * TODO Investigate having fetch_object() return
>  * TODO error/success and stopping the music here.
> -* TODO Pass a repository struct through 
> fetch_object.
> +* TODO Pass a repository struct through 
> fetch_object,
> +* such that arbitrary repositories work.
>  */
> -   if (r != the_repository)
> -   die(_("partial clones only supported in 
> the_repository"));
> fetch_object(repository_format_partial_clone, 
> real->hash);
> already_retried = 1;
> continue;
> 
> Thanks,
> Stefan

v3 looks good, thanks for taking care of this.

> 
> v2:
> 
> * fixed the sha1/oid typo
> * removed spurious new line
> * Brandon and Jonthan discovered another dependency that I missed due
>   to cherrypicking that commit from a tree before partial clone was a thing.
>   We error out when attempting to use fetch_object for repos that are not
>   the_repository.
> 
> Thanks,
> Stefan
> 
> v1:
> This applies on top of origin/sb/object-store-replace and is available as
> https://github.com/stefanbeller/git/tree/oid_object_info
> 
> This continues the work of sb/packfiles-in-repository,
> extending the layer at which we have to pass in an explicit
> repository object to oid_object_info.
> 
> A test merge to next shows only a minor merge conflicit (adding
> different #include lines in one c file), so this might be a good next
> step for the object store series.
> 
> Notes on further object store series:
> I plan on converting the "parsed object store" next,
> which would be {alloc, object, tree, commit, tag}.c as that is a prerequisite
> for migrating shallow (which is intermingled with grafts) information to the
> object store.
> 
> There is currently work going on in allocation (mempool - Jameson Miller)
> and grafts (deprecate grafts - DScho), which is why I am sending this
> series first. I think it can go in parallel to the "parsed object store"
> that is coming next.
> 
> Thanks,
> Stefan
> 
> Jonathan Nieder (1):
>   packfile: add repository argument to packed_object_info
> 
> Stefan Beller (8):
>   cache.h: add repository argument to oid_object_info_extended
>   cache.h: add repository argument to oid_object_info
>   packfile: add repository argument to retry_bad_packed_offset
>   packfile: add repository argument to packed_to_object_type
>   packfile: add repository argument to read_object
>   packfile: add repository argument to unpack_entry
>   packfile: add repository argument to cache_or_unpack_entry
>   cache.h: allow oid_object_info to handle arbitrary repositories
> 
>  archive-tar.c|  2 +-
>  archive-zip.c|  3 ++-
>  blame.c  |  4 ++--
>  builtin/blame.c  |  2 +-
>  builtin/cat-file.c   | 12 ++--
>  builtin/describe.c   |  2 +-
>  builtin/fast-export.c|  2 +-
>  builtin/fetch.c  |  2 +-
>  builtin/fsck.c   |  3 ++-
>  builtin/index-pack.c |  4 ++--
>  builtin/ls-tree.c|  2 +-
>  builtin/mktree.c |  2 +-
>  builtin/pack-objects.c   | 11 +++
>  builtin/prune.c  |  3 ++-
>  builtin/replace.c| 11 ++-
>  builtin/tag.c|  4 ++--
>  builtin/unpack-objects.c |  2 +-
>  cache.h  |  7 +--
>  diff.c   |  3 ++-
>  fast-import.c| 16 ++--
>  list-objects-filter.c|  2 +-
>  object.c |  2 +-
>  pack-bitmap-write.c  |  3 ++-
>  pack-check.c |  3 ++-
>  packfile.c   | 40 +++-
>  packfile.h   |  6 --
>  reachable.c  |  2 +-
>  refs.c   |  2 +-
>  remote.c |  2 +-
>  sequencer.c  |  3 ++-
>  sha1_file.c  | 37 +
>  sha1_name.c  | 12 ++--
>  streaming.c  |  2 +-
>  submodule.c  |  2 +-
>  tag.c|

Re: [PATCHv3 0/9] object store: oid_object_info is the next contender

2018-04-25 Thread Jonathan Tan 
On Wed, 25 Apr 2018 11:20:57 -0700
Stefan Beller  wrote:

> v3:
> * fixed and extended the commit message of last commit
> * fixed the last patch, as Jonathan Tan suggested, see interdiff:

Thanks, all my comments have been addressed.

Reviewed-by: Jonathan Tan

[PATCHv3 0/9] object store: oid_object_info is the next contender

2018-04-25 Thread Stefan Beller 
v3:
* fixed and extended the commit message of last commit
* fixed the last patch, as Jonathan Tan suggested, see interdiff:

$ git diff remotes/origin/sb/oid-object-info (which is v2)
diff --git c/sha1_file.c w/sha1_file.c
index 94123e0299..dcd6b879ac 100644
--- c/sha1_file.c
+++ w/sha1_file.c
@@ -1289,14 +1289,13 @@ int oid_object_info_extended(struct repository *r, 
const struct object_id *oid,
 
/* Check if it is a missing object */
if (fetch_if_missing && repository_format_partial_clone &&
-   !already_retried) {
+   !already_retried && r == the_repository) {
/*
 * TODO Investigate having fetch_object() return
 * TODO error/success and stopping the music here.
-* TODO Pass a repository struct through 
fetch_object.
+* TODO Pass a repository struct through 
fetch_object,
+* such that arbitrary repositories work.
 */
-   if (r != the_repository)
-   die(_("partial clones only supported in 
the_repository"));
fetch_object(repository_format_partial_clone, 
real->hash);
already_retried = 1;
continue;

Thanks,
Stefan

v2:

* fixed the sha1/oid typo
* removed spurious new line
* Brandon and Jonthan discovered another dependency that I missed due
  to cherrypicking that commit from a tree before partial clone was a thing.
  We error out when attempting to use fetch_object for repos that are not
  the_repository.

Thanks,
Stefan

v1:
This applies on top of origin/sb/object-store-replace and is available as
https://github.com/stefanbeller/git/tree/oid_object_info

This continues the work of sb/packfiles-in-repository,
extending the layer at which we have to pass in an explicit
repository object to oid_object_info.

A test merge to next shows only a minor merge conflicit (adding
different #include lines in one c file), so this might be a good next
step for the object store series.

Notes on further object store series:
I plan on converting the "parsed object store" next,
which would be {alloc, object, tree, commit, tag}.c as that is a prerequisite
for migrating shallow (which is intermingled with grafts) information to the
object store.

There is currently work going on in allocation (mempool - Jameson Miller)
and grafts (deprecate grafts - DScho), which is why I am sending this
series first. I think it can go in parallel to the "parsed object store"
that is coming next.

Thanks,
Stefan

Jonathan Nieder (1):
  packfile: add repository argument to packed_object_info

Stefan Beller (8):
  cache.h: add repository argument to oid_object_info_extended
  cache.h: add repository argument to oid_object_info
  packfile: add repository argument to retry_bad_packed_offset
  packfile: add repository argument to packed_to_object_type
  packfile: add repository argument to read_object
  packfile: add repository argument to unpack_entry
  packfile: add repository argument to cache_or_unpack_entry
  cache.h: allow oid_object_info to handle arbitrary repositories

 archive-tar.c|  2 +-
 archive-zip.c|  3 ++-
 blame.c  |  4 ++--
 builtin/blame.c  |  2 +-
 builtin/cat-file.c   | 12 ++--
 builtin/describe.c   |  2 +-
 builtin/fast-export.c|  2 +-
 builtin/fetch.c  |  2 +-
 builtin/fsck.c   |  3 ++-
 builtin/index-pack.c |  4 ++--
 builtin/ls-tree.c|  2 +-
 builtin/mktree.c |  2 +-
 builtin/pack-objects.c   | 11 +++
 builtin/prune.c  |  3 ++-
 builtin/replace.c| 11 ++-
 builtin/tag.c|  4 ++--
 builtin/unpack-objects.c |  2 +-
 cache.h  |  7 +--
 diff.c   |  3 ++-
 fast-import.c| 16 ++--
 list-objects-filter.c|  2 +-
 object.c |  2 +-
 pack-bitmap-write.c  |  3 ++-
 pack-check.c |  3 ++-
 packfile.c   | 40 +++-
 packfile.h   |  6 --
 reachable.c  |  2 +-
 refs.c   |  2 +-
 remote.c |  2 +-
 sequencer.c  |  3 ++-
 sha1_file.c  | 37 +
 sha1_name.c  | 12 ++--
 streaming.c  |  2 +-
 submodule.c  |  2 +-
 tag.c|  2 +-
 35 files changed, 124 insertions(+), 93 deletions(-)

-- 
2.17.0.441.gb46fe60e1d-goog