[Buildroot] [PATCH v3 1/1] yad: new package

[Buildroot] [PATCH v3 1/1] yad: new package

[James Knight]

Signed-off-by: James Knight <james.knight@rockwellcollins.com>
---
Changes v2 -> v3:
  - Increment to version 0.31.1 (removed need for HTML patch introduced
     in v2 change; suggested by Vicente Olivert Riera).

Changes v1 -> v2:
  - Increment to version 0.29.0 (with updated hash and patch fix).
  - Removed complex gtk library selection. Defaults to libgtk3, if
     available; elsewise, uses libgtk2 (suggested from comments by
     Baruch Siach, Thomas Petazzoni and Peter Korsgaard).
---
 package/Config.in     |  1 +
 package/yad/Config.in | 13 +++++++++++++
 package/yad/yad.hash  |  2 ++
 package/yad/yad.mk    | 24 ++++++++++++++++++++++++
 4 files changed, 40 insertions(+)
 create mode 100644 package/yad/Config.in
 create mode 100644 package/yad/yad.hash
 create mode 100644 package/yad/yad.mk

diff --git a/package/Config.in b/package/Config.in
index 3794f44..5d4eb7d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -290,6 +290,7 @@ comment "X applications"
 	source "package/xscreensaver/Config.in"
 	source "package/xterm/Config.in"
 	source "package/xvkbd/Config.in"
+	source "package/yad/Config.in"
 
 comment "X libraries and helper libraries"
 	source "package/libsexy/Config.in"
diff --git a/package/yad/Config.in b/package/yad/Config.in
new file mode 100644
index 0000000..1e6776b
--- /dev/null
+++ b/package/yad/Config.in
@@ -0,0 +1,13 @@
+config BR2_PACKAGE_YAD
+	bool "yad"
+	depends on BR2_PACKAGE_XORG7
+	depends on BR2_PACKAGE_LIBGTK2 || BR2_PACKAGE_LIBGTK3
+	select BR2_PACKAGE_LIBGLIB2
+	help
+	  YAD (yet another dialog) is a tool for create graphical dialogs
+	  from shell scripts.
+
+	  http://sourceforge.net/projects/yad-dialog/
+
+comment "yad needs libgtk2 or libgtk3"
+	depends on !BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3
diff --git a/package/yad/yad.hash b/package/yad/yad.hash
new file mode 100644
index 0000000..fdde908
--- /dev/null
+++ b/package/yad/yad.hash
@@ -0,0 +1,2 @@
+# Locally computed
+sha256 3716d71861009add49e0a8a2269b181feffd0449e4cdf4f317f873dfb8c94b82 yad-0.31.1.tar.xz
diff --git a/package/yad/yad.mk b/package/yad/yad.mk
new file mode 100644
index 0000000..7615cf6
--- /dev/null
+++ b/package/yad/yad.mk
@@ -0,0 +1,24 @@
+################################################################################
+#
+# yad
+#
+################################################################################
+
+YAD_VERSION = 0.31.1
+YAD_SOURCE = yad-$(YAD_VERSION).tar.xz
+YAD_SITE = http://sourceforge.net/projects/yad-dialog/files
+YAD_LICENSE = GPLv3
+YAD_LICENSE_FILES = COPYING
+YAD_DEPENDENCIES = host-intltool host-pkgconf libglib2
+
+YAD_CONF_OPTS = --enable-html=no
+
+ifeq ($(BR2_PACKAGE_LIBGTK3),y)
+YAD_DEPENDENCIES += libgtk3
+YAD_CONF_OPTS += --with-gtk=gtk3
+else
+YAD_DEPENDENCIES += libgtk2
+YAD_CONF_OPTS += --with-gtk=gtk2
+endif
+
+$(eval $(autotools-package))
-- 
1.9.5.msysgit.1

[Buildroot] [PATCH v3 1/1] yad: new package

[Vicente Olivert Riera]

Dear James Knight,

thanks for taking into account my review. I have added more comments
below, so please, keep reading.

On 10/05/2015 09:28 PM, James Knight wrote:
[snip]
> diff --git a/package/yad/Config.in b/package/yad/Config.in
> new file mode 100644
> index 0000000..1e6776b
> --- /dev/null
> +++ b/package/yad/Config.in
> @@ -0,0 +1,13 @@
> +config BR2_PACKAGE_YAD
> +	bool "yad"
> +	depends on BR2_PACKAGE_XORG7
> +	depends on BR2_PACKAGE_LIBGTK2 || BR2_PACKAGE_LIBGTK3

In the next line you are going to select BR2_PACKAGE_LIBGLIB2, so you
need to propagate its dependencies to this package as well. That means
you need to add right here the following lines:

depends on BR2_USE_WCHAR # libglib2
depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
depends on BR2_USE_MMU # libglib2

> +	select BR2_PACKAGE_LIBGLIB2
> +	help
> +	  YAD (yet another dialog) is a tool for create graphical dialogs
> +	  from shell scripts.

The word "from" still fits in the first line as it will not exceed the
72 characters limit.

> +
> +	  http://sourceforge.net/projects/yad-dialog/
> +
> +comment "yad needs libgtk2 or libgtk3"
> +	depends on !BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3

And you also need to update this comment since now you have the libglib2
dependencies as well. Something like this would be ok:

comment "yad needs libgtk2 or libgtk3 and a toolchain w/ wchar, threads"
  depends on BR2_USE_MMU
  depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS || \
    (!BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3)

Be careful if you copy-paste the above text. You have to use tabs for
indentation instead of spaces :-)

Regards,

Vincent.

[Buildroot] [PATCH v3 1/1] yad: new package

[James Knight]

Vincent,

On Wed, Oct 7, 2015 at 6:08 AM, Vicente Olivert Riera
<Vincent.Riera@imgtec.com> wrote:
> In the next line you are going to select BR2_PACKAGE_LIBGLIB2, so you
> need to propagate its dependencies to this package as well. That means
> you need to add right here the following lines:
>
> depends on BR2_USE_WCHAR # libglib2
> depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
> depends on BR2_USE_MMU # libglib2

Gotcha. I'll get that fixed.

On this point, is the common practice to copy the `depends` of
packages you `select` on (ex. for this yad package, if I select on
`BR2_PACKAGE_LIBGLIB2`, I then check the libglib2 package and copy the
`depends` references it has and add the command "libglib2" to each
line)?

> The word "from" still fits in the first line as it will not exceed the
> 72 characters limit.

Does it? For myself I have three (3) characters left after "dialogs"
(4+2+69+3). Is a four (4) character tab not the expected length or is
it something else? Curious if I've been messing up all my Config.in
files. :o

>> +
>> +       http://sourceforge.net/projects/yad-dialog/
>> +
>> +comment "yad needs libgtk2 or libgtk3"
>> +     depends on !BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3
>
> And you also need to update this comment since now you have the libglib2
> dependencies as well. Something like this would be ok:
>
> comment "yad needs libgtk2 or libgtk3 and a toolchain w/ wchar, threads"
>   depends on BR2_USE_MMU
>   depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS || \
>     (!BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3)

Will do. Thanks.

> Be careful if you copy-paste the above text. You have to use tabs for
> indentation instead of spaces :-)

Always paranoid about this; unfortunately my paranoia doesn't catch it
all the time. :(

[Buildroot] [PATCH v3 1/1] yad: new package

[Vicente Olivert Riera]

Dear James Knight,

On 10/07/2015 04:46 PM, James Knight wrote:
> Vincent,
> 
> On Wed, Oct 7, 2015 at 6:08 AM, Vicente Olivert Riera
> <Vincent.Riera@imgtec.com> wrote:
>> In the next line you are going to select BR2_PACKAGE_LIBGLIB2, so you
>> need to propagate its dependencies to this package as well. That means
>> you need to add right here the following lines:
>>
>> depends on BR2_USE_WCHAR # libglib2
>> depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
>> depends on BR2_USE_MMU # libglib2
> 
> Gotcha. I'll get that fixed.
> 
> On this point, is the common practice to copy the `depends` of
> packages you `select` on (ex. for this yad package, if I select on
> `BR2_PACKAGE_LIBGLIB2`, I then check the libglib2 package and copy the
> `depends` references it has and add the command "libglib2" to each
> line)?

yes, that's mandatory. When you select a package, you have to propagate
its dependencies to the package which is selecting it. Otherwise, that
package could select the other one despite of not satisfying the
dependencies. For instance, in your case, if you select the "yad"
package, it will automatically select the "libglib2" package. And that
could be done even if your toolchain doesn't have threads support. Then,
buildroot will try to build "libglib2" and it will fail.

A package shouldn't be able to select another one if the dependencies of
the other package are not satisfied.

>> The word "from" still fits in the first line as it will not exceed the
>> 72 characters limit.
> 
> Does it? For myself I have three (3) characters left after "dialogs"
> (4+2+69+3). Is a four (4) character tab not the expected length or is
> it something else? Curious if I've been messing up all my Config.in
> files. :o

Note that the tab counts as only one character, despite of the
configuration of your text editor which displays the tabs as 4 spaces.
If you use vim, is worth to have these options in the ~/.vimrc file when
you write commit logs:

set textwidth=72
set wrap

This will wrap lines automatically when you reach the 72 characters limit.

>>> +
>>> +       http://sourceforge.net/projects/yad-dialog/
>>> +
>>> +comment "yad needs libgtk2 or libgtk3"
>>> +     depends on !BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3
>>
>> And you also need to update this comment since now you have the libglib2
>> dependencies as well. Something like this would be ok:
>>
>> comment "yad needs libgtk2 or libgtk3 and a toolchain w/ wchar, threads"
>>   depends on BR2_USE_MMU
>>   depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS || \
>>     (!BR2_PACKAGE_LIBGTK2 && !BR2_PACKAGE_LIBGTK3)
> 
> Will do. Thanks.
> 
>> Be careful if you copy-paste the above text. You have to use tabs for
>> indentation instead of spaces :-)
> 
> Always paranoid about this; unfortunately my paranoia doesn't catch it
> all the time. :(

hehe, hopefully we will catch it during the review process :P

Regards,

Vincent.