[PATCH 2 of 2 rfc] subrepos: make subrepositories sticky

[Martin Geisler]

Erik Zielke <ez at aragost.com> writes:

> # HG changeset patch
> # User Erik Zielke <ez at aragost.com>
> # Date 1294241235 -3600
> # Node ID a0f44721987f64620669e59ec9d638b74d2c41ec
> # Parent  29633b15e5841f444bce6f6b33822d24e91d134f
> subrepos: make subrepositories sticky
>
>
> +            else:
> +                if dst != cur:

I think you could use elif above?

> +                    self._repo.ui.warn(_('warning: subrepository %s left in '
> +                                         'revision %s\n') %
> +                                         (subrelpath(self), cur))
> +                    w = self._repo[None]
> +                    if w.dirty():
> +                        self._repo.ui.warn(_('After committing the'
> +                                            ' modifications to %s'
> +                                            ' use "hg merge" in %s to '
> +                                             'merge them with the intended '
> +                                             'revision %s\n') %
> +                                              (subrelpath(self),
> +                                               subrelpath(self), dst))

Our warnings do not looke like that -- they start with a lower-case word
ad are short, normally just a single line of <80 characters.

> +                    else:
> +                        self._repo.ui.warn(_('(use "hg update -C" to update %s '
> +                                             'to the intended revision %s)\n')
> +                                              % (subrelpath(self), dst))
>          elif anc == dst:
> -            self._repo.ui.debug("skipping subrepo %s\n" % subrelpath(self))
> +            if not self.dirty():
> +                self._repo.ui.debug("skipping subrepo %s\n"
> +                                    % subrelpath(self))
> +            else:
> +                self._repo.ui.debug("updating subrepo %s\n"
> +                                    % subrelpath(self))
> +                hg.update(self._repo, state[1])
>          else:
> -            self._repo.ui.debug("merging subrepo %s\n" % subrelpath(self))
> +            self._repo.ui.debug("merging subrepo %s\n"
> +                                % subrelpath(self))

Please don't make unrelated changes -- the patch should only contain the
hunks necessary to implement the new feature. Cleanup can come in a
later patch.

>              hg.merge(self._repo, state[1], remind=False)
>  
> +

Here too.

>      def push(self, force):
>          # push subrepos depth-first for coherent ordering
>          c = self._repo['']
> @@ -607,11 +635,23 @@
>          if overwrite:
>              self._svncommand(['revert', '--recursive', self._path])
>  
> -    def merge(self, state):
> +    def merge(self, state, overwrite=False):
>          old = int(self._state[1])
>          new = int(state[1])
> -        if new > old:
> -            self.get(state)
> +        if self._wcrev() == self._state[1]:
> +            self.get(state, overwrite)
> +        else:
> +            self._ui.warn(_('warning: subrepository %s left in '
> +                                         'revision %s\n') %
> +                                         (subrelpath(self),
> +                                          old))
> +                    

There is trailing whitespace above and the indention is... uneven :)

> +            if self._wcchanged()[0]:
> +                self._ui.warn(_('Some hint???\n'))

Perhaps the hint could be

  subrepository %s not updated: dirty working copy

if I've understood the code correctly.

> @@ -791,7 +831,7 @@
>          for b in branches:
>              if b == 'refs/heads/master':
>                  # master trumps all other branches
> -                checkout(['checkout', 'refs/heads/master'])
> +                self._gitcommand(['checkout', 'refs/heads/master'])
>                  return
>              if not firstlocalbranch and not b.startswith('refs/remotes/'):
>                  firstlocalbranch = b

I guess this belongs in the first patch.

-- 
Martin Geisler

aragost Trifork
Professional Mercurial support
http://aragost.com/en/services/mercurial/blog/