Page MenuHomePhabricator

windows: add windows behavior on broken pager
ClosedPublic

Authored by Alphare on Jul 7 2021, 9:14 AM.

Details

Summary

Apparently, Windows has "better" behavior than Unix in this case. This is an
edge case that led me down a rabbit hole, only to find a bug in the Python
documentation... I am not planning on trying to reproduce the same behavior
on Unix systems since it's not really useful, but other people are welcome to!

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Jul 7 2021, 9:14 AM
marmoute accepted this revision.Jul 7 2021, 11:57 AM

Is there any more context you can/want to supply about the bug, what's better about Windows, the rabbit hole, etc?

Is there any more context you can/want to supply about the bug, what's better about Windows, the rabbit hole, etc?

My understanding (@Alphare can correct me) is that windows do -not- have the bug. The test is showing and documenting a bug. And This works fine on Windows, so I don't think we need extra documentation.

Is there any more context you can/want to supply about the bug, what's better about Windows, the rabbit hole, etc?

Sure. The expected behavior is apparently to not paginate if the pager is broken, but still show the contents. I didn't understand that going in, so I was trying to figure out why Windows' behavior was differing from Linux'.
The pager code makes use of os.dup, whose documentation says "On Windows, when duplicating a standard stream (0: stdin, 1: stdout, 2: stderr), the new file descriptor is inheritable.", which is not true (at least in 3.9) as os.get_inheritable(os.dup(1)) evaluates to False. This is made worse by the fact that os.dup and its sibiling os.dup2 work differently on Python 2 and 3... but also in different versions of Python 3 because of bugfixes, which should explain why the doc is wrong (see https://bugs.python.org/issue37267 and related bugs). I had help from Victor Stinner who initially championed the Python 3 changes.

But turns out this had nothing to do with the problem since Windows "behaves properly", maybe by accident?

I hope that makes sense.

mharbison72 added inline comments.Jul 9 2021, 11:08 AM
tests/test-pager.t
223

I have no idea why, but now this output is being removed on Windows with py3.8.1 and py3.9.0. I *know* I was seeing this output on this system before, but I can't figure out what's changed. I'm running this on top of 57bdecf4322c.

baymax updated this revision to Diff 29073.Jul 9 2021, 9:21 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 29093.Jul 9 2021, 11:41 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare added inline comments.Jul 13 2021, 11:42 AM
tests/test-pager.t
223

It appears that this is flaky somehow, but I've had many runs with the text appearing. We can deal with the flakyness later I think, once we're sure about how frequent it is.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.