All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Antonio Ospite <ao2@ao2.it>
Cc: gitster@pobox.com, git@vger.kernel.org,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Stefan Beller" <sbeller@google.com>, "Jeff King" <peff@peff.net>,
	"SZEDER Gábor" <szeder.dev@gmail.com>
Subject: Re: [PATCH v7 09/10] submodule: support reading .gitmodules when it's not in the working tree
Date: Tue, 30 Oct 2018 10:57:09 +0100 (STD)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1810301053540.4546@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20181025161813.17252-10-ao2@ao2.it>

Hi Antonio,

On Thu, 25 Oct 2018, Antonio Ospite wrote:

> diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh
> new file mode 100755
> index 0000000000..21a86b89c6
> --- /dev/null
> +++ b/t/t7418-submodule-sparse-gitmodules.sh
> @@ -0,0 +1,122 @@
> +#!/bin/sh
> +#
> +# Copyright (C) 2018  Antonio Ospite <ao2@ao2.it>
> +#
> +
> +test_description='Test reading/writing .gitmodules when not in the working tree
> +
> +This test verifies that, when .gitmodules is in the current branch but is not
> +in the working tree reading from it still works but writing to it does not.
> +
> +The test setup uses a sparse checkout, however the same scenario can be set up
> +also by committing .gitmodules and then just removing it from the filesystem.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'sparse checkout setup which hides .gitmodules' '
> +	git init upstream &&
> +	git init submodule &&
> +	(cd submodule &&
> +		echo file >file &&
> +		git add file &&
> +		test_tick &&
> +		git commit -m "Add file"
> +	) &&
> +	(cd upstream &&
> +		git submodule add ../submodule &&
> +		test_tick &&
> +		git commit -m "Add submodule"
> +	) &&
> +	git clone upstream super &&
> +	(cd super &&
> +		cat >.git/info/sparse-checkout <<-\EOF &&
> +		/*
> +		!/.gitmodules
> +		EOF
> +		git config core.sparsecheckout true &&
> +		git read-tree -m -u HEAD &&
> +		test_path_is_missing .gitmodules
> +	)
> +'
> +
> +test_expect_success 'reading gitmodules config file when it is not checked out' '
> +	echo "../submodule" >expect &&
> +	git -C super submodule--helper config submodule.submodule.url >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'not writing gitmodules config file when it is not checked out' '
> +	test_must_fail git -C super submodule--helper config submodule.submodule.url newurl &&
> +	test_path_is_missing super/.gitmodules
> +'
> +
> +test_expect_success 'initialising submodule when the gitmodules config is not checked out' '
> +	test_must_fail git -C super config submodule.submodule.url &&
> +	git -C super submodule init &&
> +	git -C super config submodule.submodule.url >actual &&
> +	echo "$PWD/submodule" >expect &&

Would you mind squashing this fixup in?

-- snip --
diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh
index 21a86b89c6cb..3f7f27188313 100755
--- a/t/t7418-submodule-sparse-gitmodules.sh
+++ b/t/t7418-submodule-sparse-gitmodules.sh
@@ -55,7 +55,7 @@ test_expect_success 'initialising submodule when the gitmodules config is not ch
 	test_must_fail git -C super config submodule.submodule.url &&
 	git -C super submodule init &&
 	git -C super config submodule.submodule.url >actual &&
-	echo "$PWD/submodule" >expect &&
+	echo "$(pwd)/submodule" >expect &&
 	test_cmp expect actual
 '
 
-- snap --

On Windows, `$PWD` and `$(pwd)` are *not* synonymous. The former
reflects the "Unix path" which is understood by the Bash script (and
only by the Bash script, *not* by `git.exe`!) while the latter refers to
the actual Windows path.

Without this fix, your new test case will fail on Windows all the time,
see e.g.
https://git-for-windows.visualstudio.com/git/_build/results?buildId=22913&view=logs

Thanks,
Johannes


> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'updating submodule when the gitmodules config is not checked out' '
> +	test_path_is_missing super/submodule/file &&
> +	git -C super submodule update &&
> +	test_cmp submodule/file super/submodule/file
> +'
> +
> +ORIG_SUBMODULE=$(git -C submodule rev-parse HEAD)
> +ORIG_UPSTREAM=$(git -C upstream rev-parse HEAD)
> +ORIG_SUPER=$(git -C super rev-parse HEAD)
> +
> +test_expect_success 're-updating submodule when the gitmodules config is not checked out' '
> +	test_when_finished "git -C submodule reset --hard $ORIG_SUBMODULE;
> +			    git -C upstream reset --hard $ORIG_UPSTREAM;
> +			    git -C super reset --hard $ORIG_SUPER;
> +			    git -C upstream submodule update --remote;
> +			    git -C super pull;
> +			    git -C super submodule update --remote" &&
> +	(cd submodule &&
> +		echo file2 >file2 &&
> +		git add file2 &&
> +		test_tick &&
> +		git commit -m "Add file2 to submodule"
> +	) &&
> +	(cd upstream &&
> +		git submodule update --remote &&
> +		git add submodule &&
> +		test_tick &&
> +		git commit -m "Update submodule"
> +	) &&
> +	git -C super pull &&
> +	# The --for-status options reads the gitmodules config
> +	git -C super submodule summary --for-status >actual &&
> +	rev1=$(git -C submodule rev-parse --short HEAD) &&
> +	rev2=$(git -C submodule rev-parse --short HEAD^) &&
> +	cat >expect <<-EOF &&
> +	* submodule ${rev1}...${rev2} (1):
> +	  < Add file2 to submodule
> +
> +	EOF
> +	test_cmp expect actual &&
> +	# Test that the update actually succeeds
> +	test_path_is_missing super/submodule/file2 &&
> +	git -C super submodule update &&
> +	test_cmp submodule/file2 super/submodule/file2 &&
> +	git -C super status --short >output &&
> +	test_must_be_empty output
> +'
> +
> +test_expect_success 'not adding submodules when the gitmodules config is not checked out' '
> +	git clone submodule new_submodule &&
> +	test_must_fail git -C super submodule add ../new_submodule &&
> +	test_path_is_missing .gitmodules
> +'
> +
> +# This test checks that the previous "git submodule add" did not leave the
> +# repository in a spurious state when it failed.
> +test_expect_success 'init submodule still works even after the previous add failed' '
> +	git -C super submodule init
> +'
> +
> +test_done

  reply	other threads:[~2018-10-30  9:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-25 16:18 [PATCH v7 00/10] Make submodules work if .gitmodules is not checked out Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 01/10] submodule: add a print_config_from_gitmodules() helper Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 02/10] submodule: factor out a config_set_in_gitmodules_file_gently function Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 03/10] t7411: merge tests 5 and 6 Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 04/10] t7411: be nicer to future tests and really clean things up Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 05/10] submodule--helper: add a new 'config' subcommand Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 06/10] submodule: use the 'submodule--helper config' command Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 07/10] t7506: clean up .gitmodules properly before setting up new scenario Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 08/10] submodule: add a helper to check if it is safe to write to .gitmodules Antonio Ospite
2018-10-25 16:18 ` [PATCH v7 09/10] submodule: support reading .gitmodules when it's not in the working tree Antonio Ospite
2018-10-30  9:57   ` Johannes Schindelin [this message]
2018-10-30 11:16     ` Antonio Ospite
2018-10-31  6:01       ` Junio C Hamano
2018-10-25 16:18 ` [PATCH v7 10/10] t/helper: add test-submodule-nested-repo-config Antonio Ospite
2018-10-25 18:49 ` [PATCH v7 00/10] Make submodules work if .gitmodules is not checked out Stefan Beller
2018-10-26  1:59   ` Junio C Hamano
2018-10-26 18:43     ` Stefan Beller

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

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

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.1810301053540.4546@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=ao2@ao2.it \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    --cc=szeder.dev@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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.