[PATCH libinput] tools: fake-build the measure touch-pressure/size sources
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.
meson.build | 18 ++
tools/libinput-measure-touch-size| 14 ++
tools/libinput-measure-touchpad-pressure | 14 ++
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index b650bbde..c6ba982d 100644
--- a/meson.build
+++ b/meson.build
@@ -541,16 +541,34 @@ configure_file(input :
'tools/libinput-measure-touchpad-tap.man',
install_dir : join_paths(get_option('mandir'), 'man1')
)
+config_builddir = configuration_data()
+config_builddir.set('BUILDDIR', meson.build_root())
+# libinput-measure-touchpad-pressure gets built but we install_data the
+# source instead. The built file is the one that uses the local list-quirks
+# and should be used for debugging.
install_data('tools/libinput-measure-touchpad-pressure',
install_dir : libinput_tool_path)
+configure_file(input: 'tools/libinput-measure-touchpad-pressure',
+ output: 'libinput-measure-touchpad-pressure',
+ configuration : config_builddir,
+ install_dir : '')
+
configure_file(input : 'tools/libinput-measure-touchpad-pressure.man',
output : 'libinput-measure-touchpad-pressure.1',
configuration : man_config,
install : true,
install_dir : join_paths(get_option('mandir'), 'man1')
)
+# libinput-measure-touch-size gets built but we install_data the source
+# instead. The built file is the one that uses the local list-quirks and
+# should be used for debugging.
install_data('tools/libinput-measure-touch-size',
install_dir : libinput_tool_path)
+configure_file(input: 'tools/libinput-measure-touch-size',
+ output: 'libinput-measure-touch-size',
+ configuration : config_builddir,
+ install_dir : '')
+
configure_file(input : 'tools/libinput-measure-touch-size.man',
output : 'libinput-measure-touch-size.1',
configuration : man_config,
diff --git a/tools/libinput-measure-touch-size
b/tools/libinput-measure-touch-size
index 9ff65eaa..f2ee0b31 100755
--- a/tools/libinput-measure-touch-size
+++ b/tools/libinput-measure-touch-size
@@ -24,6 +24,7 @@
# DEALINGS IN THE SOFTWARE.
#
+import os
import sys
import subprocess
import argparse
@@ -221,10 +222,15 @@ class Device(object):
sys.exit(1)
def _init_thresholds_from_quirks(self):
-# FIXME: this uses the system-installed version
-# but we should really auto-detect when this is started
-# from the builddir
-command = ['libinput', 'list-quirks', self.path]
+# This is replaced for the version in builddir but left as-is for
+# the installed version. For the builddir one we need to run the
+# local list-quirks
+builddir = '@BUILDDIR@'
+if builddir != '@' + 'BUILDDIR' + '@':
+command = [os.path.join(builddir, 'libinput-list-quirks')]
+else:
+command = ['libinput', 'list-quirks']
+command.append(self.path)
cmd = subprocess.run(command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
if cmd.returncode != 0:
print("Error querying quirks:
{}".format(cmd.stderr.decode('utf-8')), file=sys.stderr)
diff --git a/tools/libinput-measure-touchpad-pressure
b/tools/libinput-measure-touchpad-pressure
index 765e7997..4026e6ff 100755
--- a/tools/libinput-measure-touchpad-pressure
+++ b/tools/libinput-measure-touchpad-pressure
@@ -24,6 +24,7 @@
# DEALINGS IN THE SOFTWARE.
#
+import os
import sys
import subprocess
import argparse
@@ -199,10 +200,15 @@ class Device(object):
sys.exit(1)
def _init_thresholds_from_quirks(self):
-# FIXME: this uses the system-installed version
-# but we should really auto-detect when this is started
-# from the builddir
-command = ['libinput', 'list-quirks', self.path]
+# This is replaced for the version in builddir but left as-is for
+# the installed version. For the builddir one we need to run the
+# local list-quirks
+builddir = '@BUILDDIR@'
+if builddir != '@' + 'BUILDDIR' + '@':
+command = [os.path.join(builddir, 'libinput-list-quirks')]
+else:
+command = ['libinput', 'list-quirks']
+command.append(self.path)
cmd = subprocess.run(command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
if cmd.returncode != 0:
print("Error querying quirks:
{}".format(cmd.stderr.decode('utf-8')), file=sys.stderr)
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
Re: how to handle the last_error of wl_display
Hi pekka
I find the last_error is caused by
wl_display_flush()-->wl_connection_flush(). The sendmsg() return fail.
Many applications write code like this:
redraw()
{
wl_surface_commit();
wl_display_sync();
wl_display_flush();
}
while (1) {
redraw();
}
The code will block to wait buffer release event if there is no free
buffer. But if display->last_error happen, the wl_display_dispatch_queue()
fail too. Does the application use the wl_display_flush() right?
Thank you!
Best Regards
Nancy
2018-06-14 18:33 GMT+08:00 Pekka Paalanen :
> On Thu, 14 Jun 2018 16:59:35 +0800
> zou lan wrote:
>
> > Dear pekka & All:
> >
> > I find a wayland application report error about allocate memory fail.
> > I check the code, after request to weston to allocate dma buffer, it call
> > wl_display_roundtrip().
> > But this function return very fast, it doesn't block until the create
> > events back.
> >
> > I suspect the display->last_error is not 0. Then the
> wl_display_roundtrip()
> > return fast because of the last_error. Does it right?
>
> Hi,
>
> sounds plausible.
>
> > I find the following log:
> >
> > wl_display@1: error 0: invalid object 10
>
> Your application has a bug.
>
> > Does the application need to handle the display errors? How could I make
> > the last_error not impact wl_display_roundtrip?
>
> All protocol errors are always fatal: they cause the client to be
> disconnected and render the client wl_display unusable. The application
> can only report the error somewhere and quit.
>
> Protocol errors always imply a bug. Usually the bug is in the client
> code, but it could be in compositor code as well.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Session suspension and restoration protocol
As a general remark I want to expand upon, why I did work on this protocol although Mike's xdg-session-management protocol already existed: The xdg-session-management protocol defines only how certain sessions / surfaces can be saved for later restoration and how to restore them. But it does not define conditions on if saving is suitable at the moment and how long the compositor needs to remember the saved restoration info. I believe in particular the last point is crucial, and the client needs to negotiate this with the compositor somehow depending on what it wants to do with the restoration info. My proposal tries to solve this by defining very tight concepts (sleep, quit, shutdown) on what the saved surface info is intended to be used for and allows the compositor to remove surface data, that has not been recovered in the boundaries of these concepts. And having DBus-Activation (or the Exec line of desktop files) defined in the protocol as the way to restore an app after it has quit the Wayland connection has the advantages that all information are in one place and that client developers have it easier, since they can rest assure that if they target this one spec and the compositor supports the suspension level, the workspace will at some point reactivate their app in a predictable manner. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Session suspension and restoration protocol
On Tue, Jun 19, 2018 at 11:19 AM Markus Ongyerth wrote: > > Hey Roman, > > first a general remark: > I don't see any mention of which state is supposed to be restored. The > workspace (virtual desktops), geometry (session-restore), just client-side > window (the embedded usecase)? > Is this left implementation-defined intentionally? If so, I think it would be > nice to have a note about that. Should allow restoration of workspace and geometry by compositor and client-side window by client. > While the sleep version of this has a way for a client ot overwrite and > restore itself, I'm missing that functionality on quit/shutdown. > The current state is always compositor triggered on those. While it might make > sense for e.g. multi-window applications or other fancy things to use a > restoration protocol (at least for window geometry) on their own startup. > It *should* work as is, but I'd like to see it explicitly. The rational was that as soon as the client has agreed to the quit/shutdown state, it should have no business anymore to change this. Because in general it is meant to now quit the client connection and its own process only waiting for the Restore call by the compositor. If there is no process, it can no longer cancel the state. So it's not allowed. A client which will quit itself after some internal process ended should therefore not go into the quit state, otherwise it might get restored at some later point although it has nothing more to do. > On 2018/6月/18 05:05, Roman Gilg wrote: > > * using D-Bus interface only to secure against sandboxed clients > What? Why exactly? When I first read this, I expected that the client is > supposed to use the portal stuff to call out of a sandbox, but reading through > the actual protocol, I think the only usage of D-Bus is to launch the > applications from the compositor? > Since this needs explicit client support either way, IMO this could be > implmented by adding a required cmdline argument (e.g. > --org-freedsktop-restore=UID) instead. > Parsing .desktop files, and launching applications in a helper shouldn't bee > too much of a burdon. > I know that GNOME/KDE employ D-Bus either way, but others want to avoid it as > much as possible. The first draft was written exactly this way: parse the desktop file, execute the Exec line with a special argument. But GNOME people said, that D-Bus Activation should be used. And I think they are right in theory. But if there is a large potion of potential users not willing to support D-Bus Activation one could offer the possibility to execute the Exec line or do D-Bus Activation. Client and compositor just need to make sure that they support whatever method they want to use (and if they don't use at least one of the methods at the same time, don't try to use the protocol). Should be easy to do with another event in the manager interface and another argument to its register_session_suspension call. And personally I would at least recommend in the specs to use D-Bus Activation instead of Exec. > > > > > >> summary="an object for the reference already exists"/> > >> summary="parent object destroyed before child objects"/> > >value="2" might be better to differentiate this from defunct_children :) > > summary="unallowed request sent while session suspended"/> > I'd suggest another entry: > summary="client tried to create a suspesion > object that's not > uspported by the compositor"/> My plan in this case was to just not send the respective event (i.e let the client create the sleep object but not send the sleep event if the compositor does not support sleep). But probably it's nicer to put out an error directly. > > [0] https://specifications.freedesktop.org/desktop-entry-spec > > > >>summary="register a new restore session"/> > >>summary="for restoration must match client_id of previous > > session"/> > What if there is no previous session? Then it should be 0. If we decide to use strings instead, it's an empty string. Needs to get mentioned in the description though. > > Calling this request on an xdg_toplevel for which a > > zxdg_session_surface > > object already exists results in a protocol error. > > > >>summary="session surface object"/> > > > > > What's your rational to have the surface_restore_id server allocated and sent > as event? This should be in a "client namespace" (i.e. bound to the > protocol-global restore_id) either way, so the client can savely manage them > as well. You are right, that the surface ids could be generated by the client, since they are bound by the restore_id. In this case the compositor would need to make sure though, that the client does not send for different surfaces the same id. Maybe there doesn't need to be sent
Re: Session suspension and restoration protocol
What I forgot: the Restore method should probably take some similar argument like Activate's platform_data argument instead of a single integer/string. This way the argument can be extended with additional information, like with the Wayland server socket name, what Markus brought up. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Session suspension and restoration protocol
On Mon, Jun 18, 2018 at 6:01 PM Simon McVittie wrote: > > This document might be useful for the D-Bus side: > https://dbus.freedesktop.org/doc/dbus-api-design.html Hi Simon, thanks for the link. I have read it. Was a good D-Bus overview/introduction I have looked for already for quite some time. Wondering why I never found it before. Should enable me to misuse terminology less often. Thanks for the numerous corrections you did on that in your reply. I won't reply to all of them here, but will integrate them if there is a final proposal. > Do you intend the D-Bus side of the protocol to be equally useful to > X11 apps, or is this a Wayland-only thing? I have only thought about Wayland apps until now, because for X11 / Xwayland in Plasma we have XSMP support. But I'm not against having the mechanism work for X11 apps as well as a replacment for XSMP since GNOME doesn't use it at the moment and thought it was lacking. So yea, if it would work for X11 apps as well, that's fine with me and I will try to help make it happen. > > > and the method being called by the > > compositor on this interface is simply called 'restore' > > D-Bus method names are conventionally in camel-case, so 'Restore'. > > When we discussed this on #gnome-hackers we were talking about doing > restoration by passing the restored session ID as platform_data to the > Activate method defined by > https://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#dbus > which would mean that for trivial/stateless apps, it would be enough to > ignore it and just start up, which DBusActivatable implementations like > GtkApplication will already do. Is there a reason why you've opted to define > a new method instead? I think it makes sense to have a separate method, because a client can dbus-activate with the protocol any dbus-activatable app. Imagine a rogue client tries to dbus-activate an app, which is not written to use this extension but provides the Activate method. The compositor tries to dbus-activate it and gets no feedback that this was an operation not doing what it supposed to do. If there is no Restore method, I assume D-Bus would tell it so. The compositor can then show this information to the user. In this sense the protocol should be extended with the information that the Restore method should also return an error, if the restoration id is not found. A rogue client would that way also not be able to silently start a different client, that is supports the protocol (for that to make it secure the restore IDs should be generated through a hash function though). ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 0/4] Preparing for Meson
On Tue, 19 Jun 2018 11:46:10 +0100 Daniel Stone wrote: > Hi Pekka, > > On Mon, 18 Jun 2018 at 15:41, Pekka Paalanen wrote: > > these changes are intended to make using Meson easier. Daniel's patches > > are v2, mine are v1. > > > > Daniel Stone (2): > > tests: Don't rely on build directory layout > > tests: Reshuffle IVI layout tests > > > > Pekka Paalanen (2): > > shared: remove weston_config_get_libexec_dir() > > tests: remove WESTON_BUILD_DIR from env > > Thanks a lot for picking these up; series is: > Reviewed-by: Daniel Stone Hi, I made the changes Emil suggested: const int name_len and mention the caveat in patch 1, and a note in patch 2. Added a note from Emre in patch 1. All pushed now: 002f0c56..78a42116 master -> master Thanks, pq pgppFcUBelEyo.pgp Description: OpenPGP digital signature ___ 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()
On Wed, 20 Jun 2018 12:56:16 +0100 Emil Velikov wrote: > 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 Yes. Actually, this is config-parser. It's not supposed to be exported by libweston at all, but it gets pulled in because of libshared.la. Looks like there is more to clean up. Thanks, pq pgpxCRtvwnz5b.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: EXT: [PATCH weston 0/1] Allow forcing outputs on
On Tue, 19 Jun 2018 14:07:08 +0200 Tomasz Olszak wrote: > Hi, > > While I was testing this patch I ended up with: > > interface: 'wl_output', version: 3, name: 14 > x: 2732, y: 0, scale: 1, > > > This happened when I used force-on on second "HDMI-A-2" display. > > When I used force-on on HDMI-A-1 everything worked as expected. anx x=y=0 Hi, I did a quick test on a device with two connectors, both unplugged, and forcing each one on at a time. I didn't see the issue. I'll try to see how it could still happen. Would you have a weston log of the failure case along with full weston-info output? Thanks, pq pgpLuthEFU8X1.pgp Description: OpenPGP digital signature ___ 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()
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 4/4] tests: Reshuffle IVI layout tests
On Tue, 19 Jun 2018 18:39:09 +0100 Emil Velikov wrote: > 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? True, it would be better that way, but if you don't insist, I won't bother, since three people with you included have already reviewed the patch. > Either way, patch looks spot on and is > Reviewed-by: Emil Velikov Cheers, pq pgpw6lSbseUkd.pgp Description: OpenPGP digital signature ___ 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()
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. Thanks, pq pgpB9NwTwgMDg.pgp Description: OpenPGP digital signature ___ 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
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: [PATCH weston 0/4] Preparing for Meson
On Tue, 19 Jun 2018 14:49:27 + "Ucan, Emre (ADITG/ESB)" wrote: > Hi Pekka, > > All your patches are reviewed by me: > Reviewed-by: Emre Ucan > > Don't relying on build directory has the major advantage that it is > lot easier to run weston unit-tests in target environment if weston > is cross compiled. Cool, that didn't even occur to me. Thanks, pq pgp3v5Xn8Ny7U.pgp Description: OpenPGP digital signature ___ 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
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 weston 1/4] tests: Don't rely on build directory layout
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.
> 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.
Thanks,
pq
pgpviFqZ5hZpz.pgp
Description: OpenPGP digital signature
___
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
On Tue, 19 Jun 2018 17:58:41 +0100 Emil Velikov wrote: > 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. The section Review starts with a paragraph about that: one R-b is required, plus someone with commit access must agree which could be the same R-b or an additional R-b, A-b or S-o-b. It doesn't help people with commit access to decide if they can agree or not though, but I would start by relying on people's own judgement to not ack things they're not sure of. Thanks, pq pgp_vtkH_O2iX.pgp Description: OpenPGP digital signature ___ 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
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. I can accept that. Is anyone against this change? Thanks, pq > > -Emil > > [1] https://gcc.gnu.org/c99status.html > [2] https://gcc.gnu.org/releases.html pgpFvYAZUwePL.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] util: abort if we try to allocate more than a MB
On Wed, Jun 20, 2018 at 09:12:54AM +0200, Jan Engelhardt wrote: > On Wednesday 2018-06-20 02:22, Matheus Santana wrote: > > >Reviewed-by: Matheus Santana > > > >The check for negatives isn't needed anymore? > > It indeed is not, since a (size_t)-1 is generally greater than 1048576. still good to leave it in as test case though, just in case we end up taking the limit out at some point. Whether it's justified or not depends on the use-case then, but tripping up during a test run is always good. Cheers, Peter > > > Let's put a cap on for one MB, anything above that is likely some > > memory > > corruption and should be caught early. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] util: abort if we try to allocate more than a MB
On Wednesday 2018-06-20 02:22, Matheus Santana wrote: >Reviewed-by: Matheus Santana > >The check for negatives isn't needed anymore? It indeed is not, since a (size_t)-1 is generally greater than 1048576. > Let's put a cap on for one MB, anything above that is likely some memory > corruption and should be caught early. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
