Re: [PATCHv3 00/23] Bring more repository handles into our code base

[Derrick Stolee]

On 11/13/2018 7:12 PM, Stefan Beller wrote:

Please have a look at the last 4 patches specifically as they were new in
the last iteration (but did not receive any comment), as they demonstrate
and fix a problem that is only exposed when using GIT_TEST_COMMIT_GRAPH=1
for the test suite.


I took a look at these last four and didn't find anything wrong. Rest of 
the series looks good.


While all of the TODOs in the last patch are an imperfect solution, I 
think it's probably the best we can do for now.


Thanks,
-Stolee

Re: [PATCHv3 00/23] Bring more repository handles into our code base

[Jonathan Tan]

> Please have a look at the last 4 patches specifically as they were new in
> the last iteration (but did not receive any comment), as they demonstrate
> and fix a problem that is only exposed when using GIT_TEST_COMMIT_GRAPH=1
> for the test suite.

Thanks. I only reviewed patches 18 and 20-23, as only those are
different from the previous iteration according to the range-diff.

I've written my comments about patch 18 already [1], and the other
patches look good to me.

In patch 21, I could go either way about whether it's more desirable to
pass the pool or the repository to the freeing functions.

Thanks for discovering the issue that patch 23 illustrates. I thought
that the tests were written carefully enough in that the_repository
didn't have any relevant objects or configurations (all relevant data
was in a path that is not the default repository), but apparently some
still slipped through.

[1] 
https://public-inbox.org/git/20181115195416.21890-1-jonathanta...@google.com/

[PATCHv3 00/23] Bring more repository handles into our code base

[Stefan Beller]

This resends origin/sb/more-repo-in-api.

Unlike the previous resend (v2), this is not rebased to a newer base.
This doesn't contain the patch for pending semantic changes, as that
seems to go in its own topic branch.

Please have a look at the last 4 patches specifically as they were new in
the last iteration (but did not receive any comment), as they demonstrate
and fix a problem that is only exposed when using GIT_TEST_COMMIT_GRAPH=1
for the test suite.

Previous discussion at
https://public-inbox.org/git/20181030220817.61691-1-sbel...@google.com/

Thanks,
Stefan

Stefan Beller (23):
  sha1_file: allow read_object to read objects in arbitrary repositories
  packfile: allow has_packed_and_bad to handle arbitrary repositories
  object-store: allow read_object_file_extended to read from any repo
  object-store: prepare read_object_file to deal with any repo
  object-store: prepare has_{sha1, object}_file to handle any repo
  object: parse_object to honor its repository argument
  commit: allow parse_commit* to handle any repo
  commit-reach.c: allow paint_down_to_common to handle any repo
  commit-reach.c: allow merge_bases_many to handle any repo
  commit-reach.c: allow remove_redundant to handle any repo
  commit-reach.c: allow get_merge_bases_many_0 to handle any repo
  commit-reach: prepare get_merge_bases to handle any repo
  commit-reach: prepare in_merge_bases[_many] to handle any repo
  commit: prepare get_commit_buffer to handle any repo
  commit: prepare repo_unuse_commit_buffer to handle any repo
  commit: prepare logmsg_reencode to handle arbitrary repositories
  pretty: prepare format_commit_message to handle arbitrary repositories
  submodule: use submodule repos for object lookup
  submodule: don't add submodule as odb for push
  commit-graph: convert remaining functions to handle any repo
  commit: prepare free_commit_buffer and release_commit_memory for any
repo
  path.h: make REPO_GIT_PATH_FUNC repository agnostic
  t/helper/test-repository: celebrate independence from the_repository

 builtin/fsck.c|   3 +-
 builtin/log.c |   6 +-
 builtin/rev-list.c|   3 +-
 cache.h   |   2 +
 commit-graph.c|  40 +++--
 commit-reach.c|  73 +
 commit-reach.h|  38 +++--
 commit.c  |  41 ++---
 commit.h  |  43 +-
 .../coccinelle/the_repository.pending.cocci   | 144 ++
 object-store.h|  35 -
 object.c  |   8 +-
 packfile.c|   5 +-
 packfile.h|   2 +-
 path.h|   2 +-
 pretty.c  |  28 ++--
 pretty.h  |   7 +-
 sha1-file.c   |  34 +++--
 streaming.c   |   2 +-
 submodule.c   |  76 ++---
 t/helper/test-repository.c|  10 ++
 21 files changed, 452 insertions(+), 150 deletions(-)
 create mode 100644 contrib/coccinelle/the_repository.pending.cocci

Range-diff:
 1:  1b9b5c695e =  1:  ca9fece80e sha1_file: allow read_object to read objects 
in arbitrary repositories
 2:  33b94066f2 =  2:  8eac25fe32 packfile: allow has_packed_and_bad to handle 
arbitrary repositories
 3:  5217b6b1e1 !  3:  06e5f83b66 object-store: allow read_object_file_extended 
to read from arbitrary repositories
@@ -1,6 +1,6 @@
 Author: Stefan Beller 
 
-object-store: allow read_object_file_extended to read from arbitrary 
repositories
+object-store: allow read_object_file_extended to read from any repo
 
 read_object_file_extended is not widely used, so migrate it all at 
once.
 
 4:  2b7239b55b !  4:  6167722608 object-store: prepare read_object_file to 
deal with arbitrary repositories
@@ -1,6 +1,6 @@
 Author: Stefan Beller 
 
-object-store: prepare read_object_file to deal with arbitrary 
repositories
+object-store: prepare read_object_file to deal with any repo
 
 As read_object_file is a widely used function (which is also regularly 
used
 in new code in flight between master..pu), changing its signature is 
painful
@@ -13,16 +13,14 @@
 e675765235 (diff.c: remove implicit dependency on the_index, 
2018-09-21)
 
 Add a coccinelle patch to convert existing callers, but do not apply
-the resulting patch from 'make coccicheck' to keep the diff of this
-patch small.
+the resulting patch to keep the diff of this patch small.
 
 Signed-off-by: Stefan Beller 
-Signed-off-by: Junio C Hamano