diff mbox

[v4,3/3] package/gtksharp: new package

Message ID 1423748972-17471-4-git-send-email-angelo.compagnucci@gmail.com
State Superseded
Headers show

Commit Message

Angelo Compagnucci Feb. 12, 2015, 1:49 p.m. UTC 
This patch adds gtksharp package for mono.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/Config.in            |  5 +++++
 package/gtksharp/Config.in   |  9 +++++++++
 package/gtksharp/gtksharp.mk | 15 +++++++++++++++
 3 files changed, 29 insertions(+)
 create mode 100644 package/gtksharp/Config.in
 create mode 100644 package/gtksharp/gtksharp.mk

Comments

Thomas Petazzoni Feb. 14, 2015, 7:44 a.m. UTC | #1 
Dear Angelo Compagnucci,

On Thu, 12 Feb 2015 14:49:32 +0100, Angelo Compagnucci wrote:

> diff --git a/package/gtksharp/Config.in b/package/gtksharp/Config.in
> new file mode 100644
> index 0000000..8f3af2e
> --- /dev/null
> +++ b/package/gtksharp/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_GTKSHARP
> +	bool "gtk#"
> +	depends on BR2_PACKAGE_MONO

Not needed, since this Config.in file is already only included if Mono
is enabled, as per your change in package/Config.in.

> +	select BR2_PACKAGE_LIBGTK3

You can't just select libgtk3, you also need to inherit all its
dependencies. However, I am not sure in such situation if we really
want to "select" libgtk3 or to "depend" on it. The reason is that by
using "select", the user may not necessarily go to the libgtk3 option
to select the display backend. And by default, the display backend is
the Broadway backend, which most likely isn't what the user wants.

So for this case, I would personally lean towards using "depends on
BR2_PACKAGE_LIBGTK3", especially since it's quite obvious for the user
who wants gtk# that the gtk library should be installed.

> +	select BR2_TARGET_TZ_INFO

More details about why tz information are needed would be useful. In
any case, this should be:

	select BR2_TARGET_TZ_INFO if BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_UCLIBC

because the BR2_TARGET_TZ_INFO option is not available for Musl toolchains.

> +	help
> +	  Gtk# is a .NET language binding for the GTK+ toolkit.
> +
> +	  http://ftp.gnome.org/pub/gnome/sources/gtk-sharp/2.99/

In the Config.in help text, we want the main web site of the project,
not the download URL. So most likely, this should instead be:

	  http://www.mono-project.com/docs/gui/gtksharp/

> diff --git a/package/gtksharp/gtksharp.mk b/package/gtksharp/gtksharp.mk
> new file mode 100644
> index 0000000..948db19
> --- /dev/null
> +++ b/package/gtksharp/gtksharp.mk
> @@ -0,0 +1,15 @@
> +#############################################################
> +#
> +# gtksharp
> +#
> +#############################################################

80 # signs are the convention.

> +
> +GTKSHARP_VERSION = 2.99.3
> +GTKSHARP_SITE = http://ftp.gnome.org/pub/gnome/sources/gtk-sharp/2.99/

Please use the GTKSHARP_VERSION_MAJOR thing used in several other packages:

GTKSHARP_VERSION_MAJOR = 2.99
GTKSHAP_VERSION = $(GTKSHARP_VERSION_MAJOR).3
GTKSHARP_SITE = ......./gtk-sharp/$(GTKSHARP_VERSION_MAJOR)/

> +GTKSHARP_SOURCE = gtk-sharp-$(GTKSHARP_VERSION).tar.xz
> +GTKSHARP_LICENSE = LGPLv2
> +GTKSHARP_LICENSE_FILES = COPYING
> +GTKSHARP_INSTALL_STAGING = YES
> +GTKSHARP_DEPENDENCIES += mono libgtk3

Minor detail: the += sign is not needed here, you could just do '='.

Other than that looks good. Really, the only thing a bit weird about
this series is the time zone related thing.

Thanks,

Thomas
Angelo Compagnucci Feb. 14, 2015, 8:28 a.m. UTC | #2 
Dear Thomas Petazzoni,

2015-02-14 8:44 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear Angelo Compagnucci,
>
> On Thu, 12 Feb 2015 14:49:32 +0100, Angelo Compagnucci wrote:
>
>> diff --git a/package/gtksharp/Config.in b/package/gtksharp/Config.in
>> new file mode 100644
>> index 0000000..8f3af2e
>> --- /dev/null
>> +++ b/package/gtksharp/Config.in
>> @@ -0,0 +1,9 @@
>> +config BR2_PACKAGE_GTKSHARP
>> +     bool "gtk#"
>> +     depends on BR2_PACKAGE_MONO
>
> Not needed, since this Config.in file is already only included if Mono
> is enabled, as per your change in package/Config.in.

Ok!

>> +     select BR2_PACKAGE_LIBGTK3
>
> You can't just select libgtk3, you also need to inherit all its
> dependencies. However, I am not sure in such situation if we really
> want to "select" libgtk3 or to "depend" on it. The reason is that by
> using "select", the user may not necessarily go to the libgtk3 option/she need to enable
> to select the display backend. And by default, the display backend is
> the Broadway backend, which most likely isn't what the user wants.
>
> So for this case, I would personally lean towards using "depends on
> BR2_PACKAGE_LIBGTK3", especially since it's quite obvious for the user
> who wants gtk# that the gtk library should be installed.

Honestly, I will stay with "select" but expanding it with all libgtk3
selects and depens is impractical, so let's go for depend.
For the backend side, users should be a little bit aware of what they
are doing and selecting a proper backend.

We can default on another backend for libgtk3, but this is not
strictly related to gtk#. I think that having an X11 default backend
for libgtk3 is more sensible than broadway.

>
>> +     select BR2_TARGET_TZ_INFO
>
> More details about why tz information are needed would be useful. In
> any case, this should be:
>
>         select BR2_TARGET_TZ_INFO if BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_UCLIBC
>
> because the BR2_TARGET_TZ_INFO option is not available for Musl toolchains.

Please refer to the thread regarding "[PATCH v4 2/3] system:
Defaulting TZ_LOCALTIME to UTC" for a better explanation.

>
>> +     help
>> +       Gtk# is a .NET language binding for the GTK+ toolkit.
>> +
>> +       http://ftp.gnome.org/pub/gnome/sources/gtk-sharp/2.99/
>
> In the Config.in help text, we want the main web site of the project,
> not the download URL. So most likely, this should instead be:
>
>           http://www.mono-project.com/docs/gui/gtksharp/

Ok!

>> diff --git a/package/gtksharp/gtksharp.mk b/package/gtksharp/gtksharp.mk
>> new file mode 100644
>> index 0000000..948db19
>> --- /dev/null
>> +++ b/package/gtksharp/gtksharp.mk
>> @@ -0,0 +1,15 @@
>> +#############################################################
>> +#
>> +# gtksharp
>> +#
>> +#############################################################
>
> 80 # signs are the convention.

Ok, I just copied from another package!

>
>> +
>> +GTKSHARP_VERSION = 2.99.3
>> +GTKSHARP_SITE = http://ftp.gnome.org/pub/gnome/sources/gtk-sharp/2.99/
>
> Please use the GTKSHARP_VERSION_MAJOR thing used in several other packages:
>
> GTKSHARP_VERSION_MAJOR = 2.99
> GTKSHAP_VERSION = $(GTKSHARP_VERSION_MAJOR).3
> GTKSHARP_SITE = ......./gtk-sharp/$(GTKSHARP_VERSION_MAJOR)/

Ok!

>> +GTKSHARP_SOURCE = gtk-sharp-$(GTKSHARP_VERSION).tar.xz
>> +GTKSHARP_LICENSE = LGPLv2
>> +GTKSHARP_LICENSE_FILES = COPYING
>> +GTKSHARP_INSTALL_STAGING = YES
>> +GTKSHARP_DEPENDENCIES += mono libgtk3
>
> Minor detail: the += sign is not needed here, you could just do '='.

OK!

> Other than that looks good. Really, the only thing a bit weird about
> this series is the time zone related thing.
>
> Thanks,

Thank you too Thomas!

>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index fe3d3d0..27daf2a 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -464,6 +464,11 @@  menu "Lua libraries/modules"
 endmenu
 endif
 	source "package/mono/Config.in"
+if BR2_PACKAGE_MONO
+menu "Mono libraries/modules"
+	source "package/gtksharp/Config.in"
+endmenu
+endif
 	source "package/nodejs/Config.in"
 	source "package/perl/Config.in"
 if BR2_PACKAGE_PERL
diff --git a/package/gtksharp/Config.in b/package/gtksharp/Config.in
new file mode 100644
index 0000000..8f3af2e
--- /dev/null
+++ b/package/gtksharp/Config.in
@@ -0,0 +1,9 @@ 
+config BR2_PACKAGE_GTKSHARP
+	bool "gtk#"
+	depends on BR2_PACKAGE_MONO
+	select BR2_PACKAGE_LIBGTK3
+	select BR2_TARGET_TZ_INFO
+	help
+	  Gtk# is a .NET language binding for the GTK+ toolkit.
+
+	  http://ftp.gnome.org/pub/gnome/sources/gtk-sharp/2.99/
diff --git a/package/gtksharp/gtksharp.mk b/package/gtksharp/gtksharp.mk
new file mode 100644
index 0000000..948db19
--- /dev/null
+++ b/package/gtksharp/gtksharp.mk
@@ -0,0 +1,15 @@ 
+#############################################################
+#
+# gtksharp
+#
+#############################################################
+
+GTKSHARP_VERSION = 2.99.3
+GTKSHARP_SITE = http://ftp.gnome.org/pub/gnome/sources/gtk-sharp/2.99/
+GTKSHARP_SOURCE = gtk-sharp-$(GTKSHARP_VERSION).tar.xz
+GTKSHARP_LICENSE = LGPLv2
+GTKSHARP_LICENSE_FILES = COPYING
+GTKSHARP_INSTALL_STAGING = YES
+GTKSHARP_DEPENDENCIES += mono libgtk3
+
+$(eval $(autotools-package))