[PATCH STABLE?] subrepo: only do clean update when overwrite is set

[Mads Kiilerich]

Simon Heimberg wrote, On 10/23/2012 10:11 AM:
> # HG changeset patch
> # User Simon Heimberg <simohe at besonet.ch>
> # Date 1350979706 -7200
> # Node ID 3e66922817ac1a904e7b56bb2ae75074c818db94
> # Parent  891a489443ed8c9daaf37197f7d3a0c8c904e27c
> subrepo: only do clean update when overwrite is set
>
> Files in subrepo are overwritten on update. But this should only happen when -C
> is specified.

I would use "were" instead of "are" to make it clear for future readers 
that you are describing how it was before this changeset. But there is 
no clear guideline for this.

> Use overwrite parameter introduced for svn subrepos in c19b9282d3a7 to decide
> whether to do the same as update (merge files) or clean (remove changes).
>
> This fixes issue3276

This should be in the subject line - see
http://mercurial.selenic.com/wiki/ContributingChanges#Submission_checklist
http://mercurial.selenic.com/wiki/ContributingChanges#Patch_descriptions

> test-subrepo is extended to test if an untracked file is overwritten.
> The first two chunks are debugging output which has changed. (Because overwrite
> is not always true anymore for subrepos)
> No other test needs any change.
>
> diff -r 891a489443ed -r 3e66922817ac mercurial/hg.py
> --- a/mercurial/hg.py	Mon Okt 15 23:32:28 2012 +0200
> +++ b/mercurial/hg.py	Die Okt 23 10:08:26 2012 +0200
> @@ -446,6 +446,12 @@
>   # naming conflict in clone()
>   _update = update
>   
> +def updatesub(subrepo, node, overwrite=False):
> +    """Do update or clean, but do not display anything
> +
> +    returns stats"""
> +    return mergemod.update(subrepo, node, False, overwrite, None)

This function is only used by subrepos but is in no way specific for 
subrepos. Couldn't it be generalized? Or perhaps even better: Just do 
more like we do for ordinary update:
     if clean:
         ret = hg.clean(repo, rev)
     else:
         ret = hg.update(repo, rev)

That could require adding a show_stats flag to update ... but I'm not 
sure why we don't want stats in this case.

Just returning the result of mergemod.update would also make abstraction 
in hg.py leaky. It is not used so there is no reason to return it at all.

> +
>   def clean(repo, node, show_stats=True):
>       """forcibly switch the working directory to node, clobbering changes"""
>       stats = mergemod.update(repo, node, False, True, None)
> diff -r 891a489443ed -r 3e66922817ac mercurial/subrepo.py
> --- a/mercurial/subrepo.py	Mon Okt 15 23:32:28 2012 +0200
> +++ b/mercurial/subrepo.py	Die Okt 23 10:08:26 2012 +0200
> @@ -519,7 +519,7 @@
>           self._get(state)
>           source, revision, kind = state
>           self._repo.ui.debug("getting subrepo %s\n" % self._path)
> -        hg.clean(self._repo, revision, False)
> +        hg.updatesub(self._repo, revision, overwrite)
>   
>       def merge(self, state):
>           self._get(state)
> diff -r 891a489443ed -r 3e66922817ac tests/test-subrepo.t
> --- a/tests/test-subrepo.t	Mon Okt 15 23:32:28 2012 +0200
> +++ b/tests/test-subrepo.t	Die Okt 23 10:08:26 2012 +0200
> @@ -210,9 +210,10 @@
>     subrepo merge f0d2028bf86d+ 1831e14459c4 1f14a2e2d3ec
>       subrepo t: other changed, get t:6747d179aa9a688023c4b0cad32e4c92bb7f34ad:hg
>     getting subrepo t
> +    searching for copies back to rev 1
>     resolving manifests
> -   overwrite: True, partial: False
> -   ancestor: 60ca1237c194+, local: 60ca1237c194+, remote: 6747d179aa9a
> +   overwrite: False, partial: False
> +   ancestor: 60ca1237c194, local: 60ca1237c194+, remote: 6747d179aa9a
>      t: remote is newer -> g
>     updating: t 1/1 files (100.00%)
>     getting t
> @@ -580,16 +581,37 @@
>     $ hg -q -R repo2/s push
>     $ hg -R repo2/s up -C 0
>     1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> -  $ echo 2 > repo2/s/a
> -  $ hg -R repo2/s ci -m3
> +  $ echo 2 > repo2/s/b
> +  $ hg -R repo2/s ci -m3 -A
> +  adding b
>     created new head
>     $ hg -R repo2 ci -m3
>     $ hg -q -R repo2 push
> -  abort: push creates new remote head 9d66565e64e1!
> +  abort: push creates new remote head cc505f09a8b2!
>     (did you forget to merge? use push -f to force)
>     [255]
>     $ hg -R repo update
>     0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> +test if untracked file is not overwritten
> +
> +  $ echo issue3276_ok > repo/s/b
> +  $ hg -R repo2 push -f -q
> +  $ hg -R repo update
> +  b: untracked file differs
> +  abort: untracked files in working directory differ from files in requested revision
> +  [255]
> +
> +  $ cat repo/s/b
> +  issue3276_ok
> +  $ rm repo/s/b
> +  $ hg -R repo revert --all
> +  reverting repo/.hgsubstate
> +  reverting subrepo s
> +  $ hg -R repo update
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ cat repo/s/b
> +  2
>     $ rm -rf repo2 repo

You mention 'up -C' in the description but do not add any tests using 
it. Is it already tested somewhere else, or will revert --all test the 
same code path?

/Mads