[PATCH 2/5] Remove wayland_protocoldir definition

[Pekka Paalanen]

On Tue, 27 May 2014 09:49:28 +0200
Thierry Reding <thierry.reding at gmail.com> wrote:

> On Tue, May 27, 2014 at 09:43:09AM +0300, Pekka Paalanen wrote:
> > On Mon, 19 May 2014 17:12:40 +0200
> > Thierry Reding <thierry.reding at gmail.com> wrote:
> > 
> > > From: Thierry Reding <treding at nvidia.com>
> > > 
> > > This is mostly useless and can be confusing in makefiles. The pattern
> > > rules defined in the makefile snippet are generic enough to allow the
> > > protocol sources to reside in subdirectories.
> > > 
> > > Signed-off-by: Thierry Reding <treding at nvidia.com>
> > > ---
> > >  wayland-scanner.m4 | 2 --
> > >  wayland-scanner.mk | 6 +++---
> > >  2 files changed, 3 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/wayland-scanner.m4 b/wayland-scanner.m4
> > > index 4e4222ad4d2f..e7f383a4c74f 100644
> > > --- a/wayland-scanner.m4
> > > +++ b/wayland-scanner.m4
> > > @@ -8,6 +8,4 @@ AC_DEFUN([WAYLAND_SCANNER_RULES], [
> > >  
> > >      wayland_scanner_rules=`$PKG_CONFIG --variable=pkgdatadir wayland-scanner`/wayland-scanner.mk
> > >      AC_SUBST_FILE([wayland_scanner_rules])
> > > -
> > > -    AC_SUBST([wayland_protocoldir], [$1])
> > >  ])
> > > diff --git a/wayland-scanner.mk b/wayland-scanner.mk
> > > index 0a72062baa59..b6e0f444aaa5 100644
> > > --- a/wayland-scanner.mk
> > > +++ b/wayland-scanner.mk
> > > @@ -1,8 +1,8 @@
> > > -%-protocol.c : $(wayland_protocoldir)/%.xml
> > > +%-protocol.c: %.xml
> > >  	$(AM_V_GEN)$(wayland_scanner) code < $< > $@
> > >  
> > > -%-server-protocol.h : $(wayland_protocoldir)/%.xml
> > > +%-server-protocol.h: %.xml
> > >  	$(AM_V_GEN)$(wayland_scanner) server-header < $< > $@
> > >  
> > > -%-client-protocol.h : $(wayland_protocoldir)/%.xml
> > > +%-client-protocol.h: %.xml
> > >  	$(AM_V_GEN)$(wayland_scanner) client-header < $< > $@
> > 
> > I think the original purpose here was, that the .xml was not in the
> > same directory as where you wanted to put the generated .c and .h files.
> > For instance, Weston could install some .xml files to the system, and a
> > third party project would then generate the C code from them.
> 
> That's for cases where some packages provided a set of standard
> protocols that some other package would want to implement?

Implement or use, yes. DE-specific extensions come to mind as first. We
have been meaning to do this, but so far extensions in Weston are
either private or experimental, and Mesa's wl_drm is sort of meant to
be private, so people are probably used to just copying the .xml files
from project to another. That's like copying C header files...

Both the server and client side needs to be built with the .xml files,
so it would kind of make sense for the server (or a third project) to
install the .xml files.

> > Would that still work?
> 
> I don't think it would. But if you set wayland_protocoldir to some
> absolute path it will prevent any such package from providing its own
> set of protocol files and use the same make snippet since those rules
> will then only look at the absolute wayland_protocoldir but not inside
> the source tree.

Yes, that was quite likely one of the major problems with the
wayland-scanner.mk.

> So I think shipping a common snippet will either be able to support one
> or the other use case, but not both at the same time.

Right, which is why I'd drop the makefile snippet altogether.

> > And out-of-tree builds?
> 
> For out-of-tree builds the above still works because srcdir is added to
> the VPATH automatically and therefore make will find the protocol files
> there.

Ok, I suspected that.

> > Another thing is, I'm not sure we can change these?
> > 
> > wayland-scanner.mk and wayland-scanner.m4 are installed files, and I'm
> > so unclear of what guarantees we have for them. They are used for build
> > only, so I suppose dependent projects test for the minimum libwayland
> > version, but I think we need to maintain backward compatibility.
> > 
> > OTOH, I still think that we should drop wayland-scanner.mk completely.
> > Can we do that? Both Weston and Mesa just open-code what
> > wayland-scanner.mk would have provided, because wayland-scanner.mk was
> > inconvenient to use.
> 
> Like you said, it doesn't seem like these are currently used by anyone
> so there isn't much opportunity to break anything. In fact the whole

That is why I can even suggest the delayed removal of these files. ;-)

> reason for this series was to make the m4/mk files more usable so that
> other projects could use them.

> > The rest of this series pretty much depends on the decision here.
> > 
> > How about the following:
> > - keep the existing wayland-scanner.{mk,m4} as is
> > - add warnings to both that they will be going away after a few
> >   wayland releases, so that they will yell at build time
> > - add a new .m4 file for what you have in patch 3
> > - patch 3 should provide two things:
> >   * set WAYLAND_SCANNER to the exectable
> >   * set another variable to the directory of the installed .xml files?
> > 
> > Then after enough time passes, we could just remove
> > wayland-scanner.{mk,m4} as per the warning/threat.
> 
> I'm not too thrilled by the idea of having everyone using their own copy
> of the makefile snippets, but as discussed above I don't see a way to
> make it work for the general case, so maybe it's not worth it.

It's just three pairs of lines, and the wayland-scanner CLI is
basically set in stone anyway. To me not sharing this tiny bit of code
is much simpler and cleaner.

> FWIW, I don't think we necessarily need to remove wayland-scanner.m4.
> The interface isn't changed and we can mark the makefile snippet as
> deprecated using $(warning ...) for example. Then once nobody's using
> the makefile snippet any longer we can simply remove the m4 pieces that
> set it up along with the snippet itself.

But you are renaming $(wayland_scanner) to $(WAYLAND_SCANNER), wouldn't
that break anything? Ok, yeah, we could probably just keep all the old
stuff, too, until the warning has had its time.


Thanks,
pq