git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Elijah Newren <newren@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Derrick Stolee <stolee@gmail.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH 0/6] easy bulk commit creation in tests
Date: Sun, 30 Jun 2019 02:34:11 -0400	[thread overview]
Message-ID: <20190630063410.GA31264@sigill.intra.peff.net> (raw)
In-Reply-To: <CABPp-BEyq-9sj_9wxLdh66BJqqjQ80a8sCpXd_cMCArAHnM7kA@mail.gmail.com>

On Sat, Jun 29, 2019 at 10:38:43AM -0600, Elijah Newren wrote:

> >   - 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

  reply	other threads:[~2019-06-30  6:34 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-27 17:05 Git Test Coverage Report (Thurs. June 27) Derrick Stolee
2019-06-27 17:35 ` Derrick Stolee
2019-06-28  6:41   ` Jeff King
2019-06-28  9:37     ` [PATCH 0/6] easy bulk commit creation in tests Jeff King
2019-06-28  9:39       ` [PATCH 1/6] test-lib: introduce test_commit_bulk Jeff King
2019-06-28 12:35         ` Derrick Stolee
2019-06-28 18:05           ` Junio C Hamano
2019-06-29  0:09           ` Jeff King
2019-06-28 17:53         ` Junio C Hamano
2019-06-29  0:14           ` Jeff King
2019-06-28 18:44         ` Ævar Arnfjörð Bjarmason
2019-06-29  0:19           ` Jeff King
2019-06-28 21:32         ` Eric Sunshine
2019-06-28 23:04           ` SZEDER Gábor
2019-06-28 23:46             ` Eric Sunshine
2019-06-29  0:26               ` Jeff King
2019-06-29  8:24               ` SZEDER Gábor
2019-07-01 17:42                 ` Junio C Hamano
2019-06-29  0:25           ` Jeff King
2019-06-28  9:39       ` [PATCH 2/6] t5310: increase the number of bitmapped commits Jeff King
2019-06-28  9:41       ` [PATCH 3/6] t3311: use test_commit_bulk Jeff King
2019-06-28  9:41       ` [PATCH 4/6] t5702: " Jeff King
2019-06-28  9:42       ` [PATCH 5/6] t5703: " Jeff King
2019-06-28  9:42       ` [PATCH 6/6] t6200: " Jeff King
2019-06-28 12:53       ` [PATCH 0/6] easy bulk commit creation in tests Johannes Schindelin
2019-06-29  0:30         ` Jeff King
2019-06-29 16:38           ` Elijah Newren
2019-06-30  6:34             ` Jeff King [this message]
2019-06-28 18:49       ` Ævar Arnfjörð Bjarmason
2019-06-29  0:45         ` Jeff King
2019-06-29  4:53       ` [PATCH v2 1/6] test-lib: introduce test_commit_bulk Jeff King
2019-07-01 22:24         ` Junio C Hamano
2019-07-02  5:16           ` Jeff King
2019-07-01 22:28         ` Junio C Hamano
2019-07-02  5:22           ` Jeff King
2019-06-28  6:45   ` Git Test Coverage Report (Thurs. June 27) Jeff King
2019-06-28 12:23     ` Derrick Stolee
2019-06-28 23:59       ` Jeff King
2019-06-29  1:36         ` Derrick Stolee
2019-06-29  5:15           ` Jeff King
2019-06-28  9:47   ` Duy Nguyen
2019-06-28 12:39     ` Derrick Stolee
2019-06-28 13:39   ` Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190630063410.GA31264@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).