Message ID | 1356633918-16114-2-git-send-email-stefan.froberg@petroprogram.com |
---|---|
State | Rejected |
Headers | show |
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
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
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
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
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
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 +
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