Patchwork [v2] push: warn after pushing draft changesets with tags

login
register
mail settings
Submitter Nathan Goldbaum
Date March 11, 2016, 5:38 p.m.
Message ID <189ea0e68f3c50c28bbb.1457717888@ROUS2>
Download mbox | patch
Permalink /patch/13799/
State Superseded
Headers show

Comments

Nathan Goldbaum - March 11, 2016, 5:38 p.m.
# HG changeset patch
# User Nathan Goldbaum <ngoldbau@illinois.edu>
# Date 1457664319 21600
#      Thu Mar 10 20:45:19 2016 -0600
# Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
# Parent  88738948f5d27cebe673ceb655c6161c34efda7d
push: warn after pushing draft changesets with tags
Sean Farley - March 11, 2016, 7:32 p.m.
Nathan Goldbaum <nathan12343@gmail.com> writes:

> # HG changeset patch
> # User Nathan Goldbaum <ngoldbau@illinois.edu>
> # Date 1457664319 21600
> #      Thu Mar 10 20:45:19 2016 -0600
> # Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
> # Parent  88738948f5d27cebe673ceb655c6161c34efda7d
> push: warn after pushing draft changesets with tags

I don't know if a warning is going to be enough. After dealing with
users over the last year, I'm inclined to think that 'hg tag foo' should
just mark the tagged commit as public (or possibly the signed commit).
Augie Fackler - March 12, 2016, 12:58 a.m.
On Fri, Mar 11, 2016 at 11:32:54AM -0800, Sean Farley wrote:
>
> Nathan Goldbaum <nathan12343@gmail.com> writes:
>
> > # HG changeset patch
> > # User Nathan Goldbaum <ngoldbau@illinois.edu>
> > # Date 1457664319 21600
> > #      Thu Mar 10 20:45:19 2016 -0600
> > # Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
> > # Parent  88738948f5d27cebe673ceb655c6161c34efda7d
> > push: warn after pushing draft changesets with tags
>
> I don't know if a warning is going to be enough. After dealing with
> users over the last year, I'm inclined to think that 'hg tag foo' should
> just mark the tagged commit as public (or possibly the signed commit).

I have a foggy memory that 'hg tag' was supposed to make the tagged
revision public. That could be nuts though, as the context I remember
it in was one of the sprints in Denmark.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Matt Harbison - March 12, 2016, 1:41 a.m.
On Fri, 11 Mar 2016 19:58:20 -0500, Augie Fackler <raf@durin42.com> wrote:

> On Fri, Mar 11, 2016 at 11:32:54AM -0800, Sean Farley wrote:
>>
>> Nathan Goldbaum <nathan12343@gmail.com> writes:
>>
>> > # HG changeset patch
>> > # User Nathan Goldbaum <ngoldbau@illinois.edu>
>> > # Date 1457664319 21600
>> > #      Thu Mar 10 20:45:19 2016 -0600
>> > # Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
>> > # Parent  88738948f5d27cebe673ceb655c6161c34efda7d
>> > push: warn after pushing draft changesets with tags
>>
>> I don't know if a warning is going to be enough. After dealing with
>> users over the last year, I'm inclined to think that 'hg tag foo' should
>> just mark the tagged commit as public (or possibly the signed commit).
>
> I have a foggy memory that 'hg tag' was supposed to make the tagged
> revision public. That could be nuts though, as the context I remember
> it in was one of the sprints in Denmark.

If the simple warning isn't enough, can making the tagged revision public  
be deferred to when the tag creating changeset is pushed?

My workflow is to push to a non publishing repo regularly, tag a build  
occasionally, sanity test, and release.  Once I found a bug and needed to  
amend + evolve, and redo the build, which had its own issue [1].  If  
tagging instantly made it public, there wouldn't be any easy way to tell  
what was actually in the wild.  Even outgoing to the non publishing repo  
may not indicate that, if it is holding the draft revisions.

[1]  
https://www.mercurial-scm.org/pipermail/mercurial-devel/2015-February/066189.html

(I guess I should resubmit this patch, because even if tagging makes  
something instantly public, it can be forced back to draft and cause the  
issue in the local repo.  I just wasn't sure if Pierre-Yves' concern about  
tag caching was sufficiently addressed.)
Pierre-Yves David - March 14, 2016, 1:46 p.m.
On 03/12/2016 12:58 AM, Augie Fackler wrote:
> On Fri, Mar 11, 2016 at 11:32:54AM -0800, Sean Farley wrote:
>>
>> Nathan Goldbaum <nathan12343@gmail.com> writes:
>>
>>> # HG changeset patch
>>> # User Nathan Goldbaum <ngoldbau@illinois.edu>
>>> # Date 1457664319 21600
>>> #      Thu Mar 10 20:45:19 2016 -0600
>>> # Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
>>> # Parent  88738948f5d27cebe673ceb655c6161c34efda7d
>>> push: warn after pushing draft changesets with tags
>>
>> I don't know if a warning is going to be enough. After dealing with
>> users over the last year, I'm inclined to think that 'hg tag foo' should
>> just mark the tagged commit as public (or possibly the signed commit).
>
> I have a foggy memory that 'hg tag' was supposed to make the tagged
> revision public. That could be nuts though, as the context I remember
> it in was one of the sprints in Denmark.

This have been mention multiple time but we never really made our mind 
on this. Mistake will taking are not unheard of and it is useful to be 
able to fix them while still local only.

But publishing anything tagged make sense and also help the issue of 
repository that all purely local and grow full draft for ever.

The current motivation for this is that we don't do any rewriting of the 
tag itself when a tagged changeset is rewritten. I wonder to what extend 
this would be easy to introduce.

We could also introduce a special guard to rewrite tagged changeset, the 
same as we are introducing something making harder to do history 
rewriting that create divergence, we could warn about rewritting stuff 
under a tag?
Sean Farley - March 14, 2016, 6:53 p.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> On 03/12/2016 12:58 AM, Augie Fackler wrote:
>> On Fri, Mar 11, 2016 at 11:32:54AM -0800, Sean Farley wrote:
>>>
>>> Nathan Goldbaum <nathan12343@gmail.com> writes:
>>>
>>>> # HG changeset patch
>>>> # User Nathan Goldbaum <ngoldbau@illinois.edu>
>>>> # Date 1457664319 21600
>>>> #      Thu Mar 10 20:45:19 2016 -0600
>>>> # Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
>>>> # Parent  88738948f5d27cebe673ceb655c6161c34efda7d
>>>> push: warn after pushing draft changesets with tags
>>>
>>> I don't know if a warning is going to be enough. After dealing with
>>> users over the last year, I'm inclined to think that 'hg tag foo' should
>>> just mark the tagged commit as public (or possibly the signed commit).
>>
>> I have a foggy memory that 'hg tag' was supposed to make the tagged
>> revision public. That could be nuts though, as the context I remember
>> it in was one of the sprints in Denmark.
>
> This have been mention multiple time but we never really made our mind 
> on this. Mistake will taking are not unheard of and it is useful to be 
> able to fix them while still local only.

Perhaps. My fear is that delaying stuff until push time (in general) is
too late. I always get bitten in the ass by accidentally pushing to a
publishing repo and now everything locally is public. Perhaps at the
sprint we can talk about some warning flags and also about tagged
commits?

> But publishing anything tagged make sense and also help the issue of 
> repository that all purely local and grow full draft for ever.

Righto.

> The current motivation for this is that we don't do any rewriting of the 
> tag itself when a tagged changeset is rewritten. I wonder to what extend 
> this would be easy to introduce.

I've experimented with this during converts. I think it's tricky /
fragile but might be possible.
Nathan Goldbaum - March 14, 2016, 7:26 p.m.
So I guess I'm getting a little lost here.

Is the conclusion here that my patch isn't the way we want to go, and
instead we should implement something that either marks the commit public
once the tag is created or does so on push.

On Mon, Mar 14, 2016 at 1:53 PM, Sean Farley <sean@farley.io> wrote:

>
> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
>
> > On 03/12/2016 12:58 AM, Augie Fackler wrote:
> >> On Fri, Mar 11, 2016 at 11:32:54AM -0800, Sean Farley wrote:
> >>>
> >>> Nathan Goldbaum <nathan12343@gmail.com> writes:
> >>>
> >>>> # HG changeset patch
> >>>> # User Nathan Goldbaum <ngoldbau@illinois.edu>
> >>>> # Date 1457664319 21600
> >>>> #      Thu Mar 10 20:45:19 2016 -0600
> >>>> # Node ID 189ea0e68f3c50c28bbb060c8ef59beef43c8a8d
> >>>> # Parent  88738948f5d27cebe673ceb655c6161c34efda7d
> >>>> push: warn after pushing draft changesets with tags
> >>>
> >>> I don't know if a warning is going to be enough. After dealing with
> >>> users over the last year, I'm inclined to think that 'hg tag foo'
> should
> >>> just mark the tagged commit as public (or possibly the signed commit).
> >>
> >> I have a foggy memory that 'hg tag' was supposed to make the tagged
> >> revision public. That could be nuts though, as the context I remember
> >> it in was one of the sprints in Denmark.
> >
> > This have been mention multiple time but we never really made our mind
> > on this. Mistake will taking are not unheard of and it is useful to be
> > able to fix them while still local only.
>
> Perhaps. My fear is that delaying stuff until push time (in general) is
> too late. I always get bitten in the ass by accidentally pushing to a
> publishing repo and now everything locally is public. Perhaps at the
> sprint we can talk about some warning flags and also about tagged
> commits?
>
> > But publishing anything tagged make sense and also help the issue of
> > repository that all purely local and grow full draft for ever.
>
> Righto.
>
> > The current motivation for this is that we don't do any rewriting of the
> > tag itself when a tagged changeset is rewritten. I wonder to what extend
> > this would be easy to introduce.
>
> I've experimented with this during converts. I think it's tricky /
> fragile but might be possible.
>
Sean Farley - March 14, 2016, 8:27 p.m.
Nathan Goldbaum <nathan12343@gmail.com> writes:

> So I guess I'm getting a little lost here.
>
> Is the conclusion here that my patch isn't the way we want to go, and
> instead we should implement something that either marks the commit public
> once the tag is created or does so on push.

It probably means we'll talk about it at the sprint this Friday.
Pierre-Yves David - March 26, 2016, 1:10 a.m.
On 03/14/2016 01:27 PM, Sean Farley wrote:
>
> Nathan Goldbaum <nathan12343@gmail.com> writes:
>
>> So I guess I'm getting a little lost here.
>>
>> Is the conclusion here that my patch isn't the way we want to go, and
>> instead we should implement something that either marks the commit public
>> once the tag is created or does so on push.
>
> It probably means we'll talk about it at the sprint this Friday.

And we forgot to :-/

At that point we probably need something a bit more long lived than a 
thread on the mailing list to track issues, proposals and opinions. 
Maybe a wikipage or a bug.

Cheers,

Patch

diff -r 88738948f5d2 -r 189ea0e68f3c mercurial/commands.py
--- a/mercurial/commands.py	Fri Mar 11 13:00:20 2016 +0000
+++ b/mercurial/commands.py	Thu Mar 10 20:45:19 2016 -0600
@@ -5865,6 +5865,26 @@  def push(ui, repo, dest=None, **opts):
         elif not result and pushop.bkresult:
             result = 2
 
+    # warn if any draft changesets with tags were pushed
+    if not result:
+        tag_and_draft = "(%ln - public()) and tagged()"
+        warned = False
+        for rev in repo.revs(tag_and_draft, pushop.outgoing.missing):
+            ctx = repo[rev]
+            tag_names = ctx.tags()
+            if 'tip' in tag_names:
+                tag_names.remove('tip')
+            if tag_names and ctx.phase() == phases.draft:
+                tag_warning = '"%s"' % tag_names[0]
+                ntags = len(tag_names)
+                if ntags > 1:
+                    tag_warning += ' and %s other tags' % (ntags - 1)
+                warned = True
+                ui.warn(_('pushed tags %s as draft\n') % tag_warning)
+        if warned:
+            ui.warn(_('(use "hg phase --public" to publish changes, '
+                      'then push again)\n'))
+
     return result
 
 @command('recover', [])
diff -r 88738948f5d2 -r 189ea0e68f3c tests/test-push-warn.t
--- a/tests/test-push-warn.t	Fri Mar 11 13:00:20 2016 +0000
+++ b/tests/test-push-warn.t	Thu Mar 10 20:45:19 2016 -0600
@@ -88,6 +88,24 @@  Specifying a revset that evaluates to nu
   adding file changes
   added 2 changesets with 1 changes to 1 files
 
+Warn about pushing a draft, tagged changeset to a non-publishing repository
+
+  $ echo "[phases]" >> ../a/.hg/hgrc
+  $ echo "publish = False" >> ../a/.hg/hgrc
+  $ echo "foobar" > t5
+  $ hg add t5
+  $ hg commit -m "5"
+  $ hg tag tag-name
+  $ hg push ../a
+  pushing to ../a
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 2 changesets with 2 changes to 2 files
+  pushed tags "tag-name" as draft
+  (use "hg phase --public" to publish changes, then push again)
+
   $ cd ..
 
   $ hg init c