[qbzr] Re: [RFC] push and --no-strict and QBzr

[Alexander Belchenko]

John Arbash Meinel пишет:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Alexander Belchenko wrote:
>> Recently bzr 1.17 has changed default behavior of push command, and now
>> it checks working tree for uncommitted changes and even prevent user to
>> push if working tree is out of date (sic!).
>>
>> This is very serious change and I hope there was really serious reasons
>> for this. Unfortunately now all development activity going behind the
>> scenes in LP merge requests, so I have no idea if there was discussion
>> on this topic or not, but I don't remember any discussion in mailing
>> list on this topic. Maybe I've missed it. Can anybody point me on
>> rationale for this change?
> 
> Vincent was the one who implemented it, and unfortunately he is on
> vacation this week and next. However, here are some key points:
> 
> 1) If your working tree is out of date, doing "bzr push" would push the
> *branch* tip, and not the revision that the working tree was at. (There
> is no way to push the working tree tip.) This (IMO) is more of a
> pre-existing bug, as it certainly seems to me that you would be pushing
> something you thought was something else. I believe 'bzr push
> - --no-strict' preserves the old behavior.
> 
> 2)
> 
>> However this change has very bad side effect on QBzr and makes our qpush
>> dialog very hard to use, therefore for non-clean tree it's "broken" and
>> I'm in trouble to fix it.
>>
>> I don't think it makes sense to add visible checkbox to qpush to reflect
>> this option, although we can do it. But this change should be
>> conditional, because QBzr supports and bzr < 1.17.
> 
> I think it makes more sense for QBzr to natively provide push support,
> rather than spawning a process for 'bzr push'. (The amount of code in
> cmd_push.run() is pretty small.)

This may be right in the ideal world, but we using push in subprocess to 
properly separate GUI rendering (progress bar) and ability to Cancel 
operation in progress from actual command execution. Currently bzrlib 
does not expose the way to repaint UI and Cancel operation other than in 
the PB ticks. This has very undesirable side effects on slow network 
operations when PB updated not very often.

> This allows you to implement strict/no-strict support directly, and also
> allows a bit better operation of the command anyway. (--remember,
> - --stacked, --stacked-on, etc are probably more accurate if you manage
> them more directly.)

I don't see why direct manipulations with all the above can be done more 
accurate using bzrlib API than bzr CLI. Really.

I'm just wonder: do any of qbzr users really needed features like 
--strict, --stacked, --stacked-on?
We control --remember in qpush pretty well.

>> Another way is to force --no-strict inside qpush always and get old
>> behaviour for free IIUC. I'm not sure is it right either.
>>
>> Another way will be: don't do anything and teach all qbzr users how to
>> setup alias with --no-strict:
>>
>> bzr alias push="push --no-strict"
>>
>> This one dont sound right for me at all.
> 
> There is a configuration setting "push_strict = False" that you can set.
> It can be set per-branch I believe, but if not in 'bazaar.conf' will
> certainly handle it.

If it documented somewhere? Help on push command has no clue about this 
configuration.

>> I'm very confused by the change in core push command and I don't
>> understand it (this is all about of teach newbies through the pain,
>> maybe?), so I'd like to hear some advices how we should "fix" qpush now,
>> especially from qbzr users.
>>
> 
> So... why are your working trees so often out of date with what you want
> to push that you are seeing the issue so often?

Well, because I'm working with lightweight checkouts of bzr plugins 
code, and qpush has used branch root as push source, not checkout root. 
This problem is fixed in qbzr trunk.

But my RFC is more broad: what's default we want to provide to our users?

> We certainly can
> re-evaluate what the default setting should be. Robert opened a bug on
> it, asking for it to be reversed. I was about 50/50 originally, changing
> the default seemed ok, I'm probably leaning slightly towards going back,
> but I don't want to do it "because that's the way it was". I want to do
> it because it is the better way. (Or at least even and "that's the way
> it was".)

Hmm :-/

> The general idea was that people would be more confused when "bzr push"
> didn't push what they thought they were pushing (I forgot to commit,
> etc), rather than having to supply a new argument when they really did
> want to push something out of the ordinary.
> 
> There was also a strong push from MySQL. I don't know if they wanted it
> to be the default, they *definitely* wanted to be able to have it as the
> default for some people. (push_strict=True). As one of the reoccurring
> issues for MySQL was people pushing and not realizing that they had not
> committed. (I think some of this stemmed from using BK which
> automatically commits when you 'merge', while we require it as a
> separate step.)

I went to the similar conclusion about this option, but I still think 
this change has introduced too much penalty to existing experienced bzr 
users, and we have to pay this price for the sake of newbies safety.
I'm not agreed with such price somewhere deep inside me.

> 
> John
> =:->
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (Cygwin)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAkpobTEACgkQJdeBCYSNAAPLKACfRzMFEsvGWKCK3jUxlpMiUXVz
> vT0Anj/pJeX/T8Yvf61sn/9dBP4j6Bq+
> =PaTl
> -----END PGP SIGNATURE-----
> 
> 
>