Re: [PATCH] coroutine: add libucontext as external library

[Joelle van Dyne <j@getutm.app>]

On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > iOS does not support ucontext natively for aarch64 and the sigaltstack is
> > also unsupported (even worse, it fails silently, see:
> > https://openradar.appspot.com/13002712 )
> >
> > As a workaround we include a library implementation of ucontext and add it
> > as a build option.
> >
> > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > ---
> >  configure                 | 21 ++++++++++++++++++---
> >  meson.build               | 12 +++++++++++-
> >  util/coroutine-ucontext.c |  9 +++++++++
> >  .gitmodules               |  3 +++
> >  MAINTAINERS               |  6 ++++++
> >  meson_options.txt         |  2 ++
> >  subprojects/libucontext   |  1 +
> >  7 files changed, 50 insertions(+), 4 deletions(-)
> >  create mode 160000 subprojects/libucontext
> >
> > diff --git a/configure b/configure
> > index 34fccaa2ba..5f225894a9 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1773,7 +1773,7 @@ Advanced options (experts only):
> >    --oss-lib                path to OSS library
> >    --cpu=CPU                Build for host CPU [$cpu]
> >    --with-coroutine=BACKEND coroutine backend. Supported options:
> > -                           ucontext, sigaltstack, windows
> > +                           ucontext, libucontext, sigaltstack, windows
>
> This approach mixes the concept of the coroutine backend (ucontext,
> sigaltstack, etc) with the optional libucontext library dependency.
>
> libucontext is not a coroutine backend. The patch had to introduce
> $coroutine_impl in addition to $coroutine in order to work around this.
> Let's avoid combining these two independent concepts into
> --with-coroutine=.
>
> I suggest treating libucontext as an optional library dependency in
> ./configure with explicit --enable-libucontext/--disable-libucontext
> options. Most of the time neither option will be provided by the user
> and ./configure should automatically decide whether libucontext is
> needed or not.
>
> > +case $coroutine in
> > +libucontext)
> > +  git_submodules="${git_submodules} subprojects/libucontext"
> > +  mkdir -p libucontext
>
> Why is this mkdir necessary?

That is a typo, will fix.

Thanks to all the feedback in this thread. I will shelve this patchset
for now and see if it's possible to fix ucontext on Darwin. Or if we
go with gcoroutine that would work as well. Either way it seems like
this isn't ready yet.

-j