git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "W. Trevor King" <wking@tremily.us>
To: Jens Lehmann <Jens.Lehmann@web.de>, Junio C Hamano <gitster@pobox.com>
Cc: Phil Hord <phil.hord@gmail.com>, Git <git@vger.kernel.org>,
	Heiko Voigt <hvoigt@hvoigt.net>, Jeff King <peff@peff.net>,
	Shawn Pearce <spearce@spearce.org>,
	Nahor <nahor.j+gmane@gmail.com>
Subject: Re: [RFC] remove/deprecate 'submodule init' and 'sync'
Date: Sun, 02 Dec 2012 14:09:29 -0500	[thread overview]
Message-ID: <20121202190929.GG9401@odin.tremily.us> (raw)
In-Reply-To: <7vy5hhmcwp.fsf@alter.siamese.dyndns.org> <50BA3412.60309@web.de> <20121201163714.GC4823@odin.tremily.us> <20121201181643.GF4823@odin.tremily.us>

[-- Attachment #1: Type: text/plain, Size: 5933 bytes --]

Before I get into the details, I'd like to point out that I actually
understand the purpose of `submodule init` now ;).  To avoid further
confusion, my current one-line command summaries would be:

  init:   mark a submodule as active for future submodule operation
  deinit: mark a submodule as inactive for future submodule operation
  sync:   update remote.<name>.origin in submodules to reflect changes
          in .gitmodules or the superproject's remote URL.

I don't think we disagree on that, we just don't agree on how to
implement it.

Currently, Git uses submodule.<name>.url in the superproject's local
configuration as a marker for submodule activation.  This is not (as
far as I know) discussed in the docs, which is why I initially
missunderstood the purpose of `init` to be “setup the superproject's
local configuration so we don't have to keep resolving the submodules
URL relative to the superproject's upstream URL”.  With the proposed
`deinit` docs, this role for the submodule.<name>.url is mentioned,
but not in a place where casual users will be able to easily connect
it to the purpose of `init`.

I floated using submodule.<name>.update (with 'none' for inactive and
anything else for active) as an alternative marker:

On Sat, Dec 01, 2012 at 01:16:43PM -0500, W. Trevor King wrote:
> On Sat, Dec 01, 2012 at 07:04:05PM +0100, Jens Lehmann wrote:
> > Am 01.12.2012 18:49, schrieb W. Trevor King:
> > > I think removing `init` will cause some compatibility issues anyway,
> > > so I was re-imaging how you do it.  I don't think update='none' and
> > > "don't populate my submodule" are distinct ideas, while a locally
> > > configured url="somwhere" and "please populate my submodule" are (with
> > > the blank-url case defaulting to the superproject itself).
> > 
> > Why would we want to remove "init"? It still has to copy the "url"
> > setting (and it would be a compatibility nightmare if we would change
> > that, imagine different git versions used on the same work tree).
> 
> In my init-less rewrite, it doesn't have to copy the url setting.
> People using older versions of Git would need to run `init` using
> their old version.  Having the url defined in .git/config won't break
> my init-less submodule commands, it just means that the value in
> .gitmodules will be masked.

but that doesn't seem to be going over very well.  Junio may have been
weighing in obliquely with:

On Sat, Dec 01, 2012 at 06:00:06PM -0800, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> > [snip v1 deinit commit message]
> 
> I fully agree with your analysis on the reason why the "url" element
> is special and has to be copied to $GIT_DIR/config, but when you
> deinit (or uninit) a submodule to say you are no longer interested
> in it and do not want it populated in the context of the
> superproject, I am not sure if removing only submodule.$name.url (so
> that when you later decide to "init" it again, you will keep the
> values for submodule.$name.update and other things from the previous
> life) is the sane thing to do, or it is better to remove
> submodule.$name.* altogether as if an earlier "init" has never
> happened.  Would it be worth analyzing the pros-and-cons here?

Let me take another stab at presenting my argument in favor of a
different activity marker.

Proposal:

Add a new boolean option, submodule.<name>.active, to explicitly mark
submodules as active (with “active” defined as “to be returned by
module_list()”).  Strip down `init` (and the --init part of `update
--init`) to just setting this option to true.  `deinit` only sets this
option to false (but a `deinit --clean` could remove the whole
submodule.<name> section).

With this in place, extracting URLs for submodule operations be
similar to the extraction of other variables (.gitmodules defaults
with superproject-local .git/config overrides).  This also makes it
easier to track maintenance updates in .gitmodules-defined URLs,
because you aren't forced to bake overrides into your local
.git/config

The upgrade path from earlier versions of Git is easy: if
submodule.<name>.active is unset, use the presence of
submodule.<name>.url to determine its initial value.

In the case where you check out an earlier superproject commit which
is missing a particular submodule (or remove a submodule without
deinit-ing), the presense of an active setting in .git/config should
not cause an error, which they currently seem to:

On Sat, Dec 01, 2012 at 11:37:14AM -0500, W. Trevor King wrote:
> On Sat, Dec 01, 2012 at 04:56:02PM +0100, Jens Lehmann wrote:
> > Am 01.12.2012 00:52, schrieb Phil Hord:
> > > If I never 'submodule init' a submodule, it does not get visited by
> > > 'git submodule foreach', among others.  I think some people use this
> > > behavior explicitly.
> > >
> > > On the other hand, I've also notice that a submodule which I have
> > > removed does not get de-inited later one.  It causes my 'git submodule
> > > foreach' to emit errors.  :-(
> >
> > I'm currently hacking on "git submodule deinit" which removes the 'url'
> > setting from git/config. This should do the trick for you, right?
> >
> > Just removing that submodule automagically would not work that well, as
> > it would deinitialize a submodule when you switch to a branch where it
> > isn't present and you'd have to reinitialize it when you come back.
>
> I think this is another case where we should be looping through
> submodules based on the revision-specific .gitmodules content, and
> querying the local config only to determine if the user wants to
> update them (to drop into them with foreach, etc.).

Thoughts?

Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2012-12-02 19:10 UTC|newest]

Thread overview: 156+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-22 16:34 [PATCH] git-submodule add: Record branch name in .gitmodules W. Trevor King
     [not found] ` <CABURp0pqg7XC6makK2OcundMabV9AtcBNGNK6Q0TMZfJbt3anw@mail.gmail.com>
2012-10-22 22:55   ` W. Trevor King
2012-10-24 18:12     ` Phil Hord
2012-10-25  1:12       ` W. Trevor King
2012-10-26 13:37         ` Jeff King
2012-10-23 19:16 ` Nahor
2012-10-23 19:44   ` W. Trevor King
2012-10-23 20:44     ` W. Trevor King
2012-10-23 21:57       ` [PATCH v2] git-submodule add: Add -r/--record option W. Trevor King
2012-10-24 19:15         ` Jens Lehmann
2012-10-25  0:53           ` W. Trevor King
2012-10-28 20:48             ` Jens Lehmann
2012-10-28 21:16               ` W. Trevor King
2012-10-28 21:59               ` Shawn Pearce
2012-10-28 22:34                 ` W. Trevor King
2012-10-29  5:34                   ` Jeff King
2012-10-29 10:45                     ` W. Trevor King
2012-10-29 10:58                       ` Jeff King
2012-10-29 11:29                         ` W. Trevor King
2012-10-29 11:43                           ` Jeff King
2012-10-29 17:38                             ` Phil Hord
2012-10-29 21:36                               ` Jeff King
2012-10-29 22:21                                 ` Phil Hord
2012-10-29 22:27                                   ` Jeff King
2012-11-09  3:35                                     ` [PATCH v3 0/3] " W. Trevor King
2012-11-09  3:35                                     ` [PATCH v3 1/3] " W. Trevor King
2012-11-09  7:34                                       ` Junio C Hamano
2012-11-09 16:29                                         ` Heiko Voigt
2012-11-10 19:02                                           ` W. Trevor King
2012-11-17 15:04                                             ` Heiko Voigt
2012-11-17 19:20                                               ` W. Trevor King
2012-11-17 21:31                                                 ` Heiko Voigt
2012-11-17 22:00                                                   ` W. Trevor King
2012-11-20  0:49                                                 ` Junio C Hamano
2012-11-20  1:16                                                   ` W. Trevor King
2012-11-20  5:39                                                     ` Junio C Hamano
2012-11-20 12:19                                                       ` W. Trevor King
2012-11-20 19:52                                                         ` Junio C Hamano
2012-11-23 15:55                                                           ` Heiko Voigt
2012-11-23 16:23                                                             ` W. Trevor King
2012-11-23 16:30                                                               ` W. Trevor King
2012-11-23 17:54                                                               ` W. Trevor King
2012-11-26 21:00                                                                 ` [PATCH v4 0/4] git-submodule add: Add --local-branch option W. Trevor King
2012-11-26 21:00                                                                   ` [PATCH v4 1/4] " W. Trevor King
2012-11-26 21:00                                                                   ` [PATCH v4 2/4] git-submodule init: Record submodule.<name>.branch in repository config W. Trevor King
2012-11-27 23:19                                                                     ` Jens Lehmann
2012-11-28  0:40                                                                       ` W. Trevor King
2012-11-30 17:53                                                                       ` [RFC] remove/deprecate 'submodule init' and 'sync' W. Trevor King
2012-11-30 18:17                                                                         ` W. Trevor King
2012-11-30 23:52                                                                         ` Phil Hord
2012-12-01 12:48                                                                           ` W. Trevor King
2012-12-01 15:42                                                                             ` Jens Lehmann
2012-12-01 15:56                                                                           ` Jens Lehmann
2012-12-01 16:37                                                                             ` W. Trevor King
2012-12-01 16:45                                                                             ` [PATCH] submodule: add 'deinit' command Jens Lehmann
2012-12-02  2:00                                                                               ` Junio C Hamano
2012-12-02 19:55                                                                                 ` Jens Lehmann
2012-12-03  7:58                                                                                   ` Junio C Hamano
2012-12-04 21:48                                                                                     ` [PATCH v2] " Jens Lehmann
2012-12-04 23:06                                                                                       ` Junio C Hamano
2012-12-12 15:08                                                                                       ` Michael J Gruber
2012-12-12 17:22                                                                                         ` Jens Lehmann
2012-12-12 19:32                                                                                           ` Junio C Hamano
2012-12-12 22:25                                                                                             ` Jens Lehmann
2012-12-12 22:34                                                                                               ` Junio C Hamano
2012-12-12 23:09                                                                                                 ` W. Trevor King
2012-12-12 23:35                                                                                                   ` Junio C Hamano
2012-12-13  0:28                                                                                                     ` W. Trevor King
2012-12-13 15:47                                                                                               ` Marc Branchaud
2012-12-01 15:38                                                                         ` [RFC] remove/deprecate 'submodule init' and 'sync' Jens Lehmann
2012-12-01 16:30                                                                           ` W. Trevor King
2012-12-01 17:25                                                                             ` Jens Lehmann
2012-12-01 17:49                                                                               ` W. Trevor King
2012-12-01 18:04                                                                                 ` Jens Lehmann
2012-12-01 18:16                                                                                   ` W. Trevor King
2012-12-02 19:09                                                                                     ` W. Trevor King [this message]
2012-12-02 20:29                                                                                       ` Jens Lehmann
2012-12-02 21:11                                                                                         ` W. Trevor King
2012-12-01 16:54                                                                         ` W. Trevor King
2012-12-03 15:38                                                                           ` W. Trevor King
2012-11-26 21:00                                                                   ` [PATCH v4 3/4] git-submodule update: Add --branch option W. Trevor King
2012-11-27 18:51                                                                     ` Heiko Voigt
2012-11-27 20:21                                                                       ` W. Trevor King
2012-11-26 21:00                                                                   ` [PATCH v4 4/4] Hack fix for 'submodule update does not fetch already present commits' W. Trevor King
2012-11-27 18:31                                                                   ` [PATCH v4 0/4] git-submodule add: Add --local-branch option Heiko Voigt
2012-11-27 19:04                                                                     ` W. Trevor King
2012-11-27 19:16                                                                     ` Heiko Voigt
2012-11-27 19:01                                                                 ` W. Trevor King
2012-11-27 21:18                                                                   ` [PATCH v4 4/4] Hack fix for 'submodule update does not fetch already present commits' W. Trevor King
2012-11-27 23:28                                                                   ` Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Heiko Voigt
2012-11-28  2:42                                                                     ` W. Trevor King
2012-11-29 18:51                                                                       ` Phil Hord
2012-11-23 17:24                                                             ` [PATCH v3 1/3] git-submodule add: Add -r/--record option Sascha Cunz
2012-11-23 16:03                                                         ` Heiko Voigt
2012-11-28 13:09                                           ` [PATCH v4 0/4] git-submodule add: Add --local-branch option (summary) W. Trevor King
2012-11-28 16:53                                             ` W. Trevor King
2012-11-28 19:30                                               ` [PATCH v5 0/2] submodule update: add --remote for submodule's upstream changes W. Trevor King
2012-11-28 19:30                                                 ` [PATCH v5 1/2] " W. Trevor King
2012-11-28 19:30                                                 ` [PATCH v5 2/2] submodule add: If --branch is given, record it in .gitmodules W. Trevor King
2012-11-29 16:12                                                 ` [RFC] git-submodule update: Add --commit option W. Trevor King
2012-11-29 16:21                                                   ` W. Trevor King
2012-11-29 16:27                                                   ` W. Trevor King
2012-11-29 19:13                                                 ` [PATCH v5 0/2] submodule update: add --remote for submodule's upstream changes W. Trevor King
2012-11-30  1:11                                                   ` Phil Hord
2012-11-30  3:27                                                     ` W. Trevor King
2012-12-02  3:17                                                       ` [PATCH v6 0/4] " W. Trevor King
2012-12-02  3:17                                                         ` [PATCH v6 1/4] submodule: add get_submodule_config helper funtion W. Trevor King
2012-12-03 19:30                                                           ` Junio C Hamano
2012-12-04  0:17                                                             ` W. Trevor King
2012-12-11 18:58                                                               ` [PATCH v7 0/3] submodule update: add --remote for submodule's upstream changes W. Trevor King
2012-12-11 18:58                                                                 ` [PATCH v7 1/3] submodule: add get_submodule_config helper funtion W. Trevor King
2012-12-11 18:58                                                                 ` [PATCH v7 2/3] submodule update: add --remote for submodule's upstream changes W. Trevor King
2012-12-12 17:43                                                                   ` Phil Hord
2012-12-12 19:54                                                                     ` Junio C Hamano
2012-12-12 23:02                                                                     ` W. Trevor King
2012-12-19 16:03                                                                       ` [PATCH v8 0/3] " wking
2012-12-19 16:03                                                                         ` [PATCH v8 1/3] submodule: add get_submodule_config helper funtion wking
2012-12-21  8:20                                                                           ` Heiko Voigt
2012-12-21 11:04                                                                             ` W. Trevor King
2012-12-19 16:03                                                                         ` [PATCH v8 2/3] submodule update: add --remote for submodule's upstream changes wking
2012-12-19 16:03                                                                         ` [PATCH v8 3/3] submodule add: If --branch is given, record it in .gitmodules wking
2012-12-19 17:43                                                                           ` Junio C Hamano
2012-12-21  8:18                                                                         ` [PATCH v8 0/3] submodule update: add --remote for submodule's upstream changes Heiko Voigt
2012-12-11 18:58                                                                 ` [PATCH v7 3/3] submodule add: If --branch is given, record it in .gitmodules W. Trevor King
2012-12-12  5:42                                                                 ` [PATCH v7 0/3] submodule update: add --remote for submodule's upstream changes Junio C Hamano
2012-12-12 15:24                                                                   ` W. Trevor King
2012-12-12 18:19                                                                     ` Junio C Hamano
2012-12-12 22:44                                                                       ` W. Trevor King
2012-12-02  3:17                                                         ` [PATCH v6 2/4] " W. Trevor King
2012-12-03 16:46                                                           ` Junio C Hamano
2012-12-03 18:15                                                             ` W. Trevor King
2012-12-03 18:38                                                               ` W. Trevor King
2012-12-03 20:29                                                                 ` Junio C Hamano
2012-12-02  3:17                                                         ` [PATCH v6 3/4] submodule add: If --branch is given, record it in .gitmodules W. Trevor King
2012-12-02  3:17                                                         ` [PATCH v6 4/4] submodule update: add submodule.<name>.remote config option W. Trevor King
2012-12-02 19:32                                                       ` [PATCH v5 0/2] submodule update: add --remote for submodule's upstream changes Jens Lehmann
2012-11-10 18:44                                         ` [PATCH v3 1/3] git-submodule add: Add -r/--record option W. Trevor King
2012-11-11 10:33                                           ` Junio C Hamano
2012-11-11 15:00                                             ` W. Trevor King
2012-11-17 15:30                                               ` Heiko Voigt
2012-11-28 19:42                                             ` W. Trevor King
2012-11-28 20:08                                               ` Junio C Hamano
     [not found]                                       ` <20121109104607.GC4406@ftbfs.org>
2012-11-10 19:11                                         ` W. Trevor King
2012-11-09  3:35                                     ` [PATCH v3 2/3] git-submodule foreach: export .gitmodules settings as variables W. Trevor King
2012-11-09 16:45                                       ` Heiko Voigt
2012-11-10 19:21                                         ` W. Trevor King
2012-11-09  3:35                                     ` [PATCH v3 3/3] git-submodule: Motivate --record with an example use case W. Trevor King
2012-10-25 22:14         ` [PATCH v2] git-submodule add: Add -r/--record option W. Trevor King
2012-10-26 14:00           ` Jeff King
2012-10-23 21:45     ` [PATCH] git-submodule add: Record branch name in .gitmodules Nahor
2012-10-23 20:36   ` Jens Lehmann
2012-10-23 20:55     ` W. Trevor King
2012-10-23 22:02     ` Nahor
2012-10-24 19:10       ` Jens Lehmann
  -- strict thread matches above, loose matches on Subject: below --
2012-11-28 18:28 [PATCH] submodule update: document exisiting -r form for --rebase W. Trevor King
2012-11-28 19:02 ` Junio C Hamano

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=20121202190929.GG9401@odin.tremily.us \
    --to=wking@tremily.us \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=nahor.j+gmane@gmail.com \
    --cc=peff@peff.net \
    --cc=phil.hord@gmail.com \
    --cc=spearce@spearce.org \
    /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).