[PATCH wayland-web] Added depencies and bug fixes to build instructions

[Pekka Paalanen]

On Mon, 19 May 2014 20:31:28 -0700
Bill Spitzak <spitzak at gmail.com> wrote:

> 
> 
> On 05/19/2014 12:39 AM, Pekka Paalanen wrote:
> > On Fri, 16 May 2014 13:43:50 -0700
> > spitzak at gmail.com wrote:
> >
> >> From: spitzak <spitzak at gmail.com>
> >>
> >> This is based on several experimental runs on two different
> >> Ubuntu 12.04 machines. Latest version includes instructions for
> >> a recent update of Mesa (which requires llvm-3.1) and that you
> >> must compile Cairo to avoid bugs in the system-supplied version.
> >
> > Hi,
> >
> > This is v3 of the same patch, right?
> > Please mention that, so no-one goes reviewing the outdated versions,
> > especially when it's this long.
> 
> Okay, I will try to get that right. Still trying to get git format-patch 
> and send-email to work correctly. The only method I have found to 
> reliably set the subject and text is to edit the patch file after it is 
> produced. You probably saw some very screwy stuff I posted trying to 
> make the --compose switch work, before I realized I better send 
> everything to myself first.
> 
> I would like to produce instructions on how to make and send a patch to 
> the mailing list too, so I hope I am getting it right and not missing 
> something obvious.

Right, here are some details on how I do it.

First the setup, let's say for Weston. It is nice to note the
project/repository in the [] part of the subject, so it is obvious to
which git repo the patch applies. For Weston I have set the git config
key:
	format.subjectprefix=PATCH weston
I think you can set that with:
	$ git config format.subjectprefix "PATCH weston"
That is only a per-repo default, which you can override for both
git-format-patch, and git-send-email if not using git-format-patch, on
the command line with --subject-prefix="PATCH foofoo".

When you do a revised patch series, there is a --reroll-count option,
but it also works if you do it manually with --subject-prefix="PATCH
foofoo v5".

If I have a series that needs a cover letter, I first store the
formatted patches into a directory:
	$ git format-patch --cover-letter -o outdir ...
Go edit the cover letter, check it's all right, and then
	$ git send-email --to=... [--cc=...] outdir
Something like that.

If the series is simple, I might just call git-send-email directly with
all of the above command line options as necessary.

For an RFC series, I set the subject-prefix to have RFC instead of
PATCH. RFC series is not meant to be merged as is, but is sent out for
comments.

> >
> > General comments:
> >
> > - Drop all the re-line-wrapping that does not actually change the
> >    content. It just makes this patch daunting to review.
> 
> Will try to make the patch again without this.
> 
> > - I think we should keep the main building page distribution agnostic.
> >    Otherwise it would be specific to Fedora rawhide or latest release or
> >    something, since Fedora is what many main developers use, IIRC (not
> >    me, though). Certainly choosing an ancient distribution like Ubuntu
> >    12.04 is far too old to be considered in detail on or as the base of
> >    the main building page. The same goes for the xserver page.
> >
> > I suppose you could make a sub-page for Ubuntu specifics, but I think
> > it might be better on some ubuntu-specific wiki where end users can
> > update it easily. We could still link there.
> 
> The problem is that without the "what is the name of the package 
> containing foo" information, it is pretty near impossible for a beginner 
> to compile this. Trust me, having worked on this literally for four 
> years now, it is NOT easy and this information is valuable. I did assume 
> the package names were relevant to other systems.

What is the target audience of the build guide?

I'm not actually too sure, but I would like to assume, that the reader
is familiar with building stuff from git and dealing with configure
complaining about missing dependencies. Installing missing dependencies
is distribution specific. Distributions use different package names,
and sometimes split a project into several packages differently.

What we at upstream need to do, is to make sure that the complaints
from configure are understandable.

The question "what package in distribution Foo provides laalaa.pc?" is
usually best answered by Google, without writing instructions specific
to each distribution. And that becomes a pretty generic guide on "How
to build software", so if there is a site which already explains all
these basics, we can link to it, but I don't think we should write that
guide on the Wayland site.

If the user is not familiar in general on building software manually
from source, then he should be looking for pre-packaged Wayland and
Weston for his particular distribution.

There is one reason to have a more thorough guide though: end users
reporting bugs, who are then asked to test a patch or the latest git
version. But every project out there has this problem, surely someone
has written a relatively generic guide?

The thing is, I would like to keep the build guide short and to the
point. If someone needs more hand-holding with the basics, we should
have links to more thorough guides.

> I think instructions lacking the "install this package" commands are not 
> usable. I suppose I could change it to "install xyz, ddd and foobar" but 
> I'm not sure if that is any better than showing a command that works on 
> one platform at least.

I think a verbal description instead of commands would be better,
because it is plain English. When you write commands, you assume that
the reader knows what those commands do, and realizes he may need to
adjust the package names.

> > I bet the next person fixing these instructions will not be on Ubuntu
> > 12.04, so better drop this, or it gets outdated quick when the person
> > forgets to remove this.
> 
> I am unable to change any machine I am using off of Ubuntu 12.04 as they 
> are being used for commercial software development and that software 
> must run on that system, since this is really what people use. I don't 
> think I am the only software developer in this position.
> 
> There have also been a number of emails sent to this list from people 
> complaining that they could not figure out how to compile it. When they 
> identified the system it seems they were all using either Raspberry pi 
> or Ubuntu 12.04.

I still think we should keep the Wayland web site instructions mostly
generic, at least the main build guide, and have another page or
external site for distribution specfic guides.

It is a question of maintenance, and who gets the blame when the page
goes out of date.

Also the process of editing the Wayland web site is much more involved
than e.g. a wiki has, so it is more likely end users don't bother
contributing.

> >> -export WLD LD_LIBRARY_PATH PKG_CONFIG_PATH ACLOCAL ACLOCAL_PATH
> >> +<pre>
> >> +export WLD=$HOME/install   <font color=#800># change this to another location if you prefer</font>
> >> +export LD_LIBRARY_PATH=$WLD/lib
> >> +export PKG_CONFIG_PATH=$WLD/lib/pkgconfig/:$WLD/share/pkgconfig/
> >> +export ACLOCAL_PATH="$WLD/share/aclocal"
> >> +export ACLOCAL="aclocal -I $ACLOCAL_PATH"
> >>   </pre>
> >
> > This is a good hunk IMO, and the boxing is nice, but I'd like to keep
> > (and add) the $ in front for manually typed commands.
> 
> I removed the $ signs so that multiple lines could be cut & pasted to a 
> terminal. I agree it looks better with them, but this seems much more 
> important. Maybe there is some trick so that both can work but I have 
> not come up with one.

I think you don't want to paste multiple commands in one go. If one
fails, the following will fail even harder, and maybe obscure the first
error.

> >> +mkdir -p $WLD/share/aclocal <font color=#800># avoid a bug in aclocal</font>
> >
> > A good addition, though might also use $ACLOCAL_PATH directly.
> 
> Yes good idea.
> 
> Previously I have compiled without setting any of the ACLOCAL variables 
> at all, and it worked (at least as well as this), so I'm wondering if 
> they really are necessary.

They are likely necessary for anything that still uses
wayland-scanner.m4.

> 
> >> +sudo apt-get install libpciaccess-dev <font color=#800># needed by drm</font>
> >
> > Distribution specifics.
> 
> Yes but this is a good example. The error message said only "pci" I 
> think. I had to do a lot of searching and test-installs before I found 
> that this package (with the word "lib" on the start, and "access-dev" on 
> the end) was the one. That is not something we should require everybody 
> to repeat. I think it is making people give up on wayland long before 
> they get anywhere.

Are you sure it is needed?

Nothing in Weston refers to pciaccess.h or pciaccess.pc.

Maybe it was the Xorg server when it still was used for xwayland, but
nowadays we have Xwayland server that I don't think should be requiring
that.

> >> (all the proto git packages)
> >
> > These should not usually be needed. I think it would be enough to
> > just mention --disable-dri3 in case someone has a problem with these
> > dependencies. The rest are only because you used such an old
> > distribution as a base. I don't think we should go that far in the
> > generic guide.
> 
> Some of those git repositories were updated at the same time xserver 
> was. About 2 months ago it compiled with the packaged ones, and it does 
> not now, and in fact in one day another package was added. Since it 
> changes that fast I doubt any distribution is going to be up to date.

Do you think we can keep this guide up-to-date? Would you commit to
that?

Another idea: keep the main build guide short, but have another page
for "configure failed with .... " sections with hints how to solve it.
That way people only need to read what they actually require.

I'd like to avoid manual building of dependencies for which the
distribution packages would be just fine.

> I did have --disable-dri3 in there for a while, but eventually the 
> xserver required most of the same packages as mesa, so I removed that, 
> wanting to configure mesa exactly like you are doing.

Maybe worth re-checking, if building only the Xwayland servers gets rid
of some of those.

> >> +<p>And finally you can compile Mesa:</p>
> >> +
> >> +<pre>
> >> +git clone git://anongit.freedesktop.org/mesa/mesa
> >> +cd mesa
> >> +./autogen.sh --prefix=$WLD --enable-gles2 --disable-gallium-egl \
> >> + --with-egl-platforms=x11,wayland,drm --enable-gbm --enable-shared-glapi \
> >> + --with-gallium-drivers=r300,r600,swrast,nouveau \
> >> + --disable-llvm-shared-libs <font color=#800># this may be a bug in the llvm package</font>
> >> +make && make install
> >> +cd ..
> >
> > I'm not sure should tell people to use --disable-llvm-shared-libs, the
> > Mesa default must be good enough, or it is a distribution problem with
> > the dependencies. Anyway, this is not really a Mesa build guide but
> > just enough details to configure Mesa properly for Wayland needs in
> > general.
> 
>  From what I read on the web it is a problem with the llvm packages. I 
> feel like this is not fixable without recompiling llvm from source...

In a specfic distribution, right?

> The configure error is pretty clear that adding 
> "--disable-llvm-shared-libs" will work so I suppose removing it is not 
> that bad. Despite what I am trying to do, mesa changes so fast that it 
> is very unlikely the next code update will compile without changing 
> these instructions anyway. Maybe putting it in a comment will help.

Sure.

> I also discovered that it does not complain about a missing llvm if you 
> remove the r300 driver. However I think this also silently removed all 
> shader compilation from all backends, right?

Err, I don't know. I wouldn't think so. Something has to build the
shaders, but I'm not sure what.

> > Distribution specifics. Just drop the Cairo and Pixman parts.
> 
> The versions included with my system have bugs that I thought were bugs 
> in wayland or the wayland xserver. It might be a good idea that people 
> don't see such errors the first time they run it.

Did these bugs appear with cairo-image, or cairo-gl only? If cairo-gl
only, then we can just forget about it. If cairo-image is affected, we
should consider bumping the version requirements in Weston's
configure.ac.

> 
> >> +sudo apt-get install libmtdev-dev libpam0g-dev
> >
> > Distribution specifics.
> 
> Again these are good examples of mystery package names that I had to 
> figure out by trial and error and installing a lot of incorrect 
> packages. Nobody is going to guess that pam has "0g" on the end. This is 
> the important information I was trying to add to the instructions.
> 
> Compiling these from git might be more acceptable?

libpam I would say definitely better to use the distribution version.
OTOH mtdev is not that critical.

But still, this is distribution specific all, and would be better on a
separate ubuntu 12.04 page.

> >> +PATH=$WLD/bin:$PATH ./autogen.sh --prefix=$WLD --disable-setuid-install
> >
> > No need for PATH, we have a proper fix coming.
> 
> Well it sounds like there is some disagreement about the fix. I think I 
> should leave the text in there until I can confirm the version in git 
> has this fixed.

Yes indeed. Now it seem like the PATH is actually the better way, and
it would also work without patching Weston, so I take that back. You
could add PATH into the generic environment setup section.

> >> +<font color=#800># remove --disable-setuid-install if installing system-wide</font>
> >
> > No, system-wide has nothing to do with needing or not needing
> > setuid-install. weston-launch needs to be setuid if it used, at least
> > without proper systemd I believe.
> 
> Okay got that. Does it only set setuid on the weston-launch executable? 
> I have not gotten any indication that this works at all. I only get the 
> error about needing to add myself to the weston-launch group, which I 
> certainly did!

Yes, only weston-launch binary will be setuid-root. This is to ensure,
that it has proper rights to access all input devices and the VT/TTY,
when systemd does not provide those services. There is also
drmSetMaster(), but I am not sure how that works here.

It is also possible that a distribution would be configured to give
access to the locally logged in user anyway, in which case setuid-root
would not be needed.


Thanks,
pq