public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
@ 2016-09-23  3:23 Kerin Millar
  0 siblings, 0 replies; 6+ messages in thread
From: Kerin Millar @ 2016-09-23  3:23 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

Duly updated to use any instead of ==, as recommended by Brian Dolbec.

-- 
Kerin Millar <kfm@plushkava.net>

[-- Attachment #2: portage-wsl-support-v2.patch --]
[-- Type: application/octet-stream, Size: 1730 bytes --]

commit aa759f7a506206c4ff7e9929d7a79fabae45475b
Author: Kerin Millar <kfm@plushkava.net>
Date:   Fri Sep 23 01:55:10 2016 +0000

    AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
    
    As of Windows 10 build 14393, WSL is unable to support EbuildIpcDaemon
    correctly. The presence of /dev/lxss - as a character device - indicates
    that we are running under WSL, in which case the use of the daemon should be
    disabled.
    
    Note that stat calls directed at /dev/lxss are currently doomed to fail with
    EPERM. Thus, this specific error is also used as a means to detect WSL.
    Should Microsoft render this device node accessible in future builds, WSL
    will still be detected correctly.

diff --git a/pym/_emerge/AbstractEbuildProcess.py b/pym/_emerge/AbstractEbuildProcess.py
index 8bd30a6..af2c289 100644
--- a/pym/_emerge/AbstractEbuildProcess.py
+++ b/pym/_emerge/AbstractEbuildProcess.py
@@ -128,7 +128,17 @@ class AbstractEbuildProcess(SpawnProcess):
 			# since we're not displaying to a terminal anyway.
 			self.settings['NOCOLOR'] = 'true'
 
-		if self._enable_ipc_daemon:
+		# Determine whether we are running under WSL (Windows Subsystem for Linux).
+		# Trying to stat /dev/lxss under WSL will always fail with EPERM (for now).
+		running_wsl = False
+		try:
+			if platform.system() in ['Linux'] and stat.S_ISCHR(os.stat('/dev/lxss')):
+				running_wsl = True
+		except OSError as e:
+			if (e.errno in [errno.EPERM]):
+				running_wsl = True
+
+		if self._enable_ipc_daemon and not running_wsl:
 			self.settings.pop('PORTAGE_EBUILD_EXIT_FILE', None)
 			if self.phase not in self._phases_without_builddir:
 				if 'PORTAGE_BUILDDIR_LOCKED' not in self.settings:

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
@ 2016-09-23  3:48 Zac Medico
  2016-09-26 22:55 ` Kerin Millar
  0 siblings, 1 reply; 6+ messages in thread
From: Zac Medico @ 2016-09-23  3:48 UTC (permalink / raw
  To: gentoo-portage-dev

On Thu, Sep 22, 2016 at 8:23 PM, Kerin Millar <kfm@plushkava.net> wrote:
> Duly updated to use any instead of ==, as recommended by Brian Dolbec.
>
> --
> Kerin Millar <kfm@plushkava.net>

My first choice would be to use a small test case to detect when ipc
is broken, and disable it dynamically. A good example of such a test
is the can_poll_device function here:

https://gitweb.gentoo.org/proj/portage.git/tree/pym/portage/util/_eventloop/EventLoop.py?h=portage-2.3.1#n597

If it's not possible to use a test similar to the above, maybe it's
best to use /proc/version or /proc/sys/kernel/osrelease as mentioned
in the following issue:

https://github.com/Microsoft/BashOnWindows/issues/423

Thanks,
Zac


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
@ 2016-09-26  5:49 Zac Medico
  2016-09-26 22:58 ` Kerin Millar
  0 siblings, 1 reply; 6+ messages in thread
From: Zac Medico @ 2016-09-26  5:49 UTC (permalink / raw
  To: gentoo-portage-dev

On Thu, Sep 22, 2016 at 8:48 PM, Zac Medico <zmedico@gentoo.org> wrote:
> On Thu, Sep 22, 2016 at 8:23 PM, Kerin Millar <kfm@plushkava.net> wrote:
>> Duly updated to use any instead of ==, as recommended by Brian Dolbec.
>>
>> --
>> Kerin Millar <kfm@plushkava.net>
>
> My first choice would be to use a small test case to detect when ipc
> is broken, and disable it dynamically. A good example of such a test
> is the can_poll_device function here:
>
> https://gitweb.gentoo.org/proj/portage.git/tree/pym/portage/util/_eventloop/EventLoop.py?h=portage-2.3.1#n597
>
> If it's not possible to use a test similar to the above, maybe it's
> best to use /proc/version or /proc/sys/kernel/osrelease as mentioned
> in the following issue:
>
> https://github.com/Microsoft/BashOnWindows/issues/423
>
> Thanks,
> Zac

I've started playing around with WSL, and I've discovered that
portage's ipc actually works if we use fcntl.flock instead of
fcntl.lockf!!! Simply set _default_lock_fn = fcntl.flock in
pym/portage/locks.py, and watch the tests succeed:

$ pym/portage/tests/runTests.py pym/portage/tests/ebuild/test_ipc_daemon.py
testIpcDaemon (portage.tests.ebuild.test_ipc_daemon.IpcDaemonTestCase) ... ok

----------------------------------------------------------------------
Ran 1 test in 1.282s

OK

So, I'm thinking that we should add a dynamic test inside
pym/portage/locks.py which checks for a broken fcntl.lockf, and falls
back to fcntl.flock in that case.

Thanks,
Zac


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
  2016-09-23  3:48 [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux Zac Medico
@ 2016-09-26 22:55 ` Kerin Millar
  0 siblings, 0 replies; 6+ messages in thread
From: Kerin Millar @ 2016-09-26 22:55 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

Hi Zac,

On Thu, 22 Sep 2016 20:48:26 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> On Thu, Sep 22, 2016 at 8:23 PM, Kerin Millar <kfm@plushkava.net> wrote:
> > Duly updated to use any instead of ==, as recommended by Brian Dolbec.
> >
> > --
> > Kerin Millar <kfm@plushkava.net>
> 
> My first choice would be to use a small test case to detect when ipc
> is broken, and disable it dynamically. A good example of such a test
> is the can_poll_device function here:
> 
> https://gitweb.gentoo.org/proj/portage.git/tree/pym/portage/util/_eventloop/EventLoop.py?h=portage-2.3.1#n597

This is a good idea. I suspect a faulty implementation of (e)poll, but the exact manner in which WSL is falling short is something that eludes me. Should I manage to figure out, I'll likely propose an alternative workaround.

> 
> If it's not possible to use a test similar to the above, maybe it's
> best to use /proc/version or /proc/sys/kernel/osrelease as mentioned
> in the following issue:
> 
> https://github.com/Microsoft/BashOnWindows/issues/423

I had read this prior but didn't find it convincing. However unlikely it may seem, someone could add Microsoft to EXTRAVERSION - for an Azure guest perhaps - and break such a heuristic. By contrast, the chances of /dev/lxss existing outside of the (L)inu(X) (S)ub(S)ystem are infinitesimally small.

-- 
Kerin Millar <kfm@plushkava.net>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
  2016-09-26  5:49 Zac Medico
@ 2016-09-26 22:58 ` Kerin Millar
  2016-09-27 16:26   ` Zac Medico
  0 siblings, 1 reply; 6+ messages in thread
From: Kerin Millar @ 2016-09-26 22:58 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

On Sun, 25 Sep 2016 22:49:59 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> On Thu, Sep 22, 2016 at 8:48 PM, Zac Medico <zmedico@gentoo.org> wrote:
> > On Thu, Sep 22, 2016 at 8:23 PM, Kerin Millar <kfm@plushkava.net> wrote:
> >> Duly updated to use any instead of ==, as recommended by Brian Dolbec.
> >>
> >> --
> >> Kerin Millar <kfm@plushkava.net>
> >
> > My first choice would be to use a small test case to detect when ipc
> > is broken, and disable it dynamically. A good example of such a test
> > is the can_poll_device function here:
> >
> > https://gitweb.gentoo.org/proj/portage.git/tree/pym/portage/util/_eventloop/EventLoop.py?h=portage-2.3.1#n597
> >
> > If it's not possible to use a test similar to the above, maybe it's
> > best to use /proc/version or /proc/sys/kernel/osrelease as mentioned
> > in the following issue:
> >
> > https://github.com/Microsoft/BashOnWindows/issues/423
> >
> > Thanks,
> > Zac
> 
> I've started playing around with WSL, and I've discovered that
> portage's ipc actually works if we use fcntl.flock instead of
> fcntl.lockf!!! Simply set _default_lock_fn = fcntl.flock in
> pym/portage/locks.py, and watch the tests succeed:
> 
> $ pym/portage/tests/runTests.py pym/portage/tests/ebuild/test_ipc_daemon.py
> testIpcDaemon (portage.tests.ebuild.test_ipc_daemon.IpcDaemonTestCase) ... ok
> 
> ----------------------------------------------------------------------
> Ran 1 test in 1.282s
> 
> OK

How strange!

> So, I'm thinking that we should add a dynamic test inside
> pym/portage/locks.py which checks for a broken fcntl.lockf, and falls
> back to fcntl.flock in that case.

Sounds good to me. Thank you for looking into it.

-- 
Kerin Millar <kfm@plushkava.net>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux
  2016-09-26 22:58 ` Kerin Millar
@ 2016-09-27 16:26   ` Zac Medico
  0 siblings, 0 replies; 6+ messages in thread
From: Zac Medico @ 2016-09-27 16:26 UTC (permalink / raw
  To: Kerin Millar; +Cc: gentoo-portage-dev

On Mon, Sep 26, 2016 at 3:58 PM, Kerin Millar <kfm@plushkava.net> wrote:
> On Sun, 25 Sep 2016 22:49:59 -0700
> Zac Medico <zmedico@gentoo.org> wrote:
>
>> On Thu, Sep 22, 2016 at 8:48 PM, Zac Medico <zmedico@gentoo.org> wrote:
>> > On Thu, Sep 22, 2016 at 8:23 PM, Kerin Millar <kfm@plushkava.net> wrote:
>> >> Duly updated to use any instead of ==, as recommended by Brian Dolbec.
>> >>
>> >> --
>> >> Kerin Millar <kfm@plushkava.net>
>> >
>> > My first choice would be to use a small test case to detect when ipc
>> > is broken, and disable it dynamically. A good example of such a test
>> > is the can_poll_device function here:
>> >
>> > https://gitweb.gentoo.org/proj/portage.git/tree/pym/portage/util/_eventloop/EventLoop.py?h=portage-2.3.1#n597
>> >
>> > If it's not possible to use a test similar to the above, maybe it's
>> > best to use /proc/version or /proc/sys/kernel/osrelease as mentioned
>> > in the following issue:
>> >
>> > https://github.com/Microsoft/BashOnWindows/issues/423
>> >
>> > Thanks,
>> > Zac
>>
>> I've started playing around with WSL, and I've discovered that
>> portage's ipc actually works if we use fcntl.flock instead of
>> fcntl.lockf!!! Simply set _default_lock_fn = fcntl.flock in
>> pym/portage/locks.py, and watch the tests succeed:
>>
>> $ pym/portage/tests/runTests.py pym/portage/tests/ebuild/test_ipc_daemon.py
>> testIpcDaemon (portage.tests.ebuild.test_ipc_daemon.IpcDaemonTestCase) ... ok
>>
>> ----------------------------------------------------------------------
>> Ran 1 test in 1.282s
>>
>> OK
>
> How strange!

While the ebuild-ipc helper waits for a response, it uses non-blocking
lock calls to poll for liveliness, so it's critical that the locking
api be usable here. We've seen a similar issue in the past with PyPy,
where fcntl.lockf was broken while fcntl.flock worked just fine.

Thanks,
Zac


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-27 16:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-23  3:48 [gentoo-portage-dev] [PATCH v2] AbstractEbuildProcess: disable ipc_daemon under Windows Subsystem for Linux Zac Medico
2016-09-26 22:55 ` Kerin Millar
  -- strict thread matches above, loose matches on Subject: below --
2016-09-26  5:49 Zac Medico
2016-09-26 22:58 ` Kerin Millar
2016-09-27 16:26   ` Zac Medico
2016-09-23  3:23 Kerin Millar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox