Re: [PATCH 0/6] easy bulk commit creation in tests
- From
Jeff King <peff@peff.net>
- Date
- Jun 30, 2019, 06:34 UTC
- Message-ID
- <20190630063410.GA31264@sigill.intra.peff.net>
- In-Reply-To
- <CABPp-BEyq-9sj_9wxLdh66BJqqjQ80a8sCpXd_cMCArAHnM7kA@mail.gmail.com>
On Sat, Jun 29, 2019 at 10:38:43AM -0600, Elijah Newren wrote:
Show 11 quoted lines
> > - add a feature to fast-import to say "build on top of ref X", instead > > of using to use rev-parse to manually generates a "reset" line > > (maybe this is even possible already; I searched for it, but not > > very hard). > > It already exists; quoting the fast-import documentation: > > "The special case of restarting an incremental import from the > current branch value should be written as: > > from refs/heads/branch^0
Thank you! I looked over the documentation several times for this, but I was looking for an individual command similar to "reset".
Unfortunately, I'm not sure we can use this to save ourselves a process. What I really want to say is "if it does not exist, start from scratch and otherwise build on the existing branch".
I couldn't figure out a way to do that without first finding out myself if the branch exists (incurring a process) and then modifying my fast-import stream appropriately.
So I don't think it actually shaves off our processes, but as I argued elsewhere, I think it's probably not that important anyway. I do think the end result is a bit simpler to read, too, as the while-loop now generates the input in its entirety (I didn't reindent it yet in the diff below):
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 9fd0fa2a89..4233f408e8 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -305,14 +305,11 @@ test_commit_bulk () { done total=$1 - { - # A "reset ... from" instructs fastimport to build on an - # existing branch tip rather than trying to overwrite. - if tip=$(git -C "$indir" rev-parse --verify "$ref" 2>/dev/null) - then - echo "reset $ref" - echo "from $tip" - fi + add_from= + if git rev-parse --verify "$ref" >/dev/null 2>&1 + then + add_from=t + fi while test "$total" -gt 0 do @@ -329,16 +326,16 @@ test_commit_bulk () { echo "data <<EOF" printf "$message\n" $n echo "EOF" + test -n "$add_from" && echo "from $ref^0" printf "M 644 inline $filename\n" $n echo "data <<EOF" printf "$contents\n" $n echo "EOF" echo + add_from= n=$((n + 1)) total=$((total - 1)) - done - - } >"$tmpfile" + done >"$tmpfile" git -C "$indir" \ -c fastimport.unpacklimit=0 \ Actually, thinking about it more, avoiding the $() probably does save us a subshell fork, too. > > The third one is a little less elegant to me, because there are a lot of > > questions about how to checkout (e.g., with "-f", what happens to > > deleted files, etc). > > There's a question with deleted files? Why wouldn't you just delete > them from the index and working tree? The more interesting questions > to me in this case is what to do if the index or working tree were > dirty before the import started; that seems like a mess, though maybe > it's just a case where you abort before even importing. On a similar > note, though, there could have been an untracked file that is in the > way of a now-to-be-tracked file that you might not want to lose. Sorry, by deleted I meant files that were already deleted in the working tree or index, not ones our fast-import stream deleted. I.e,. the same dirty case you're asking about. But modifications have the same problem, too (I was thinking we'd just overwrite them as if the user had done "cat >dirty-file" as part of their fast-import, but that only applies to files they actually touched). So "dirty" is definitely the right way to think about it. -Peff