On 2020-07-24 14:31, Pierre-Yves David wrote:
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).
At client side, we can’t detect this unless we pull first. Should the
server detect and abort?
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
I thought you folded it already. I just pushed your changesets with my
changes included.
(https://foss.heptapod.net/octobus/mercurial-devel/-/commit/286cfe4ac350a4d003743390b0c1cf13caa2e2e0
and ancestors)
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)
I don’t have a strong opinion on that. The current approach seems good
enough.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
