Ability to use git refs migrate --ref-format=reftable - #769

I can confirm, that the build artifacts works properly.
As it changes in gitx/objective-git#118 to libgit2 v2.0.0.0-rc1 I just want to ask if you @goneng are fine with it ?
At least I'm fine, as it's just a number cosmetic

I can confirm, that the build artifacts works properly. As it changes in gitx/objective-git#118 to libgit2 v2.0.0.0-rc1 I just want to ask if you @goneng are fine with it ? At least I'm fine, as it's just a number cosmetic

Hi @hannesa2, I think Reftable support is a nice addition, and the integration tests (bare repos, linked worktrees, the SHA-256 rejection) look thorough.
I built the PR head locally and the full unit suite passes (352 tests, including the 5 reftable ones).

On your question about libgit2 v2.0.0-rc.1: I'm fine with moving to it for reftable support.
(I saw that the 1.9.x security fixes are present in rc.1 and that no public function was removed.)

A few things I'd still like to see:

Before merging

• Have noticed that External/objective-git pin (f238100) is on the branch of Ability to use git refs migrate --ref-format=reftable objective-git#118,
  which is still open - I assume once this is merged, you're going to re-point the bump here
  at the merged commit.
• Ability to use git refs migrate --ref-format=reftable objective-git#118 adds #import <git2/deprecated.h> to GTOID.m and to the public header GTRepository.h.
  Master has no such imports in ObjectiveGit/ today, so this re-grows the shim debt from objective-git still relies on libgit2's deprecated API shims #652.
  For GTOID.m, git_odb_hash has a non-deprecated replacement, git_object_id_from_buffer.
  In GTRepository.h, GIT_REPOSITORY_INIT_NO_DOTGIT_DIR is now defined as 0,
  which turns GTRepositoryInitWithoutDotGit into a no-op flag.
  Could you handle those in Ability to use git refs migrate --ref-format=reftable objective-git#118 instead of importing the deprecated header?
• A short note in the PR description that the pin is a release candidate would help later readers.

Smaller suggestions

• README: the libgit2 section names revision 0551dfd4, but the pin is the v2.0.0-rc.1 tag, which contains it.
  Naming the tag would be clearer.
• README: "Apple's bundled Git may be too old" - Apple Git 2.50.1 (macOS here) handles reftable repositories fine.
  Maybe state the Git 2.45 minimum and name the macOS versions that ship an older Git.
• PBGitRepository.m:90: the project avoids explanatory inline comments, and
  the error text already says what is unsupported.
• PBReftableRepositoryTests could live in its own file instead of PBRepositoryFinderTests.m.
  (That needs a pbxproj entry, so it is fine to skip, if you prefer to keep it as-is.

Happy to re-review once gitx/objective-git#118 lands.

I applied all suggestions as good as I can @goneng

• Have noticed that External/objective-git pin (f238100) is on the branch of Ability to use git refs migrate --ref-format=reftable objective-git#118,
  which is still open - I assume once this is merged, you're going to re-point the bump here
  at the merged commit.

Sure, as soon this one if fine, I'll first merge gitx/objective-git#118 then I'll change this one again with right submodule pointer

• I followed you suggestion concerning the README.md
• I merged Ability to use git refs migrate --ref-format=reftable objective-git#118 to master and use the lastest master now here