Re: [PATCH v9 09/10] Add reftable library

> > +((cd reftable-repo && git fetch origin && git checkout origin/master ) ||
> > +git clone https://github.com/google/reftable reftable-repo) && \
>
> I think you can discard most of the '\' at the end of the line, as
> the shell knows, when you stop a line with &&, you haven't finished
> talking to it yet.  That would make the result slightly less noisy
> to follow.  We don't rely on "set -e" in this project, but this is a
> borrowed file from our point of view---as long as we are using -e,
> wouldn't it make more sense to take full advantage of it and do
> without &&?

> > +cp reftable-repo/c/*.[ch] reftable/ && \
> > +cp reftable-repo/c/include/*.[ch] reftable/ && \
> > +cp reftable-repo/LICENSE reftable/ &&
> > +git --git-dir reftable-repo/.git show --no-patch origin/master \
> > +> reftable/VERSION && \
>
> > +sed -i~ 's|if REFTABLE_IN_GITCORE|if 1 /* REFTABLE_IN_GITCORE */|' reftable/system.h
>
> "sed -i" is probably GNUism that does not port well.

> > +rm reftable/*_test.c reftable/test_framework.* reftable/compat.*
>
> I presume that this is because we copied everything that match
> c/*.[ch] and is not about removing what used to exist in reftable/
> directory locally?  Not complaining, but checking if I am reading
> the intention of the code correctly.

> > +git add reftable/*.[ch]
>
> As we flattened when copying from c/include/*.[ch], this "git add"
> can also be flat, which makes sense.  Don't you need to also add the
> LICENSE file in there?

> Two comments on the design:
>
>  - The list of "what to copy in" largely depends on the upstream
>    reftable library; hardcoding the patterns here would make it
>    harder to reorganize the files for the upstream project.  I
>    wonder if it makes more sense to reserve a single path in the
>    reftable-repo and hardcode the knowledge of what to copy out
>    there?  That would mean the resulting update.sh here can become
>
>         (git -C reftable-repo pull --ff-only && git clone ...) &&
>         reftable-repo/copy-out-to-git-core.sh reftable/
>
>    and the copy-out-to-git-core.sh script that comes with the
>    upstream reftable library is the only one that needs to know
>    where the license and C source files are in the same version as
>    itself, and how to produce $1/VERSION file using the commit log
>    message of the commit it comes from.
>
>  - The above procedure (with or without the "who should know what to
>    copy" change) would cope well with modified and new files in the
>    reftable upstream, but does not remove a library file that used
>    to exist (and copied to the git-core project) that no longer is
>    needed and shipped.  It appears that there has to be some way to
>    remove them.