[Buildroot] [PATCH v2 1/2] rxvt-unicode: new package

[Buildroot] [PATCH v2 1/2] rxvt-unicode: new package

[Stefan Fröberg]

Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
---
 package/Config.in                    |    1 +
 package/rxvt-unicode/Config.in       |    9 +++++++++
 package/rxvt-unicode/rxvt-unicode.mk |   12 ++++++++++++
 3 files changed, 22 insertions(+), 0 deletions(-)
 create mode 100644 package/rxvt-unicode/Config.in
 create mode 100644 package/rxvt-unicode/rxvt-unicode.mk

diff --git a/package/Config.in b/package/Config.in
index 66445ff..73bab6e 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -169,6 +169,7 @@ source "package/leafpad/Config.in"
 source "package/midori/Config.in"
 source "package/pcmanfm/Config.in"
 source "package/rdesktop/Config.in"
+source "package/rxvt-unicode/Config.in"
 source "package/sylpheed/Config.in"
 source "package/synergy/Config.in"
 source "package/torsmo/Config.in"
diff --git a/package/rxvt-unicode/Config.in b/package/rxvt-unicode/Config.in
new file mode 100644
index 0000000..3d819ce
--- /dev/null
+++ b/package/rxvt-unicode/Config.in
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_RXVT_UNICODE
+	bool "rxvt-unicode"
+	select BR2_PACKAGE_FONTCONFIG
+	select BR2_PACKAGE_NCURSES
+	depends on BR2_PACKAGE_XORG7
+	help
+	  rxvt clone with xft and unicode support
+
+	  http://software.schmorp.de/pkg/rxvt-unicode
diff --git a/package/rxvt-unicode/rxvt-unicode.mk b/package/rxvt-unicode/rxvt-unicode.mk
new file mode 100644
index 0000000..009b40e
--- /dev/null
+++ b/package/rxvt-unicode/rxvt-unicode.mk
@@ -0,0 +1,12 @@
+#############################################################
+#
+# rxvt-unicode
+#
+#############################################################
+RXVT_UNICODE_VERSION = 9.15
+RXVT_UNICODE_SOURCE = rxvt-unicode-$(RXVT_UNICODE_VERSION).tar.bz2
+RXVT_UNICODE_SITE = http://dist.schmorp.de/rxvt-unicode
+RXVT_UNICODE_DEPENDENCIES = fontconfig ncurses xlib_libX11 xlib_libXrender
+RXVT_UNICODE_CONF_OPT += --enable-everything --disable-perl --with-x --with-terminfo=/usr/share/terminfo
+
+$(eval $(autotools-package))
-- 
1.7.7.6

[Buildroot] [PATCH v2 2/2] rxvt-unicode: new package

[Stefan Fröberg]

This patch will ensure that rxvt-terminfo files will be created.
Upstream-Status: Pending

Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
---
 .../rxvt-unicode/rxvt-unicode-9.15-terminfo.patch  |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
 create mode 100644 package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch

diff --git a/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch b/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
new file mode 100644
index 0000000..910ce76
--- /dev/null
+++ b/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
@@ -0,0 +1,12 @@
+diff -Naur rxvt-unicode-9.15.org/doc/Makefile.in rxvt-unicode-9.15/doc/Makefile.in
+--- rxvt-unicode-9.15.org/doc/Makefile.in	2012-04-11 23:57:47.105465883 +0300
++++ rxvt-unicode-9.15/doc/Makefile.in	2012-04-12 00:02:13.848367530 +0300
+@@ -102,7 +102,7 @@
+ 	$(INSTALL_DATA) rxvt.7.man  $(DESTDIR)$(man7dir)/$(RXVTNAME).$(man7ext)
+ @IF_PERL@	$(INSTALL) -d $(DESTDIR)$(man3dir)
+ @IF_PERL@	$(INSTALL_DATA) rxvtperl.3.man $(DESTDIR)$(man3dir)/$(RXVTNAME)perl.$(man3ext)
+-	@TIC@ $(srcdir)/etc/rxvt-unicode.terminfo
++	@TIC@ $(srcdir)/etc/rxvt-unicode.terminfo -o $(DESTDIR)/usr/share/terminfo
+ 
+ distdepend: alldoc
+ 
-- 
1.7.7.6

[Buildroot] [PATCH v2 2/2] rxvt-unicode: new package

[Thomas Petazzoni]

Dear Stefan Fr?berg,

On Thu, 27 Dec 2012 20:45:18 +0200, Stefan Fr?berg wrote:
> This patch will ensure that rxvt-terminfo files will be created.
> Upstream-Status: Pending
> 
> Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>

Please merge this patch into the previous patch.

It seems that there a misunderstanding on how patches should be
submitted. If your new package requires a certain number of patches to
build or behave properly, then those patches should be part of the
patch adding the package.

Generally, for simple packages, only one patch to Buildroot is needed.

> ---
>  .../rxvt-unicode/rxvt-unicode-9.15-terminfo.patch  |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>  create mode 100644 package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
> 
> diff --git a/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch b/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
> new file mode 100644
> index 0000000..910ce76
> --- /dev/null
> +++ b/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
> @@ -0,0 +1,12 @@

Your patch lacks a description and Signed-off-by line.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 2/2] rxvt-unicode: new package

[Stefan Fröberg]

4.1.2013 1:27, Thomas Petazzoni kirjoitti:
> Dear Stefan Fr?berg,
>
> On Thu, 27 Dec 2012 20:45:18 +0200, Stefan Fr?berg wrote:
>> This patch will ensure that rxvt-terminfo files will be created.
>> Upstream-Status: Pending
>>
>> Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
> Please merge this patch into the previous patch.
>
> It seems that there a misunderstanding on how patches should be
> submitted. If your new package requires a certain number of patches to
> build or behave properly, then those patches should be part of the
> patch adding the package.
>
> Generally, for simple packages, only one patch to Buildroot is needed.

Yeah, Im still confused when to break it to pieces.

But Isn't it much more easier to drop some patches if I wrap them to
individual,
isolated patches ??

I mean, for example that elfutils patch set that I posted, it has 12
files and if
one of those files (except #1 and #2 ofcourse) seems to not work out
then isn't it
much more easier to just drop that one problematic patch than to
remodify one huge patch again and again? Right?

And I would never have submitted that firefox patch in one huge chunk
because
it would have been too much work for poor reviewer(s)
(thanks a million about reviewing it Arnout! i haven't forgotted about
those fixes you mentioned
and will kick new patch series out soon)

So for small packages single patch is kosher but for large (where does
the line go?) it is acceptable
to break it (like what Yan and others do) ???

Regards
Stefan



>> ---
>>  .../rxvt-unicode/rxvt-unicode-9.15-terminfo.patch  |   12 ++++++++++++
>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>  create mode 100644 package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
>>
>> diff --git a/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch b/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
>> new file mode 100644
>> index 0000000..910ce76
>> --- /dev/null
>> +++ b/package/rxvt-unicode/rxvt-unicode-9.15-terminfo.patch
>> @@ -0,0 +1,12 @@
> Your patch lacks a description and Signed-off-by line.
>
> Thanks!
>
> Thomas

[Buildroot] [PATCH v2 2/2] rxvt-unicode: new package

[Thomas Petazzoni]

Dear Stefan Fr?berg,

On Fri, 04 Jan 2013 16:42:45 +0200, Stefan Fr?berg wrote:

> Yeah, Im still confused when to break it to pieces.
> 
> But Isn't it much more easier to drop some patches if I wrap them to
> individual,
> isolated patches ??
> 
> I mean, for example that elfutils patch set that I posted, it has 12
> files and if
> one of those files (except #1 and #2 ofcourse) seems to not work out
> then isn't it
> much more easier to just drop that one problematic patch than to
> remodify one huge patch again and again? Right?
> 
> And I would never have submitted that firefox patch in one huge chunk
> because
> it would have been too much work for poor reviewer(s)
> (thanks a million about reviewing it Arnout! i haven't forgotted about
> those fixes you mentioned
> and will kick new patch series out soon)
> 
> So for small packages single patch is kosher but for large (where does
> the line go?) it is acceptable
> to break it (like what Yan and others do) ???

What you have to realize is that a patch applied to Buildroot must not
break the build. So it must be fully self-contained.

This is not the case with your elfutils patch set: the first patch adds
the package, and later patches add patches to make the package build
properly under certain conditions. This is not correct, as if we apply
only the first patch, then the build is broken.

So of course, for large packages, you can split things in several
pieces, but each piece should work on its own. So for example, you can
submit a first patch adding the package in a minimal way (supporting
only few features and options), and then gradually add more things.

See Yann's patches with Qemu: he adds a minimal support for Qemu (but
something that *builds*), and then in followup patches add more
features.

Splitting patches is *NOT* about the size of the patches. It is about
each patch carrying a logical change, that has a meaning on its own,
and that preserves the "buildability" of Buildroot.

For example, in your elfutils case, what I may end up doing is
splitting things like this:

 * One patch adds the elfutils package, with a dependency on glibc, so
   that I don't need all the uClibc specific fixes.

 * A second patch adds the uClibc specific fixes, and removes the
   dependency on glibc.

The first patch on its own works and builds. It doesn't need anything
from the second patch. The second patch simply improves things.

None of this discussion is specific to Buildroot. Is it exactly the
same story if you do Linux kernel development, U-Boot development or
anything else.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 2/2] rxvt-unicode: new package

[Stefan Fröberg]

Hi Thomas

4.1.2013 17:03, Thomas Petazzoni kirjoitti:
> Dear Stefan Fr?berg,
>
> On Fri, 04 Jan 2013 16:42:45 +0200, Stefan Fr?berg wrote:
>
>> Yeah, Im still confused when to break it to pieces.
>>
>> But Isn't it much more easier to drop some patches if I wrap them to
>> individual,
>> isolated patches ??
>>
>> I mean, for example that elfutils patch set that I posted, it has 12
>> files and if
>> one of those files (except #1 and #2 ofcourse) seems to not work out
>> then isn't it
>> much more easier to just drop that one problematic patch than to
>> remodify one huge patch again and again? Right?
>>
>> And I would never have submitted that firefox patch in one huge chunk
>> because
>> it would have been too much work for poor reviewer(s)
>> (thanks a million about reviewing it Arnout! i haven't forgotted about
>> those fixes you mentioned
>> and will kick new patch series out soon)
>>
>> So for small packages single patch is kosher but for large (where does
>> the line go?) it is acceptable
>> to break it (like what Yan and others do) ???
> What you have to realize is that a patch applied to Buildroot must not
> break the build. So it must be fully self-contained.
>
> This is not the case with your elfutils patch set: the first patch adds
> the package, and later patches add patches to make the package build
> properly under certain conditions. This is not correct, as if we apply
> only the first patch, then the build is broken.
>
> So of course, for large packages, you can split things in several
> pieces, but each piece should work on its own. So for example, you can
> submit a first patch adding the package in a minimal way (supporting
> only few features and options), and then gradually add more things.
>
> See Yann's patches with Qemu: he adds a minimal support for Qemu (but
> something that *builds*), and then in followup patches add more
> features.
>
> Splitting patches is *NOT* about the size of the patches. It is about
> each patch carrying a logical change, that has a meaning on its own,
> and that preserves the "buildability" of Buildroot.
>
> For example, in your elfutils case, what I may end up doing is
> splitting things like this:
>
>  * One patch adds the elfutils package, with a dependency on glibc, so
>    that I don't need all the uClibc specific fixes.
>
>  * A second patch adds the uClibc specific fixes, and removes the
>    dependency on glibc.
>
> The first patch on its own works and builds. It doesn't need anything
> from the second patch. The second patch simply improves things.
>
> None of this discussion is specific to Buildroot. Is it exactly the
> same story if you do Linux kernel development, U-Boot development or
> anything else.
>
> Best regards,
>
> Thomas

Ah, I see.

So for example in this elfutils case it should have been one big patch
(because all those individual
*.patch files are really needed to make it build).
 
And any extra patches in that series would just add features
(like BR2_PACKAGE_ELFUTILS_ZLiB_SUPPORT) or fix something ?

In other words, first patch is the buildable core and everything else
optional ?

Stefan

[Buildroot] [PATCH v2 2/2] rxvt-unicode: new package

[Thomas Petazzoni]

Dear Stefan Fr?berg,

On Fri, 04 Jan 2013 17:28:47 +0200, Stefan Fr?berg wrote:

> So for example in this elfutils case it should have been one big patch
> (because all those individual
> *.patch files are really needed to make it build).
>  
> And any extra patches in that series would just add features
> (like BR2_PACKAGE_ELFUTILS_ZLiB_SUPPORT) or fix something ?
> 
> In other words, first patch is the buildable core and everything else
> optional ?

Yes, we could summarize it this way.

Or, as I said, since the biggest problem with elfutils is uClibc
support, the first patch could have added a glibc-only elfutils (which
proper depends on) and then a second patch could add the necessary
additional crap to make it uClibc friendly.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 1/2] rxvt-unicode: new package

[Thomas Petazzoni]

Dear Stefan Fr?berg,

On Thu, 27 Dec 2012 20:45:17 +0200, Stefan Fr?berg wrote:
> Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
> ---
>  package/Config.in                    |    1 +
>  package/rxvt-unicode/Config.in       |    9 +++++++++
>  package/rxvt-unicode/rxvt-unicode.mk |   12 ++++++++++++
>  3 files changed, 22 insertions(+), 0 deletions(-)
>  create mode 100644 package/rxvt-unicode/Config.in
>  create mode 100644 package/rxvt-unicode/rxvt-unicode.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index 66445ff..73bab6e 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -169,6 +169,7 @@ source "package/leafpad/Config.in"
>  source "package/midori/Config.in"
>  source "package/pcmanfm/Config.in"
>  source "package/rdesktop/Config.in"
> +source "package/rxvt-unicode/Config.in"
>  source "package/sylpheed/Config.in"
>  source "package/synergy/Config.in"
>  source "package/torsmo/Config.in"
> diff --git a/package/rxvt-unicode/Config.in b/package/rxvt-unicode/Config.in
> new file mode 100644
> index 0000000..3d819ce
> --- /dev/null
> +++ b/package/rxvt-unicode/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_RXVT_UNICODE
> +	bool "rxvt-unicode"
> +	select BR2_PACKAGE_FONTCONFIG
> +	select BR2_PACKAGE_NCURSES
> +	depends on BR2_PACKAGE_XORG7

Please add:

	select BR2_PACKAGE_XLIB_LIBX11
	select BR2_PACKAGE_XLIB_LIBXRENDER

> +	help
> +	  rxvt clone with xft and unicode support

You're saying it uses Xft, but xlib_libXft is not part of your
dependencies. Is this normal?

> --- /dev/null
> +++ b/package/rxvt-unicode/rxvt-unicode.mk
> @@ -0,0 +1,12 @@
> +#############################################################
> +#
> +# rxvt-unicode
> +#
> +#############################################################
> +RXVT_UNICODE_VERSION = 9.15
> +RXVT_UNICODE_SOURCE = rxvt-unicode-$(RXVT_UNICODE_VERSION).tar.bz2
> +RXVT_UNICODE_SITE = http://dist.schmorp.de/rxvt-unicode
> +RXVT_UNICODE_DEPENDENCIES = fontconfig ncurses xlib_libX11 xlib_libXrender
> +RXVT_UNICODE_CONF_OPT += --enable-everything --disable-perl --with-x --with-terminfo=/usr/share/terminfo

Please cut this line:

RXVT_UNICODE_CONF_OPT += \
	--enable-everything \
	--disable-perl \
	--with-x \
	--with-terminfo=/usr/share/terminfo

Also, please add the RXVT_UNICODE_LICENSE and
RXVT_UNICODE_LICENSE_FILES variables.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 1/2] rxvt-unicode: new package

[Stefan Fröberg]

Hi Thomas

4.1.2013 1:25, Thomas Petazzoni kirjoitti:
> Dear Stefan Fr?berg,
>
> On Thu, 27 Dec 2012 20:45:17 +0200, Stefan Fr?berg wrote:
>> Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
>> ---
>>  package/Config.in                    |    1 +
>>  package/rxvt-unicode/Config.in       |    9 +++++++++
>>  package/rxvt-unicode/rxvt-unicode.mk |   12 ++++++++++++
>>  3 files changed, 22 insertions(+), 0 deletions(-)
>>  create mode 100644 package/rxvt-unicode/Config.in
>>  create mode 100644 package/rxvt-unicode/rxvt-unicode.mk
>>
>> diff --git a/package/Config.in b/package/Config.in
>> index 66445ff..73bab6e 100644
>> --- a/package/Config.in
>> +++ b/package/Config.in
>> @@ -169,6 +169,7 @@ source "package/leafpad/Config.in"
>>  source "package/midori/Config.in"
>>  source "package/pcmanfm/Config.in"
>>  source "package/rdesktop/Config.in"
>> +source "package/rxvt-unicode/Config.in"
>>  source "package/sylpheed/Config.in"
>>  source "package/synergy/Config.in"
>>  source "package/torsmo/Config.in"
>> diff --git a/package/rxvt-unicode/Config.in b/package/rxvt-unicode/Config.in
>> new file mode 100644
>> index 0000000..3d819ce
>> --- /dev/null
>> +++ b/package/rxvt-unicode/Config.in
>> @@ -0,0 +1,9 @@
>> +config BR2_PACKAGE_RXVT_UNICODE
>> +	bool "rxvt-unicode"
>> +	select BR2_PACKAGE_FONTCONFIG
>> +	select BR2_PACKAGE_NCURSES
>> +	depends on BR2_PACKAGE_XORG7
> Please add:
>
> 	select BR2_PACKAGE_XLIB_LIBX11
> 	select BR2_PACKAGE_XLIB_LIBXRENDER
>
>> +	help
>> +	  rxvt clone with xft and unicode support
> You're saying it uses Xft, but xlib_libXft is not part of your
> dependencies. Is this normal?
Ach! I forgot to add that...

>> --- /dev/null
>> +++ b/package/rxvt-unicode/rxvt-unicode.mk
>> @@ -0,0 +1,12 @@
>> +#############################################################
>> +#
>> +# rxvt-unicode
>> +#
>> +#############################################################
>> +RXVT_UNICODE_VERSION = 9.15
>> +RXVT_UNICODE_SOURCE = rxvt-unicode-$(RXVT_UNICODE_VERSION).tar.bz2
>> +RXVT_UNICODE_SITE = http://dist.schmorp.de/rxvt-unicode
>> +RXVT_UNICODE_DEPENDENCIES = fontconfig ncurses xlib_libX11 xlib_libXrender
>> +RXVT_UNICODE_CONF_OPT += --enable-everything --disable-perl --with-x --with-terminfo=/usr/share/terminfo
> Please cut this line:
>
> RXVT_UNICODE_CONF_OPT += \
> 	--enable-everything \
> 	--disable-perl \
> 	--with-x \
> 	--with-terminfo=/usr/share/terminfo
>
> Also, please add the RXVT_UNICODE_LICENSE and
> RXVT_UNICODE_LICENSE_FILES variables.

Ok, I will.

Thanks Thomas!

Stefan

> Thanks!
>
> Thomas

[Buildroot] [PATCH v2 1/2] rxvt-unicode: new package

[Thomas Petazzoni]

Dear Stefan Fr?berg,

On Thu, 27 Dec 2012 20:45:17 +0200, Stefan Fr?berg wrote:
> Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
> ---
>  package/Config.in                    |    1 +
>  package/rxvt-unicode/Config.in       |    9 +++++++++
>  package/rxvt-unicode/rxvt-unicode.mk |   12 ++++++++++++
>  3 files changed, 22 insertions(+), 0 deletions(-)
>  create mode 100644 package/rxvt-unicode/Config.in
>  create mode 100644 package/rxvt-unicode/rxvt-unicode.mk

Are you still interested in getting rxvt-unicode merged in Buildroot?
If so, are you willing to submit an updated version of your patch?

(I'm trying to clean up the patch queue from very old patches that
haven't seen any activity since a while).

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com