| Message ID | 20170103212932.9254-2-barbieri@profusion.mobi |
|---|---|
| State | Accepted |
| Headers | show |
Hi Gustavo, Le 03/01/2017 à 22:29, Gustavo Sverzut Barbieri a écrit : > use the correct variable (BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT) and do > not impose dependency on util-linux if not needed. > > Signed-off-by: Gustavo Sverzut Barbieri <barbieri@profusion.mobi> > --- > package/efl/Config.in | 6 +++--- > package/efl/efl.mk | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/package/efl/Config.in b/package/efl/Config.in > index c51fc564d..542c354a8 100644 > --- a/package/efl/Config.in > +++ b/package/efl/Config.in > @@ -17,9 +17,6 @@ config BR2_PACKAGE_EFL > # https://phab.enlightenment.org/T2728 > select BR2_PACKAGE_LUAJIT # Lua support broken > select BR2_PACKAGE_LZ4 > - select BR2_PACKAGE_UTIL_LINUX > - # libblkid is part of required tools, see EFL's README. > - select BR2_PACKAGE_UTIL_LINUX_LIBBLKID IIRC, There is a build issue behind this, but it was with an older efl version (1.13 or 1.14). So I have to check this. BTW do you want to be added to DEVELOPERS file and receive any build issues from Buildroot autobuilders related to efl packages ? https://buildroot.org/downloads/manual/manual.html#DEVELOPERS > select BR2_PACKAGE_ZLIB > help > Enlightenment Foundation Libraries > @@ -101,7 +98,10 @@ config BR2_PACKAGE_EFL_PULSEAUDIO > > config BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT > bool "Enable libmount support (recommended)" Maybe we should rename this option to BR2_PACKAGE_EFL_UTIL_LINUX and use "Enable util-linux (limbount + libblkid) support (recommended)" Best regards, Romain > + select BR2_PACKAGE_UTIL_LINUX > select BR2_PACKAGE_UTIL_LINUX_LIBMOUNT > + # libblkid is part of required tools, see EFL's README. > + select BR2_PACKAGE_UTIL_LINUX_LIBBLKID > default y > help > Libmount is used heavily inside Eeze for support of removable > diff --git a/package/efl/efl.mk b/package/efl/efl.mk > index 2fe140a30..ab08946c4 100644 > --- a/package/efl/efl.mk > +++ b/package/efl/efl.mk > @@ -20,7 +20,7 @@ EFL_LICENSE_FILES = \ > EFL_INSTALL_STAGING = YES > > EFL_DEPENDENCIES = host-pkgconf host-efl host-luajit dbus freetype \ > - jpeg luajit lz4 udev util-linux zlib > + jpeg luajit lz4 udev zlib > > # Configure options: > # --disable-lua-old: build elua for the target. > @@ -59,7 +59,7 @@ else > EFL_CONF_OPTS += --disable-cxx-bindings > endif > > -ifeq ($(BR2_PACKAGE_UTIL_LINUX_LIBMOUNT),y) > +ifeq ($(BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT),y) > EFL_DEPENDENCIES += util-linux > EFL_CONF_OPTS += --enable-libmount > else >
On Tue, Jan 3, 2017 at 8:37 PM, Romain Naour <romain.naour@gmail.com> wrote: > Hi Gustavo, > > Le 03/01/2017 à 22:29, Gustavo Sverzut Barbieri a écrit : >> use the correct variable (BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT) and do >> not impose dependency on util-linux if not needed. >> >> Signed-off-by: Gustavo Sverzut Barbieri <barbieri@profusion.mobi> >> --- >> package/efl/Config.in | 6 +++--- >> package/efl/efl.mk | 4 ++-- >> 2 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/package/efl/Config.in b/package/efl/Config.in >> index c51fc564d..542c354a8 100644 >> --- a/package/efl/Config.in >> +++ b/package/efl/Config.in >> @@ -17,9 +17,6 @@ config BR2_PACKAGE_EFL >> # https://phab.enlightenment.org/T2728 >> select BR2_PACKAGE_LUAJIT # Lua support broken >> select BR2_PACKAGE_LZ4 >> - select BR2_PACKAGE_UTIL_LINUX >> - # libblkid is part of required tools, see EFL's README. >> - select BR2_PACKAGE_UTIL_LINUX_LIBBLKID > > IIRC, There is a build issue behind this, but it was with an older efl version > (1.13 or 1.14). So I have to check this. I did the cumbersome "make clean" then "make all" and it worked. So I assume this is safe now, but as you see these are enabled if BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT is enabled, and that's the default. > BTW do you want to be added to DEVELOPERS file and receive any build issues from > Buildroot autobuilders related to efl packages ? > > https://buildroot.org/downloads/manual/manual.html#DEVELOPERS sounds good. Following that guideline I should add myself to that file, likely in this patch. Okay? >> select BR2_PACKAGE_ZLIB >> help >> Enlightenment Foundation Libraries >> @@ -101,7 +98,10 @@ config BR2_PACKAGE_EFL_PULSEAUDIO >> >> config BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT >> bool "Enable libmount support (recommended)" > > Maybe we should rename this option to BR2_PACKAGE_EFL_UTIL_LINUX and use > "Enable util-linux (limbount + libblkid) support (recommended)" Not sure... to tell you the truth I think libblkid is a "nice to have", not must have... our code doesn't refer to anything in libblkid, but if you want mount to auto detect the partition/filesystem, then blkid is needed to scan the block device and infer that AFAIU. However if that's what people want I can do for sure, super simple.
Le 04/01/2017 à 03:09, Gustavo Sverzut Barbieri a écrit : > On Tue, Jan 3, 2017 at 8:37 PM, Romain Naour <romain.naour@gmail.com> wrote: >> Hi Gustavo, >> >> Le 03/01/2017 à 22:29, Gustavo Sverzut Barbieri a écrit : >>> use the correct variable (BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT) and do >>> not impose dependency on util-linux if not needed. >>> >>> Signed-off-by: Gustavo Sverzut Barbieri <barbieri@profusion.mobi> >>> --- >>> package/efl/Config.in | 6 +++--- >>> package/efl/efl.mk | 4 ++-- >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/package/efl/Config.in b/package/efl/Config.in >>> index c51fc564d..542c354a8 100644 >>> --- a/package/efl/Config.in >>> +++ b/package/efl/Config.in >>> @@ -17,9 +17,6 @@ config BR2_PACKAGE_EFL >>> # https://phab.enlightenment.org/T2728 >>> select BR2_PACKAGE_LUAJIT # Lua support broken >>> select BR2_PACKAGE_LZ4 >>> - select BR2_PACKAGE_UTIL_LINUX >>> - # libblkid is part of required tools, see EFL's README. >>> - select BR2_PACKAGE_UTIL_LINUX_LIBBLKID >> >> IIRC, There is a build issue behind this, but it was with an older efl version >> (1.13 or 1.14). So I have to check this. > > I did the cumbersome "make clean" then "make all" and it worked. So I > assume this is safe now, but as you see these are enabled if > BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT is enabled, and that's the > default. I did a build test today without util-linux package and it build fine with the current efl version (1.18.4) > > >> BTW do you want to be added to DEVELOPERS file and receive any build issues from >> Buildroot autobuilders related to efl packages ? >> >> https://buildroot.org/downloads/manual/manual.html#DEVELOPERS > > sounds good. Following that guideline I should add myself to that > file, likely in this patch. Okay? Well, It seems that the recommended way to do it is to send a separate patch. The reason is: if someone need to backport a patch adding a feature or a new package, we want to avoid a conflict in DEVELOPERS file while cherry-picking the commit. Thomas, can you confirm ? > > >>> select BR2_PACKAGE_ZLIB >>> help >>> Enlightenment Foundation Libraries >>> @@ -101,7 +98,10 @@ config BR2_PACKAGE_EFL_PULSEAUDIO >>> >>> config BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT >>> bool "Enable libmount support (recommended)" >> >> Maybe we should rename this option to BR2_PACKAGE_EFL_UTIL_LINUX and use >> "Enable util-linux (limbount + libblkid) support (recommended)" > > Not sure... to tell you the truth I think libblkid is a "nice to > have", not must have... our code doesn't refer to anything in > libblkid, but if you want mount to auto detect the > partition/filesystem, then blkid is needed to scan the block device > and infer that AFAIU. > > However if that's what people want I can do for sure, super simple. I followed the README comment here and because BR2_PACKAGE_UTIL_LINUX_LIBBLKID is not selected by default when util-linux package is selected. Also libblkid.so is not so big (~300Ko), I'm not sure we really need a new option BR2_PACKAGE_EFL_UTIL_LINUX_LIBLKID config BR2_PACKAGE_EFL_UTIL_LINUX_LIBLKID bool "Enable libblkid support (recommended)" depends on BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT Ok, let's do it as you suggest Reviewed-by: Romain Naour <romain.naour@gmail.com> [Build tested] Tested-by: Romain Naour <romain.naour@gmail.com> Best regards, Romain > > > >
On Thu, Jan 5, 2017 at 9:20 PM, Romain Naour <romain.naour@gmail.com> wrote: > Le 04/01/2017 à 03:09, Gustavo Sverzut Barbieri a écrit : >> On Tue, Jan 3, 2017 at 8:37 PM, Romain Naour <romain.naour@gmail.com> wrote: [...] >>> BTW do you want to be added to DEVELOPERS file and receive any build issues from >>> Buildroot autobuilders related to efl packages ? >>> >>> https://buildroot.org/downloads/manual/manual.html#DEVELOPERS >> >> sounds good. Following that guideline I should add myself to that >> file, likely in this patch. Okay? > > Well, It seems that the recommended way to do it is to send a separate patch. > The reason is: if someone need to backport a patch adding a feature or a new > package, we want to avoid a conflict in DEVELOPERS file while cherry-picking the > commit. > > Thomas, can you confirm ? ok, will wait for Thomas to send as a separate patch or not. >>>> select BR2_PACKAGE_ZLIB >>>> help >>>> Enlightenment Foundation Libraries >>>> @@ -101,7 +98,10 @@ config BR2_PACKAGE_EFL_PULSEAUDIO >>>> >>>> config BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT >>>> bool "Enable libmount support (recommended)" >>> >>> Maybe we should rename this option to BR2_PACKAGE_EFL_UTIL_LINUX and use >>> "Enable util-linux (limbount + libblkid) support (recommended)" >> >> Not sure... to tell you the truth I think libblkid is a "nice to >> have", not must have... our code doesn't refer to anything in >> libblkid, but if you want mount to auto detect the >> partition/filesystem, then blkid is needed to scan the block device >> and infer that AFAIU. >> >> However if that's what people want I can do for sure, super simple. > > I followed the README comment here and because BR2_PACKAGE_UTIL_LINUX_LIBBLKID > is not selected by default when util-linux package is selected. > Also libblkid.so is not so big (~300Ko), I'm not sure we really need a new > option BR2_PACKAGE_EFL_UTIL_LINUX_LIBLKID > > config BR2_PACKAGE_EFL_UTIL_LINUX_LIBLKID > bool "Enable libblkid support (recommended)" > depends on BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT > > Ok, let's do it as you suggest As I said, the liblkid is not a dependency per se... is more to make libmount useful, overall. This isn't specific to EFL. Looking at package/util-linux/Config.in to write this reply I saw it's already the case: config BR2_PACKAGE_UTIL_LINUX_LIBMOUNT bool "libmount" depends on BR2_USE_MMU # fork() select BR2_PACKAGE_UTIL_LINUX_LIBBLKID :-)
Hello, On Fri, 6 Jan 2017 00:20:55 +0100, Romain Naour wrote: > > sounds good. Following that guideline I should add myself to that > > file, likely in this patch. Okay? > > Well, It seems that the recommended way to do it is to send a separate patch. > The reason is: if someone need to backport a patch adding a feature or a new > package, we want to avoid a conflict in DEVELOPERS file while cherry-picking the > commit. > > Thomas, can you confirm ? Correct. Please send a separate patch. Thanks! Thomas
Hello, On Tue, 3 Jan 2017 19:29:31 -0200, Gustavo Sverzut Barbieri wrote: > use the correct variable (BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT) and do > not impose dependency on util-linux if not needed. > > Signed-off-by: Gustavo Sverzut Barbieri <barbieri@profusion.mobi> > --- > package/efl/Config.in | 6 +++--- > package/efl/efl.mk | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) Applied to master, thanks. Thomas
diff --git a/package/efl/Config.in b/package/efl/Config.in index c51fc564d..542c354a8 100644 --- a/package/efl/Config.in +++ b/package/efl/Config.in @@ -17,9 +17,6 @@ config BR2_PACKAGE_EFL # https://phab.enlightenment.org/T2728 select BR2_PACKAGE_LUAJIT # Lua support broken select BR2_PACKAGE_LZ4 - select BR2_PACKAGE_UTIL_LINUX - # libblkid is part of required tools, see EFL's README. - select BR2_PACKAGE_UTIL_LINUX_LIBBLKID select BR2_PACKAGE_ZLIB help Enlightenment Foundation Libraries @@ -101,7 +98,10 @@ config BR2_PACKAGE_EFL_PULSEAUDIO config BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT bool "Enable libmount support (recommended)" + select BR2_PACKAGE_UTIL_LINUX select BR2_PACKAGE_UTIL_LINUX_LIBMOUNT + # libblkid is part of required tools, see EFL's README. + select BR2_PACKAGE_UTIL_LINUX_LIBBLKID default y help Libmount is used heavily inside Eeze for support of removable diff --git a/package/efl/efl.mk b/package/efl/efl.mk index 2fe140a30..ab08946c4 100644 --- a/package/efl/efl.mk +++ b/package/efl/efl.mk @@ -20,7 +20,7 @@ EFL_LICENSE_FILES = \ EFL_INSTALL_STAGING = YES EFL_DEPENDENCIES = host-pkgconf host-efl host-luajit dbus freetype \ - jpeg luajit lz4 udev util-linux zlib + jpeg luajit lz4 udev zlib # Configure options: # --disable-lua-old: build elua for the target. @@ -59,7 +59,7 @@ else EFL_CONF_OPTS += --disable-cxx-bindings endif -ifeq ($(BR2_PACKAGE_UTIL_LINUX_LIBMOUNT),y) +ifeq ($(BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT),y) EFL_DEPENDENCIES += util-linux EFL_CONF_OPTS += --enable-libmount else
use the correct variable (BR2_PACKAGE_EFL_UTIL_LINUX_LIBMOUNT) and do not impose dependency on util-linux if not needed. Signed-off-by: Gustavo Sverzut Barbieri <barbieri@profusion.mobi> --- package/efl/Config.in | 6 +++--- package/efl/efl.mk | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)