Patchwork [v2,2/2] rxvt-unicode: new package

login
register
mail settings
Submitter Stefan Fröberg
Date Dec. 27, 2012, 6:45 p.m.
Message ID <1356633918-16114-2-git-send-email-stefan.froberg@petroprogram.com>
Download mbox | patch
Permalink /patch/208350/
State Rejected
Headers show

Comments

Stefan Fröberg - Dec. 27, 2012, 6:45 p.m.
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
Thomas Petazzoni - Jan. 3, 2013, 11:27 p.m.
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
Stefan Fröberg - Jan. 4, 2013, 2:42 p.m.
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
Thomas Petazzoni - Jan. 4, 2013, 3:03 p.m.
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
Stefan Fröberg - Jan. 4, 2013, 3:28 p.m.
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
Thomas Petazzoni - Jan. 4, 2013, 4:23 p.m.
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

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
+