git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: bmwill@google.com, git@vger.kernel.org, larsxschneider@gmail.com
Subject: Re: [PATCH] pull: respect submodule update configuration
Date: Fri, 18 Aug 2017 23:24:47 -0700
Message-ID: <xmqqvalk9is0.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170818220431.22157-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> Do not override the submodule configuration in the call to update
> the submodules, but give a weaker default.
>
> Reported-by: Lars Schneider <larsxschneider@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>   
> Personally I dislike this patch, but I have no better idea for the time
> being.

The patch text from a cursory look seems reasonable to me.

It's not like you have 47 different codepaths that need to pay
attention to the .update config and they all have to pass the new
--default-update option, this is merely to fix one of them that
relates to the problem reported by Lars, and you need a similar fix
to other 46, right?

If you want the "--recurse-submodules" thing to always do the
"weaker default" thing in your project, you can choose not to set
.update to custom values in any of your submodules, so I do not
think the reason why you dislike this change is because it would
affect your use of submodules.

So I am a bit curious to learn which part of this change you dislike
and why.


>  builtin/pull.c             |  6 ++++--
>  git-submodule.sh           |  7 ++++++-
>  t/t7400-submodule-basic.sh | 22 ++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 9b86e519b1..be4f74d764 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -553,7 +553,8 @@ static int rebase_submodules(void)
>  	cp.git_cmd = 1;
>  	cp.no_stdin = 1;
>  	argv_array_pushl(&cp.args, "submodule", "update",
> -				   "--recursive", "--rebase", NULL);
> +				   "--recursive", "--default-update",
> +				   "rebase", NULL);
>  
>  	return run_command(&cp);
>  }
> @@ -565,7 +566,8 @@ static int update_submodules(void)
>  	cp.git_cmd = 1;
>  	cp.no_stdin = 1;
>  	argv_array_pushl(&cp.args, "submodule", "update",
> -				   "--recursive", "--checkout", NULL);
> +				   "--recursive", "--default-update",
> +				   "checkout", NULL);
>  
>  	return run_command(&cp);
>  }
> diff --git a/git-submodule.sh b/git-submodule.sh
> index e131760eec..6dbc32e686 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -511,6 +511,7 @@ fetch_in_submodule () (
>  cmd_update()
>  {
>  	# parse $args after "submodule ... update".
> +	default_update="checkout"
>  	while test $# -ne 0
>  	do
>  		case "$1" in
> @@ -552,6 +553,10 @@ cmd_update()
>  		--checkout)
>  			update="checkout"
>  			;;
> +		--default-update)
> +			default_update="$2"
> +			shift
> +			;;
>  		--recommend-shallow)
>  			recommend_shallow="--recommend-shallow"
>  			;;
> @@ -619,7 +624,7 @@ cmd_update()
>  			update_module=$(git config submodule."$name".update)
>  			if test -z "$update_module"
>  			then
> -				update_module="checkout"
> +				update_module="$default_update"
>  			fi
>  		fi
>  
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index dcac364c5f..ff64bf8528 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1289,4 +1289,26 @@ test_expect_success 'init properly sets the config' '
>  	test_must_fail git -C multisuper_clone config --get submodule.sub1.active
>  '
>  
> +test_expect_success 'submodule update and git pull with disabled submodule' '
> +	test_when_finished "rm -rf multisuper_clone" &&
> +	pwd=$(pwd) &&
> +	cat <<-\EOF >expect &&
> +	-sub0
> +	 sub1 (test2)
> +	 sub2 (test2)
> +	 sub3 (test2)
> +	 sub4 (test2)
> +	 sub5 (test2)
> +	EOF
> +	git clone file://"$pwd"/multisuper multisuper_clone &&
> +	(
> +		cd multisuper_clone &&
> +		git config --local submodule.sub0.update none &&
> +		git submodule update --init --recursive &&
> +		git pull --recurse-submodules &&
> +		git submodule status | cut -c 1,43- >actual
> +	) &&
> +	test_cmp expect multisuper_clone/actual
> +'
> +
>  test_done

  parent reply index

Thread overview: 38+ messages in thread (expand / mbox.gz / Atom feed / [top])
2017-08-16 18:35 Submodule regression in 2.14? Lars Schneider
2017-08-16 18:51 ` Stefan Beller
2017-08-16 18:53   ` Stefan Beller
2017-08-17 21:21   ` Lars Schneider
2017-08-17 21:55     ` Stefan Beller
2017-08-18  2:13       ` Junio C Hamano
2017-08-18  4:02         ` Stefan Beller
2017-08-18 16:50           ` Junio C Hamano
2017-08-18 19:09             ` Stefan Beller
2017-08-19  6:51               ` Junio C Hamano
2017-08-21 16:05                 ` Heiko Voigt
2017-08-21 16:42                   ` Stefan Beller
2017-08-22 15:33                     ` Heiko Voigt
2017-08-22 18:10                       ` Stefan Beller
2017-08-25  9:10                         ` Heiko Voigt
2017-08-25 16:38                           ` Stefan Beller
2017-08-25 16:53                           ` Junio C Hamano
2017-08-21 16:48                   ` Junio C Hamano
2017-08-22 15:50                     ` Heiko Voigt
2017-08-21 16:46                 ` Stefan Beller
2017-08-21 22:45                   ` Junio C Hamano
2017-08-18 13:12       ` Lars Schneider
2017-08-18 17:16         ` Stefan Beller
2017-08-18 19:10           ` Junio C Hamano
2017-08-18 22:04             ` [PATCH] pull: respect submodule update configuration Stefan Beller
2017-08-18 22:05               ` Stefan Beller
2017-08-19  6:17                 ` Junio C Hamano
2017-08-19  6:24               ` Junio C Hamano [this message]
2017-08-21 16:20                 ` Heiko Voigt
2017-08-21 16:55                   ` Stefan Beller
2017-08-21 17:20                     ` Lars Schneider
2017-08-21 17:48                       ` Stefan Beller
2017-08-21 18:21                         ` Brandon Williams
2017-08-21 22:52                           ` Junio C Hamano
2017-08-22 14:50                           ` Lars Schneider
2017-08-22 17:51                             ` Stefan Beller
2017-08-22 18:55                               ` Brandon Williams
2017-08-19 18:24           ` Submodule regression in 2.14? Lars Schneider

Reply instructions:

You may reply publically 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 to all the recipients using the --to, --cc,
  and --in-reply-to switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvalk9is0.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    --cc=sbeller@google.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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox