On 7/24/20 2:25 PM, Manuel Jacob wrote:
On 2020-07-24 14:07, Pierre-Yves David wrote:
On 7/24/20 8:19 AM, Manuel Jacob wrote:
Overall, I’m fine to backout this for now and solve the problem
during the Mercurial 5.6 development cycle.
Thanks, let do that.
I still think that this check is not the right place for preventing
the cases you were raising. The message refers to changesets included
in the push, which I read as "transferred to the server", and
including non-missing changesets in the check causes e.g. issue6372.
For situations where the changesets are already on the server, the
old check missed many cases (the new doesn’t attempt to catch them).
If you find it important to retain the shotgun that misses part of
the target and hits the wall behind it (to use a different metaphor
than the holey safety net ;)), backing this out and taking a bit more
time to fix it during the 5.6 development cycle seems reasonable.
Yeah the existing code was very ra, it is some very old code setup
quickly when there was more pressing matters. The assumption was "with
a simple but wide net, we can probably catch most of the problems, but
the problem space was not really fully mapped and tested at that time
(because other focus) hence the various flaw. Lets fix it in 5.6, even
if we dont handle every corner case having a good map of the problem
space would be very helpful.
A good start would be to define what are the limits of what we can
detect. If two clients are involved, there are situations where we can’t
really detect anything without server support. Example (I hope my email
client doesn’t mangle it, otherwise see https://dpaste.com/E8J72RMLT):
$ cat >> $HGRCPATH << EOF
> [phases]
> publish = False
> [experimental]
> evolution = all
> EOF
$ hg init server
$ cd server
$ echo root > root; hg add root; hg ci -m root
$ hg phase --public
$ echo a > a; hg add a; hg ci -m a
$ cd ..
$ hg clone server client1
updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg clone server client2
updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cd client1
$ hg branch foo -r 1
changed branch on 1 changesets
$ hg push --new-branch
pushing to $TESTTMP/server
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 0 changes to 1 files (+1 heads)
1 new obsolescence markers
obsoleted 1 changesets
$ cd ../client2
$ echo b > b; hg add b; hg ci -m b
$ hg push
pushing to $TESTTMP/server
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 1 changes to 1 files
1 new orphan changesets
Client 1 amends A and client 2 adds B on top of A. At each client, it
looks good, but when both push, we have an orphan on the server. Should
the server reject this if the client doesn’t explicitly force it?
The second push will create the orphan, we should detect the situation
at this time. (and point it to the user instead of letting him push
silently).
Backing out will reintroduce the bug that non-head outgoing
content-divergent and phase-divergent changesets are not detected. I
can send another patch that checks them.
That seems like a good idea.
About the tests in the following patches: I’ve put some small
modifications on top in
https://foss.heptapod.net/octobus/mercurial-devel/-/commits/topic/stable/push-obscheck--small-modifications.
If you like them, you can absorb them in your patches, or prune them
otherwise.
All three are great, got ahead in folding them at the appropriate
location.
If you want them to be committed like this, you’ll probably need to send
a V2. ;)
Let me know once they are folded. I'll email the V2
The current test structure is:
* case 1: test pushing
* case 2: test pushing
* case 1: test publishing
* case 2: test publishing
An alternative structure would make it easier to add more cases later:
* case 1: test pushing
* case 1: test publishing
* case 2: test pushing
* case 2: test publishing
I considered each approach. I think the "publishing" check will be a
different layer of check eventually with different sub case so it make
sense to group them. (but I don't have a super strong opinion on
that).
Yes, leaving the structure as-is might be a good idea. We can relatively
easily add a separate check for "do we publish obsolete changesets?", as
we know both the published changesets and the obsolete changesets. If
it’s a simple effective check, we don’t have to test it for every case
where we test the obsolete / unstable check.
Maybe it would make sense to factor out the setup more so that each run
is more independant (but that comes with extra runtime cost)
--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
