Re: [PATCH weston] build: don't manually parse the weston.ini.in templates
On Mon, 2 Jul 2018 08:37:52 +0100 Emil Velikov wrote: > 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 I never knew that was even a thing. Is that actually supposed to work? Wouldn't the first case be broken on all software that builds in any such paths? Thanks, pq pgpjcP1gwXhdW.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] build: don't manually parse the weston.ini.in templates
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
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.
> > 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.
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?)
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.
> 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.
> 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.
Thanks,
pq
pgpv1Py8Aziib.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] build: don't manually parse the weston.ini.in templates
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 weston] build: don't manually parse the weston.ini.in templates
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".)
Thanks,
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
--
Quentin “Sardem FF7” Glidic
___
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
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
