"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > diff --git a/reftable/update.sh b/reftable/update.sh > new file mode 100644 Shouldn't this be executable, even though the readme tells the reader to run "sh update.sh"? > index 00000000000..0fd90f89675 > --- /dev/null > +++ b/reftable/update.sh > @@ -0,0 +1,14 @@ > +#!/bin/sh > + > +set -eux Use of -e and -u are understandable but -x is a bit unusual unless one is debugging... > +((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. Thanks.
