Re: [PATCH] drm: support up to 128 drm devices

2023-07-25 Thread Emil Velikov
On Mon, 17 Jul 2023 at 14:54, Simon Ser  wrote:
>
> On Monday, July 17th, 2023 at 15:24, Emil Velikov  
> wrote:
>
> > > > For going forward, here is one way we can shave this yak:
> > > >  - update libdrm to max 64 nodes
> > > >  - roll libdrm release, nag distributions to update to it // could be
> > > > folded with the next release below
> > > >  - update libdrm to use name driven type detection
> > > >  - roll libdrm release, nag distributions to update to it
> > > >  - once ^^ release hits most distributions, merge the above proposed
> > > > kernel patch
> > > >- the commit message should explain the caveats and fixed libdrm 
> > > > version
> > > >- we should be prepared to revert the commit, if it causes user
> > > > space regression - fix (see below) and re-introduce the kernel patch
> > > > 1-2 releases later
> > >
> > > That sounds really scary to me. I'd really prefer to try not to break the
> > > kernel uAPI here.
> > >
> >
> > With part in particular? Mind you I'm not particularly happy either,
> > since in essence it's like a controlled explosion.
>
> I believe there are ways to extend the uAPI to support more devices without
> breaking the uAPI. Michał Winiarski's patch for instance tried something to
> this effect.
>
> > > The kernel rule is "do not break user-space".
> >
> > Yes, in a perfect world. In practice, there have been multiple kernel
> > changes breaking user-space. Some got reverted, some remained.
> > AFAICT the above will get us out of the sticky situation we're in with
> > the least amount of explosion.
> >
> > If there is a concrete proposal, please go ahead and sorry if I've
> > missed it. I'm supposed to be off, having fun with family when I saw
> > this whole thing explode.
> >
> > Small note: literally all the users I've seen will stop on a missing
> > node (card or render) aka if the kernel creates card0...63 and then
> > card200... then (hard wavy estimate) 80% of the apps will be broken.
>
> That's fine, because that's not a kernel regression. Supporting more than 64
> devices is a new kernel feature, and if some user-space ignores the new nodes,
> that's not a kernel regression. A regression only happens when a use-case 
> which
> works with an older kernel is broken by a newer kernel.

Won't this approach effectively hard-code/leak even more kernel uABI
into dozens of not hundreds of userspace projects? This does not sound
like a scalable solution IMHO.

I am 100% behind the "don't break userspace rule", alas very few
things in life are as black/white as your comments seem to suggest.
Thus I would suggest doing a bit of both or a compromise if you will.
Namely:
 - try the initial route outlined above
 - if there are (m)any fires, revert the kernel patch and opt for the
work by Michał

This has the benefit of fixing a bunch of the uABI abuses out there,
and leaking more uABI only on as-needed basis.

Side note: KDE folks have their own flatpak runtime and have been
quite open to backport libdrm/other fixes.

HTH
Emil


Re: [PATCH] drm: support up to 128 drm devices

2023-07-17 Thread Emil Velikov
On Mon, 17 Jul 2023 at 10:45, Simon Ser  wrote:
>
> On Monday, July 17th, 2023 at 09:30, Emil Velikov  
> wrote:
>
> > > I'm worried what might happen with old user-space, especially old libdrm.
> >
> > I also share the same concern. Although the bigger issue is not libdrm
> > - since we can update it and prod distributions to update it.
> > The biggest concern is software that cannot be rebuild/updated -
> > closed source and various open-source that has been abandoned.
>
> Well. Now that we have Flatpak and AppImage and friends, we're really likely
> to have old libdrm copies vendored all over the place, and these will stick
> around essentially forever.
>

The flatpak devs have been very helpful. So I'm pretty sure we can get
those updated - even for older flatpaks.
For AppImage, I have no experience.

> > For going forward, here is one way we can shave this yak:
> >  - update libdrm to max 64 nodes
> >  - roll libdrm release, nag distributions to update to it // could be
> > folded with the next release below
> >  - update libdrm to use name driven type detection
> >  - roll libdrm release, nag distributions to update to it
> >  - once ^^ release hits most distributions, merge the above proposed
> > kernel patch
> >- the commit message should explain the caveats and fixed libdrm version
> >- we should be prepared to revert the commit, if it causes user
> > space regression - fix (see below) and re-introduce the kernel patch
> > 1-2 releases later
>
> That sounds really scary to me. I'd really prefer to try not to break the
> kernel uAPI here.
>

With part in particular? Mind you I'm not particularly happy either,
since in essence it's like a controlled explosion.

> The kernel rule is "do not break user-space".

Yes, in a perfect world. In practice, there have been multiple kernel
changes breaking user-space. Some got reverted, some remained.
AFAICT the above will get us out of the sticky situation we're in with
the least amount of explosion.

If there is a concrete proposal, please go ahead and sorry if I've
missed it. I'm supposed to be off, having fun with family when I saw
this whole thing explode.

Small note: literally all the users I've seen will stop on a missing
node (card or render) aka if the kernel creates card0...63 and then
card200... then (hard wavy estimate) 80% of the apps will be broken.

HTH
Emil


Re: [PATCH] drm: support up to 128 drm devices

2023-07-17 Thread Emil Velikov
On Fri, 14 Jul 2023 at 11:32, Simon Ser  wrote:
>
> (cc Daniel Vetter and Pekka because this change has uAPI repercussions)
>
> On Friday, June 30th, 2023 at 13:56, James Zhu  wrote:
>
> > From: Christian König 
> >
> > This makes room for up to 128 DRM devices.
> >
> > Signed-off-by: Christian König 
> > Signed-off-by: James Zhu 
> > ---
> >  drivers/gpu/drm/drm_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 73b845a75d52..0d55c6f5 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -137,7 +137,7 @@ static int drm_minor_alloc(struct drm_device *dev, 
> > unsigned int type)
> >   r = idr_alloc(_minors_idr,
> >   NULL,
> >   64 * type,
> > - 64 * (type + 1),
> > + 64 * (type + 2),
>
> The type comes from this enum:
>
> enum drm_minor_type {
> DRM_MINOR_PRIMARY,
> DRM_MINOR_CONTROL,
> DRM_MINOR_RENDER,
> DRM_MINOR_ACCEL = 32,
> };
>
> Before this patch, 0..63 are for primary, 64..127 are for control (never
> exposed by the kernel), 128..191 are for render, 2048..2112 are for accel.
> After this patch, 0..127 are for primary, 64..191 are for control (never
> exposed by the kernel), 128..255 are for render, 2048..2176 are for accel.
>
> I'm worried what might happen with old user-space, especially old libdrm.

I also share the same concern. Although the bigger issue is not libdrm
- since we can update it and prod distributions to update it.
The biggest concern is software that cannot be rebuild/updated -
closed source and various open-source that has been abandoned.

As mentioned in the gitlab ticket - the current style of embedding the
uABI/uAPI in each and every application is not great IMHO. That is why
I've introduced the `drmGetDevices2` API, to consolidate most of the
chaos in a single place.

For going forward, here is one way we can shave this yak:
 - update libdrm to max 64 nodes
 - roll libdrm release, nag distributions to update to it // could be
folded with the next release below
 - update libdrm to use name driven type detection
 - roll libdrm release, nag distributions to update to it
 - once ^^ release hits most distributions, merge the above proposed
kernel patch
   - the commit message should explain the caveats and fixed libdrm version
   - we should be prepared to revert the commit, if it causes user
space regression - fix (see below) and re-introduce the kernel patch
1-2 releases later

In parallel to all the above, as a community we should collectively
audit and update open-source applications to the `drmDevices2` API.

As with other legacy (DRI1), this one will take some time but we can get there.

HTH
Emil


Re: wayland-protocols scope and governance

2019-05-06 Thread Emil Velikov
Hi Drew,

On Mon, 6 May 2019 at 01:41, Drew DeVault  wrote:
>
> Here's an updated governance document for everyone to consider. Changes
> from the first version:
>
> - Use wayland-devel instead of a dedicated mailing list
> - Use Gitlab for reviewing new protocols
> - Extend discussion period for governance amendments from 30 days to 60
> - Permit either 1 or 2 points of contact for a wayland-protocols member
> - Clarify who's affected by the cool-down period after a failed
>   membership removal vote
>
> I chose not to change the wording of the xdg namespace definition,
> despite Daniel's objection. I couldn't come up with a wording that I
> think would make everyone happy - feedback welcome. Under Daniel's
> proposed wording of "catch-all window management", a case is easily made
> for wlr-foreign-toplevel-management:
>
> https://github.com/swaywm/wlr-protocols/blob/master/unstable/wlr-foreign-toplevel-management-unstable-v1.xml
>
> I expect this would be controversial. Or perhaps it wouldn't be, and
> would fit into this namespace, but would just be NACK'd by some folks.
> Depends on how strongly integrated desktop folks want to gatekeep the
> XDG namespace. Thoughts?
>
>   wayland-protocols governance
>
> This document governs the maintenance of wayland-protocols and serves to 
> outline
> the broader process for standardization of protocol extensions in the Wayland
> ecosystem.
>
>  1. Membership
>
Disclaimer: I did not read through the prior discussion - the volume
was tad much :-(

What exactly is the goal of having membership?

At the end of the day if a project proposes a protocol and is willing
to tweak it based on community feedback - great.
If they don't well - not much one can do about it :-\

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v4 1/5] build/doc: Ensure destination dir exists despite VPATH

2018-08-29 Thread Emil Velikov
Hi Dan,

On 28 August 2018 at 23:19, Daniel Stone  wrote:
> Make considers a variable called VPATH when trying to satisfy
> dependencies, e.g. for a target 'foo', it will consider the target
> extant if VPATH is '../../bar' and '../../bar/foo' exists.
>
> Part of the doc build, the '$(alldirs)' target, exists to create the
> target directories if they do not exist. For example, before generating
> xml/wayland-architecture.png, it will ensure the 'xml' target is
> considered up-to-date thanks to the target dependency.
>
> Creating $(srcdir)/doc/doxygen/xml thus means that the 'xml' dependency
> will be satisfied, so we'll never create the output directory, and the
> doc build will fail.
>
> Change the alldirs target list to be absolute paths, so VPATH will not
> be consulted and defeat the entire point of what we're trying to do.
> This fixes the Meson build, where we later create
> doc/doxygen/xml/meson.build.
>
Have you tried something as trivial as the below sed?
s/$(AM_V_GEN)/$(AM_V_GEN)$(MKDIR_P) $@/g

It will allow you to remove the, dare I say it, bonkers "let's make a
target that only creates a folder".
Plus avoid all the complexity that you've proposing.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 6/6] tests: Remove memory leak checking infrastructure

2018-08-29 Thread Emil Velikov
On 29 August 2018 at 07:17, Daniel Stone  wrote:
> There are far better ways to detect memory leaks, such as either
> valgrind or ASan. Having Meson makes it really easy to use these tools
> in our tests, and we can do that in CI as well.
>
> Having these local wrappers actually completely broke ASan usage, so
> remove them in favour of using the more powerful options.
>
Fwiw adding valgrind to any build-system is a 2-5 line task ;-)
Sanitizers on the other hand (esp. on shared libraries) is a pain.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 5/6] tests: Overly elaborate compiler warning workaround

2018-08-29 Thread Emil Velikov
Hi Dan,

On 29 August 2018 at 07:17, Daniel Stone  wrote:
> Clang will rightly point out that example_sockaddr_un in socket-test
> will get discarded from the compilation unit as it is completely unused.
> Put in a couple of lines which of no value other than stopping Clang
> from complaining.
>
> Signed-off-by: Daniel Stone 
> ---
>  tests/socket-test.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/socket-test.c b/tests/socket-test.c
> index e9705628..8d39edce 100644
> --- a/tests/socket-test.c
> +++ b/tests/socket-test.c
> @@ -42,7 +42,7 @@
>   * See `man 7 unix`.
>   */
>
> -static const struct sockaddr_un example_sockaddr_un;
> +static struct sockaddr_un example_sockaddr_un;
>
>  #define TOO_LONG (1 + sizeof example_sockaddr_un.sun_path)
>
> @@ -69,6 +69,11 @@ TEST(socket_path_overflow_client_connect)
> d = wl_display_connect(path);
> assert(d == NULL);
> assert(errno == ENAMETOOLONG);
> +
> +   /* This is useless, but prevents a warning about example_sockaddr_un
> +* being discarded from the compilation unit. */
> +   strcpy(example_sockaddr_un.sun_path, "happy now clang?");
> +   assert(example_sockaddr_un.sun_path[0] != '\0');

Why don't you add _attrubute__((used)) instead of doing the blame
game? We already use it in the repo.

Side note: the manpage says "sun_path[108]" and also points out that
"some implementations have sun_path as short as 92 bytes"
The check in wl_socket_init_for_display_name uses sizeof, yet prints a
"exceeds 108 bytes" message.

I guess the message should be fixed?

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] cursor: use memfd_create or shm_open for anonymous in-memory files

2018-08-16 Thread Emil Velikov
On 15 August 2018 at 15:14, Simon Ser  wrote:
> On Linux, try using memfd_create and file sealing. Fallback to
> shm_open on old kernels.
>
> On FreeBSD, use shm_open with SHM_ANON.
>
> Otherwise, use shm_open with a random name, making sure the name
> isn't already taken.
>
Thinking out loud:

I wonder if the next POSIX standard will finally standardise
random/anonymous shm files.
Be that shm_open+SHM_ANON (FreeBSD), shm_mk*temp (OpenBSD) or any
other solution.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: How fwrd-compatible is using dlopen and core only?

2018-08-14 Thread Emil Velikov
On 14 August 2018 at 08:57, Pekka Paalanen  wrote:
> On Fri, 10 Aug 2018 18:31:03 +
> ferreiradaselva  wrote:
>
>> Oh,
>>
>> I solved the problem with the extern variable
>> (wl_registry_interface), I could get a pointer to it using
>> `dlsym(handler, "wl_registry_interface")`.
>>
>> The other question remains: can I use the content of the inline
>> functions directly (to avoid the need to include protocol headers)?
>> Is forward-compatibility guaranteed?
>
> Hi,
>
> yes, you can.
>
> The inline functions and everything that is in the installed headers of
> libwayland, generated or not, will always get built into user programs.
> We try hard to not break any already compiled user programs (this is
> the promise of a stable library ABI), which means you should be safe.
>
> The above naturally also applies to headers generated by
> wayland-scanner for extensions from e.g. wayland-protocols. A user
> project may get the XML file from a wayland-protocols dependency, but
> it is only used at build time. Once the user project is built,
> libwayland must guarantee that it will keep working ABI wise.
>
> I belive e.g. libSDL is already doing what you intend, see
> src/video/wayland/.
>
> Another design would be to make your own plugin to your library and
> dlopen that plugin which was linked to libwayland during the build as
> usual. Whether that's better or worse depends on your scope.
>
I would strongly stay away from copying misc inline functions from the
wayland headers.
Instead one can follow the SDL and Waffle (much simpler IMHO)
approach. Here is quick how-to:

- declare mangled symbol pointers, define the normal symbols in terms
of the mangled ones [1]
- have some init function that populates the mangled symbol pointers [2]
- make sure you include the 'wrapper' header before the normal wayland
headers [3]

HTH
Emil

[1] 
https://github.com/waffle-gl/waffle/blob/master/src/waffle/wayland/wayland_wrapper.h
[2] 
https://github.com/waffle-gl/waffle/blob/master/src/waffle/wayland/wayland_wrapper.c
[3] 
https://github.com/waffle-gl/waffle/blob/master/src/waffle/wayland/wayland_window.c#L31
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 1/2] build: Remove execinfo.h check

2018-07-26 Thread Emil Velikov
On 25 July 2018 at 18:36, Derek Foreman  wrote:
> On 2018-07-21 06:13 AM, Daniel Stone wrote:
>> The check for the execinfo.h header is only advisory; the build will not
>> fail if it is not present, and set HAVE_EXECINFO_H if it is. The check
>> was added in commit 5cfdbef3d299 ("build: Allow disabling building of
>> wayland libraries") with no obvious use or reasoning.
>
> 5cfdbe3d299 just moved it, it was added in commit bc3e020475
>
> It appears to have been accidentally included there, as it doesn't look
> relevant to the bug ticket referenced in the commit.
>
>> Remove the no-op check.
>>
>> Signed-off-by: Daniel Stone 
>
> Reviewed-by: Derek Foreman 
>
> Though, I wouldn't mind seeing a more accurate commit log. ;)
>
It's always better to not get called out for other's copy/paste
mistakes. If you guys like Fixes tags, here is one

Fixes: bc3e020475e ("build: Add declaration checks to check for
required syscall flags")

Fwiw, series is
Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] simple-dmabuf-drm: use GBM generic calls

2018-07-24 Thread Emil Velikov
Hi everyone,

On 11 July 2018 at 09:33, Daniel Stone  wrote:
> Hi,
> On Wed, 11 Jul 2018 at 09:16, Pekka Paalanen  wrote:
>> On Tue, 10 Jul 2018 17:53:15 +0200 Emilio Pozuelo Monfort 
>>  wrote:
>> > No need to write libdrm driver specific code for each supported
>> > driver, we can just let GBM call the right one for us now.
>> >
>> > Signed-off-by: Emilio Pozuelo Monfort 
>> > ---
>> >
>> > Hi,
>> >
>> > This simplifies the code a lot, using gbm_bo as Emil suggested. Some 
>> > problems
>> > I still see:
>> >
>> > - NV12 doesn't work, it seems that 
>> > backends/dri/gbm_dri.c:gbm_format_to_dri_format()
>> >   doesn't support it.
>>
>> I think NV12 and other less common formats, should someone add them in
>> this program, should not be lost. That may be part of the reason GBM
>> wasn't used: the need to test YUV formats, and non-GPU-renderable
>> formats in general.
>
> Honestly, most of the reason was because I wrote the original client
> in about 15 minutes on a very short bus trip. You can probably see
> that in comments like 'XXX: would be nice to draw something that
> changes here'. So I don't think you can really read too much into the
> original code.
>
> Other great reasons include:
>   - we wanted to explicitly get modifiers allocated, and this was
> before the GBM modifiers interface existed
>   - at the time, I was doing bring-up of a GBM implementation which
> wasn't usable by generic clients (privileged allocation only)
>   - YUV format support
>
> I'm pretty ambivalent about it though. V4L2 and Vivid might well cover
> the YUV case well enough, and even if not, GBM should still be able to
> allocate R/RG buffers.
>
Darn, I did not spot NV12 in weston. Mesa is perfectly capable of
using YUV formats, so it should be a matter of adding the odd GBM
bits.

That said, it might be more involving than what you have time for, so
feel free to put this patch on hold.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v16 15/23] compositor-drm: Support modifiers with GBM

2018-07-06 Thread Emil Velikov
Hi Dan,

On 5 July 2018 at 18:16, Daniel Stone  wrote:
> Use the extended GBM allocator interface to support modifiers and
> multi-planar BOs.
>
> Signed-off-by: Daniel Stone 
> Tested-by: Emre Ucan 
> ---
>  configure.ac   |  3 ++
>  libweston/compositor-drm.c | 61 +++---
>  2 files changed, 53 insertions(+), 11 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index c550198ae..357b6471e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -215,6 +215,9 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
> [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports 
> modifier advertisement])],
> [AC_MSG_WARN([libdrm does not support modifier 
> advertisement])])
> +  PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM_MODIFIERS, [gbm >= 17.1],
> +   [AC_DEFINE([HAVE_GBM_MODIFIERS], 1, [GBM supports 
> modifiers])],
> +   [AC_MSG_WARN([GBM does not support modifiers])])

Instead of such lovely checks and multiple #ifdef blocks through in
the code, one can use weak symbols.
See kmscube code has some examples.

That said, it's something that could be handled at a later stage.
There's no point in delaying the series over that detail.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH libinput 1/5] tools: if the execdir is the builddir, add it to the path

2018-07-04 Thread Emil Velikov
Hi Peter,

On 28 June 2018 at 00:51, Peter Hutterer  wrote:
> When running libinput tools from the builddir, look up the subtools in the
> builddir as well. Otherwise, add the install prefix to the list of lookup
> locations.
>
> This ensures that a) we're running builddir stuff against builddir stuff, but
> also b) that we're not running builddir stuff against installed stuff because
> that may give us false positives.
>
> Signed-off-by: Peter Hutterer 
> ---
> Similar-ish to the git approach Emil suggested in [1] but git is a lot more
> involved here. Since we only need this for debugging tools, we can pick the
> simpler approach here and simply ignore most of the corner cases.
>
I think that git does thing in that particular way is to ensure a
consistent lookup approach.
Personally tools_execdir_is_builddir reads like it should return a
bool, sadly no better name comes to mind.

More of an jfyi, than anything else.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl

2018-07-03 Thread Emil Velikov
Hi Emilio,

On 2 July 2018 at 16:22, Emilio Pozuelo Monfort  wrote:
> Signed-off-by: Emilio Pozuelo Monfort 
> ---
> I tried a build with --disable-egl as I didn't have the headers
> installed, and it broke here. The EGL usage here seemed optional so I
> did that, but I didn't run-test the result. If it would make more sense
> to disable the client if EGL support is disabled I can do that.
>
>  clients/simple-dmabuf-drm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
Fairly orthogonal question, aimed at wayland devs:

Why does this "simple" app has per-device code instead of using
gbm_bo_map/unmap?
API has been around for 2 years and every half recent Mesa driver has
support for it. Only the really old ones do not radeon (r100), r200,
nouveau_vieux and i915.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 4/8] fixup! scanner: use c99 initializers for the request/events arrays

2018-07-03 Thread Emil Velikov
On 3 July 2018 at 12:28, Pekka Paalanen  wrote:
> On Thu, 14 Jun 2018 16:49:41 +0100
> Emil Velikov  wrote:
>
>> ---
>>  tests/data/example-code.c   | 238 
>> 
>>  tests/data/small-code-core.c|   8 +-
>>  tests/data/small-code.c |   8 +-
>>  tests/data/small-private-code.c |   8 +-
>>  4 files changed, 131 insertions(+), 131 deletions(-)
>
>> diff --git a/tests/data/small-private-code.c 
>> b/tests/data/small-private-code.c
>> index 35fa653..84f77de 100644
>> --- a/tests/data/small-private-code.c
>> +++ b/tests/data/small-private-code.c
>> @@ -54,13 +54,13 @@ static const struct wl_interface *types[] = {
>>  };
>>
>>  static const struct wl_message intf_A_requests[] = {
>> - { "rq1", "sun", types + 0 },
>> - { "rq2", "nsiufho", types + 1 },
>> - { "destroy", "", types + 0 },
>> + { .name = "rq1", .signature = "sun", .types = [0] },
>> + { .name = "rq2", .signature = "nsiufho", .types = [1] },
>> + { .name = "destroy", .signature = "", .types = [0] },
>>  };
>
> Hi,
>
> this change doesn't look more readable to me than the old version.
> Adding the field names only clutters things and makes the lines longer
> for no added information: it is always the exact same fields set in the
> same order, and there are only three of them.
>
> The change from 'types + i' to '[i]' seems ok though.
>
Seems like these dead trivial patches are inciting some bike-shedding.
Proceed as you wish, sadly I'm out of patience.

Peace
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 8/8] fixup! scanner: initialize .{method, event}_count via ARRAY_SIZE

2018-07-03 Thread Emil Velikov
On 3 July 2018 at 12:45, Pekka Paalanen  wrote:
> On Thu, 14 Jun 2018 16:49:45 +0100
> Emil Velikov  wrote:
>
>> ---
>>  tests/data/example-code.c   | 73 
>> +
>>  tests/data/small-code-core.c|  5 +--
>>  tests/data/small-code.c |  5 +--
>>  tests/data/small-private-code.c |  5 +--
>>  4 files changed, 46 insertions(+), 42 deletions(-)
>>
>> diff --git a/tests/data/example-code.c b/tests/data/example-code.c
>> index 2e1f73b..65d9651 100644
>> --- a/tests/data/example-code.c
>> +++ b/tests/data/example-code.c
>> @@ -146,6 +146,7 @@ static const struct wl_interface *types[] = {
>>   [94] = _surface_interface,
>>  };
>>
>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>  static const struct wl_message wl_display_requests[] = {
>>   { .name = "sync", .signature = "n", .types = [8] },
>>   { .name = "get_registry", .signature = "n", .types = [9] },
>> @@ -158,8 +159,8 @@ static const struct wl_message wl_display_events[] = {
>>
>>  WL_EXPORT const struct wl_interface wl_display_interface = {
>>   .name = "wl_display", .version = 1,
>> - .method_count = 2, .methods = wl_display_requests,
>> - .event_count = 2, .events = wl_display_events,
>> + .method_count = ARRAY_SIZE(wl_display_requests), .methods = 
>> wl_display_requests,
>> + .event_count = ARRAY_SIZE(wl_display_events), .events = 
>> wl_display_events,
>>  };
>
> Hi,
>
> this change is not an obvious improvement to me. "method_count = 2" is
> pretty clear, that combined with "methods = foo" does not seem to
> leave anything to be desired.
>
> If this code was hand-written, then I would be cheering for ARRAY_SIZE
> for sure, but it's not. It all comes from a generator that gets the
> count right.
>
> I suppose we'd need an opinion from someone who is less familiar with
> Wayland C bindings.
>
Sure ARRAY_SIZE is used for getting thins "right" although it also
improves _readability_.

Pre-patch (series) it took me an embarrassing amount of time trying to
understand any of the magic created by the scanner.
Perhaps I'm just silly in trying to spare the next reader some time...

It's trivial change and basic coding practise.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] configure.ac: bump libdrm requirement to 2.4.68

2018-07-02 Thread Emil Velikov
On 2 July 2018 at 10:41, Emil Velikov  wrote:
> On 2 July 2018 at 03:07, Peter Hutterer  wrote:
>> On Mon, Jun 11, 2018 at 11:02:19AM +0100, Daniel Stone wrote:
>>> Hi,
>>>
>>> On 11 June 2018 at 10:25, Pekka Paalanen  wrote:
>>> > On Mon, 11 Jun 2018 09:29:49 +1000
>>> > Peter Hutterer  wrote:
>>> >> +PKG_CHECK_MODULES(LIBDRM, [libdrm >= 2.4.68], [], [AC_MSG_ERROR([
>>> >>   libdrm is a hard build-time dependency for libweston core,
>>> >>   but a sufficient version was not found. However, libdrm
>>> >>   is not a runtime dependency unless you have features
>>> >
>>> > this patch is correct, but I would like to hear more opinions if we
>>> > want to bump the hard build-time dependency from release of Jan 2012 to
>>> > Apr 2016.
>>> >
>>> > If we do this bump, we could remove the fallback definitions for
>>> > DRM_FORMAT_R8 and DRM_FORMAT_GR88.
>>> >
>>> > In case we want to consider bumping even further, DRM_FORMAT_MOD_* were
>>> > introduced in 2.4.83 (Aug 2017), and atomic in 2.4.78 (Apr 2017).
>>>
>>> I'd be in favour of bumping all the way to .83. It's available in
>>> Debian testing and unstable, as well as for stable (stretch) via the
>>> backports.debian.org repository. It's available for Ubuntu 16.04 LTS
>>> via the xenial-updates repository, and releases since. It's available
>>> in Yocto Rocko (Oct 2017), but unsurprisingly not in Pyro (May 2017);
>>> I believe there are a few BSPs based on Pyro and earlier still, but
>>> they would be pretty trivial bumps to include if vendors want to
>>> upgrade Weston.
>>>
>>> Either patch is:
>>> Acked-by: Daniel Stone 
>>
>> pekka: gentle ping, if you're happy with this one
>>
> On Debian front:
> - oldstable: 2.4.74 in backports
> - stable: 2.4.74 in main, 2.4.91 in backports
>
> Ubuntu:
>  - 16.04: 2.4.83 in xenial-updates
>  - 14.04: 2.4.67 in trusty-updates
>
> In other words, Trusty is def. out of the question: I'd go with this
> patch for now to unblock.
> Might want to bump to .83 closer to the release/branch point.
>
Should have added: As-is patch is
Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] configure.ac: bump libdrm requirement to 2.4.68

2018-07-02 Thread Emil Velikov
On 2 July 2018 at 03:07, Peter Hutterer  wrote:
> On Mon, Jun 11, 2018 at 11:02:19AM +0100, Daniel Stone wrote:
>> Hi,
>>
>> On 11 June 2018 at 10:25, Pekka Paalanen  wrote:
>> > On Mon, 11 Jun 2018 09:29:49 +1000
>> > Peter Hutterer  wrote:
>> >> +PKG_CHECK_MODULES(LIBDRM, [libdrm >= 2.4.68], [], [AC_MSG_ERROR([
>> >>   libdrm is a hard build-time dependency for libweston core,
>> >>   but a sufficient version was not found. However, libdrm
>> >>   is not a runtime dependency unless you have features
>> >
>> > this patch is correct, but I would like to hear more opinions if we
>> > want to bump the hard build-time dependency from release of Jan 2012 to
>> > Apr 2016.
>> >
>> > If we do this bump, we could remove the fallback definitions for
>> > DRM_FORMAT_R8 and DRM_FORMAT_GR88.
>> >
>> > In case we want to consider bumping even further, DRM_FORMAT_MOD_* were
>> > introduced in 2.4.83 (Aug 2017), and atomic in 2.4.78 (Apr 2017).
>>
>> I'd be in favour of bumping all the way to .83. It's available in
>> Debian testing and unstable, as well as for stable (stretch) via the
>> backports.debian.org repository. It's available for Ubuntu 16.04 LTS
>> via the xenial-updates repository, and releases since. It's available
>> in Yocto Rocko (Oct 2017), but unsurprisingly not in Pyro (May 2017);
>> I believe there are a few BSPs based on Pyro and earlier still, but
>> they would be pretty trivial bumps to include if vendors want to
>> upgrade Weston.
>>
>> Either patch is:
>> Acked-by: Daniel Stone 
>
> pekka: gentle ping, if you're happy with this one
>
On Debian front:
- oldstable: 2.4.74 in backports
- stable: 2.4.74 in main, 2.4.91 in backports

Ubuntu:
 - 16.04: 2.4.83 in xenial-updates
 - 14.04: 2.4.67 in trusty-updates

In other words, Trusty is def. out of the question: I'd go with this
patch for now to unblock.
Might want to bump to .83 closer to the release/branch point.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] build: don't manually parse the weston.ini.in templates

2018-07-02 Thread Emil Velikov
Hi All,

Tl;Dr: Silly brain of mine auto-expanded $(foo). Patch might work, but
as Quentin mentioned it won't majority of the time.
Please consider this binned.

On 29 June 2018 at 08:37, Pekka Paalanen  wrote:
> On Thu, 28 Jun 2018 18:59:01 +0100
> Emil Velikov  wrote:
>
>> On 28 June 2018 at 10:58, Quentin Glidic
>>  wrote:
>> > On 6/27/18 3:04 PM, Emil Velikov wrote:
>> >>
>> >> From: Emil Velikov 
>> >>
>> >> Adding those to configure.ac ensures that:
>> >>   - the weston.ini files are {re,}generated only when needed
>> >>   - the .in files are shipped in the tarball
>> >>   - all the manual handling of the above can be removed ;-)
>> >
>> >
>> > Did you actually test that?
>> > In configure.ac, "bindir" is "${prefix}/bin" (default value), so you’d end
>> > up with "path=${prefix}/bin/weston-flower", and obviously, it won’t launch.
Weirdly enough, I do actually set the folders passed to configure when
building a local distro package.

Turns out that brain of mine auto-expanded $[prefix}, going foobar
with your comment.
The variable is stored literally, thus patch will work only in corner cases.

Thank you for this.

>> > Also, though it’s not that used, you can override directories at "make"
>> > time.
>> > In the end, Makefile is the only place we know the full usable value for
>> > directories. (Otherwise, the only case it’ll work is when you pass all the
>> > directories as full paths to "./configure".)
>> >
>> Hmm I think a good point is to step back a bit and say how the
>> weston.ini files should be used.
>> Are they meant for builddir only usage (a), are they sort of a
>> template that one should copy (b) or other (c).
>
> I believe the weston.ini files in root and ivi-shell/ directories are
> examples, which could be used as templates for a custom config
> primarily. It would be good if they are correct and usable as is, that
> is, they use the directories used by 'make install' when DESTDIR is
> *not* given. That way a user can copy the file, have a test run that
> works, and then tweak further.
>
> Weston will pick weston.ini from CWD only if it finds it nowhere else,
> see weston.ini.man.
>
Right, I will send a patch later adding a small comment at the top of the files.

 "This file is a drop-in template. See `man weston.ini' for more"

> If weston cannot find any weston.ini, it will still run with built-in
> defaults, which include one launcher icon that should start
> weston-terminal. So there is a precedent of a built-in path already
> that cannot change at install time. (Whether that should rely on PATH
> instead is another question - maybe it should?)
>
Relying on PATH seems reasonable. But it's not my call at the end of the day.

> IMO it would be fine to just remove weston-flower and any client that
> is normally not installed from the example ini files. weston-terminal
> is the most important launcher.
>
AFAICT weston-flower is the only instance that needs a
builddir->install dir fix.
It's one of the base demos.

>> The patch from Emre suggest (b). My current assumption is on the same
>> page, based on the bindir/weston-foo (and friends) instances in
>> weston.ini.in.
>>
>> I wonder if "make allows you to override everything" is not it's bane.
>> Just because you can, don't mean one should.
>> All in all people who thinker with that should really know what they're 
>> doing.
>
> If Quentin was referring to DESTDIR only, then there should be no
> problem. DESTDIR is used for installing into a staging tree which
> cannot be executed from. One is expected to copy that into the proper
> $prefix before running is possible.
>
Agreed, DESTDIR in itself should be (and is) fine.
Some illustrations on the "disaster" mentioned earlier.

git clean
./configure
make // implicit all - weston.ini is generated
make bindir=foo install // weston.ini is not regenerated, bindir is ignored

git clean
./configure
make bindir=foo
make install // the previous bindir is used, even when you did not ask for it

git clean
./configure
make bindir=foo install // implicit all, bindir is used

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] build: don't manually parse the weston.ini.in templates

2018-06-28 Thread Emil Velikov
On 28 June 2018 at 10:58, Quentin Glidic
 wrote:
> On 6/27/18 3:04 PM, Emil Velikov wrote:
>>
>> From: Emil Velikov 
>>
>> Adding those to configure.ac ensures that:
>>   - the weston.ini files are {re,}generated only when needed
>>   - the .in files are shipped in the tarball
>>   - all the manual handling of the above can be removed ;-)
>
>
> Did you actually test that?
> In configure.ac, "bindir" is "${prefix}/bin" (default value), so you’d end
> up with "path=${prefix}/bin/weston-flower", and obviously, it won’t launch.
> Also, though it’s not that used, you can override directories at "make"
> time.
> In the end, Makefile is the only place we know the full usable value for
> directories. (Otherwise, the only case it’ll work is when you pass all the
> directories as full paths to "./configure".)
>
Hmm I think a good point is to step back a bit and say how the
weston.ini files should be used.
Are they meant for builddir only usage (a), are they sort of a
template that one should copy (b) or other (c).

The patch from Emre suggest (b). My current assumption is on the same
page, based on the bindir/weston-foo (and friends) instances in
weston.ini.in.

I wonder if "make allows you to override everything" is not it's bane.
Just because you can, don't mean one should.
All in all people who thinker with that should really know what they're doing.

Having the weston.ini files generated at "make all" means that those
variables are honoured only at "make all".
Aka relying that you can override them is a recipe for disaster.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] contributing: review rules for bugs

2018-06-28 Thread Emil Velikov
On 28 June 2018 at 10:12, Pekka Paalanen  wrote:
> On Wed, 27 Jun 2018 17:49:34 +0100
> Emil Velikov  wrote:
>
>> Hi Pekka,
>>
>> A couple small ideas come to mind:
>>
>> On 27 June 2018 at 14:47, Pekka Paalanen  wrote:
>> > From: Pekka Paalanen 
>> >
>> > Half of the ideas came from Daniel but most of them are reworded, the
>> > rest are my thoughts.
>> >
>> > Mention compiler warnings specifically, and be more explicit on what
>> > kind of code or bugs or bug fixes are acceptable or not. Clarify commit
>> > scope.
>> >
>> > Cc: Daniel Stone 
>> > Signed-off-by: Pekka Paalanen 
>> > ---
>> >  CONTRIBUTING.md | 19 ---
>> >  1 file changed, 16 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
>> > index 70d0eca..51bef89 100644
>> > --- a/CONTRIBUTING.md
>> > +++ b/CONTRIBUTING.md
>> > @@ -223,11 +223,24 @@ include tests excercising the additions in the test 
>> > suite.
>> >  - The code fits the existing software architecture, e.g. no layering
>> >  violations.
>> >
>> > -- The code is correct and does not ignore corner-cases.
>> > +- The code is correct and does not introduce new failures for existing 
>> > users,
>> > +does not add new corner-case bugs, and does not introduce new compiler
>> > +warnings.
>> Compiler warnings vary greatly on the compiler and it's version. It
>> does become a nuisance when committer/reviewer is using a newer
>> version which flags dozens of warnings.
>
> Yes. There is no need to get out of one's way to hunt for possible
> warnings, but if someone does notice new warnings, they should be
> fixed. If a reviewer gets them but the author/submitter doesn't, the
> reviewer can help with them. It's common sense to me.
>
> I've been wondering if the CI build should actually turn -Werror on,
> but that's better to discuss after if/when we migrate to MR-based work
> flow.
>
>> That aside, worth loosening this for patches where existing code is
>> copied or moved?
>
> It is already loosened: they are not new warnings if they were there
> before. If code motion results in new warnings, they should ideally be
> fixed. If the fix makes the code motion harder to review, it's ok to
> fix it in another patch before or after.
>
> I think we also need to be picky on how much details we are going to
> write down here. It is very easy to extend the guidelines with details
> so far that it becomes too long to read. That is why I try to be as
> terse as possible.
>
> It's the spirit of the guidelines that matters, they are not to be
> interpreted by the letter, and there may always be special
> circumstances where the reasonable action is to overlook something.
> Even with the guidelines, we still rely very much on the individual
> judgement. The important thing is to get people to the right mindset in
> general.
>
> Perhaps the above would be worth a paragraph in CONTRUBUTING.md?
>
It was meant as food for thought, inspired by some long threads I've
seen where devs discuss various compiler warnings.
You're spot on being terse and providing a feel (or spirit as you call
it) is the key point.

Apologies for the distraction this has caused :-\

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] contributing: review rules for bugs

2018-06-27 Thread Emil Velikov
Hi Pekka,

A couple small ideas come to mind:

On 27 June 2018 at 14:47, Pekka Paalanen  wrote:
> From: Pekka Paalanen 
>
> Half of the ideas came from Daniel but most of them are reworded, the
> rest are my thoughts.
>
> Mention compiler warnings specifically, and be more explicit on what
> kind of code or bugs or bug fixes are acceptable or not. Clarify commit
> scope.
>
> Cc: Daniel Stone 
> Signed-off-by: Pekka Paalanen 
> ---
>  CONTRIBUTING.md | 19 ---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 70d0eca..51bef89 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -223,11 +223,24 @@ include tests excercising the additions in the test 
> suite.
>  - The code fits the existing software architecture, e.g. no layering
>  violations.
>
> -- The code is correct and does not ignore corner-cases.
> +- The code is correct and does not introduce new failures for existing users,
> +does not add new corner-case bugs, and does not introduce new compiler
> +warnings.
Compiler warnings vary greatly on the compiler and it's version. It
does become a nuisance when committer/reviewer is using a newer
version which flags dozens of warnings.
That aside, worth loosening this for patches where existing code is
copied or moved?

>
> -- In a patch series, every intermediate step produces correct code as well.
> +- In a patch series, every intermediate step produces correct and 
> warning-free
> +code as well.
>
It makes sense to move this as the final bullet-point and let it refer
to the whole section.
Namely:

- In a patch series, every intermediate step adheres to the guidelines above.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston] build: don't manually parse the weston.ini.in templates

2018-06-27 Thread Emil Velikov
From: Emil Velikov 

Adding those to configure.ac ensures that:
 - the weston.ini files are {re,}generated only when needed
 - the .in files are shipped in the tarball
 - all the manual handling of the above can be removed ;-)

Note: the abs_top_builddir for weston-flower was swapped with the correct
bindir. Squashed in here since it was never worked correctly :-\

Signed-off-by: Emil Velikov 
---
Shout if you feel strongly about splitting the weston-flower fix.

Based on top of Emre's "ivi-shell: use install paths in example config"
patch.
---
 Makefile.am   | 22 ++
 configure.ac  |  2 +-
 weston.ini.in |  2 +-
 3 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 637dd239..2095aa5a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -12,23 +12,7 @@ BUILT_SOURCES =
 
 AM_DISTCHECK_CONFIGURE_FLAGS = --disable-setuid-install
 
-EXTRA_DIST = weston.ini.in ivi-shell/weston.ini.in
-
-weston.ini : $(srcdir)/weston.ini.in
-   $(AM_V_GEN)$(SED) \
-   -e 's|@bindir[@]|$(bindir)|g' \
-   -e 's|@abs_top_builddir[@]|$(abs_top_builddir)|g' \
-   -e 's|@libexecdir[@]|$(libexecdir)|g' \
-   $< > $@
-
-ivi-shell/weston.ini : $(srcdir)/ivi-shell/weston.ini.in
-   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(SED) \
-   -e 's|@bindir[@]|$(bindir)|g' \
-   -e 's|@libexecdir[@]|$(libexecdir)|g' \
-   -e 's|@westondatadir[@]|$(westondatadir)|g' \
-   $< > $@
-
-all-local : weston.ini ivi-shell/weston.ini
+EXTRA_DIST =
 
 AM_CFLAGS = $(GCC_CFLAGS)
 
@@ -43,9 +27,7 @@ AM_CPPFLAGS = \
-DLIBEXECDIR='"$(libexecdir)"'  \
-DBINDIR='"$(bindir)"'
 
-CLEANFILES = weston.ini\
-   ivi-shell/weston.ini\
-   internal-screenshot-00.png  \
+CLEANFILES = internal-screenshot-00.png\
$(BUILT_SOURCES)
 
 # Libtool race fix
diff --git a/configure.ac b/configure.ac
index 2a62b62a..9b7071e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -680,7 +680,7 @@ if test "x$enable_systemd_notify" = "xyes"; then
   PKG_CHECK_MODULES(SYSTEMD_DAEMON, [libsystemd])
 fi
 
-AC_CONFIG_FILES([Makefile libweston/version.h compositor/weston.pc])
+AC_CONFIG_FILES([Makefile weston.ini ivi-shell/weston.ini libweston/version.h 
compositor/weston.pc])
 
 # AC_CONFIG_FILES needs the full name when running autoconf, so we need to use
 # libweston_abi_version here, and outside [] because of m4 quoting rules
diff --git a/weston.ini.in b/weston.ini.in
index 257c4ec4..e743cc49 100644
--- a/weston.ini.in
+++ b/weston.ini.in
@@ -38,7 +38,7 @@ path=/usr/bin/google-chrome
 
 [launcher]
 icon=/usr/share/icons/gnome/24x24/apps/arts.png
-path=@abs_top_builddir@/weston-flower
+path=@bindir@/weston-flower
 
 [input-method]
 path=@libexecdir@/weston-keyboard
-- 
2.18.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] ivi-shell: use install paths in example config

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 12:12, Emil Velikov  wrote:
> On 27 June 2018 at 12:01, Michael Tretter  wrote:
>> Hi,
>>
>> On Fri, 25 May 2018 08:46:16 +0200, Michael Tretter wrote:
>>> On Thu, 24 May 2018 17:08:47 +0200, Emre Ucan wrote:
>>> > The example weston.ini file uses source and build
>>> > directory paths. Therefore, it is only useful when
>>> > used on the same system that is used to build Weston.
>>> >
>>> > We can use install paths instead of build/source paths
>>> > to fix this problem.
>>> >
>>> > v2 changes:
>>> > - use $(westondatadir) instead of $(datadir)
>>> >
>>> > Reported-by: Michael Tretter 
>>> > Signed-off-by: Emre Ucan 
>>>
>>> Reviewed-by: Michael Tretter 
>>
>> Some variables that this patch touches have already been removed, but I
>> think it would still make change to rename 'abs_top_builddir' and
>> 'abs_top_srcdir'. Is there anything preventing this patch from being
>> applied?
>>
> Is this still required with the recently landed builddir changes by Dan/Pekka?
> I'll give it a look after lunch and can offer some feedback, after
> lunch. Gut feeling atm is that this breaks make check.
>
My gut feeling and coffee deprived brain were miles off.
Patch is spot on and is
Reviewed-by: Emil Velikov 

An extra cool cleanup incoming ;-)
-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] parse_modeline: Ignore case of {h,v}sync flags

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 12:42, Guido Günther  wrote:
> Hi,
> On Wed, Jun 27, 2018 at 11:30:40AM +0100, Emil Velikov wrote:
>> On 26 June 2018 at 19:40, Guido Günther  wrote:
>> > Some modeline generators put out e.g. +HSync instead of +hsync. Accept
>> > that too since it's not ambigous.
>> >
>> Hmm which generator is that? The cvt one, given as an example seems to
>> produce lowercase ones.
>
> The xorg.conf manpage uses +HSync and a calculator generating just that is
>
>   https://arachnoid.com/modelines/
>
You are right, xorg.conf does list the camelcase instances. At the
same time nearly anything in that file is case insensitive.

> and I don't see why doing this case insensitive would hurt anyone and
> make things less error prone for users.
Yes it is trivial and more importantly it's not my call to make [as I
mentioned earlier].

Having "modeline" and "less error prone" in the same context is an
interesting oxymoron.
Instead of having a per display server modelines, a more consistent
and robust solution is to utilise the kernel command line [1] or via a
custom edid blob [2].

HTH
Emil

[1] video=... see the format
https://gitlab.freedesktop.org/lima/linux/blob/a01c47737a9ca118ab75c6fd6e75739b824de830/drivers/gpu/drm/drm_modes.c#L1350

[2] drm_kms_helper.edid_firmware=...
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] ivi-shell: use install paths in example config

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 12:01, Michael Tretter  wrote:
> Hi,
>
> On Fri, 25 May 2018 08:46:16 +0200, Michael Tretter wrote:
>> On Thu, 24 May 2018 17:08:47 +0200, Emre Ucan wrote:
>> > The example weston.ini file uses source and build
>> > directory paths. Therefore, it is only useful when
>> > used on the same system that is used to build Weston.
>> >
>> > We can use install paths instead of build/source paths
>> > to fix this problem.
>> >
>> > v2 changes:
>> > - use $(westondatadir) instead of $(datadir)
>> >
>> > Reported-by: Michael Tretter 
>> > Signed-off-by: Emre Ucan 
>>
>> Reviewed-by: Michael Tretter 
>
> Some variables that this patch touches have already been removed, but I
> think it would still make change to rename 'abs_top_builddir' and
> 'abs_top_srcdir'. Is there anything preventing this patch from being
> applied?
>
Is this still required with the recently landed builddir changes by Dan/Pekka?
I'll give it a look after lunch and can offer some feedback, after
lunch. Gut feeling atm is that this breaks make check.

Do check the in-flight "commit access" patches from Pekka.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v7 5/6] compositor-fbdev: detect the first fb device in the seat

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 00:39, nerdopolis  wrote:
> This adds a function to detect the first framebuffer device in the
> current seat. Instead of hardcoding /dev/fb0, detect the device
> with udev, favoring the boot_vga device, and falling back to the
> first framebuffer device in the seat if there is none. This is very
> similar to what compositor-drm does to find display devices
> ---
>  libweston/compositor-fbdev.c | 83 ++--
>  1 file changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index e3777495..616300dc 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -777,6 +777,77 @@ session_notify(struct wl_listener *listener, void *data)
> }
>  }
>
> +static char *
> +find_framebuffer_device(struct fbdev_backend *b, const char *seat)
> +{
> +   struct udev_enumerate *e;
> +   struct udev_list_entry *entry;
> +   const char *path, *device_seat, *id, *fb_device_path;
> +   struct udev_device *device, *fb_device, *pci;
> +
> +   e = udev_enumerate_new(b->udev);
> +   udev_enumerate_add_match_subsystem(e, "graphics");
> +   udev_enumerate_add_match_sysname(e, "fb[0-9]*");
> +
> +   udev_enumerate_scan_devices(e);
> +   fb_device = NULL;
> +   udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e)) {
> +   bool is_boot_vga = false;
> +
> +   path = udev_list_entry_get_name(entry);
> +   device = udev_device_new_from_syspath(b->udev, path);
> +   if (!device)
> +   continue;
> +   device_seat = udev_device_get_property_value(device, 
> "ID_SEAT");
> +   if (!device_seat)
> +   device_seat = default_seat;
> +   if (strcmp(device_seat, seat)) {
> +   udev_device_unref(device);
> +   continue;
> +   }
> +
> +   pci = udev_device_get_parent_with_subsystem_devtype(device,
> +   "pci", NULL);
> +   if (pci) {
> +   id = udev_device_get_sysattr_value(pci, "boot_vga");
> +   if (id && !strcmp(id, "1"))
> +   is_boot_vga = true;
> +   }
> +
> +   /* If a framebuffer device was found, and this device isn't
> +* the boot-VGA device, don't use it. */
> +   if (!is_boot_vga && fb_device) {
> +   udev_device_unref(device);
> +   continue;
> +   }
> +
> +   /* There can only be one boot_vga device. Try to use it
> +* at all costs. */
> +   if (is_boot_vga) {
> +   if (fb_device)
> +   udev_device_unref(fb_device);
> +   fb_device = device;
> +   break;
> +   }
> +
> +   /* Per the (!is_boot_vga && fb_device) test above, only
> +* trump existing saved devices with boot-VGA devices, so if
> +* the test ends up here, this must be the first device seen. 
> */
> +   assert(!fb_device);
> +   fb_device = device;
> +   }
> +
> +   udev_enumerate_unref(e);
> +
This function (barring the if bellow) seems mostly identical to the
one in compositor-drm.c
Might be a good idea to make it a helper vaguely alike

struct udev_device *
foo(struct backend *b, const char *seat, const char *subsys, const
char *sysname,
  int (*predicate)(struct backend b*, struct udev_device *d))

The callers will be as trivial as
foo(b, seat, "drm", "card[0-9]*", drm_device_is_kms);
foo(b, seat, "graphics", "fb[0-9]*", NULL);

You'd want to do this at some point, since close to nobody tests
fbdev. I think that people porting any drm fixes to fbdev is even less
;-)

> +   if (fb_device)
> +   {
> +   fb_device_path=strdup(udev_device_get_devnode(fb_device));
> +   udev_device_unref(fb_device);
> +   }
> +
Obviously this trivial hunk will have to stay in fbdev.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v7 0/6] Make Weston multiseat aware

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 00:39, nerdopolis  wrote:
> These patches make Weston handle multiple seats. Fixes from the last
> attempt include updating fbdev_set_screen_info , updating some fuzz,
> and making the selection of the framebuffer device similar to
> compositor-drm.c by favoring the boot_vga device, and making
> requested changes
>
With the leak plugged the series is
Reviewed-by: Emil Velikov 

I would strongly recommend the boot_vga refactor as well, but I don't
think it's worth blocking on it.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v7 3/6] launcher-logind: only get a VT on seat0, as only seat0 supports VTs

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 00:39, nerdopolis  wrote:
> As only seat0 supports TTYs, this changes the logind launcher where
> it detects a TTY, only if the seat is seat0. This has only been
> tested for logind
> ---
>  libweston/launcher-logind.c | 22 --
>  libweston/launcher-util.c   |  4 
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/libweston/launcher-logind.c b/libweston/launcher-logind.c
> index d0559c8f..36a4e642 100644
> --- a/libweston/launcher-logind.c
> +++ b/libweston/launcher-logind.c
> @@ -762,18 +762,20 @@ launcher_logind_connect(struct weston_launcher **out, 
> struct weston_compositor *
> free(t);
> goto err_session;
> }
> -   free(t);
>
> -   r = weston_sd_session_get_vt(wl->sid, >vtnr);
> -   if (r < 0) {
> -   weston_log("logind: session not running on a VT\n");
> -   goto err_session;
> -   } else if (tty > 0 && wl->vtnr != (unsigned int )tty) {
> -   weston_log("logind: requested VT --tty=%d differs from real 
> session VT %u\n",
> -  tty, wl->vtnr);
> -   r = -EINVAL;
> -   goto err_session;
> +   if (!strcmp(t, "seat0")) {
> +   r = weston_sd_session_get_vt(wl->sid, >vtnr);
> +   if (r < 0) {
> +   weston_log("logind: session not running on a VT\n");
> +   goto err_session;
> +   } else if (tty > 0 && wl->vtnr != (unsigned int )tty) {
> +   weston_log("logind: requested VT --tty=%d differs 
> from real session VT %u\n",
> +  tty, wl->vtnr);
> +   r = -EINVAL;
> +   goto err_session;
> +   }
> }
> +   free(t);
>
t is leaked in the error paths. The briefest way to handle is
  r = strcmp(...);
  free(t);
  if (r == 0) {
existing_get_vt_code
  }

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] parse_modeline: Ignore case of {h,v}sync flags

2018-06-27 Thread Emil Velikov
On 26 June 2018 at 19:40, Guido Günther  wrote:
> Some modeline generators put out e.g. +HSync instead of +hsync. Accept
> that too since it's not ambigous.
>
Hmm which generator is that? The cvt one, given as an example seems to
produce lowercase ones.
Personally I'm inclined to suggest fixing the generator or using cvt,
instead of working around it here.

That said, more experienced developers should make the call.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH 0/5] Implement support for drm properties "GAMMA_LUT" and "CTM"

2018-06-26 Thread Emil Velikov
On 26 June 2018 at 17:47, Matheus Santana  wrote:
>
>
> On Tue, Jun 26, 2018 at 11:51 AM, Emil Velikov 
> wrote:
>>
>> On 25 June 2018 at 19:48, Matheus Santana  wrote:
>> > Hi Harsha,
>> >
>> >> Weston needs to be rebuilt. But, no new library versions dependencies.
>> >
>> >
>> > All right.
>> >
>> > On Mon, Jun 25, 2018 at 10:21 AM, Harsha Manjula Mallikarjun (RBEI/ECF3)
>> >  wrote:
>> >>
>> >> Hi Matheus,
>> >>
>> >> >From: Matheus Santana [mailto:e...@cin.ufpe.br]
>> >> >Sent: Friday, June 22, 2018 8:33 PM
>> >> >To: Harsha Manjula Mallikarjun (RBEI/ECF3)
>> >> > 
>> >> >Cc: wayland-devel@lists.freedesktop.org
>> >> >Subject: Re: [PATCH 0/5] Implement support for drm properties
>> >> > "GAMMA_LUT"
>> >> > and "CTM"
>> >> >
>> >> >Hi Harsha,
>> >> >I am unable to run the tests you introduced. Tests execution gets
>> >> > stuck
>> >> > after reporting success for surface-global-test.
>> >>
>> >> Surface-global-test terminates the compositor. May be this is the
>> >> reason.
>> >> I do not see a stuck test rather a terminated compositor.
>> >>
>> >> For testing I modified Weston.ini as follows:
>> >> modules=ivi-controller.so,
>> >> surface-global-test.so,gamma-test.so,ctm-test.so
>> >>
>> >> Are you running tests in an another way?
>> >
>> >
>> > I'm running them by just issuing `make check` after building weston. I
>> > basically execute three commands (within weston's root dir):
>> >
>> > $ ./autogen.sh --prefix=$WLD
>> > $ make
>> > $ make check
>> >
>> > The output for `autogen.sh` (which is the same regardless patches
>> > application) is attached.
>> > All tests in suite run fine (24 passed, 1 skipped) when running these
>> > commands without the paches.
>> >
>> > I'm applying the patches by downloading the mbox and `git am
>> > patches.mbox`.
>> >
>> > Are you able to run all tests in suite by just `make check` after
>> > building
>> > the project?
>> >
>> Matheus your assumption is correct. make check should pass, without
>> any local changes.
>>
>> Quick look points the following:
>>  - the drm (compositor-drm) backend supports the new functionality
>>  - the default backend (see tests/weston-tests-env) is headless
>> (compositor-headless)
>
>
> So we shouldn't run drm-specific tests with `make check`, maybe?
>
Personally I would try some of the following:
 a) see if a test specific ini file (see [0] and draw inspiration from
existing ini files) won't help
 b) enhance the testing infra to allow selecting the backend

Then again, might be better to see what more experienced developers prefer.

>>
>>  - xwayland is used - not sure if the xwayland codebase (which lives
>> in the Xserver repo) needs updates
>
>
> Would you please point out what does indicate that xwayland is in use?
>
This line [1] says "all tests with .la extension, will be run via
weston-tests-env". While this hunk [2] does the execution and you can
--xwayland in there.

HTH
Emil

[0] 
https://gitlab.freedesktop.org/wayland/weston/blob/78a42116ae92f93a01539a785ce95cc478189608/tests/weston-tests-env#L41
[1] 
https://gitlab.freedesktop.org/wayland/weston/blob/78a42116ae92f93a01539a785ce95cc478189608/Makefile.am#L1264
[2] 
https://gitlab.freedesktop.org/wayland/weston/blob/78a42116ae92f93a01539a785ce95cc478189608/tests/weston-tests-env#L67
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH 0/5] Implement support for drm properties "GAMMA_LUT" and "CTM"

2018-06-26 Thread Emil Velikov
On 25 June 2018 at 19:48, Matheus Santana  wrote:
> Hi Harsha,
>
>> Weston needs to be rebuilt. But, no new library versions dependencies.
>
>
> All right.
>
> On Mon, Jun 25, 2018 at 10:21 AM, Harsha Manjula Mallikarjun (RBEI/ECF3)
>  wrote:
>>
>> Hi Matheus,
>>
>> >From: Matheus Santana [mailto:e...@cin.ufpe.br]
>> >Sent: Friday, June 22, 2018 8:33 PM
>> >To: Harsha Manjula Mallikarjun (RBEI/ECF3)
>> > 
>> >Cc: wayland-devel@lists.freedesktop.org
>> >Subject: Re: [PATCH 0/5] Implement support for drm properties "GAMMA_LUT"
>> > and "CTM"
>> >
>> >Hi Harsha,
>> >I am unable to run the tests you introduced. Tests execution gets stuck
>> > after reporting success for surface-global-test.
>>
>> Surface-global-test terminates the compositor. May be this is the reason.
>> I do not see a stuck test rather a terminated compositor.
>>
>> For testing I modified Weston.ini as follows:
>> modules=ivi-controller.so,
>> surface-global-test.so,gamma-test.so,ctm-test.so
>>
>> Are you running tests in an another way?
>
>
> I'm running them by just issuing `make check` after building weston. I
> basically execute three commands (within weston's root dir):
>
> $ ./autogen.sh --prefix=$WLD
> $ make
> $ make check
>
> The output for `autogen.sh` (which is the same regardless patches
> application) is attached.
> All tests in suite run fine (24 passed, 1 skipped) when running these
> commands without the paches.
>
> I'm applying the patches by downloading the mbox and `git am patches.mbox`.
>
> Are you able to run all tests in suite by just `make check` after building
> the project?
>
Matheus your assumption is correct. make check should pass, without
any local changes.

Quick look points the following:
 - the drm (compositor-drm) backend supports the new functionality
 - the default backend (see tests/weston-tests-env) is headless
(compositor-headless)
 - xwayland is used - not sure if the xwayland codebase (which lives
in the Xserver repo) needs updates

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] man: remove redundant word in weston.ini(5)

2018-06-26 Thread Emil Velikov
On 22 June 2018 at 22:00, Matheus Santana  wrote:
> Signed-off-by: Matheus Santana 
> ---
>  man/weston.ini.man | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man/weston.ini.man b/man/weston.ini.man
> index 027eae0..02c9b03 100644
> --- a/man/weston.ini.man
> +++ b/man/weston.ini.man
> @@ -468,7 +468,7 @@ denoting the scaling multiplier for the output.
>  .RE
>  .TP 7
>  .BI "seat=" name
> -The logical seat name that that this output should be associated with. If 
> this
> +The logical seat name that this output should be associated with. If this
>  is set then the seat's input will be confined to the output that has the seat
>  set on it. The expectation is that this functionality will be used in a
>  multiheaded environment with a single compositor for multiple output and 
> input

Nice catch.

Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH libinput] tools: fake-build the measure touch-pressure/size sources

2018-06-26 Thread Emil Velikov
On 21 June 2018 at 05:56, Peter Hutterer  wrote:
> This way we can make them execute the list-quirks from the builddir.
>
> Signed-off-by: Peter Hutterer 
> ---
> If anyone has any good ideas for how to do something similar for C source
> file, I'd appreciate it.
>
Here is the approach used by git (with git-foo as an example)

a) stat `dirname git`../git-core/git-foo
b) stat (for each in $PATH)/git-foo
c) $(pwd)/git-foo

With the first one found, being executed.
So when installed a) kicks in (99% of the time), and while in builddir c) does.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-web 2/3] building: use enable-llvm option

2018-06-26 Thread Emil Velikov
On 21 June 2018 at 19:54, Matheus Santana  wrote:
> Fix
>
> error: --enable-llvm is required when building r300
>
Fwiw, this error is strictly for performance reasons. If having a
LLVM-free driver is more important than performance, one can remove
the check.
That aside, the series is

Reviewed-by: Emil Velikov 

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 2/4] shared: remove weston_config_get_libexec_dir()

2018-06-20 Thread Emil Velikov
On 20 June 2018 at 12:04, Pekka Paalanen  wrote:
> On Tue, 19 Jun 2018 18:02:16 +0100
> Emil Velikov  wrote:
>
>> On 18 June 2018 at 15:40, Pekka Paalanen  wrote:
>> > From: Pekka Paalanen 
>> >
>> > Now that WESTON_MODULE_MAP supersedes WESTON_BUILD_DIR for libexec
>> > binaries, we don't need to check in WESTON_BUILD_DIR anymore.
>> >
>> > There was only one user of weston_config_get_libexec_dir(), so remove
>> > the whole function. There is no reason to export it.
>> >
>> Removal of public API should be accompanied with changing the API/ABI 
>> version.
>> Bump of the DSO ABI is one way to handle it.
>
> That was already done in:
> https://gitlab.freedesktop.org/wayland/weston/commit/01f60211b2ff3d12bd8bc6a008ba07c30a666760
> which put us to libweston 5.0.0. We don't bump major twice between
> releases.
>
Indeed, I did not spot that one. IMHO a small note vaguely like the
following is a good call.

"The ABI major was bumped since last release, so there's no need to do
so again."

Regardless, the patch is
Reviewed-by: Emil Velikov 

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 1/4] tests: Don't rely on build directory layout

2018-06-20 Thread Emil Velikov
On 20 June 2018 at 11:51, Pekka Paalanen  wrote:
> On Tue, 19 Jun 2018 18:30:13 +0100
> Emil Velikov  wrote:
>
>> Hi Pekka,
>>
>> On 18 June 2018 at 15:40, Pekka Paalanen  wrote:
>> > From: Daniel Stone 
>> >
>> > Rather than having a hardcoded dependency on the build-directory layout,
>> > use an explicit module-map environment variable, which rewrites requests
>> > for modules and helper/libexec binaries to specific paths.
>> >
>> > This will help with migration to Meson where setting up the paths
>> > according to autotools would be painful and unnecessary.
>> >
>> Surely one didn't need a build system, to make nice improvements.
>
> The driving force behind a change should be stated in the commit
> message. Granted, this paragraph was added by me, Daniel wrote only the
> first paragraph. I didn't see the first paragraph as sufficient
> rationale. The patch also allows loading external backends which we
> specifically do not want to support at the time.
>
Right, this extra information seems a lot better selling point than
build system A/B.

>> I guess, sometimes, tools do need to give us a wake-up call.
>>
>>
>> > +WL_EXPORT size_t
>> > +weston_module_path_from_env(const char *name, char *path, size_t path_len)
>> > +{
>> > +   const char *mapping = getenv("WESTON_MODULE_MAP");
>> > +   const char *end;
>> > +
>> strlen(name) once and use it throughout?
>>
>
> Might as well, since 'name' doesn't change.
>
>>
>> > +   if (!mapping)
>> > +   return 0;
>> > +
>> > +   end = mapping + strlen(mapping);
>> > +   while (mapping < end && *mapping) {
>> > +   const char *filename, *next;
>> > +
>> > +   /* early out: impossibly short string */
>> > +   if ((size_t)(end - mapping) < strlen(name) + 1)
>> > +   return 0;
>> > +
>> > +   filename = [strlen(name) + 1];
>> > +   next = strchrnul(mapping, ';');
>> > +
>> > +   if (strncmp(mapping, name, strlen(name)) == 0 &&
>> > +   mapping[strlen(name)] == '=') {
>> > +   size_t file_len = next - filename; /* no trailing 
>> > NUL */
>> > +   if (file_len >= path_len)
>> > +   return 0;
>> > +   strncpy(path, filename, file_len);
>> > +   path[file_len] = '\0';
>> A simple stat() can, more or less, ensure that we're not giving random
>> rubbish to the caller.
>> Worth it, or more of an overkill?
>
> If a mapping exists, we need to return the mapped-to path. If we return
> nothing, then the caller will use the default path and a broken module
> map could go unnoticed if there actually is something in the default
> path.
>
Hmm good point - shout early and loud is a good thing to have.
The transient fallbacks tend to do more harm than good.

With the "allow external modules" in the commit message + const
[int,ssize_t] foo = strlen(name), the patch is
Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCHv2 wayland 0/8] wayland-scanner: produce code with c99 initializers

2018-06-20 Thread Emil Velikov
On 20 June 2018 at 11:23, Pekka Paalanen  wrote:
> On Tue, 19 Jun 2018 17:43:47 +0100
> Emil Velikov  wrote:
>
>> On 18 June 2018 at 11:36, Pekka Paalanen  wrote:
>> > On Thu, 14 Jun 2018 16:49:37 +0100
>> > Emil Velikov  wrote:
>> >
>> >> Hi all,
>> >>
>> >> Here's a take v2 of the series, with the following changes:
>> >>  - don't trim trailing NULL entries from the wl_interfaces* array
>> >>  - updated tests - separate patches to ease review, to be squashed
>> >>
>> >> On the question of why, despite the aesthetics these patches make the
>> >> generated files actually understandable by a human being...
>> >
>> > Hi Emil,
>> >
>> > on the previous round, this concern was raised:
>> >
>> Thanks, did not spot that one.
>>
>>
>> > On Tue, 13 Feb 2018 13:36:06 +
>> > Daniel Stone  wrote:
>> >
>> >> But that being said, my worry is that we don't actually control the
>> >> compilation environment for the scanner output. Scanner output
>> >> currently compiles with '-pedantic -ansi -Wall -Wextra' (at least,
>> >> when inline is defined). This patch changes that requirement, and I
>> >> worry that - like previous discussions on changing scanner output -
>> >> that upgrading Wayland would lead to people hitting compilation
>> >> failures.
>> >
>> > What is your rationale for that being a non-issue?
>> >
>> According to [1] GCC has supported designated initializers since v3.0,
>> released some 17 years ago [2].
>> Clang has supported them from a very early age. On the Windows front
>> MSVC 2013 introduced support and it EOL.
>>
>> Other less common compilers (say the Sun/Oracle or Intel ones) are
>> fine as well - although I cannot give you exact details.
>>
>> In other words unless someone does one of the following two they're
>> perfectly fine.
>>  - uses unsupported (ancient?) compiler, or
>>  - explicitly sets -pedantic -ansi _and_ -Werror
>>
>> In the case they do, they should seriously reconsider what they're
>> inflicting on themselves.
>> Both from functionality and security POV.
>
> Ok. So '-pedantic -ansi' will still compile, even if with warnings?
> Ansi being equivalent to -std=c90 it seems.
>
You can easily check that with something like the following.

cd $wayland-repo
apply patches
cd tests/data
echo "#include "small-server.h" >> ff.c
echo "int main(void) { return 0; }" >> ff.c
gcc -pedantic -ansi -Dinline="" -I. example-code.c ff.c
-lwayland-server -o foobar


-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 1/2] contributing: add review guidelines

2018-06-19 Thread Emil Velikov
Hi Pekka,

On 18 June 2018 at 14:42, Pekka Paalanen  wrote:

> +- Stable ABI or API is not broken.
> +
I think I've just caught one of those ;-)

Thanks for the vast, yet concise writeup. Fwiw
Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 4/4] tests: Reshuffle IVI layout tests

2018-06-19 Thread Emil Velikov
On 18 June 2018 at 15:40, Pekka Paalanen  wrote:
> From: Daniel Stone 
>
> Rename the IVI tests to be more consistent with the others, and invert
> the naming of plugin/client to make it slightly more clear what's going
> to happen. Handle the renaming by using wet_get_binary_path to rewrite
> the local binaries.
>
> As a side-effect, weston.ini ivi-shell-user-interface no longer needs to
> be given as an absolute path.
>
> Signed-off-by: Daniel Stone 
>
> v2:
>
> Call ivi-layout.ivi as ivi-layout-test-client.ivi to keep the same name
> in both the file and the lookup, so that the module map does not need to
> change the name.
>
> Update code comments to reflect the new names.
>
> Rename ivi_layout-test-plugin.c to ivi-layout-test-plugin.c.
>
About half of the patch is the mechanical rename - worth keeping that
separate from the rest?
Either way, patch looks spot on and is
Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 1/4] tests: Don't rely on build directory layout

2018-06-19 Thread Emil Velikov
Hi Pekka,

On 18 June 2018 at 15:40, Pekka Paalanen  wrote:
> From: Daniel Stone 
>
> Rather than having a hardcoded dependency on the build-directory layout,
> use an explicit module-map environment variable, which rewrites requests
> for modules and helper/libexec binaries to specific paths.
>
> This will help with migration to Meson where setting up the paths
> according to autotools would be painful and unnecessary.
>
Surely one didn't need a build system, to make nice improvements.
I guess, sometimes, tools do need to give us a wake-up call.


> +WL_EXPORT size_t
> +weston_module_path_from_env(const char *name, char *path, size_t path_len)
> +{
> +   const char *mapping = getenv("WESTON_MODULE_MAP");
> +   const char *end;
> +
strlen(name) once and use it throughout?


> +   if (!mapping)
> +   return 0;
> +
> +   end = mapping + strlen(mapping);
> +   while (mapping < end && *mapping) {
> +   const char *filename, *next;
> +
> +   /* early out: impossibly short string */
> +   if ((size_t)(end - mapping) < strlen(name) + 1)
> +   return 0;
> +
> +   filename = [strlen(name) + 1];
> +   next = strchrnul(mapping, ';');
> +
> +   if (strncmp(mapping, name, strlen(name)) == 0 &&
> +   mapping[strlen(name)] == '=') {
> +   size_t file_len = next - filename; /* no trailing NUL 
> */
> +   if (file_len >= path_len)
> +   return 0;
> +   strncpy(path, filename, file_len);
> +   path[file_len] = '\0';
A simple stat() can, more or less, ensure that we're not giving random
rubbish to the caller.
Worth it, or more of an overkill?

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 2/4] shared: remove weston_config_get_libexec_dir()

2018-06-19 Thread Emil Velikov
On 18 June 2018 at 15:40, Pekka Paalanen  wrote:
> From: Pekka Paalanen 
>
> Now that WESTON_MODULE_MAP supersedes WESTON_BUILD_DIR for libexec
> binaries, we don't need to check in WESTON_BUILD_DIR anymore.
>
> There was only one user of weston_config_get_libexec_dir(), so remove
> the whole function. There is no reason to export it.
>
Removal of public API should be accompanied with changing the API/ABI version.
Bump of the DSO ABI is one way to handle it.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] contributing: commit rights

2018-06-19 Thread Emil Velikov
On 18 June 2018 at 14:42, Pekka Paalanen  wrote:
> From: Pekka Paalanen 
>
> This has been copied from
> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/CONTRIBUTING?id=eccae1360d6d01e73c6af2bd97122cef708207ef
> and slightly edited to better with Wayland and Weston.
>
> The intention is to make it easier to give out commit access to new
> people, let them know what is expected of them, and help the community
> to grow. Hopefully this will in time improve the patch review throughput
> and timeliness.
>
> The original text was introduced in
> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/commit/CONTRIBUTING?id=0350f0e7f6a0e07281445fc3082aa70419f4aac7
>
> Signed-off-by: Pekka Paalanen 
> ---
>  CONTRIBUTING.md | 33 +
>  1 file changed, 33 insertions(+)
>
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 6e74b6d..70d0eca 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -246,5 +246,38 @@ clarity suffers.
>  - The code adheres to the style guidelines.
>
>
> +Commit rights
> +=
> +
> +Commit rights will be granted to anyone who requests them and fulfills the
> +below criteria:
> +
> +- Submitted some (10 as a rule of thumb) non-trivial (not just simple
> +  spelling fixes and whitespace adjustment) patches that have been merged
> +  already.
> +
> +- Are actively participating in public discussions about their work (on the
> +  mailing list or IRC). This should not be interpreted as a requirement to
> +  review other peoples patches but just make sure that patch submission isn't
> +  one-way communication. Cross-review is still highly encouraged.
> +
> +- Will be regularly contributing further patches. This includes regular
> +  contributors to other parts of the open source graphics stack who only
> +  do the occasional development in this project.
> +
> +- Agrees to use their commit rights in accordance with the documented merge
> +  criteria, tools, and processes.
> +
> +To apply for commit rights, create a new issue in gitlab for the respective
> +project and give it the "accounts" label.
> +
> +Committers are encouraged to request their commit rights get removed when 
> they
> +no longer contribute to the project. Commit rights will be reinstated when 
> they
> +come back to the project.
> +
> +Maintainers and committers should encourage contributors to request commit
> +rights, especially junior contributors tend to underestimate their skills.
> +
> +
Yay, glad to see some documentation on the topic. I wonder when it
would have came out, if it wasn't for the gitlab move.
Either way, thanks for doing this. Fwiw:

Reviewed-by: Emil Velikov 

Aside: it would be a good idea who's review is considered sufficient
to get merge patches.
It would be great to spare the fun experiences of wayland-egl.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCHv2 wayland 0/8] wayland-scanner: produce code with c99 initializers

2018-06-19 Thread Emil Velikov
On 18 June 2018 at 11:36, Pekka Paalanen  wrote:
> On Thu, 14 Jun 2018 16:49:37 +0100
> Emil Velikov  wrote:
>
>> Hi all,
>>
>> Here's a take v2 of the series, with the following changes:
>>  - don't trim trailing NULL entries from the wl_interfaces* array
>>  - updated tests - separate patches to ease review, to be squashed
>>
>> On the question of why, despite the aesthetics these patches make the
>> generated files actually understandable by a human being...
>
> Hi Emil,
>
> on the previous round, this concern was raised:
>
Thanks, did not spot that one.


> On Tue, 13 Feb 2018 13:36:06 +
> Daniel Stone  wrote:
>
>> But that being said, my worry is that we don't actually control the
>> compilation environment for the scanner output. Scanner output
>> currently compiles with '-pedantic -ansi -Wall -Wextra' (at least,
>> when inline is defined). This patch changes that requirement, and I
>> worry that - like previous discussions on changing scanner output -
>> that upgrading Wayland would lead to people hitting compilation
>> failures.
>
> What is your rationale for that being a non-issue?
>
According to [1] GCC has supported designated initializers since v3.0,
released some 17 years ago [2].
Clang has supported them from a very early age. On the Windows front
MSVC 2013 introduced support and it EOL.

Other less common compilers (say the Sun/Oracle or Intel ones) are
fine as well - although I cannot give you exact details.

In other words unless someone does one of the following two they're
perfectly fine.
 - uses unsupported (ancient?) compiler, or
 - explicitly sets -pedantic -ansi _and_ -Werror

In the case they do, they should seriously reconsider what they're
inflicting on themselves.
Both from functionality and security POV.

-Emil

[1] https://gcc.gnu.org/c99status.html
[2] https://gcc.gnu.org/releases.html
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] Makefile.am: add include dir for AC_CONFIG_MACRO_DIR to work

2018-06-18 Thread Emil Velikov
On 15 June 2018 at 15:06, Maciej Wolny  wrote:
> On 05/06/18 11:53, Maciej Wolny wrote:
>> da331647269ee9d73c4008ae901d107320bdc8d1 added a compatiblity macro for
>> old versions of pkg-config. However, the file in which that macro
>> resides was not included. From the autoconf docs: "Note that if you use
>> aclocal from Automake to generate aclocal.m4, you must also set
>> ACLOCAL_AMFLAGS = -I dir in your top-level Makefile.am.".
>> ---
>>  Makefile.am | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 4b9a901..1aa13cf 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -1,3 +1,5 @@
>> +ACLOCAL_AMFLAGS = -I m4
>> +
>>  unstable_protocols =
>>  \
>>   unstable/pointer-gestures/pointer-gestures-unstable-v1.xml 
>>  \
>>   unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml     
>>  \
>>
>
> I'm sorry, I didn't specify - this is a patch for wayland-protocols.
That helps clear some confusion. Thanks

Reviewed-by: Emil Velikov 

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] .gitlab-ci.yml: collect the distcheck error logs

2018-06-14 Thread Emil Velikov
From: Emil Velikov 

Currently we issue both check and distcheck, as reportedly there has
been cases in the past one works, while the other doesn't.

Yet we only collect the check artefacts (test logs).

Correct that, by picking the distcheck ones as well.
Note: the build-*/wayland*/ directory is purged by distcheck if it runs
successfully.

Signed-off-by: Emil Velikov 
---
 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index bc1a005..2489665 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -31,5 +31,6 @@ build-native:
 when: always
 paths:
 - build-*/wayland-*.tar.xz
+- build-*/wayland*/_build/sub/*.log
 - build-*/*.log
 - prefix-*
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 8/8] fixup! scanner: initialize .{method, event}_count via ARRAY_SIZE

2018-06-14 Thread Emil Velikov
---
 tests/data/example-code.c   | 73 +
 tests/data/small-code-core.c|  5 +--
 tests/data/small-code.c |  5 +--
 tests/data/small-private-code.c |  5 +--
 4 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/tests/data/example-code.c b/tests/data/example-code.c
index 2e1f73b..65d9651 100644
--- a/tests/data/example-code.c
+++ b/tests/data/example-code.c
@@ -146,6 +146,7 @@ static const struct wl_interface *types[] = {
[94] = _surface_interface,
 };
 
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 static const struct wl_message wl_display_requests[] = {
{ .name = "sync", .signature = "n", .types = [8] },
{ .name = "get_registry", .signature = "n", .types = [9] },
@@ -158,8 +159,8 @@ static const struct wl_message wl_display_events[] = {
 
 WL_EXPORT const struct wl_interface wl_display_interface = {
.name = "wl_display", .version = 1,
-   .method_count = 2, .methods = wl_display_requests,
-   .event_count = 2, .events = wl_display_events,
+   .method_count = ARRAY_SIZE(wl_display_requests), .methods = 
wl_display_requests,
+   .event_count = ARRAY_SIZE(wl_display_events), .events = 
wl_display_events,
 };
 
 static const struct wl_message wl_registry_requests[] = {
@@ -173,8 +174,8 @@ static const struct wl_message wl_registry_events[] = {
 
 WL_EXPORT const struct wl_interface wl_registry_interface = {
.name = "wl_registry", .version = 1,
-   .method_count = 1, .methods = wl_registry_requests,
-   .event_count = 2, .events = wl_registry_events,
+   .method_count = ARRAY_SIZE(wl_registry_requests), .methods = 
wl_registry_requests,
+   .event_count = ARRAY_SIZE(wl_registry_events), .events = 
wl_registry_events,
 };
 
 static const struct wl_message wl_callback_events[] = {
@@ -183,7 +184,7 @@ static const struct wl_message wl_callback_events[] = {
 
 WL_EXPORT const struct wl_interface wl_callback_interface = {
.name = "wl_callback", .version = 1,
-   .event_count = 1, .events = wl_callback_events,
+   .event_count = ARRAY_SIZE(wl_callback_events), .events = 
wl_callback_events,
 };
 
 static const struct wl_message wl_compositor_requests[] = {
@@ -193,7 +194,7 @@ static const struct wl_message wl_compositor_requests[] = {
 
 WL_EXPORT const struct wl_interface wl_compositor_interface = {
.name = "wl_compositor", .version = 4,
-   .method_count = 2, .methods = wl_compositor_requests,
+   .method_count = ARRAY_SIZE(wl_compositor_requests), .methods = 
wl_compositor_requests,
 };
 
 static const struct wl_message wl_shm_pool_requests[] = {
@@ -204,7 +205,7 @@ static const struct wl_message wl_shm_pool_requests[] = {
 
 WL_EXPORT const struct wl_interface wl_shm_pool_interface = {
.name = "wl_shm_pool", .version = 1,
-   .method_count = 3, .methods = wl_shm_pool_requests,
+   .method_count = ARRAY_SIZE(wl_shm_pool_requests), .methods = 
wl_shm_pool_requests,
 };
 
 static const struct wl_message wl_shm_requests[] = {
@@ -217,8 +218,8 @@ static const struct wl_message wl_shm_events[] = {
 
 WL_EXPORT const struct wl_interface wl_shm_interface = {
.name = "wl_shm", .version = 1,
-   .method_count = 1, .methods = wl_shm_requests,
-   .event_count = 1, .events = wl_shm_events,
+   .method_count = ARRAY_SIZE(wl_shm_requests), .methods = wl_shm_requests,
+   .event_count = ARRAY_SIZE(wl_shm_events), .events = wl_shm_events,
 };
 
 static const struct wl_message wl_buffer_requests[] = {
@@ -231,8 +232,8 @@ static const struct wl_message wl_buffer_events[] = {
 
 WL_EXPORT const struct wl_interface wl_buffer_interface = {
.name = "wl_buffer", .version = 1,
-   .method_count = 1, .methods = wl_buffer_requests,
-   .event_count = 1, .events = wl_buffer_events,
+   .method_count = ARRAY_SIZE(wl_buffer_requests), .methods = 
wl_buffer_requests,
+   .event_count = ARRAY_SIZE(wl_buffer_events), .events = wl_buffer_events,
 };
 
 static const struct wl_message wl_data_offer_requests[] = {
@@ -251,8 +252,8 @@ static const struct wl_message wl_data_offer_events[] = {
 
 WL_EXPORT const struct wl_interface wl_data_offer_interface = {
.name = "wl_data_offer", .version = 3,
-   .method_count = 5, .methods = wl_data_offer_requests,
-   .event_count = 3, .events = wl_data_offer_events,
+   .method_count = ARRAY_SIZE(wl_data_offer_requests), .methods = 
wl_data_offer_requests,
+   .event_count = ARRAY_SIZE(wl_data_offer_events), .events = 
wl_data_offer_events,
 };
 
 static const struct wl_message wl_data_source_requests[] = {
@@ -272,8 +273,8 @@ static const struct wl_message wl_data_source_events[] = {
 
 WL_EXPORT const struct wl_interface wl_data_source_interface = {
.name = "wl_data_source", .version = 3,
-   .method_count = 3, .methods = wl_data_source_requests,
-   .event_count = 6, .events = wl_data_source_events,
+   .method_count = 

[PATCH wayland 6/8] fixup! scanner: use c99 initializers for the interface symbols

2018-06-14 Thread Emil Velikov
---
 tests/data/example-code.c   | 124 +++-
 tests/data/small-code-core.c|   6 +-
 tests/data/small-code.c |   6 +-
 tests/data/small-private-code.c |   6 +-
 4 files changed, 67 insertions(+), 75 deletions(-)

diff --git a/tests/data/example-code.c b/tests/data/example-code.c
index 4a41b8b..2e1f73b 100644
--- a/tests/data/example-code.c
+++ b/tests/data/example-code.c
@@ -157,9 +157,9 @@ static const struct wl_message wl_display_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_display_interface = {
-   "wl_display", 1,
-   2, wl_display_requests,
-   2, wl_display_events,
+   .name = "wl_display", .version = 1,
+   .method_count = 2, .methods = wl_display_requests,
+   .event_count = 2, .events = wl_display_events,
 };
 
 static const struct wl_message wl_registry_requests[] = {
@@ -172,9 +172,9 @@ static const struct wl_message wl_registry_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_registry_interface = {
-   "wl_registry", 1,
-   1, wl_registry_requests,
-   2, wl_registry_events,
+   .name = "wl_registry", .version = 1,
+   .method_count = 1, .methods = wl_registry_requests,
+   .event_count = 2, .events = wl_registry_events,
 };
 
 static const struct wl_message wl_callback_events[] = {
@@ -182,9 +182,8 @@ static const struct wl_message wl_callback_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_callback_interface = {
-   "wl_callback", 1,
-   0, NULL,
-   1, wl_callback_events,
+   .name = "wl_callback", .version = 1,
+   .event_count = 1, .events = wl_callback_events,
 };
 
 static const struct wl_message wl_compositor_requests[] = {
@@ -193,9 +192,8 @@ static const struct wl_message wl_compositor_requests[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_compositor_interface = {
-   "wl_compositor", 4,
-   2, wl_compositor_requests,
-   0, NULL,
+   .name = "wl_compositor", .version = 4,
+   .method_count = 2, .methods = wl_compositor_requests,
 };
 
 static const struct wl_message wl_shm_pool_requests[] = {
@@ -205,9 +203,8 @@ static const struct wl_message wl_shm_pool_requests[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_shm_pool_interface = {
-   "wl_shm_pool", 1,
-   3, wl_shm_pool_requests,
-   0, NULL,
+   .name = "wl_shm_pool", .version = 1,
+   .method_count = 3, .methods = wl_shm_pool_requests,
 };
 
 static const struct wl_message wl_shm_requests[] = {
@@ -219,9 +216,9 @@ static const struct wl_message wl_shm_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_shm_interface = {
-   "wl_shm", 1,
-   1, wl_shm_requests,
-   1, wl_shm_events,
+   .name = "wl_shm", .version = 1,
+   .method_count = 1, .methods = wl_shm_requests,
+   .event_count = 1, .events = wl_shm_events,
 };
 
 static const struct wl_message wl_buffer_requests[] = {
@@ -233,9 +230,9 @@ static const struct wl_message wl_buffer_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_buffer_interface = {
-   "wl_buffer", 1,
-   1, wl_buffer_requests,
-   1, wl_buffer_events,
+   .name = "wl_buffer", .version = 1,
+   .method_count = 1, .methods = wl_buffer_requests,
+   .event_count = 1, .events = wl_buffer_events,
 };
 
 static const struct wl_message wl_data_offer_requests[] = {
@@ -253,9 +250,9 @@ static const struct wl_message wl_data_offer_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_data_offer_interface = {
-   "wl_data_offer", 3,
-   5, wl_data_offer_requests,
-   3, wl_data_offer_events,
+   .name = "wl_data_offer", .version = 3,
+   .method_count = 5, .methods = wl_data_offer_requests,
+   .event_count = 3, .events = wl_data_offer_events,
 };
 
 static const struct wl_message wl_data_source_requests[] = {
@@ -274,9 +271,9 @@ static const struct wl_message wl_data_source_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_data_source_interface = {
-   "wl_data_source", 3,
-   3, wl_data_source_requests,
-   6, wl_data_source_events,
+   .name = "wl_data_source", .version = 3,
+   .method_count = 3, .methods = wl_data_source_requests,
+   .event_count = 6, .events = wl_data_source_events,
 };
 
 static const struct wl_message wl_data_device_requests[] = {
@@ -295,9 +292,9 @@ static const struct wl_message wl_data_device_events[] = {
 };
 
 WL_EXPORT const struct wl_interface wl_data_device_interface = {
-   "wl_data_device", 3,
-   3, wl_data_device_requests,
-   6, wl_data_device_events,
+   .name = "wl_data_device", .version = 3,
+   .method_count = 3, .methods = wl_data_device_requests,
+   .event_count = 6, .events = wl_data_device_events,
 };
 
 static const struct wl_message wl_data_device_manager_requests[] = {
@@ -306,9 +303,8 @@ static const struct wl_message 
wl_data_device_manager_requests[] = {
 };
 
 WL_EXPORT const struct wl_interface 

[PATCH wayland 7/8] scanner: initialize .{method, event}_count via ARRAY_SIZE

2018-06-14 Thread Emil Velikov
From: Emil Velikov 

Demystify the final magic value within the generated files.

Signed-off-by: Emil Velikov 
---
 src/scanner.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/scanner.c b/src/scanner.c
index 22b1daa..8f469e9 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -1779,6 +1779,7 @@ emit_code(struct protocol *protocol, enum visibility vis)
}
printf("};\n\n");
 
+   printf("#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))\n");
wl_list_for_each_safe(i, next, >interface_list, link) {
 
emit_messages(>request_list, i, "requests");
@@ -1790,12 +1791,12 @@ emit_code(struct protocol *protocol, enum visibility 
vis)
   symbol_visibility, i->name, i->name, i->version);
 
if (!wl_list_empty(>request_list))
-   printf("\t.method_count = %d, .methods = 
%s_requests,\n",
-  wl_list_length(>request_list), i->name);
+   printf("\t.method_count = ARRAY_SIZE(%s_requests), 
.methods = %s_requests,\n",
+  i->name, i->name);
 
if (!wl_list_empty(>event_list))
-   printf("\t.event_count = %d, .events = %s_events,\n",
-  wl_list_length(>event_list), i->name);
+   printf("\t.event_count = ARRAY_SIZE(%s_events), .events 
= %s_events,\n",
+  i->name, i->name);
 
printf("};\n\n");
 
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 4/8] fixup! scanner: use c99 initializers for the request/events arrays

2018-06-14 Thread Emil Velikov
---
 tests/data/example-code.c   | 238 
 tests/data/small-code-core.c|   8 +-
 tests/data/small-code.c |   8 +-
 tests/data/small-private-code.c |   8 +-
 4 files changed, 131 insertions(+), 131 deletions(-)

diff --git a/tests/data/example-code.c b/tests/data/example-code.c
index ccfa9ef..4a41b8b 100644
--- a/tests/data/example-code.c
+++ b/tests/data/example-code.c
@@ -147,13 +147,13 @@ static const struct wl_interface *types[] = {
 };
 
 static const struct wl_message wl_display_requests[] = {
-   { "sync", "n", types + 8 },
-   { "get_registry", "n", types + 9 },
+   { .name = "sync", .signature = "n", .types = [8] },
+   { .name = "get_registry", .signature = "n", .types = [9] },
 };
 
 static const struct wl_message wl_display_events[] = {
-   { "error", "ous", types + 0 },
-   { "delete_id", "u", types + 0 },
+   { .name = "error", .signature = "ous", .types = [0] },
+   { .name = "delete_id", .signature = "u", .types = [0] },
 };
 
 WL_EXPORT const struct wl_interface wl_display_interface = {
@@ -163,12 +163,12 @@ WL_EXPORT const struct wl_interface wl_display_interface 
= {
 };
 
 static const struct wl_message wl_registry_requests[] = {
-   { "bind", "usun", types + 0 },
+   { .name = "bind", .signature = "usun", .types = [0] },
 };
 
 static const struct wl_message wl_registry_events[] = {
-   { "global", "usu", types + 0 },
-   { "global_remove", "u", types + 0 },
+   { .name = "global", .signature = "usu", .types = [0] },
+   { .name = "global_remove", .signature = "u", .types = [0] },
 };
 
 WL_EXPORT const struct wl_interface wl_registry_interface = {
@@ -178,7 +178,7 @@ WL_EXPORT const struct wl_interface wl_registry_interface = 
{
 };
 
 static const struct wl_message wl_callback_events[] = {
-   { "done", "u", types + 0 },
+   { .name = "done", .signature = "u", .types = [0] },
 };
 
 WL_EXPORT const struct wl_interface wl_callback_interface = {
@@ -188,8 +188,8 @@ WL_EXPORT const struct wl_interface wl_callback_interface = 
{
 };
 
 static const struct wl_message wl_compositor_requests[] = {
-   { "create_surface", "n", types + 10 },
-   { "create_region", "n", types + 11 },
+   { .name = "create_surface", .signature = "n", .types = [10] },
+   { .name = "create_region", .signature = "n", .types = [11] },
 };
 
 WL_EXPORT const struct wl_interface wl_compositor_interface = {
@@ -199,9 +199,9 @@ WL_EXPORT const struct wl_interface wl_compositor_interface 
= {
 };
 
 static const struct wl_message wl_shm_pool_requests[] = {
-   { "create_buffer", "nu", types + 12 },
-   { "destroy", "", types + 0 },
-   { "resize", "i", types + 0 },
+   { .name = "create_buffer", .signature = "nu", .types = [12] },
+   { .name = "destroy", .signature = "", .types = [0] },
+   { .name = "resize", .signature = "i", .types = [0] },
 };
 
 WL_EXPORT const struct wl_interface wl_shm_pool_interface = {
@@ -211,11 +211,11 @@ WL_EXPORT const struct wl_interface wl_shm_pool_interface 
= {
 };
 
 static const struct wl_message wl_shm_requests[] = {
-   { "create_pool", "nhi", types + 18 },
+   { .name = "create_pool", .signature = "nhi", .types = [18] },
 };
 
 static const struct wl_message wl_shm_events[] = {
-   { "format", "u", types + 0 },
+   { .name = "format", .signature = "u", .types = [0] },
 };
 
 WL_EXPORT const struct wl_interface wl_shm_interface = {
@@ -225,11 +225,11 @@ WL_EXPORT const struct wl_interface wl_shm_interface = {
 };
 
 static const struct wl_message wl_buffer_requests[] = {
-   { "destroy", "", types + 0 },
+   { .name = "destroy", .signature = "", .types = [0] },
 };
 
 static const struct wl_message wl_buffer_events[] = {
-   { "release", "", types + 0 },
+   { .name = "release", .signature = "", .types = [0] },
 };
 
 WL_EXPORT const struct wl_interface wl_buffer_interface = {
@@ -239,17 +239,17 @@ WL_EXPORT const struct wl_interface wl_buffer_interface = 
{
 };
 
 static const struct wl_message wl_data_offer_requests[] = {
-   { "accept", "u?s", types + 0 },
-   { "receive", "sh", types + 0 },
-   { "destroy", "", types + 0 },
-   { "finish", "3", types + 0 },
-   { "set_actions", "3uu", types + 0 },
+   { .name = "accept", .signature = "u?s", .types = [0] },
+   { .name = "receive", .signature = "sh", .types = [0] },
+   { .name = "destroy", .signature = "", .types = [0] },
+   { .name = "finish", .signature = "3", .types = [0] },
+   { .name = "set_actions", .signature = "3uu", .types = [0] },
 };
 
 static const struct wl_message wl_data_offer_events[] = {
-   { "offer", "s", types + 0 },
-   { "source_actions", "3u", types + 0 },
-   { "action", "3u", types + 0 },
+   { .name = "offer", .signature = "s", .types = [0] },
+   { .name = "source_actions", .signature = "3u", .types = [0] },
+   { .name = "action", 

[PATCH wayland 5/8] scanner: use c99 initializers for the interface symbols

2018-06-14 Thread Emil Velikov
From: Emil Velikov 

Provides some clarity and removes the unnecessary zero/NULL
initialization.

Signed-off-by: Emil Velikov 
---
 src/scanner.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/scanner.c b/src/scanner.c
index fb3c808..22b1daa 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -1786,20 +1786,16 @@ emit_code(struct protocol *protocol, enum visibility 
vis)
 
printf("%s const struct wl_interface "
   "%s_interface = {\n"
-  "\t\"%s\", %d,\n",
+  "\t.name = \"%s\", .version = %d,\n",
   symbol_visibility, i->name, i->name, i->version);
 
if (!wl_list_empty(>request_list))
-   printf("\t%d, %s_requests,\n",
+   printf("\t.method_count = %d, .methods = 
%s_requests,\n",
   wl_list_length(>request_list), i->name);
-   else
-   printf("\t0, NULL,\n");
 
if (!wl_list_empty(>event_list))
-   printf("\t%d, %s_events,\n",
+   printf("\t.event_count = %d, .events = %s_events,\n",
   wl_list_length(>event_list), i->name);
-   else
-   printf("\t0, NULL,\n");
 
printf("};\n\n");
 
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCHv2 wayland 0/8] wayland-scanner: produce code with c99 initializers

2018-06-14 Thread Emil Velikov
Hi all,

Here's a take v2 of the series, with the following changes:
 - don't trim trailing NULL entries from the wl_interfaces* array
 - updated tests - separate patches to ease review, to be squashed

On the question of why, despite the aesthetics these patches make the
generated files actually understandable by a human being...

Thanks
Emil

Emil Velikov (8):
  scanner: use c99 initializers for the wl_interface * array
  fixup! scanner: use c99 initializers for the wl_interface * array
  scanner: use c99 initializers for the request/events arrays
  fixup! scanner: use c99 initializers for the request/events arrays
  scanner: use c99 initializers for the interface symbols
  fixup! scanner: use c99 initializers for the interface symbols
  scanner: initialize .{method,event}_count via ARRAY_SIZE
  fixup! scanner: initialize .{method,event}_count via ARRAY_SIZE

 src/scanner.c   |  32 +--
 tests/data/example-code.c   | 557 
 tests/data/small-code-core.c|  31 +--
 tests/data/small-code.c |  31 +--
 tests/data/small-private-code.c |  31 +--
 5 files changed, 339 insertions(+), 343 deletions(-)

-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 3/8] scanner: use c99 initializers for the request/events arrays

2018-06-14 Thread Emil Velikov
From: Emil Velikov 

Makes things a bit less magical, should you be looking in the file.

Signed-off-by: Emil Velikov 
---
 src/scanner.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/scanner.c b/src/scanner.c
index 4b49593..fb3c808 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -1675,7 +1675,7 @@ emit_messages(struct wl_list *message_list,
   interface->name, suffix);
 
wl_list_for_each(m, message_list, link) {
-   printf("\t{ \"%s\", \"", m->name);
+   printf("\t{ .name = \"%s\", .signature = \"", m->name);
 
if (m->since > 1)
printf("%d", m->since);
@@ -1714,7 +1714,7 @@ emit_messages(struct wl_list *message_list,
break;
}
}
-   printf("\", types + %d },\n", m->type_index);
+   printf("\", .types = [%d] },\n", m->type_index);
}
 
printf("};\n\n");
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 2/8] fixup! scanner: use c99 initializers for the wl_interface * array

2018-06-14 Thread Emil Velikov
---
 tests/data/example-code.c   | 190 
 tests/data/small-code-core.c|  16 ++--
 tests/data/small-code.c |  16 ++--
 tests/data/small-private-code.c |  16 ++--
 4 files changed, 119 insertions(+), 119 deletions(-)

diff --git a/tests/data/example-code.c b/tests/data/example-code.c
index bc03fe5..ccfa9ef 100644
--- a/tests/data/example-code.c
+++ b/tests/data/example-code.c
@@ -49,101 +49,101 @@ extern const struct wl_interface wl_surface_interface;
 extern const struct wl_interface wl_touch_interface;
 
 static const struct wl_interface *types[] = {
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   _callback_interface,
-   _registry_interface,
-   _surface_interface,
-   _region_interface,
-   _buffer_interface,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   _shm_pool_interface,
-   NULL,
-   NULL,
-   _data_source_interface,
-   _surface_interface,
-   _surface_interface,
-   NULL,
-   _data_source_interface,
-   NULL,
-   _data_offer_interface,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   _data_offer_interface,
-   _data_offer_interface,
-   _data_source_interface,
-   _data_device_interface,
-   _seat_interface,
-   _shell_surface_interface,
-   _surface_interface,
-   _seat_interface,
-   NULL,
-   _seat_interface,
-   NULL,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   NULL,
-   _output_interface,
-   _seat_interface,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   NULL,
-   _output_interface,
-   _buffer_interface,
-   NULL,
-   NULL,
-   _callback_interface,
-   _region_interface,
-   _region_interface,
-   _output_interface,
-   _output_interface,
-   _pointer_interface,
-   _keyboard_interface,
-   _touch_interface,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   NULL,
-   _surface_interface,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   _surface_interface,
-   NULL,
-   NULL,
-   NULL,
-   _subsurface_interface,
-   _surface_interface,
-   _surface_interface,
-   _surface_interface,
-   _surface_interface,
+   [0] = NULL,
+   [1] = NULL,
+   [2] = NULL,
+   [3] = NULL,
+   [4] = NULL,
+   [5] = NULL,
+   [6] = NULL,
+   [7] = NULL,
+   [8] = _callback_interface,
+   [9] = _registry_interface,
+   [10] = _surface_interface,
+   [11] = _region_interface,
+   [12] = _buffer_interface,
+   [13] = NULL,
+   [14] = NULL,
+   [15] = NULL,
+   [16] = NULL,
+   [17] = NULL,
+   [18] = _shm_pool_interface,
+   [19] = NULL,
+   [20] = NULL,
+   [21] = _data_source_interface,
+   [22] = _surface_interface,
+   [23] = _surface_interface,
+   [24] = NULL,
+   [25] = _data_source_interface,
+   [26] = NULL,
+   [27] = _data_offer_interface,
+   [28] = NULL,
+   [29] = _surface_interface,
+   [30] = NULL,
+   [31] = NULL,
+   [32] = _data_offer_interface,
+   [33] = _data_offer_interface,
+   [34] = _data_source_interface,
+   [35] = _data_device_interface,
+   [36] = _seat_interface,
+   [37] = _shell_surface_interface,
+   [38] = _surface_interface,
+   [39] = _seat_interface,
+   [40] = NULL,
+   [41] = _seat_interface,
+   [42] = NULL,
+   [43] = NULL,
+   [44] = _surface_interface,
+   [45] = NULL,
+   [46] = NULL,
+   [47] = NULL,
+   [48] = NULL,
+   [49] = NULL,
+   [50] = _output_interface,
+   [51] = _seat_interface,
+   [52] = NULL,
+   [53] = _surface_interface,
+   [54] = NULL,
+   [55] = NULL,
+   [56] = NULL,
+   [57] = _output_interface,
+   [58] = _buffer_interface,
+   [59] = NULL,
+   [60] = NULL,
+   [61] = _callback_interface,
+   [62] = _region_interface,
+   [63] = _region_interface,
+   [64] = _output_interface,
+   [65] = _output_interface,
+   [66] = _pointer_interface,
+   [67] = _keyboard_interface,
+   [68] = _touch_interface,
+   [69] = NULL,
+   [70] = _surface_interface,
+   [71] = NULL,
+   [72] = NULL,
+   [73] = NULL,
+   [74] = _surface_interface,
+   [75] = NULL,
+   [76] = NULL,
+   [77] = NULL,
+   [78] = _surface_interface,
+   [79] = NULL,
+   [80] = _surface_interface,
+   [81] = NULL,
+   [82] = NULL,
+   [83] = _surface_interface,
+   [84] = NULL,
+   [85] = NULL,
+   [86] = 

[PATCH wayland 1/8] scanner: use c99 initializers for the wl_interface * array

2018-06-14 Thread Emil Velikov
From: Emil Velikov 

Makes it a bit more obvious, should you be reading through the generated
code.

We might even drop the NULL entries (at a later stage) making things
shorter and easier to read.

v2: don't alter the array size by discarding trailing NULL entries

Signed-off-by: Emil Velikov 
---
 src/scanner.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/scanner.c b/src/scanner.c
index 205c28a..4b49593 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -1620,7 +1620,7 @@ emit_null_run(struct protocol *protocol)
int i;
 
for (i = 0; i < protocol->null_run_length; i++)
-   printf("\tNULL,\n");
+   printf("\t[%d] = NULL,\n", i);
 }
 
 static void
@@ -1628,6 +1628,7 @@ emit_types(struct protocol *protocol, struct wl_list 
*message_list)
 {
struct message *m;
struct arg *a;
+   int index;
 
wl_list_for_each(m, message_list, link) {
if (m->all_null) {
@@ -1638,21 +1639,23 @@ emit_types(struct protocol *protocol, struct wl_list 
*message_list)
m->type_index =
protocol->null_run_length + protocol->type_index;
protocol->type_index += m->arg_count;
+   index = m->type_index;
 
wl_list_for_each(a, >arg_list, link) {
switch (a->type) {
case NEW_ID:
case OBJECT:
if (a->interface_name)
-   printf("\t&%s_interface,\n",
-  a->interface_name);
+   printf("\t[%d] = &%s_interface,\n",
+  index, a->interface_name);
else
-   printf("\tNULL,\n");
+   printf("\t[%d] = NULL,\n", index);
break;
default:
-   printf("\tNULL,\n");
+   printf("\t[%d] = NULL,\n", index);
break;
}
+   index++;
}
}
 }
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 2/2] Add .gitlab-ci.yml

2018-06-07 Thread Emil Velikov
On 7 June 2018 at 08:46, Daniel Stone  wrote:
> Hi,
>
> On 6 June 2018 at 16:41, Emil Velikov  wrote:
>> On 6 June 2018 at 15:47, Simon McVittie  wrote:
>>> On Wed, 06 Jun 2018 at 15:33:13 +0100, Emil Velikov wrote:
>>>> On 5 June 2018 at 23:06, Daniel Stone  wrote:
>>>> > +  - apt-get -y --no-install-recommends install build-essential automake 
>>>> > autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev 
>>>> > libpixman-1-dev libpng-dev libjpeg-dev libcolord-dev mesa-common-dev 
>>>> > libglu1-mesa-dev libegl1-mesa-dev libgles2-mesa-dev libwayland-dev 
>>>> > libxcb1-dev libxcb-composite0-dev libxcb-xfixes0-dev libxcb-xkb-dev 
>>>> > libx11-xcb-dev libx11-dev libudev-dev libgbm-dev libxkbcommon-dev 
>>>> > libcairo2-dev libpango1.0-dev libgdk-pixbuf2.0-dev libxcursor-dev 
>>>> > libmtdev-dev libpam0g-dev libvpx-dev libsystemd-dev libinput-dev 
>>>> > libwebp-dev libjpeg-dev libva-dev liblcms2-dev git
>>>>
>>>> I think this can be simplify the explicit list to:
>>>> apt-get -y --no-install-recommends build-dep wayland
>>>
>>> Only if the latest version of wayland has the same build-dependencies
>>> as the (likely much older) version visible in the Sources index
>>> for the container's apt configuration. If the container is
>>> Debian 9 'stretch' (released about a year ago) then apt-get build-dep
>>> will get the build-dependencies of wayland 1.12.0, according to
>>> <https://tracker.debian.org/pkg/wayland>.
>>>
>> Sure, there can be differences. Adding the odd extra piece tends to be
>> shorter and easier (imho) than trying to keep the huge list in sync.
>> Distribution maintainers already that laborious job, why not make use of it.
>
> There are a couple of reasons I typed out the full dependency list.
> One of them is what Simon said, and over time as we get further and
> further away from Stretch, the list of extra stuff we have to add
> grows. Even if we move on to newer Debian, there's also the risk that
> we build things they don't, or some kind of transient dependency drops
> out, so we end up with some breakage. So I prefer the explicit set for
> that reason: it means we can be sure about what we're doing, rather
> than effectively CI'ing the Debian packaging.
>
One could consider build-deps from backports. One could even help the
backports team?
I guess I'm too naive trying to spare the duplication effort?

> Secondly, Stretch was just the easiest starting point: I knew how to
> get it up quickly, in a way where the runtime was also fast (unlike
> with e.g. yum/dnf, where I don't know the necessary runes to speed
> them up). But it's entirely likely we'll want to be testing on other
> distros, or even a very much from-scratch build if we are able to
> reuse the work done on GitLab CI for Mesa here:
>   https://gitlab.com/igalia/graphics/mesa/pipelines/22874845
>
> Either way, having the list of dependencies explictly typed out is
> helpful for whoever's doing the conversion, and it's easy to make sure
> the lists stay in sync if we do have multiple lists.
>
IIRC of all the Linux distros only Arch does not have an equivalent of
build-deps.
Furthermore package names and split varies across distros.

>>>> > +  - make -j4
>>>> > +  - make check
>>>> > +  - make install
>>>> > +  - make distcheck
>>>> I was under the impression that "make -j4 distcheck" is enough
>>>
>>> install and check both imply (depend on) a normal build, so that one is
>>> redundant.
>>>
>>> distcheck runs the tests like check does, but differently-configured,
>>> so for full coverage you might want to do both.
>>>
>>> Similarly, distcheck runs "make install", but into a temporary directory,
>>> not the requested $PREFIX.
>>
>> Right - did not spot that "prefix-*" is also collected as artefacts.
>
> Not just prefix, but also as far as I can see there's no way to
> collect test suite logs from a distcheck run?
If you're want the when logs everything passes, no there isn't a way.
Otherwise, yes - just tweak the regex (example below).

> We've also had
> inexplicable bugs in the past where 'make check' failed but 'make
> distcheck' succeeded (or was it vice-versa?), and given the test
> runtime is still quite short, seemed reasonable enough to explicitly
> test out all of those.
>
Everything aside, this in itself is a fairly valid reason. Currently
the distcheck logs are not fetched though.
The following should help ;-)

- buil

Re: [PATCH weston 2/2] Add .gitlab-ci.yml

2018-06-06 Thread Emil Velikov
On 6 June 2018 at 15:47, Simon McVittie  wrote:
> On Wed, 06 Jun 2018 at 15:33:13 +0100, Emil Velikov wrote:
>> On 5 June 2018 at 23:06, Daniel Stone  wrote:
>> > +  - apt-get -y --no-install-recommends install build-essential automake 
>> > autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev 
>> > libpixman-1-dev libpng-dev libjpeg-dev libcolord-dev mesa-common-dev 
>> > libglu1-mesa-dev libegl1-mesa-dev libgles2-mesa-dev libwayland-dev 
>> > libxcb1-dev libxcb-composite0-dev libxcb-xfixes0-dev libxcb-xkb-dev 
>> > libx11-xcb-dev libx11-dev libudev-dev libgbm-dev libxkbcommon-dev 
>> > libcairo2-dev libpango1.0-dev libgdk-pixbuf2.0-dev libxcursor-dev 
>> > libmtdev-dev libpam0g-dev libvpx-dev libsystemd-dev libinput-dev 
>> > libwebp-dev libjpeg-dev libva-dev liblcms2-dev git
>>
>> I think this can be simplify the explicit list to:
>> apt-get -y --no-install-recommends build-dep wayland
>
> Only if the latest version of wayland has the same build-dependencies
> as the (likely much older) version visible in the Sources index
> for the container's apt configuration. If the container is
> Debian 9 'stretch' (released about a year ago) then apt-get build-dep
> will get the build-dependencies of wayland 1.12.0, according to
> <https://tracker.debian.org/pkg/wayland>.
>
Sure, there can be differences. Adding the odd extra piece tends to be
shorter and easier (imho) than trying to keep the huge list in sync.
Distribution maintainers already that laborious job, why not make use of it.

>> > +  - make -j4
>> > +  - make check
>> > +  - make install
>> > +  - make distcheck
>> I was under the impression that "make -j4 distcheck" is enough
>
> install and check both imply (depend on) a normal build, so that one is
> redundant.
>
> distcheck runs the tests like check does, but differently-configured,
> so for full coverage you might want to do both.
>
> Similarly, distcheck runs "make install", but into a temporary directory,
> not the requested $PREFIX.
>
Right - did not spot that "prefix-*" is also collected as artefacts.

>> Alternatively it's worth setting MAKEFLAGS="-jX" once so it's used across 
>> all.
>
> Using -j would definitely help for "make distcheck".
>
> If parallel install works, it would be good to test it so that it stays
> that way: quite a lot of projects can build correctly in parallel, but
> don't work reliably for "make -jX install".
>
There has been issues in the past and everything ran fine last time
I've checked.
That's why I'm suggesting adding the toggle ;-)

I'm slightly confused if the reply is a) for, b) against or c) simply
provides more info.
In either way, the background info is always appreciated.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 2/2] Add .gitlab-ci.yml

2018-06-06 Thread Emil Velikov
On 6 June 2018 at 09:56, Pekka Paalanen  wrote:
> On Wed, 6 Jun 2018 09:22:59 +0100
> Daniel Stone  wrote:
>
>> On 6 June 2018 at 09:12, Daniel Stone  wrote:
>> > On 6 June 2018 at 09:03, Pekka Paalanen  wrote:
>> >> Distcheck does not --disable-xwayland-test, does it? So should we match
>> >> it with the autogen.sh arguments?
>> >
distcheck uses AM_DISTCHECK_CONFIGURE_FLAGS (if set in the
Makefile.am) to override the defaults.
Plus the user can further tweak things via the non AM_ version. For example

DISTCHECK_CONFIGURE_FLAGS="--disable-xwayland-test" make distcheck

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 2/2] Add .gitlab-ci.yml

2018-06-06 Thread Emil Velikov
Hi Dan,

On 5 June 2018 at 23:06, Daniel Stone  wrote:

> +  - apt-get -y --no-install-recommends install build-essential automake 
> autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev 
> libpixman-1-dev libpng-dev libjpeg-dev libcolord-dev mesa-common-dev 
> libglu1-mesa-dev libegl1-mesa-dev libgles2-mesa-dev libwayland-dev 
> libxcb1-dev libxcb-composite0-dev libxcb-xfixes0-dev libxcb-xkb-dev 
> libx11-xcb-dev libx11-dev libudev-dev libgbm-dev libxkbcommon-dev 
> libcairo2-dev libpango1.0-dev libgdk-pixbuf2.0-dev libxcursor-dev 
> libmtdev-dev libpam0g-dev libvpx-dev libsystemd-dev libinput-dev libwebp-dev 
> libjpeg-dev libva-dev liblcms2-dev git

I think this can be simplify the explicit list to:
apt-get -y --no-install-recommends build-dep wayland

> +  - mkdir -p /tmp/.X11-unix
> +  - chmod 777 /tmp/.X11-unix
> +
> +build-native:
> +  stage: build
> +  script:
> +  - git clone --depth=1 
> git://anongit.freedesktop.org/git/wayland/wayland-protocols
> +  - export WAYLAND_PROTOCOLS_DIR="$(pwd)/prefix-wayland-protocols"
> +  - export 
> PKG_CONFIG_PATH="$WAYLAND_PROTOCOLS_DIR/share/pkgconfig:$PKG_CONFIG_PATH"
> +  - cd wayland-protocols
> +  - git show -s HEAD
Is this needed?

> +  - mkdir build
> +  - cd build
> +  - ../autogen.sh --prefix="$WAYLAND_PROTOCOLS_DIR"
> +  - make install
> +  - cd ../../
> +  - export XDG_RUNTIME_DIR="$(mktemp -p $(pwd) -d xdg-runtime-XX)"
> +  - export BUILD_ID="weston-$CI_JOB_NAME_$CI_COMMIT_SHA-$CI_JOB_ID"
> +  - export PREFIX="$(pwd)/prefix-$BUILD_ID"
> +  - export BUILDDIR="$(pwd)/build-$BUILD_ID"
> +  - mkdir "$BUILDDIR" "$PREFIX"
Autotools creates $PREFIX for you.

> +  - make -j4
> +  - make check
> +  - make install
> +  - make distcheck
I was under the impression that "make -j4 distcheck" is enough and the
four separate make calls are not needed?
Alternatively it's worth setting MAKEFLAGS="-jX" once so it's used across all.

Note the artefacts regex would need a tweak if only distcheck is used.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [Mesa-dev] [PATCH i-g-t] [RFC] CONTRIBUTING: commit rights docs

2018-04-26 Thread Emil Velikov
On 24 April 2018 at 20:14, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
> On Tue, Apr 24, 2018 at 7:30 PM, Emil Velikov <emil.l.veli...@gmail.com> 
> wrote:
>> On 13 April 2018 at 11:00, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
>>> This tries to align with the X.org communities's long-standing
>>> tradition of trying to be an inclusive community and handing out
>>> commit rights fairly freely.
>>>
>>> We also tend to not revoke commit rights for people no longer
>>> regularly active in a given project, as long as they're still part of
>>> the larger community.
>>>
>>> Finally make sure that commit rights, like anything happening on fd.o
>>> infrastructre, is subject to the fd.o's Code of Conduct.
>>>
>>> v2: Point at MAINTAINERS for contact info (Daniel S.)
>>>
>>> v3:
>>> - Make it clear that commit rights are voluntary and that committers
>>>   need to acknowledge positively when they're nominated by someone
>>>   else (Keith).
>>> - Encourage committers to drop their commit rights when they're no
>>>   longer active, and make it clear they'll get readded (Keith).
>>> - Add a line that maintainers and committers should actively nominate
>>>   new committers (me).
>>>
>>> v4: Typo (Petri).
>>>
>>> v5: Typo (Sean).
>>>
>>> v6: Wording clarifications and spelling (Jani).
>>>
>>> v7: Require an explicit commitment to the documented merge criteria
>>> and rules, instead of just the implied one through the Code of Conduct
>>> threat (Jani).
>>>
>>> Acked-by: Alex Deucher <alexander.deuc...@amd.com>
>>> Acked-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
>>> Acked-by: Daniel Stone <dan...@fooishbar.org>
>>> Acked-by: Eric Anholt <e...@anholt.net>
>>> Acked-by: Gustavo Padovan <gust...@padovan.org>
>>> Acked-by: Petri Latvala <petri.latv...@intel.com>
>>> Cc: Alex Deucher <alexander.deuc...@amd.com>
>>> Cc: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
>>> Cc: Ben Widawsky <b...@bwidawsk.net>
>>> Cc: Daniel Stone <dan...@fooishbar.org>
>>> Cc: Dave Airlie <airl...@gmail.com>
>>> Cc: Eric Anholt <e...@anholt.net>
>>> Cc: Gustavo Padovan <gust...@padovan.org>
>>> Cc: Jani Nikula <jani.nik...@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
>>> Cc: Keith Packard <kei...@keithp.com>
>>> Cc: Kenneth Graunke <kenn...@whitecape.org>
>>> Cc: Kristian H. Kristensen <hoegsb...@google.com>
>>> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>>> Cc: Petri Latvala <petri.latv...@intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
>>> Cc: Sean Paul <seanp...@chromium.org>
>>> Reviewed-by: Keith Packard <kei...@keithp.com>
>>> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
>>> ---
>>> If you wonder about the wide distribution list for an igt patch: I'd
>>> like to start a discussions about x.org community norms around commit
>>> rights at large, at least for all the shared repos. I plan to propose
>>> the same text for drm-misc and libdrm too, and hopefully others like
>>> mesa/xserver/wayland would follow.
>>>
>> I think the idea is pretty good, simply highlighting some bits.
>>
>> What you've outlined in this patch has been in practise for many years:
>>  a) undocumented, applicable to most xorg projects [1]
>>  b) documented, mesa
>
> Hm, I chatted with a few mesa devs about this, and I wasn't aware
> there's explicit documentation for mesa. Where is it? I'd very much
> want to align as much as we can.
>
See the "Developer git Access" section in [1]. FWIW I prefer the
wording used in this patch and the CoC reference is a big plus.

HTH
Emil

[1] https://www.mesa3d.org/repository.html
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [Mesa-dev] [PATCH i-g-t] [RFC] CONTRIBUTING: commit rights docs

2018-04-24 Thread Emil Velikov
On 13 April 2018 at 11:00, Daniel Vetter  wrote:
> This tries to align with the X.org communities's long-standing
> tradition of trying to be an inclusive community and handing out
> commit rights fairly freely.
>
> We also tend to not revoke commit rights for people no longer
> regularly active in a given project, as long as they're still part of
> the larger community.
>
> Finally make sure that commit rights, like anything happening on fd.o
> infrastructre, is subject to the fd.o's Code of Conduct.
>
> v2: Point at MAINTAINERS for contact info (Daniel S.)
>
> v3:
> - Make it clear that commit rights are voluntary and that committers
>   need to acknowledge positively when they're nominated by someone
>   else (Keith).
> - Encourage committers to drop their commit rights when they're no
>   longer active, and make it clear they'll get readded (Keith).
> - Add a line that maintainers and committers should actively nominate
>   new committers (me).
>
> v4: Typo (Petri).
>
> v5: Typo (Sean).
>
> v6: Wording clarifications and spelling (Jani).
>
> v7: Require an explicit commitment to the documented merge criteria
> and rules, instead of just the implied one through the Code of Conduct
> threat (Jani).
>
> Acked-by: Alex Deucher 
> Acked-by: Arkadiusz Hiler 
> Acked-by: Daniel Stone 
> Acked-by: Eric Anholt 
> Acked-by: Gustavo Padovan 
> Acked-by: Petri Latvala 
> Cc: Alex Deucher 
> Cc: Arkadiusz Hiler 
> Cc: Ben Widawsky 
> Cc: Daniel Stone 
> Cc: Dave Airlie 
> Cc: Eric Anholt 
> Cc: Gustavo Padovan 
> Cc: Jani Nikula 
> Cc: Joonas Lahtinen 
> Cc: Keith Packard 
> Cc: Kenneth Graunke 
> Cc: Kristian H. Kristensen 
> Cc: Maarten Lankhorst 
> Cc: Petri Latvala 
> Cc: Rodrigo Vivi 
> Cc: Sean Paul 
> Reviewed-by: Keith Packard 
> Signed-off-by: Daniel Vetter 
> ---
> If you wonder about the wide distribution list for an igt patch: I'd
> like to start a discussions about x.org community norms around commit
> rights at large, at least for all the shared repos. I plan to propose
> the same text for drm-misc and libdrm too, and hopefully others like
> mesa/xserver/wayland would follow.
>
I think the idea is pretty good, simply highlighting some bits.

What you've outlined in this patch has been in practise for many years:
 a) undocumented, applicable to most xorg projects [1]
 b) documented, mesa

IMHO having this explicitly documented and others
(wayland/weston/wayland-protocols and xserver repos) following suite
is a big plus.

HTH
Emil

[1] As in all of https://cgit.freedesktop.org/xorg but xserver
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

2018-03-21 Thread Emil Velikov
On 21 March 2018 at 09:28, Daniel Stone  wrote:
> Hi,
>
> On 21 March 2018 at 08:27, Pekka Paalanen  wrote:
>> On Tue, 20 Mar 2018 11:46:32 +, Daniel Stone  
>> wrote:
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> how about we make it a coding style rule to not have any symbols
>> starting with an underscore?
>>
>> I don't recall any already being in the code for either Wayland or
>> Weston, but I didn't check. They certainly are not public ABI as the
>> ABI check script would have listed such.
>
> Sure, works for me.
>
Indeed very good suggestion.

>> Aren't symbols (functions, variables) starting with an underscore
>> reserved in C so should not be used anyway?
>
> My recollection is that double-underscore and underscore-uppercase is
> forbidden for use apart from the implementation / standard library,
> and anything else underscore (underscore-number or
> underscore-lowercase) can only be used for static symbols. But I'm
> sure Simon will be along shortly to correct the record. :)
>
The spec does not explicitly state "static symbols" although you're
spot on overall.

-Emil

https://stackoverflow.com/a/10688187
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

2018-03-20 Thread Emil Velikov
On 20 March 2018 at 15:09, Derek Foreman <der...@osg.samsung.com> wrote:
> On 2018-03-20 10:02 AM, Emil Velikov wrote:
>> On 20 March 2018 at 14:50, Derek Foreman <der...@osg.samsung.com> wrote:
>>> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>>>> On 20 March 2018 at 11:55, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>>>>> On 20 March 2018 at 11:46, Daniel Stone <dan...@fooishbar.org> wrote:
>>>>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>>>>> MIPS anyway:
>>>>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>>>>
>>>>>> And probably some more for ARM toolchains using other linkers:
>>>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>>>>
>>>>>> Doing a quick check across all the architectures on Debian shows that
>>>>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>>>>> need to respin for those as well, but I think at this point the
>>>>>> inconvenience of maintaining a list of every linker's implementation
>>>>>> detail on every platform, outweighs the risk of an exported
>>>>>> underscore-prefixed symbol slipping through review.
>>>>>
>>>>> I think you're preemptively worried about this. Plus there's no need
>>>>> to add every symbol a search could find.
>>>>>
>>>>> Even then, ask yourself the question:
>>>>> Which is better - updating an ugly looking list once in a blue moon or
>>>>> having apps use internal API?
>>>>>
>>>>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>>>>> We've already spend more time researching than what the [expected]
>>>>> maintenance for 1 year would be ;-)
>>>>
>>>> It all depends on how you view scope and probability.
>>>>
>>>> I don't agree my concern is preemptive: we've just shipped a beta
>>>> where 'make check' fails on a few architectures, and the proposed fix
>>>> (adding several symbols to the platform list) will still fail on
>>>> architectures Debian ships on, until it also adds _fbss, _fdata and
>>>> _ftext. I think my suggestion is reasonable: it fixes the very real
>>>> problem, in a way which avoids expanding the scope of our ABI checker
>>>> to include the implementation details of every linker/architecture
>>>> pair that people use Wayland on. The cost of this is the probability
>>>> that someone manages to add a new underscored symbol marked with
>>>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>>>> notice that in review. I think that's a reasonable tradeoff.
>>>
>>> I'm inclined to agree with Daniel here.
>>>
>>> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
>>> review seems reasonably unlikely.
>>>
>>> And after reading this thread I'm still not entirely sure we have a
>>> complete list of what should be on that whitelist, and there are only a
>>> limited number of systems I can test build on right now.
>>>
>> Sharing a few ideas that are less obvious. Having the partial list
>> allows you to:
>>  - see active developers/users - be that by merging their patches or
>> skimming through the report
>
> I don't imagine we'll be adding new API to that library frequently
> enough to mine that data productively.
>
I am talking about the C runtime symbols.

>>  - have contact point for people with unusual platforms/setups
>
> By defaulting to a broken build state for unusual platforms, do you
> really think we'll attract many contact points?
>
Quite a serious typo here - s/build/check/. From experience - yes, you will.

>>  - is fairly trivial newbie task ;-)
>
> I don't think we should be going out of our way to generate problems for
> newbies to solve.  There are plenty of worthwhile tasks for new
> participants already.
>
Might want to make the list more obvious. I've been [sort of] around
and haven't seen any.

> I've confirmed that we once again pass make check on arm (something I
> really should've done prior to the beta release.  sigh.) and have pushed
> this patch.
>
You can never have 'enough' tests before a release ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

2018-03-20 Thread Emil Velikov
On 20 March 2018 at 14:50, Derek Foreman <der...@osg.samsung.com> wrote:
> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>> On 20 March 2018 at 11:55, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>>> On 20 March 2018 at 11:46, Daniel Stone <dan...@fooishbar.org> wrote:
>>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>>> MIPS anyway:
>>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>>
>>>> And probably some more for ARM toolchains using other linkers:
>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>>
>>>> Doing a quick check across all the architectures on Debian shows that
>>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>>> need to respin for those as well, but I think at this point the
>>>> inconvenience of maintaining a list of every linker's implementation
>>>> detail on every platform, outweighs the risk of an exported
>>>> underscore-prefixed symbol slipping through review.
>>>
>>> I think you're preemptively worried about this. Plus there's no need
>>> to add every symbol a search could find.
>>>
>>> Even then, ask yourself the question:
>>> Which is better - updating an ugly looking list once in a blue moon or
>>> having apps use internal API?
>>>
>>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>>> We've already spend more time researching than what the [expected]
>>> maintenance for 1 year would be ;-)
>>
>> It all depends on how you view scope and probability.
>>
>> I don't agree my concern is preemptive: we've just shipped a beta
>> where 'make check' fails on a few architectures, and the proposed fix
>> (adding several symbols to the platform list) will still fail on
>> architectures Debian ships on, until it also adds _fbss, _fdata and
>> _ftext. I think my suggestion is reasonable: it fixes the very real
>> problem, in a way which avoids expanding the scope of our ABI checker
>> to include the implementation details of every linker/architecture
>> pair that people use Wayland on. The cost of this is the probability
>> that someone manages to add a new underscored symbol marked with
>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>> notice that in review. I think that's a reasonable tradeoff.
>
> I'm inclined to agree with Daniel here.
>
> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
> review seems reasonably unlikely.
>
> And after reading this thread I'm still not entirely sure we have a
> complete list of what should be on that whitelist, and there are only a
> limited number of systems I can test build on right now.
>
Sharing a few ideas that are less obvious. Having the partial list
allows you to:
 - see active developers/users - be that by merging their patches or
skimming through the report
 - have contact point for people with unusual platforms/setups
 - is fairly trivial newbie task ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: T480s Trackpoint drifts

2018-03-20 Thread Emil Velikov
On 20 March 2018 at 04:05, Peter Hutterer  wrote:
> On Mon, Mar 19, 2018 at 04:28:20PM +0800, Kai Hendry wrote:
>> Hi there,
>>
>> I'm an Archlinux user who has a new T480s Thinkpad and unfortunately
>> the Trackpoint can (not always or reproducibly) "drift". I hope that
>> makes sense!
>
> what exactly is drift in this context? it moves on its own to the edge of
> the screen?
>
FWIW I've experienced something that sounds like this.

Sharing some experience - it's _not_ mean to hijack the thread.

While the cursor is drifting in direction X, one can hardly move it
any another direction.
Unsurprisingly, the drift continues even after the cursor reaches the
end of the screen.

The only way to get it unstuck is by briefly moving the trackpoint in
the exact same direction where the drift is.

I've experienced that on x220 and X1C3 machines. Both running
Archlinux, in case it matters.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

2018-03-20 Thread Emil Velikov
On 20 March 2018 at 12:11, Daniel Stone <dan...@fooishbar.org> wrote:
> On 20 March 2018 at 11:55, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 20 March 2018 at 11:46, Daniel Stone <dan...@fooishbar.org> wrote:
>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>> MIPS anyway:
>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>
>>> And probably some more for ARM toolchains using other linkers:
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>
>>> Doing a quick check across all the architectures on Debian shows that
>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>> need to respin for those as well, but I think at this point the
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> I think you're preemptively worried about this. Plus there's no need
>> to add every symbol a search could find.
>>
>> Even then, ask yourself the question:
>> Which is better - updating an ugly looking list once in a blue moon or
>> having apps use internal API?
>>
>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>> We've already spend more time researching than what the [expected]
>> maintenance for 1 year would be ;-)
>
> It all depends on how you view scope and probability.
>
> I don't agree my concern is preemptive: we've just shipped a beta
> where 'make check' fails on a few architectures, and the proposed fix
> (adding several symbols to the platform list) will still fail on
> architectures Debian ships on, until it also adds _fbss, _fdata and
> _ftext. I think my suggestion is reasonable: it fixes the very real
> problem, in a way which avoids expanding the scope of our ABI checker
> to include the implementation details of every linker/architecture
> pair that people use Wayland on. The cost of this is the probability
> that someone manages to add a new underscored symbol marked with
> WL_EXPORT into a library which is currently 105 LoC, and have no-one
> notice that in review. I think that's a reasonable tradeoff.
>
Right, hopefully there won't be any internals leaked/used by other projects.
I reserve myself the right of the lovely "I told you so".

Feel free to continue with whichever solution you feel comfortable with.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

2018-03-20 Thread Emil Velikov
On 20 March 2018 at 11:46, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Emil,
>
> On 20 March 2018 at 11:30, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 20 March 2018 at 11:01, Daniel Stone <dani...@collabora.com> wrote:
>>> Rather than a hard-coded list of platform symbols, just ignore anything
>>> prefaced with an underscore. This fixes breakage on ARM, which declares
>>> several slightly different platform symbols to x86.
>>
>> FWIW I've explicitly opted against this kind of heuristics for a few reasons:
>>
>>  - projects use single/double underscored symbols for internal API
>>  - other projects have been using such symbols knowingly that it's internal 
>> API
>> Last but not least
>>  - the symbols exposed by the C runtime should stay stable
>>
>> In other words - prevents others from going silly things, only the
>> list might need 1-2 updates.
>> If the latter turns out to be false we can nuke it.
>
> Sure. As on IRC though, we definitely need to add at least _ftext for
> MIPS anyway:
> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>
> And probably some more for ARM toolchains using other linkers:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>
> Doing a quick check across all the architectures on Debian shows that
> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
> need to respin for those as well, but I think at this point the
> inconvenience of maintaining a list of every linker's implementation
> detail on every platform, outweighs the risk of an exported
> underscore-prefixed symbol slipping through review.
>
I think you're preemptively worried about this. Plus there's no need
to add every symbol a search could find.

Even then, ask yourself the question:
Which is better - updating an ugly looking list once in a blue moon or
having apps use internal API?

As said above: _if_ it turns out to be labour intensive - fine remove it.
We've already spend more time researching than what the [expected]
maintenance for 1 year would be ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

2018-03-20 Thread Emil Velikov
On 20 March 2018 at 11:01, Daniel Stone  wrote:
> Rather than a hard-coded list of platform symbols, just ignore anything
> prefaced with an underscore. This fixes breakage on ARM, which declares
> several slightly different platform symbols to x86.
>
FWIW I've explicitly opted against this kind of heuristics for a few reasons:

 - projects use single/double underscored symbols for internal API
 - other projects have been using such symbols knowingly that it's internal API
Last but not least
 - the symbols exposed by the C runtime should stay stable

In other words - prevents others from going silly things, only the
list might need 1-2 updates.
If the latter turns out to be false we can nuke it.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 2/2] .gitignore: add wayland-egl-abi-check

2018-03-20 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Instruct git go ignore the file, in case we've done an in-tree build.

Cc: Derek Foreman <der...@osg.samsung.com>
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
I've opted for the shortest fix - add an entry instead of renaming
files, updating build rules, etc.

If people really want to we can opt for that route.
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 8da9861..eadea12 100644
--- a/.gitignore
+++ b/.gitignore
@@ -43,5 +43,6 @@ Makefile
 Makefile.in
 exec-fd-leak-checker
 fixed-benchmark
+/wayland-egl-abi-check
 /wayland-scanner
 protocol/*.[ch]
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 1/2] wayland-egl-symbols-check: add ARM specific glib entrypoints

2018-03-20 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Seems like glibc on ARM (32 or 64 bit) exports a few extra symbols.
Add them to the list.

Quick look reveals that those are aliases of the already listed symbols,
and have been added to glibc since day 1 :-\

Fixes: 21b1f22eb05 ("wayland-egl: enhance the symbol test")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105620
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl-symbols-check | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 70fe1f4..6e6655e 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -17,7 +17,11 @@ fi
 AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
 
 # Platform specific symbols.
-PLAT_FUNCS="__bss_start
+PLAT_FUNCS="__bss_end__
+__bss_start
+__bss_start__
+__end__
+_bss_end__
 _edata
 _end
 _fini
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Emil Velikov
On 19 March 2018 at 13:39, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Emil,
>
> On 19 March 2018 at 13:27, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 19 March 2018 at 13:20, Daniel Stone <dan...@fooishbar.org> wrote:
>>> Me neither really, but it seemed best for consistency with the rest of
>>> the file which used test rather than [.
>>
>> Sounds fine either way - but the "test ||" -> "if test" changes seems 
>> spurious.
>> If they stand out so much, guess one could have pointed it out?
>
> Not so much spurious as just broken? The final line, as written, will
> only exit if _both_ added and removed are non-empty. If one but not
> the other is non-empty, then the test will exit success[0].
>
Right s/||/&&/ should address that

> I tried to think of a rewrite which would work, but in the end decided
> making it explicit was the least error-prone thing to do, since this
> one managed to slip past review with no-one noticing.
>
Fair point.

Thanks!
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Emil Velikov
On 19 March 2018 at 13:20, Daniel Stone  wrote:
> Hi Quentin,
> Thanks for the quick review!
>
> On 19 March 2018 at 13:06, Quentin Glidic
>  wrote:
>> On 3/19/18 1:31 PM, Daniel Stone wrote:
>>> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>>> index c47026b2..d1a4a6be 100755
>>> --- a/egl/wayland-egl-symbols-check
>>> +++ b/egl/wayland-egl-symbols-check
>>> @@ -1,11 +1,17 @@
>>>   #!/bin/sh
>>>   set -eu
>>>   +RET=0
>>>   LIB=${WAYLAND_EGL_LIB}
>>>   -if [ ! -f "$LIB" ]; then
>>> -   echo "The test binary \"$LIB\" does no exist"
>>> -   exit 1
>>> +if ! test -f "$LIB"; then
>>> +   echo "Test binary \"$LIB\" does not exist"
>>> +   exit 99
>>> +fi
>>
>> Not a big fan of replacing [!-f] with !test-f but it’s a cosmetic issue so
>> not a big deal.
>
> Me neither really, but it seemed best for consistency with the rest of
> the file which used test rather than [.
>
Sounds fine either way - but the "test ||" -> "if test" changes seems spurious.
If they stand out so much, guess one could have pointed it out?

>>> +if ! test -x "$NM"; then
>>> +   echo "nm binary \"$NM\" does not exist"
>>> +   exit 99
>>>   fi
>>
>> Here, however, you are introducing an -x test, which is not good for all the
>> people (including packagers) that set NM to e.g. -nm (so not the
>> full path).
>
> I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe
> replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it
> works if it's non-empty is fine too.
>
I'd go with non-empty - everything else seems like an overkill.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-19 Thread Emil Velikov
On 19 March 2018 at 09:56, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Fri, 16 Mar 2018 16:18:57 +0000
> Emil Velikov <emil.l.veli...@gmail.com> wrote:
>
>> On 16 March 2018 at 13:52, Pekka Paalanen <ppaala...@gmail.com> wrote:
>> > On Thu, 15 Mar 2018 14:30:27 +
>> > Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> >
>> >> From: Emil Velikov <emil.veli...@collabora.com>
>> >>
>> >> Based on a similar patch (in Mesa) by Eric Engestrom.
>> >>
>> >> v2: Rebase on top of $NM patch
>> >> v3: Rebase
>> >>
>> >> Reviewed-by: Eric Engestrom <e...@engestrom.ch> (v1)
>> >> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
>> >> ---
>> >>  egl/wayland-egl-symbols-check | 10 +-
>> >>  1 file changed, 9 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> >> index 6ad28f3..8b3d711 100755
>> >> --- a/egl/wayland-egl-symbols-check
>> >> +++ b/egl/wayland-egl-symbols-check
>> >> @@ -1,6 +1,14 @@
>> >>  #!/bin/sh
>> >> +set -eu
>> >>
>> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut 
>> >> -c 3- | while read func; do
>> >> +LIB=${WAYLAND_EGL_LIB}
>> >> +
>> >> +if [ ! -f "$LIB" ]; then
>> >> + echo "The test binary \"$LIB\" does no exist"
>> >> + exit 1
>> >> +fi
>> >> +
>> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
>> >> read func; do
>> >>  ( grep -q "^$func$" || echo $func )  <> >>  wl_egl_window_resize
>> >>  wl_egl_window_create
>> >
>> > Hi Emil,
>> >
>> > this patch makes distcheck fail with:
>> >
>> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
>> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
>> >
>> Right - I was aiming to remove the bonkers envvar and forgot about
>> this preexisting bug.
>> Patch (to be applied before the series) coming in a second.
>
> Hi Emil,
>
> because this set of four patches no longer regresses, and I think they
> very much need to be in the release, I have pushed them:
>f34af17..5f5945b  master -> master
>
> However, I did a quick test: I renamed wl_egl_window_resize() into
> wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> symbol not present, extra symbol. The test suite still passes with
> success.
>
> In egl/wayland-egl-symbols-check.log I have:
>
> ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
>
Seems like a 'fun' side-effect of the $NM patch. The series predates
the patch by a week.
First suggestion that comes to mind - use $(NM-nm}

Eric any suggestions?

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] configure.ac: fixup systemd/systemd-login detection

2018-03-16 Thread Emil Velikov
On 15 March 2018 at 17:23, Michael Tretter <m.tret...@pengutronix.de> wrote:
> On Thu, 15 Mar 2018 14:20:21 +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.veli...@collabora.com>
>>
>> Current systemd/systemd-login integration requires dbus. Although that
>> is far from clear the way current checks are handled.
>>
>> Be explicit and clear, effectively fixing cases where the systemd
>> auto detection will trip when dbus is explicitly disabled.
>>
>> Using git show -w will make things easier to read ;-)
>>
>> v2: auto disable systemd as dbus is missing
>>
>> Cc: Michael Tretter <m.tret...@pengutronix.de>
>> Reported-by: Michael Tretter <m.tret...@pengutronix.de>
>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
>> ---
>> Thanks for the review Michael!
>>
>> You're right - the else hunk is only for consistency with surrounding
>> code.
>> ---
>>  configure.ac | 28 +---
>>  1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 0b326ccc..a3453d15 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -524,7 +524,12 @@ AC_ARG_ENABLE(systemd-login,
>>AS_HELP_STRING([--enable-systemd-login],
>>   [Enable logind support]),,
>>enable_systemd_login=auto)
>> -if test x$enable_systemd_login != xno -a x$have_dbus != xno; then
>> +
>> +if test x$enable_systemd_login = xyes -a x$enable_dbus = xno; then
>> +  AC_MSG_ERROR([systemd-login support explicitly requested, but dbus not 
>> available])
>> +fi
>> +
>> +if test x$enable_systemd_login = xauto -a x$enable_dbus = xno; then
>
> If systemd-login is explicitly enabled, it will not enable
> systemd-login. The second condition is also wrong, because dbus must be
> enabled (or auto). It should read
>
> if test x$enable_systemd_login != xno -a x$enable_dbus != xno; then
>
Indeed, things are fiddly and subtle.

Pekka pulled your simpler fix so this lovely patch can go in the bin.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] wayland-egl: set the correct path to libwayland-egl.so

2018-03-16 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Earlier commit changed to passing the binary name as env. variable
introducing a typo.

That went unnoticed, since we do not check if the file is present or
not.

Cc: Pukka Paalanen <ppaala...@gmail.com>
Cc: Daniel Stone <dani...@collabora.com>
Fixes: 85cb5ed64aa ("wayland-egl-symbols-check: pass the DSO name via
the build system")
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 2731ee7..6f59c36 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -196,7 +196,7 @@ AM_TESTS_ENVIRONMENT =  
\
export WAYLAND_SCANNER='$(top_builddir)/wayland-scanner'\
TEST_DATA_DIR='$(top_srcdir)/tests/data'\
TEST_OUTPUT_DIR='$(top_builddir)/tests/output'  \
-   WAYLAND_EGL_LIB='$(top_builddir)/egl/.libs/libwayland-egl.so'   \
+   WAYLAND_EGL_LIB='$(top_builddir)/.libs/libwayland-egl.so'   \
SED=$(SED)  \
;
 
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-16 Thread Emil Velikov
On 16 March 2018 at 13:52, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Thu, 15 Mar 2018 14:30:27 +0000
> Emil Velikov <emil.l.veli...@gmail.com> wrote:
>
>> From: Emil Velikov <emil.veli...@collabora.com>
>>
>> Based on a similar patch (in Mesa) by Eric Engestrom.
>>
>> v2: Rebase on top of $NM patch
>> v3: Rebase
>>
>> Reviewed-by: Eric Engestrom <e...@engestrom.ch> (v1)
>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
>> ---
>>  egl/wayland-egl-symbols-check | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> index 6ad28f3..8b3d711 100755
>> --- a/egl/wayland-egl-symbols-check
>> +++ b/egl/wayland-egl-symbols-check
>> @@ -1,6 +1,14 @@
>>  #!/bin/sh
>> +set -eu
>>
>> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 
>> 3- | while read func; do
>> +LIB=${WAYLAND_EGL_LIB}
>> +
>> +if [ ! -f "$LIB" ]; then
>> + echo "The test binary \"$LIB\" does no exist"
>> + exit 1
>> +fi
>> +
>> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
>> read func; do
>>  ( grep -q "^$func$" || echo $func )  <>  wl_egl_window_resize
>>  wl_egl_window_create
>
> Hi Emil,
>
> this patch makes distcheck fail with:
>
> The test binary "./egl/.libs/libwayland-egl.so" does no exist
> FAIL egl/wayland-egl-symbols-check (exit status: 1)
>
Right - I was aiming to remove the bonkers envvar and forgot about
this preexisting bug.
Patch (to be applied before the series) coming in a second.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protcols v2] unstable: add xdg-toplevel-decoration protocol

2018-03-16 Thread Emil Velikov
On 15 March 2018 at 15:43, Simon Ser <cont...@emersion.fr> wrote:
> On March 15, 2018 4:15 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 2 March 2018 at 15:33, Simon Ser <cont...@emersion.fr> wrote:
>> > This adds a new protocol to negotiate server- and client-side rendering of
>> > window decorations for xdg-toplevels. This allows compositors that want
>> > to draw decorations themselves to send their preference to clients, and
>> > clients that prefer server-side decorations to request them.
>> >
>> > This is inspired by a protocol from KDE [1] which has been implemented in
>> > KDE and Sway and was submitted for consideration in 2017 [2]. This patch
>> > provides an updated protocol with those concerns taken into account.
>> >
>> > Signed-off-by: Simon Ser <cont...@emersion.fr>
>> > Reviewed-by: Drew DeVault <s...@cmpwn.com>
>> > Reviewed-by: David Edmundson <da...@davidedmundson.co.uk>
>> > Reviewed-by: Alan Griffiths <alan.griffi...@canonical.com>
>> > Reviewed-by: Tony Crisci <t...@dubstepdish.com>
>> >
>> More of a fly-by comment.
>>
>> Having a quick look does not list any of these R-B tags making it to the 
>> list.
>> Was it done in private, or I failed at searching?
>
> Yeah, it was done prior to submission to wayland-devel. See the commentary 
> below
> the commit message:
>
>> This was iterated on privately between representatives of Sway and wlroots
>> (Simon Ser, Drew DeVault and Tony Crisci), KDE and Qt (David Edmundson), and
>> Mir (Alan Griffiths).
>
Right, this says "iterated". Even then an official "Reviewed-by" ought
to happen in the open.

Anyway it was a JFYI
-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protcols v2] unstable: add xdg-toplevel-decoration protocol

2018-03-15 Thread Emil Velikov
On 2 March 2018 at 15:33, Simon Ser  wrote:
> This adds a new protocol to negotiate server- and client-side rendering of
> window decorations for xdg-toplevels. This allows compositors that want
> to draw decorations themselves to send their preference to clients, and
> clients that prefer server-side decorations to request them.
>
> This is inspired by a protocol from KDE [1] which has been implemented in
> KDE and Sway and was submitted for consideration in 2017 [2]. This patch
> provides an updated protocol with those concerns taken into account.
>
> Signed-off-by: Simon Ser 
> Reviewed-by: Drew DeVault 
> Reviewed-by: David Edmundson 
> Reviewed-by: Alan Griffiths 
> Reviewed-by: Tony Crisci 
>
More of a fly-by comment.

Having a quick look does not list any of these R-B tags making it to the list.
Was it done in private, or I failed at searching?

If the former - it's very concerning practise.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [weston PATCH] simple-dmabuf-drm: support etnaviv drm as well

2018-03-15 Thread Emil Velikov
On 15 March 2018 at 13:58, Pekka Paalanen  wrote:
> On Mon, 12 Mar 2018 16:41:46 +0100
> Guido Günther  wrote:
>
>> Since freedreno and etnaviv can live in parallel allow to build in
>> different DRM backends.
>> ---
>>  Makefile.am |   6 ++-
>>  clients/simple-dmabuf-drm.c | 122 
>> +++-
>>  configure.ac|  29 +++
>>  3 files changed, 121 insertions(+), 36 deletions(-)
>
> Hi,
>
> this seems mostly fine, but there are some issues noted below. It would
> be appropriate to split this into two patches: "allow intel and
> freedreno to be built together" and "add etnaviv support".
>
Just throwing it out there - one might be able to utilise the
gbm_bo_{un,}map API.
Haven't looked at the weston code, yet ^^ was sufficient for the
piglit tests + kmscube.

Less code, fewer vendor quirks ;-)

HTH
-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v3 3/3] wayland-egl: bump the version number to 18.1.0

2018-03-15 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Seems like I was overoptimistic with my earlier assumption, namely:

"... 17.3.x should be the last version that ships the library."

Mesa 18.0.0 and its wayland-egl is about to be released any time soon,
so bump the number since it must no be smaller. As soon as we get
a wayland release I'll drop the Mesa copy but for now.

Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl.pc.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/egl/wayland-egl.pc.in b/egl/wayland-egl.pc.in
index 943442e..2e2d4c4 100644
--- a/egl/wayland-egl.pc.in
+++ b/egl/wayland-egl.pc.in
@@ -5,7 +5,7 @@ includedir=@includedir@
 
 Name: wayland-egl
 Description: Frontend wayland-egl library
-Version: 17.4.0
+Version: 18.1.0
 Requires: wayland-client
 Libs: -L${libdir} -lwayland-egl
 Cflags: -I${includedir}
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v3 2/3] wayland-egl: enhance the symbol test

2018-03-15 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

The current test had a few fall-outs:
 - it was checking only for T (.text) symbols
 - did not consider symbol removal

Fix that by fetching all the symbols and doing a bidirectional check -
for added and removed symbols. Error out with informative message for
each case.

v2: Rebase on top of $NM patch.

Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl-symbols-check | 36 
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 8b3d711..c47026b 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -8,17 +8,37 @@ if [ ! -f "$LIB" ]; then
exit 1
 fi
 
-FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
-( grep -q "^$func$" || echo $func )  <https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-15 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Based on a similar patch (in Mesa) by Eric Engestrom.

v2: Rebase on top of $NM patch
v3: Rebase

Reviewed-by: Eric Engestrom <e...@engestrom.ch> (v1)
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl-symbols-check | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 6ad28f3..8b3d711 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,14 @@
 #!/bin/sh
+set -eu
 
-FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 3- 
| while read func; do
+LIB=${WAYLAND_EGL_LIB}
+
+if [ ! -f "$LIB" ]; then
+   echo "The test binary \"$LIB\" does no exist"
+   exit 1
+fi
+
+FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
 ( grep -q "^$func$" || echo $func )  <https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v2] configure.ac: fixup systemd/systemd-login detection

2018-03-15 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Current systemd/systemd-login integration requires dbus. Although that
is far from clear the way current checks are handled.

Be explicit and clear, effectively fixing cases where the systemd
auto detection will trip when dbus is explicitly disabled.

Using git show -w will make things easier to read ;-)

v2: auto disable systemd as dbus is missing

Cc: Michael Tretter <m.tret...@pengutronix.de>
Reported-by: Michael Tretter <m.tret...@pengutronix.de>
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
Thanks for the review Michael!

You're right - the else hunk is only for consistency with surrounding
code.
---
 configure.ac | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 0b326ccc..a3453d15 100644
--- a/configure.ac
+++ b/configure.ac
@@ -524,7 +524,12 @@ AC_ARG_ENABLE(systemd-login,
   AS_HELP_STRING([--enable-systemd-login],
  [Enable logind support]),,
   enable_systemd_login=auto)
-if test x$enable_systemd_login != xno -a x$have_dbus != xno; then
+
+if test x$enable_systemd_login = xyes -a x$enable_dbus = xno; then
+  AC_MSG_ERROR([systemd-login support explicitly requested, but dbus not 
available])
+fi
+
+if test x$enable_systemd_login = xauto -a x$enable_dbus = xno; then
   PKG_CHECK_MODULES(SYSTEMD_LOGIN,
 [libsystemd >= 209],
 [have_systemd_login_209=yes;have_systemd_login=yes],
@@ -537,21 +542,22 @@ if test x$enable_systemd_login != xno -a x$have_dbus != 
xno; then
   [have_systemd_login=yes],
   [have_systemd_login=no])
 ])
+
+  if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
then
+AC_MSG_ERROR([systemd-login support explicitly requested, but can't find 
libsystemd>=209 or libsystemd-login])
+  fi
+
+  if test "x$have_systemd_login" = "xyes"; then
+AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])]
+  fi
+
+  AS_IF([test "x$have_systemd_login_209" = "xyes"],
+[AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 
209])])
 else
   have_systemd_login=no
 fi
-
-if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
then
-  AC_MSG_ERROR([systemd-login support explicitly enabled, but can't find 
libsystemd>=209, libsystemd-login or dbus])
-fi
-
-AS_IF([test "x$have_systemd_login" = "xyes"],
-  [AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])])
 AM_CONDITIONAL(HAVE_SYSTEMD_LOGIN, test "x$have_systemd_login" = "xyes")
 
-AS_IF([test "x$have_systemd_login_209" = "xyes"],
-  [AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 209])])
-
 
 # Note that other features might want libxml2, or this feature might use
 # alternative xml libraries at some point. Therefore the feature and
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v2] build: honour libinput header location

2018-03-15 Thread Emil Velikov
From: Jan Engelhardt <jeng...@inai.de>

Add the respective CFLAGS to the build, otherwise it will error out as
seen below.

src/libinput-seat.c:30:22: fatal error: libinput.h: No such file or directory

v2: add the CFLAGS only as needed, suggested by Pekka

Cc: Pekka Paalanen <pekka.paala...@collabora.co.uk>
Cc: Jan Engelhardt <jeng...@inai.de>
[Emil Velikov: polish commit message, v2]
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
Pekka feel free to pick whichever patch you feel more confortable with.
---
 Makefile.am | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index e028a2a1..7e930d2b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -178,7 +178,7 @@ weston_LDFLAGS = -export-dynamic
 weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON   \
 -DMODULEDIR='"$(moduledir)"' \
 -DXSERVER_PATH='"@XSERVER_PATH@"'
-weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
+weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBINPUT_BACKEND_CFLAGS)
 weston_LDADD = libshared.la libweston-@LIBWESTON_MAJOR@.la \
$(COMPOSITOR_LIBS) \
$(DL_LIBS) $(LIBINPUT_BACKEND_LIBS) \
@@ -346,6 +346,7 @@ x11_backend_la_SOURCES =\
shared/helpers.h
 endif
 
+INPUT_BACKEND_CFLAGS = $(LIBINPUT_BACKEND_CFLAGS)
 INPUT_BACKEND_LIBS = $(LIBINPUT_BACKEND_LIBS)
 INPUT_BACKEND_SOURCES =\
libweston/libinput-seat.c   \
@@ -369,6 +370,7 @@ drm_backend_la_CFLAGS = \
$(COMPOSITOR_CFLAGS)\
$(EGL_CFLAGS)   \
$(DRM_COMPOSITOR_CFLAGS)\
+   $(INPUT_BACKEND_CFLAGS) \
$(AM_CFLAGS)
 drm_backend_la_SOURCES =   \
libweston/compositor-drm.c  \
@@ -443,6 +445,7 @@ fbdev_backend_la_CFLAGS =   \
$(EGL_CFLAGS)   \
$(FBDEV_COMPOSITOR_CFLAGS)  \
$(PIXMAN_CFLAGS)\
+   $(INPUT_BACKEND_CFLAGS) \
$(AM_CFLAGS)
 fbdev_backend_la_SOURCES = \
libweston/compositor-fbdev.c\
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] build: honour libinput header location

2018-03-15 Thread Emil Velikov
On 15 March 2018 at 13:11, Jan Engelhardt  wrote:
>
> On Thursday 2018-03-15 13:20, Pekka Paalanen wrote:
>>> diff --git a/Makefile.am b/Makefile.am
>>> index b5c29c04..8de40e51 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -43,7 +43,8 @@ AM_CPPFLAGS =  \
>>>  -I$(top_builddir)/protocol  \
>>>  -DLIBWESTON_MODULEDIR='"$(libweston_moduledir)"' \
>>>  -DLIBEXECDIR='"$(libexecdir)"'  \
>>> --DBINDIR='"$(bindir)"'
>>> +-DBINDIR='"$(bindir)"' \
>>> +$(LIBINPUT_BACKEND_CFLAGS)
>>>
>>>  CLEANFILES = weston.ini \
>>>  ivi-shell/weston.ini\
>>
>>I don't think we want the libinput flags on *everything*
>
> It does not really matter, the libinput API is not bad.
> Not nearly as bad as some science software, anyhow.
>
> The "only" downside of using target_CPPFLAGS is that
> (a) the generated Makefile, Makefile.in gets bigger - somewhat larger release 
> tarball
> (b) sources part of multiple targets can get built twice to accomodate 
> invocations
> with different CPPFLAGS, even if the different CPPFLAGS still lead to the
> same gcc -E output.
>
Or in a Tl;Dr - there are no serious issues either way.
Proposed patch/result is shorter. Regardless, will send v2 in a moment.

>
>>Or do you think we should actually have CFLAGS of all deps with
>>everything we build, and just gate the LIBS we link with to the targets
>>that actually need it?
>
> That's the easy way, and the one requiring the fewest keystrokes IMO.
Agreed. Some projects opt for unconditionally add all CFLAGS, while
others prefer to keep it conditional.

In either case, I'd welcome if any notable changes/reshuffle do not
hold this* fix.
Thanks
Emil

* ... or analogous of course.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston] configure.ac: fixup systemd/systemd-login detection

2018-03-14 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Current systemd/systemd-login integration requires dbus. Although that
is far from clear the way current checks are handled.

Be explicit and clear, effectively fixing cases where the systemd
auto detection will trip when dbus is explicitly disabled.

Using git show -w will make things easier to read ;-)

Cc: Michael Tretter <m.tret...@pengutronix.de>
Reported-by: Michael Tretter <m.tret...@pengutronix.de>
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
Michael, can you give this a try instead?
---
 configure.ac | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index dd344d6a..89b89d3c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -524,7 +524,11 @@ AC_ARG_ENABLE(systemd-login,
   AS_HELP_STRING([--enable-systemd-login],
  [Enable logind support]),,
   enable_systemd_login=auto)
-if test x$enable_systemd_login != xno -a x$have_dbus != xno; then
+if test x$enable_systemd_login != xno; then
+  if test x$enable_systemd_login = xyes -a x$enable_dbus = xno; then
+AC_MSG_ERROR([systemd-login support explicitly requested, but dbus not 
available])
+  fi
+
   PKG_CHECK_MODULES(SYSTEMD_LOGIN,
 [libsystemd >= 209],
 [have_systemd_login_209=yes;have_systemd_login=yes],
@@ -537,20 +541,21 @@ if test x$enable_systemd_login != xno -a x$have_dbus != 
xno; then
   [have_systemd_login=yes],
   [have_systemd_login=no])
 ])
-else
-  have_systemd_login=no
-fi
 
-if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
then
-  AC_MSG_ERROR([systemd-login support explicitly enabled, but can't find 
libsystemd>=209, libsystemd-login or dbus])
-fi
+  if test "x$have_systemd_login" = "xno" -a "x$enable_systemd_login" = "xyes"; 
then
+AC_MSG_ERROR([systemd-login support explicitly requested, but can't find 
libsystemd>=209 or libsystemd-login])
+  fi
 
-AS_IF([test "x$have_systemd_login" = "xyes"],
-  [AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])])
-AM_CONDITIONAL(HAVE_SYSTEMD_LOGIN, test "x$have_systemd_login" = "xyes")
+  if test "x$have_systemd_login" = "xyes"; then
+AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])]
+  else
+have_systemd_login=no
+  fi
 
-AS_IF([test "x$have_systemd_login_209" = "xyes"],
-  [AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 209])])
+  AS_IF([test "x$have_systemd_login_209" = "xyes"],
+[AC_DEFINE([HAVE_SYSTEMD_LOGIN_209], [1], [Have systemd-login >= 
209])])
+fi
+AM_CONDITIONAL(HAVE_SYSTEMD_LOGIN, test "x$have_systemd_login" = "xyes")
 
 
 # Note that other features might want libxml2, or this feature might use
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: weston-launch: libEGL: no configs with appropriate attributes

2018-03-14 Thread Emil Velikov
On 14 March 2018 at 11:36, Pekka Paalanen  wrote:
> On Wed, 14 Mar 2018 11:55:54 +0100
> Král Gergely  wrote:
>
>> Hi,
>>
>>
>> On my old notebook I would like to set up a test environment by running
>> weston and let clients to connect to it. I am stuck in the very
>> beginning and after almost a day of googling and trying to understand
>> documentation that are over my expertise I am asking your help.
>>
>> According to the weston building page the radeon RV200 video card should
>> work with wayland and weston. The DRM looks OK to me. Am I missing
>> something? How could I further debug to see what is exactly failing?
>>
>> Any comment is appreciated.
>>
>>
>> root$ weston-launch
>> Date: 2018-03-14 CET
>> [00:08:15.465] weston 1.12.0
>> http://wayland.freedesktop.org
>> Bug reports to:
>> https://bugs.freedesktop.org/enter_bug.cgi?product=Wayland=weston=1.12.0
>> Build: 1.11.94-2-ga08dff5 configure.ac: bump to version
>> 1.12.0 for the official release (2016-09-20 12:22:46 -0700)
>> [00:08:15.465] Command line: /usr/bin/weston
>> [00:08:15.465] OS: Linux, 4.9.0-4-686-pae, #1 SMP Debian 4.9.65-3
>> (2017-12-03), i686
>> [00:08:15.466] Using config file '/root/.config/weston.ini'
>> [00:08:15.466] Output repaint window is 7 ms maximum.
>> [00:08:15.466] Loading module
>> '/usr/lib/i386-linux-gnu/libweston-1/drm-backend.so'
>> [00:08:15.475] initializing drm backend
>> [00:08:15.476] logind: not running in a systemd session
>> [00:08:15.476] logind: cannot setup systemd-logind helper (-6), using
>> legacy fallback
>> [00:08:15.479] using /dev/dri/card0
>> [00:08:15.479] Loading module
>> '/usr/lib/i386-linux-gnu/libweston-1/gl-renderer.so'
>> libGL: Can't open configuration file /root/.drirc: No such file or
>> directory.
>> [00:08:15.514] EGL client extensions: EGL_EXT_client_extensions
>> EGL_EXT_platform_base EGL_EXT_platform_wayland
>> EGL_EXT_platform_x11 EGL_MESA_platform_gbm
>> EGL_KHR_client_get_all_proc_addresses EGL_KHR_debug
>> libEGL debug: added egl_dri2 to module array
>> libEGL debug: No DRI config supports native format 0x30335258
>> libEGL debug: No DRI config supports native format 0x30335241
>
> Hi,
>
> not sure where this comes from, and I'm not sure it wouldn't cause the
> below.
>
>> libEGL debug: the best driver is DRI2
>> libEGL debug: the value (0x9) of attribute 0x3040 did not meet the
>> criteria (0x4)
>
> The attribute is EGL_RENDERABLE_TYPE, where the system has OPENGL and
> OPENGL_ES, while Weston needs OPENGL_ES2.
>
r100/r200 only supports GL 1.3 and GLES1 1.1

One can easily check the above for any card, via glxinfo | grep
"Max.*profile version" ;-)

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"

2018-03-13 Thread Emil Velikov
On 12 March 2018 at 11:25, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi,
>
> On 12 March 2018 at 11:21, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 9 March 2018 at 11:09, Daniel Stone <dan...@fooishbar.org> wrote:
>>> On 9 March 2018 at 10:59, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>>>>  - above all, the internal path is a 'dummy' fallback. anyone can
>>>> provide the binary name as an argument
>>>> $ .../wayland-egl-symbols-check .../libwayland-egl.so
>>>>  - since we have a fallback - a plain .../wayland-egl-symbols-check
>>>> will work most of the time
>>>
>>> That makes sense, running it from the build root. Is that just because
>>> 'make check' is slow, or? (sanity-test is really slow.)
>>>
>> Short back story: I was playing with OBS and getting the build
>> artefacts (the contents of a failing test) was a pain.
>> Admittedly, there may be another way to handle that, although in general:
>>
>> Passing the file as argument makes debugging a lot quicker/easier.
>
> Sure, that makes sense. I have a suggestion though: why don't we try
> argv[1] first, then fall back to $WAYLAND_EGL_LIB, and error out if
> neither are set? That way we don't hardcode internal autotools
> implementation details into our own scripts (remove fallback
> definition), allow autotools tests to run easily (environment
> variable), and allow people to use it manually for debugging
> (command-line argument). For the latter, I think being explicit is
> better: it means you can't accidentally forget to set an environment
> variable and end up testing the wrong thing.
>
In all honesty keeping things simple was my priority.
I was hoping for some compromise, but it seems like my allowance is out :-(

Modulo any objections, I'll just respin the series to drop this patch
all together.
People are open to hack things up when they hit a problem.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] configure.ac: don't install the static libraries

2018-03-13 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

One should always be using the shared libraries.

Spotted while going through the Debian packaing.

Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
Can we have this in the 1.5 release, please?
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index a53b6cc..26e517a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -44,7 +44,7 @@ AM_CONDITIONAL(ENABLE_CPP_TEST, test "x$have_cpp_compiler" = 
"xyes")
 
 # Initialize libtool
 LT_PREREQ([2.2])
-LT_INIT
+LT_INIT([disable-static])
 
 PKG_PROG_PKG_CONFIG()
 
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"

2018-03-12 Thread Emil Velikov
On 9 March 2018 at 11:09, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Emil,
>
> On 9 March 2018 at 10:59, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 9 March 2018 at 09:47, Daniel Stone <dan...@fooishbar.org> wrote:
>>> Patches 2-4 look fine and I'm happy to merge them with my review, but
>>> could you please explain some more about this patch? I very much like
>>> keeping details of the build system (specifically its internal build
>>> paths) in the build system itself and not in the script. I was
>>> assuming something in 2-4 needed this revert to be applied, but
>>> couldn't see anything. Is there something I'm missing?
>>
>> There is one word to describe it - compromise:
>>
>>  - above all, the internal path is a 'dummy' fallback. anyone can
>> provide the binary name as an argument
>> $ .../wayland-egl-symbols-check .../libwayland-egl.so
>>  - since we have a fallback - a plain .../wayland-egl-symbols-check
>> will work most of the time
>
> That makes sense, running it from the build root. Is that just because
> 'make check' is slow, or? (sanity-test is really slow.)
>
Short back story: I was playing with OBS and getting the build
artefacts (the contents of a failing test) was a pain.
Admittedly, there may be another way to handle that, although in general:

Passing the file as argument makes debugging a lot quicker/easier.

>>  - handling env. variables (as opposed to arguments) is a pain with meson
>
> Hm, not really. You just add an 'env' argument when declaring the test, e.g.:
> egl_sym_check = find_program('wayland-egl-symbols-check')
> test_egl_syms = test('egl-symbols', egl_sym_check, env: [
> 'WAYLAND_EGL_LIB=@0@'.format(lib_wayland_egl) ])
>
Once you add NM and potentially others, it does get tiny bit messier.

>>  - handling arguments (as opposed to env. variable) is a pain with
>> current testing scheme
>
> Yeah, that doesn't work.
>
>>  - the original code is shorter
>>
>> Hope you find at least some of those reasonable.
>
> It's fair enough. I'm just trying to find out the balance of these:
> for instance, if it's no problem to add environment variables with
> Meson, do you still want to push it for reason #1, or?
>
Yes, #1 is perhaps the biggest reason behind the patch.

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"

2018-03-09 Thread Emil Velikov
On 9 March 2018 at 09:47, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Emil,
>
> On 8 March 2018 at 18:32, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 28 February 2018 at 16:38, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>>> This reverts commit 85cb5ed64aa8246f4da93fc5b76dfc34096bf803.
>>>
>>> It seems like we've misread the existing code - the DSO name can be
>>> propagated via the build-system. The one available in the script was a
>>> simple fall-back.
>>
>> Humble ping on the series?
>> It should be dead trivial although do ask for details if anything is unclear.
>
> Patches 2-4 look fine and I'm happy to merge them with my review, but
> could you please explain some more about this patch? I very much like
> keeping details of the build system (specifically its internal build
> paths) in the build system itself and not in the script. I was
> assuming something in 2-4 needed this revert to be applied, but
> couldn't see anything. Is there something I'm missing?
>
There is one word to describe it - compromise:

 - above all, the internal path is a 'dummy' fallback. anyone can
provide the binary name as an argument
$ .../wayland-egl-symbols-check .../libwayland-egl.so
 - since we have a fallback - a plain .../wayland-egl-symbols-check
will work most of the time
 - handling env. variables (as opposed to arguments) is a pain with meson
 - handling arguments (as opposed to env. variable) is a pain with
current testing scheme
 - the original code is shorter

Hope you find at least some of those reasonable.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] gl-renderer: Create a high priority context

2018-03-02 Thread Emil Velikov
Hi Chris,

On 1 March 2018 at 08:28, Chris Wilson  wrote:
> EGL_IMG_context_priority allows the client to request that their
> rendering be considered high priority. For ourselves, this is important
> as we are interactive and any delay in our rendering causes input-output

> +   if (gr->has_context_priority) {
> +   EGLint value = EGL_CONTEXT_PRIORITY_MEDIUM_IMG;
> +
> +   eglQueryContext(gr->egl_display, gr->egl_context,
> +   EGL_CONTEXT_PRIORITY_LEVEL_IMG, );
> +
> +   if (value != EGL_CONTEXT_PRIORITY_HIGH_IMG) {
> +   weston_log("Failed to obtain a high priority 
> context.\n");
> +   /* Not an error, continue on as normal */
> +   }
While this (and EGL spec) says "not an error" the i965 driver will
error out as the ioctl fails.
Say, for some reason the kernel module/HW cannot do a high-priority one ATM.

Should that be changed/fixed?

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v2 2/4] wayland-egl: fail the symbol check if lib is missing

2018-02-28 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Based on a similar patch (in Mesa) by Eric Engestrom.

v2: Rebase on top of $NM patch.

Reviewed-by: Eric Engestrom <e...@engestrom.ch>
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl-symbols-check | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index e247cf3..4b8b5e4 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,14 @@
 #!/bin/sh
+set -eu
 
-FUNCS=$($NM -D --defined-only ${1-.libs/libwayland-egl.so} | grep -o "T .*" | 
cut -c 3- | while read func; do
+LIB=${1-.libs/libwayland-egl.so}
+
+if [ ! -f "$LIB" ]; then
+   echo "The test binary \"$LIB\" does no exist"
+   exit 1
+fi
+
+FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
 ( grep -q "^$func$" || echo $func )  <https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v2 3/4] wayland-egl: enhance the symbol test

2018-02-28 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

The current test had a few fall-outs:
 - it was checking only for T (.text) symbols
 - did not consider symbol removal

Fix that by fetching all the symbols and doing a bidirectional check -
for added and removed symbols. Error out with informative message for
each case.

v2: Rebase on top of $NM patch.

Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl-symbols-check | 36 
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 4b8b5e4..364cce9 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -8,17 +8,37 @@ if [ ! -f "$LIB" ]; then
exit 1
 fi
 
-FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
-( grep -q "^$func$" || echo $func )  <https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v2 4/4] wayland-egl: bump the version number to 18.1.0

2018-02-28 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Seems like I was overoptimistic with my earlier assumption, namely:

"... 17.3.x should be the last version that ships the library."

Mesa 18.0.0 and its wayland-egl is about to be released any time soon,
so bump the number since it must no be smaller. As soon as we get
a wayland release I'll drop the Mesa copy but for now.

Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 egl/wayland-egl.pc.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/egl/wayland-egl.pc.in b/egl/wayland-egl.pc.in
index 943442e..2e2d4c4 100644
--- a/egl/wayland-egl.pc.in
+++ b/egl/wayland-egl.pc.in
@@ -5,7 +5,7 @@ includedir=@includedir@
 
 Name: wayland-egl
 Description: Frontend wayland-egl library
-Version: 17.4.0
+Version: 18.1.0
 Requires: wayland-client
 Libs: -L${libdir} -lwayland-egl
 Cflags: -I${includedir}
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"

2018-02-28 Thread Emil Velikov
This reverts commit 85cb5ed64aa8246f4da93fc5b76dfc34096bf803.

It seems like we've misread the existing code - the DSO name can be
propagated via the build-system. The one available in the script was a
simple fall-back.

v2: Rebase on top of $NM patch.

Cc: Daniel Stone 
---
 Makefile.am   | 1 -
 egl/wayland-egl-symbols-check | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2731ee7..d5b3d79 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -196,7 +196,6 @@ AM_TESTS_ENVIRONMENT =  
\
export WAYLAND_SCANNER='$(top_builddir)/wayland-scanner'\
TEST_DATA_DIR='$(top_srcdir)/tests/data'\
TEST_OUTPUT_DIR='$(top_builddir)/tests/output'  \
-   WAYLAND_EGL_LIB='$(top_builddir)/egl/.libs/libwayland-egl.so'   \
SED=$(SED)  \
;
 
diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 6ad28f3..e247cf3 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 3- 
| while read func; do
+FUNCS=$($NM -D --defined-only ${1-.libs/libwayland-egl.so} | grep -o "T .*" | 
cut -c 3- | while read func; do
 ( grep -q "^$func$" || echo $func )  

[PATCH weston] build: honour libinput header location

2018-02-27 Thread Emil Velikov
From: Jan Engelhardt <jeng...@inai.de>

Add the respective CFLAGS to the build, otherwise the build will error
out as seen below.

src/libinput-seat.c:30:22: fatal error: libinput.h: No such file or directory

[Emil Velikov: polish commit message]
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
Jan, don't be shy to send patches upstream. Devs. won't bite... most of
the time ;-)
---
 Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index b5c29c04..8de40e51 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -43,7 +43,8 @@ AM_CPPFLAGS = \
-I$(top_builddir)/protocol  \
-DLIBWESTON_MODULEDIR='"$(libweston_moduledir)"' \
-DLIBEXECDIR='"$(libexecdir)"'  \
-   -DBINDIR='"$(bindir)"'
+   -DBINDIR='"$(bindir)"' \
+   $(LIBINPUT_BACKEND_CFLAGS)
 
 CLEANFILES = weston.ini\
ivi-shell/weston.ini\
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] scanner: Fix broken private-code generation

2018-02-23 Thread Emil Velikov
On 23 February 2018 at 22:24, Derek Foreman <der...@osg.samsung.com> wrote:
> Missing a closing bracket.
>
Eek, that's a embarasing.
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>

We might want to tweak the existing infra to check that, bonus points
if we also check the headers actually safe for inclusion in C and C++
sources.
A simple runner like weston-tests-env should do it.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] wayland-egl: use correct `nm` path when cross-compiling

2018-02-23 Thread Emil Velikov
From: Emil Velikov <emil.veli...@collabora.com>

Inspired by Heiko Becker and Eric's work in libdrm and Mesa
respectively.

Cc: Eric Engestrom <eric.engest...@imgtec.com>
Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
---
 configure.ac  | 1 +
 egl/wayland-egl-symbols-check | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 2542243..91f837d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -29,6 +29,7 @@ AC_PROG_CC
 AC_PROG_CXX
 AC_PROG_GREP
 AM_PROG_AS
+AC_PROG_NM
 
 # check if we have C++ compiler. This is hacky workaround,
 # for a reason why it is this way see
diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index e107362..6ad28f3 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-FUNCS=$(nm -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 3- | 
while read func; do
+FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 3- 
| while read func; do
 ( grep -q "^$func$" || echo $func )  <https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC wayland-protocols] Add contribution guidelines for desktop extensions

2018-02-22 Thread Emil Velikov
Hi Pekka,

On 22 February 2018 at 08:32, Pekka Paalanen  wrote:

> +Once there is sufficient cross-desktop support for a proposal, the Wayland
> +maintainers can accept the extension into wayland-protocols.
> +
Might be worth defining "sufficient" in a bullet point somewhere.
Say, "more than 1 of the mentioned projects", or "50%+1", "2/3", etc

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


  1   2   3   4   >