pkg-generic: support patching local source code
PATCH 1/1 pkg-generic: support patching local source code
--- package/pkg-generic.mk | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
package/pkg-generic.mk +5 -2
@@ -555,15 +555,18 @@ else # In the package override case, the sequence of steps # source, by rsyncing # depends +# patch # configure # Use an order-only dependency so the "<pkg>-clean-for-rebuild" rule # can remove the stamp file without triggering the configure step. -$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC) +$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_PATCH) $(1)-depends: $$($(2)_FINAL_DEPENDENCIES) -$(1)-patch: $(1)-rsync +$(1)-patch: $$($(2)_TARGET_PATCH) +$$($(2)_TARGET_PATCH): $$($(2)_TARGET_RSYNC) + $(1)-extract: $(1)-rsync $(1)-rsync: $$($(2)_TARGET_RSYNC)
Signature · 1 line
1.9.1
PATCH 1/1 pkg-generic: support patching local source code
Nicolas, All,
On 2015-04-08 17:28 -0700, Nicolas S. Dade spake thusly:
Quoted 1 line
Signed-off-by Nicolas S. Dade <[email protected]>
Thanks for your patch!
However, we're explicitly not allowing patching local sources, on purpose.
The use-case for using local sources is to allow people to use a development version of the package, one they are hacking on. Thus, we believe that in most cases, the patches bundled with Buildroot would most certainly fail to apply on local sources.
So, we think that, if those patches are still needed, the developper will have to carry them in its local sources.
Also, your commit log is not explaining the issue you are facing, so it is hard to udnerstand why you would need bundled patches to be applied, and the use-case that would cover.
I'm leaving this open for now, for others to speak up, but I'd be tempted to say "no" for the aforementioned reasons.
Regards,
Yann E. MORIN.
Quoted 36 lines
--- package/pkg-generic.mk | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)package/pkg-generic.mk +5
-2@@ -555,15 +555,18 @@ else # In the package override case, the sequence of steps # source, by rsyncing # depends +# patch # configure # Use an order-only dependency so the "<pkg>-clean-for-rebuild" rule # can remove the stamp file without triggering the configure step. -$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC) +$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_PATCH) $(1)-depends: $$($(2)_FINAL_DEPENDENCIES) -$(1)-patch: $(1)-rsync +$(1)-patch: $$($(2)_TARGET_PATCH) +$$($(2)_TARGET_PATCH): $$($(2)_TARGET_RSYNC) + $(1)-extract: $(1)-rsync $(1)-rsync: $$($(2)_TARGET_RSYNC)Signature · 6 lines
1.9.1 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 pkg-generic: support patching local source code
Let me explain what led me to allow patches to local source. My local source isn't mine. It's an SDK from the vendor of a part I'm using. The vendor is much better at hardware than software, and their SDK doesn't build as-is. So I need to patch it. I also need to maintain my patches separated from the SDK since the vendor publishes updates to their SDK every month or so.
So what I am doing is having the vendor's SDK source code unzip'ed in one directory, and have buildroot use [subtrees of] that directory as the local source, patch it with my fixes, and build it.
PS I also thought that not allowing patching only for local sources was a strange asymmetry in buildroot. I expected local sources to work the same as tarballs. It wasn't until I looked into the buildroot make system that I realized why my patches weren't getting applied. So if you don't accept this patch, at least you might document that local sources cannot be patched in a clear way.
Signature · 3 lines
Nicolas Dade nic.dade at mistsys.com 831-464-6843 On Thu, Apr 9, 2015 at 3:12 PM, Yann E. MORIN <[email protected]>
Quoted 77 lines · wrote:
Nicolas, All,
On 2015-04-08 17:28 -0700, Nicolas S. Dade spake thusly:
Quoted 1 line
Signed-off-by Nicolas S. Dade <[email protected]>Thanks for your patch!
However, we're explicitly not allowing patching local sources, on purpose.
The use-case for using local sources is to allow people to use a development version of the package, one they are hacking on. Thus, we believe that in most cases, the patches bundled with Buildroot would most certainly fail to apply on local sources.
So, we think that, if those patches are still needed, the developper will have to carry them in its local sources.
Also, your commit log is not explaining the issue you are facing, so it is hard to udnerstand why you would need bundled patches to be applied, and the use-case that would cover.
I'm leaving this open for now, for others to speak up, but I'd be tempted to say "no" for the aforementioned reasons.
Regards,
Yann E. MORIN.Quoted 32 lines
--- package/pkg-generic.mk | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)package/pkg-generic.mk +5
-3@@ -555,15 +555,18 @@ else # In the package override case, the sequence of steps # source, by rsyncing # depends +# patch # configure # Use an order-only dependency so the "<pkg>-clean-for-rebuild" rule # can remove the stamp file without triggering the configure step. -$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC) +$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_PATCH) $(1)-depends: $$($(2)_FINAL_DEPENDENCIES) -$(1)-patch: $(1)-rsync +$(1)-patch: $$($(2)_TARGET_PATCH) +$$($(2)_TARGET_PATCH): $$($(2)_TARGET_RSYNC) + $(1)-extract: $(1)-rsync $(1)-rsync: $$($(2)_TARGET_RSYNC) --1.9.1
Quoted 4 lines
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
Quoted 2 lines
'------------------------------^-------^------------------^--------------------'
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150409/d6058003/attachment.html>
PATCH 1/1 pkg-generic: support patching local source code
Nicolas, All,
On 2015-04-09 15:21 -0700, Nicolas Dade spake thusly:
Quoted 10 lines
Let me explain what led me to allow patches to local source. My local source isn't mine. It's an SDK from the vendor of a part I'm using. The vendor is much better at hardware than software, and their SDK doesn't build as-is. So I need to patch it. I also need to maintain my patches separated from the SDK since the vendor publishes updates to their SDK every month or so.
So what I am doing is having the vendor's SDK source code unzip'ed in one directory, and have buildroot use [subtrees of] that directory as the local source, patch it with my fixes, and build it.
Ah, OK, I see. (To be honest, I was expecting something like that!)
Well, there's not much we can do with that...
On the one hand, we maintain the current behaviour, and your use-case is broken. On the other hand, we solve a use-case like yours, but break others where the sources are too different for our bundled patches to be applied.
Needless to say we don't like breaking stuff...
Now, let us think about that...
Still, there's something I am wondering about. Why do you point those packages to the vendor's SDK in the first place? Why don't you use the ones referenced by Buildroot (e.g. from upstream)?
I can understand you'd want a few select packages from the SDK, mostly those with HW dependencies, but the rest?
Quoted 6 lines
PS I also thought that not allowing patching only for local sources was a strange asymmetry in buildroot. I expected local sources to work the same as tarballs. It wasn't until I looked into the buildroot make system that I realized why my patches weren't getting applied. So if you don't accept this patch, at least you might document that local sources cannot be patched in a clear way.
Well, I would think that's pretty well explained in the manual:
http://buildroot.net/downloads/manual/manual.html#_using_buildroot_during_development
Quoting:
When Buildroot finds that for a given package, an <pkg>_OVERRIDE_SRCDIR has been defined, it will no longer attempt to download, extract and patch the package. Instead, it will directly use the source code available in in the specified directory [...]
I don't know how to make that even more explicit... If you find a better phrasing, do not hesitate! ;-)
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
A message that is not in the archive (sent off-list or not yet received): CAB7JjA_EKRS9uEX9MhgUVpovh4VKoUF2vx1wE2Q+a9baZTRyCQ@mail.gmail.com
PATCH 1/1 pkg-generic: support patching local source code
Nicolas, All,
On 2015-04-09 17:35 -0700, Nicolas Dade spake thusly:
[--SNIP--]
Quoted 6 lines
Quoted 1 line
why not used the [packages in buildroot]
I do for those which are in buildroot. What I'm building in the SDK are the vendor's utilities which talk to their hardware part. Those aren't packages in [regular] buildroot, but I've added packages to my buildroot to build these vendor utilities.
OK, I misread what you wrote. The way you do it makes sense, of course.
Quoted 7 lines
Quoted 1 line
[documentation]
Ah, I see why I didn't find this in the docs. I'm not using the <pkg>_OVERRIDE_SRCDIR feature. I'm setting <pkg>_SITE to the subdir of the SDK I want, and <pkt>_SITE_METHOD to "local". That is described in section "17.5.2 generic-package reference", and doesn't mention patches or lack there-of.
Aha! But inder the hood, the 'local' site method hijacks the _OVERRIDE_SRCDIR infra. Hence the same behaviour.
Quoted 8 lines
This way buildroot rsyncs from the SDK subdir I want to build to output/<pkg>-<ver>/, applies my patches, and builds it there, and the SDK source tree is left untouched. My understanding of
_OVERRIDE_SRCDIRis that it overrides what would normally be output/<pkg>-<ver>/, and if I set it to point to the SDK I'd end up patching the pristine SDK source.Maybe the right thing is to patch if
_SITEand SITE_METHOD=local is used, and not if_OVERRIDE_SRCDIRis used.
Well, doing so is surely not the solution, because we'd be trying to revert a decision made previously. The best solution would be to teach the download infra to handle the 'local' site method by itself and not rely on OVERRIDE_SRCDIR to begin with.
However, as a workaround to your issue, I'd suggest you do somwething like the following (using two packages 'foo' and 'bar' as example):
foo.mk:
FOO_SOURCE = filename-of-SDK.tar.gz
FOO_SITE = file:///path/to/SDK
define FOO_BUILD_CMDS
cd $(@D)/relative/path/to/package/foo; \
make whatever
endef
# And similar for install and other _CMDS
bar.mk:
BAR_SOURCE = filename-of-SDK.tar.gz
BAR_SITE = file:///path/to/SDK
define BAR_BUILD_CMDS
cd $(@D)/relative/path/to/package/bar; \
make whatever-else
endef
# And similar for install and other _CMDS
And then you can have whatever patch you want to apply to those packages (but the patches should be relative to the top-dir of the SDK, not the packages' dirs).
Yes, this would mean having many packages using the same source file, but that's pretty much OK; it works even though we're not doing it for any package in Buildroot.
Let's continue discussing a proper solution for that. In the meantime, I'm marking your patch as "Rejected" in our Patchwork.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 pkg-generic: support patching local source code
In reply to Nicolas DadeHi Nicolas,
[Please don't top-post]
Quoted 9 lines · Nicolas Dade
Let me explain what led me to allow patches to local source. My local source isn't mine. It's an SDK from the vendor of a part I'm using. The vendor is much better at hardware than software, and their SDK doesn't build as-is. So I need to patch it. I also need to maintain my patches separated from the SDK since the vendor publishes updates to their SDK every month or so.
So what I am doing is having the vendor's SDK source code unzip'ed in one directory, and have buildroot use [subtrees of] that directory as the local source, patch it with my fixes, and build it.
Instead of unzipping the source code, leave it as an archive (if it is really a
zipfile instead of a tarball, you'll need to supply custom EXTRACT_CMDS). Then the patches still are applied. And it gives you much more flexibility to put the
Quoted 4 lines
PS I also thought that not allowing patching only for local sources was a strange asymmetry in buildroot. I expected local sources to work the same as tarballs.
We probably should get rid of the 'local' site method, since it is indeed
confusing and serves no purpose as far as I'm concerned.
Regards,
Arnout
Quoted 5 lines
It wasn't until I looked into the buildroot make system that I realized why my patches weren't getting applied. So if you don't accept this patch, at least you might document that local sources cannot be patched in a clear way.
[snip]
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
PATCH 1/1 pkg-generic: support patching local source code
Quoted 1 line
leave it as an archive...zip...tarball
In truth it's delivered to me as a zip file inside a VM image. I run the VM once to scp out the zip file. But that's no problem; I can repack it as a tarball.
I considered doing what you two both propose: treat the SDK as a regular tarball-based package and build just the pieces I wanted. However the pristine zip file unpacks to 2.4GB. I have the disk space, but unpacking that much several times still seems something to avoid.
I think I'll just continue to maintain my buildroot enhancement as a local patch to buildroot itself.
Quoted 1 line
get rid of the 'local' site method
If you do that I'll be patching it back in too, locally of course.
Thanks for the feedback.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150410/8b2f5555/attachment.html>