This is an archive of the discontinued Mercurial Phabricator instance.

logtoprocess: connect all fds to /dev/null to avoid bad interaction with pager
AbandonedPublic

Authored by lothiraldan on Nov 15 2017, 10:44 AM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Summary

We detected that pager is waiting for log-to-process script to finish, which
is annoying when adding a script on commandfinish that does an HTTP push.

There seems to be no workaround on the script side and it will make the
behavior on Linux/MacOS closer to the Windows behavior.

The drawback is that it makes the related tests more flaky as log-to-process
outputs are now really asynchronous.

If it's considered a BC change, another option would be to add a config option
for this new behavior. I personally think that the different behavior between
Windows and Linux is confusing and that it's a bug I would be fine with a new
config option.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

lothiraldan created this revision.Nov 15 2017, 10:44 AM
yuja requested changes to this revision.Nov 16 2017, 8:34 AM
yuja added a subscriber: yuja.
yuja added inline comments.
hgext/logtoprocess.py
99

stdout and stderr should be writable. We can simply pass a file
descriptor open with O_RDWR.

nullfd = os.open(os.devnull, os.O_RDWR)
Popen(stdin=nullfd, stdout=nullfd, stderr=nullfd)

and os.close(nullfd) though it doesn't mater since the process
is terminated with _exit().

This revision now requires changes to proceed.Nov 16 2017, 8:34 AM
lothiraldan added inline comments.Nov 17 2017, 3:03 AM
hgext/logtoprocess.py
99

Thx for the catch, I will make the change.

lothiraldan updated this revision to Diff 3589.Nov 17 2017, 3:25 AM
lothiraldan marked 2 inline comments as done.Nov 17 2017, 3:25 AM
lothiraldan updated this revision to Diff 3917.Nov 28 2017, 3:36 PM
lothiraldan updated this revision to Diff 4469.Dec 15 2017, 5:11 AM