Re: [PATCH v2] submodule: add 'deinit' command

2012-12-13 Thread Marc Branchaud
On 12-12-12 05:25 PM, Jens Lehmann wrote:
 
 So unless people agree that deinit should also remove the work
 tree I'll prepare some patches teaching all git commands to
 consistently ignore deinitialized submodules. Opinions?

I agree with Trevor's suggestion that deinit should restore the user to the
state he would be in if he were never interested in the submodule.  So clean
up .git/config and remove the work tree.  (Maybe just issue a warning instead
if the submodule's work tree is dirty.)

Also, given that semantic, I agree with Michael that a bare git submodule
deinit should *not* deinitialize all the submodules.  It should require a
--all for that.  The bare command should just print a usage summary.

M.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread Michael J Gruber
Jens Lehmann venit, vidit, dixit 04.12.2012 22:48:
 With git submodule init the user is able to tell git he cares about one
 or more submodules and wants to have it populated on the next call to git
 submodule update. But currently there is no easy way he could tell git he
 does not care about a submodule anymore and wants to get rid of his local
 work tree (except he knows a lot about submodule internals and removes the
 submodule.$name.url setting from .git/config himself).
 
 Help those users by providing a 'deinit' command. This removes the whole
 submodule.name section from .git/config either for the given
 submodule(s) or for all those which have been initialized if none were
 given. Complain only when for a submodule given on the command line the
 url setting can't be found in .git/config.

Whoaaa, so why not have git rm remove everything unless I specify a
file to be removed?

I know I'm exaggerating a bit, but defaulting to --all for a
destructive operation seems to be a bit harsh, especially when the
command is targeted at those users that you mention.

 Add tests and link the man pages of git submodule deinit and git rm
 to assist the user in deciding whether removing or unregistering the
 submodule is the right thing to do for him.
 
 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---
 
 Am 03.12.2012 08:58, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:

 Maybe the principle of least surprise is better followed when we
 nuke the whole section, as it might surprise the user more to have
 a setting resurrected he customized in the last life cycle of the
 submodule than seeing that after an deinit followed by an init all
 former customizations are consistently gone. So I tend to think now
 that removing the whole section would be the better solution here.

 I tend to agree; I suspect that a deinit would be mostly done
 either to

  (1) correct mistakes the user made during a recent init and
  perhaps sync; or

  (2) tell Git that the user has finished woing with this particular
  submodule and does not intend to use it for quite a while.

 For both (1) and (2), I think it would be easier to users if we gave
 them a clean slate, the same state as the one the user who never had
 ran init on it would be in.  A user in situation (1) is asking for
 a clean slate, and a user in situation (2) is better served if he
 does not have to worry about leftover entries in $GIT_DIR/config he
 has long forgotten from many months ago (during which time the way
 the project uses the particular submodule may well have changed)
 giving non-standard experience different from what other project
 participants would get.
 
 Changes in v2:
 - Remove the whole submodule section instead of only removing the
   url setting and explain why we do that in a comment
 - Reworded commit message and git-submodule.txt to reflect that
 - Extend the test to check that a custom settings are removed
 
 
  Documentation/git-rm.txt|  4 
  Documentation/git-submodule.txt | 12 ++
  git-submodule.sh| 52 
 -
  t/t7400-submodule-basic.sh  | 12 ++
  4 files changed, 79 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
 index 262436b..ec42bf5 100644
 --- a/Documentation/git-rm.txt
 +++ b/Documentation/git-rm.txt
 @@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules 
 work tree.
  Ignored files are deemed expendable and won't stop a submodule's work
  tree from being removed.
 
 +If you only want to remove the local checkout of a submodule from your
 +work tree without committing that use `git submodule deinit` instead
 +(see linkgit:git-submodule[1]).
 +
  EXAMPLES
  
  `git rm Documentation/\*.txt`::
 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index b1de3ba..08b55a7 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -13,6 +13,7 @@ SYNOPSIS
 [--reference repository] [--] repository [path]
  'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
  'git submodule' [--quiet] init [--] [path...]
 +'git submodule' [--quiet] deinit [--] [path...]
  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
 [--reference repository] [--merge] [--recursive] [--] 
 [path...]
  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) 
 n]
 @@ -134,6 +135,17 @@ init::
   the explicit 'init' step if you do not intend to customize
   any submodule locations.
 
 +deinit::
 + Unregister the submodules, i.e. remove the whole `submodule.$name`
 + section from .git/config. Further calls to `git submodule update`,
 + `git submodule foreach` and `git submodule sync` will skip any
 + unregistered submodules until they are initialized again, so use
 + this command if you don't want to have a local 

Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread Jens Lehmann
Am 12.12.2012 16:08, schrieb Michael J Gruber:
 Jens Lehmann venit, vidit, dixit 04.12.2012 22:48:
 With git submodule init the user is able to tell git he cares about one
 or more submodules and wants to have it populated on the next call to git
 submodule update. But currently there is no easy way he could tell git he
 does not care about a submodule anymore and wants to get rid of his local
 work tree (except he knows a lot about submodule internals and removes the
 submodule.$name.url setting from .git/config himself).

 Help those users by providing a 'deinit' command. This removes the whole
 submodule.name section from .git/config either for the given
 submodule(s) or for all those which have been initialized if none were
 given. Complain only when for a submodule given on the command line the
 url setting can't be found in .git/config.
 
 Whoaaa, so why not have git rm remove everything unless I specify a
 file to be removed?

Because git add doesn't add any file in that case either?

 I know I'm exaggerating a bit, but defaulting to --all for a
 destructive operation seems to be a bit harsh, especially when the
 command is targeted at those users that you mention.

All other submodule commands (except add, which only operates on a
single submodule to be) iterate over all submodules if none were
explicitly given on the command line. So I made deinit just behave
like all the others - and especially init - do. But if people really
are surprised by being consistent here I won't argue against adding
such a --all option, but currently I'm not convinced it is worth
it. Especially as I suspect the number of submodule users having
customized those in .git/config is not that high ...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Especially as I suspect the number of submodule users having
 customized those in .git/config is not that high ...

I thought the point of deinit was to say I am not interested in
having a checkout of these submodules in my working tree anymore.
The user could do rm -fr submodule  mkdir submodule to remove it
locally and keep diff and status from noticing the removal, but
the primary reason the user needs an explicit deinit is because
many subcommands of git submodule are documented to operate on all
submodules that have been inited when given no explicit submodule
names [*1*].

Your deinit is documented not to actually remove the submodule
checkout, but that very much goes against my intuition.  What is the
justification behind that choice?  We'll remove the configuration,
you remove the tree yourself will invite the mistake of running
git rm on it, which you wanted to avoid with the addition to the
git rm documentation, no?


[Footnote]

*1* In reality, the code looks at presense of .git in the submodule
path to decide if it has been inited (cf. cmd_update), but this
implementation of deinit does not seem to cause that .git to be
removed, leaving the submodule in inited state from these other
command's perspective.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread Jens Lehmann
Am 12.12.2012 20:32, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
 
 Especially as I suspect the number of submodule users having
 customized those in .git/config is not that high ...
 
 I thought the point of deinit was to say I am not interested in
 having a checkout of these submodules in my working tree anymore.

Yes. (But I'm not sure users expect that command to also remove
the work tree)

 The user could do rm -fr submodule  mkdir submodule to remove it
 locally and keep diff and status from noticing the removal, but
 the primary reason the user needs an explicit deinit is because
 many subcommands of git submodule are documented to operate on all
 submodules that have been inited when given no explicit submodule
 names [*1*].

The real reason we need deinit is that the next run of submodule
update will otherwise happily recreate the submodule checkout you
just removed as long as it finds the url setting in .git/config.

 Your deinit is documented not to actually remove the submodule
 checkout, but that very much goes against my intuition.  What is the
 justification behind that choice?

I thought it should match what submodule init does, which is to do
nothing to the work tree until the next submodule update is run.
(But I agree that analogy is somewhat flawed until we teach update
to remove a deinitialized submodule - or maybe teach it the --deinit
option which could do both). On the other hand with current git
submodule work trees always stay around anyway until you remove them
by hand (e.g. when you switch to a branch that doesn't have it), so
I'm not sure what would surprise people more here. So I just left
the work tree unchanged.

 We'll remove the configuration,
 you remove the tree yourself will invite the mistake of running
 git rm on it, which you wanted to avoid with the addition to the
 git rm documentation, no?

I think that'll happen only if git would remind them that they
still have a populated work tree, which I believe it shouldn't.

 [Footnote]
 
 *1* In reality, the code looks at presense of .git in the submodule
 path to decide if it has been inited (cf. cmd_update), but this
 implementation of deinit does not seem to cause that .git to be
 removed, leaving the submodule in inited state from these other
 command's perspective.

Nope, cmd_update() checks first if the url is found in .git/config
and skips the submodule if not. I rechecked and only summary and
foreach still recurse into a deinitialized submodule, which they
shouldn't. But a quick test shows that git status and git diff
also still inspect a deinitialized submodule, so there's some work
left to do to handle the case where the work tree is not removed.

So unless people agree that deinit should also remove the work
tree I'll prepare some patches teaching all git commands to
consistently ignore deinitialized submodules. Opinions?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 So unless people agree that deinit should also remove the work
 tree I'll prepare some patches teaching all git commands to
 consistently ignore deinitialized submodules. Opinions?

While I agree that consistency is good, deinit that does not
remove the working tree of the submodule the user explicitly said he
no longer wants to have the checkout for is a bug, and I think these
two are orthogonal issues.

In other words, Ignore deinitialized submodules even when an
earlier bug in deinit failed to remove the working tree is a
robustness issue for the other recursing commands, not an excuse for
deinit to have such a bug in the first place, I think.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread W. Trevor King
On Wed, Dec 12, 2012 at 02:34:47PM -0800, Junio C Hamano wrote:
 Jens Lehmann jens.lehm...@web.de writes:
 
  So unless people agree that deinit should also remove the work
  tree I'll prepare some patches teaching all git commands to
  consistently ignore deinitialized submodules. Opinions?
 
 While I agree that consistency is good, deinit that does not
 remove the working tree of the submodule the user explicitly said he
 no longer wants to have the checkout for is a bug, and I think these
 two are orthogonal issues.

Should `deinit` remove the submodule checkout, replace it with the
original gitlink, and clear the .git/config information then?  That
would restore the user to the state they'd be in if they were never
interested in the submodule.

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


signature.asc
Description: OpenPGP digital signature


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread Junio C Hamano
W. Trevor King wk...@tremily.us writes:

 Should `deinit` remove the submodule checkout, replace it with the
 original gitlink, and clear the .git/config information then?  That
 would restore the user to the state they'd be in if they were never
 interested in the submodule.

AFAIU, restore the user to the state is the goal.  I am not sure
what you meant by replace it with the original gitlink, though.  A
checkout with a submodule that the user is not interested in would
have an empty directory at that path, no?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] submodule: add 'deinit' command

2012-12-12 Thread W. Trevor King
On Wed, Dec 12, 2012 at 03:35:59PM -0800, Junio C Hamano wrote:
 W. Trevor King wk...@tremily.us writes:
 
  Should `deinit` remove the submodule checkout, replace it with the
  original gitlink, and clear the .git/config information then?  That
  would restore the user to the state they'd be in if they were never
  interested in the submodule.
 
 AFAIU, restore the user to the state is the goal.  I am not sure
 what you meant by replace it with the original gitlink, though.  A
 checkout with a submodule that the user is not interested in would
 have an empty directory at that path, no?

Ah yes, the gitlink is only in the index.  Sorry for the noise.

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


signature.asc
Description: OpenPGP digital signature


[PATCH v2] submodule: add 'deinit' command

2012-12-04 Thread Jens Lehmann
With git submodule init the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to git
submodule update. But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
submodule.$name.url setting from .git/config himself).

Help those users by providing a 'deinit' command. This removes the whole
submodule.name section from .git/config either for the given
submodule(s) or for all those which have been initialized if none were
given. Complain only when for a submodule given on the command line the
url setting can't be found in .git/config.

Add tests and link the man pages of git submodule deinit and git rm
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him.

Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

Am 03.12.2012 08:58, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
 
 Maybe the principle of least surprise is better followed when we
 nuke the whole section, as it might surprise the user more to have
 a setting resurrected he customized in the last life cycle of the
 submodule than seeing that after an deinit followed by an init all
 former customizations are consistently gone. So I tend to think now
 that removing the whole section would be the better solution here.
 
 I tend to agree; I suspect that a deinit would be mostly done
 either to
 
  (1) correct mistakes the user made during a recent init and
  perhaps sync; or
 
  (2) tell Git that the user has finished woing with this particular
  submodule and does not intend to use it for quite a while.
 
 For both (1) and (2), I think it would be easier to users if we gave
 them a clean slate, the same state as the one the user who never had
 ran init on it would be in.  A user in situation (1) is asking for
 a clean slate, and a user in situation (2) is better served if he
 does not have to worry about leftover entries in $GIT_DIR/config he
 has long forgotten from many months ago (during which time the way
 the project uses the particular submodule may well have changed)
 giving non-standard experience different from what other project
 participants would get.

Changes in v2:
- Remove the whole submodule section instead of only removing the
  url setting and explain why we do that in a comment
- Reworded commit message and git-submodule.txt to reflect that
- Extend the test to check that a custom settings are removed


 Documentation/git-rm.txt|  4 
 Documentation/git-submodule.txt | 12 ++
 git-submodule.sh| 52 -
 t/t7400-submodule-basic.sh  | 12 ++
 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 262436b..ec42bf5 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules 
work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.

+If you only want to remove the local checkout of a submodule from your
+work tree without committing that use `git submodule deinit` instead
+(see linkgit:git-submodule[1]).
+
 EXAMPLES
 
 `git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b1de3ba..08b55a7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
  [--reference repository] [--] repository [path]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
 'git submodule' [--quiet] init [--] [path...]
+'git submodule' [--quiet] deinit [--] [path...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
  [--reference repository] [--merge] [--recursive] [--] 
[path...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
@@ -134,6 +135,17 @@ init::
the explicit 'init' step if you do not intend to customize
any submodule locations.

+deinit::
+   Unregister the submodules, i.e. remove the whole `submodule.$name`
+   section from .git/config. Further calls to `git submodule update`,
+   `git submodule foreach` and `git submodule sync` will skip any
+   unregistered submodules until they are initialized again, so use
+   this command if you don't want to have a local checkout of the
+   submodule in your work tree anymore (but note that this command
+   does not remove the submodule work tree). If you really want to
+   remove a submodule from the repository and commit that use
+   linkgit:git-rm[1] instead.
+
 update::
Update the registered submodules, i.e. clone missing submodules and
checkout the commit