On Mon, May 11, 2020 at 4:49 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > On Fri, May 8, 2020 at 8:58 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > >> Makes sense. > >> > >> I wonder if I should take 1-3/12 as a separate "clean-up" series and > >> merge it before everything else down to 'master'? That would reduce > >> the churn somewhat. > > > > That would be great. Do I need to send them separately, or do can you > > cherry-pick the changes out of this series? > > For the past several days, my tree had two topics, hn/refs-cleanup > (4 patches---1, 2, 3 and 11 from this series) and hn/reftable (the > rest) queued separately. If everybody (including you) is happy with I think the following patches should be uncontroversial: * refs.h: clarify reflog iteration order - I tweaked it to reflect your comments * refs: document how ref_iterator_advance_fn should handle symrefs * reftable: file format documentation - I tweaked it to remove nbsp, and fixed one phrasing nit. * reftable: clarify how empty tables should be written * t: use update-ref and show-ref to reading/writing refs The patch "Iterate over the "refs/" namespace in for_each_[raw]ref" is something I want to mull over for a bit. It may be unnecessary (stay tuned). > I am wondering if we can also throw the file format documentation > into the "more or less uncontroversial" pile. There may be a lot to > dislike in the current "implementation" in reftable/*.c, especially > when viewed in the context of this project, that makes it less > appealing to review, but my understanding is that the feature as > designed by Shawn and described in the format document is already in > use in JGit, so even if the code may need a major update only to > become viewable from some reviewers' point of view, the design it > aims to implement, at least a major part of it, is more or less cast > in stone. Correct; I support this. > Unless we collectively decide that we will never support > reftable in git-core, we need to adopt the format documentation and > the way the subsystem is supposed to work as-is to be compatible, > which makes it "more or less uncontroversial". It may need some > copyediting, though. Also it is not clear to me if the base part > (without "hash id" and incremented format version) is the only thing > in such a "the other system already implements it, so compatibility > concerns leave little room to change the design at this point" > state, or the updated variant that allows us to support SHA-256 and > other hashes is also in that state. JGit supports version 1 of reftable only, and JGit doesn't do SHA-256 at all, so we have a little wiggle room with SHA-256, but the proposal I make here is the simplest way to also support SHA-256 that is in line with how it has been decided on in general (with a 4-byte hash ID). The serialization format is set in stone from Google's perspective (we have been using it in production for 2.5 years now). The repo layout on disk is recent (Code submitted last November, and updated in February to reflect Peff's compatibility advice), so we could get away with minor tweaks if we want. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
