[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Thomas De Schampheleire]

[stupid me, forgot to put the buildroot list in copy...]

On Thu, Mar 27, 2014 at 9:00 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> People in To,
>
> The buildroot community is trying to clean up the backlog of unhandled
> patches sent to the mailing list (logged in patchwork [1]). We are
> starting from the oldest patches and working our way up towards the
> present.
>
> In this mail, one or more patches you sent to the buildroot list are
> put in one of four categories:
> A. to be refreshed and resent to the list
> B. rejected
> C. we're unsure, your feedback is requested
> D. more thorough changes needed instead of the current patch
>
> If one of your patches falls into category A, it will be added to the
> buildroot todo list, meaning that someone will eventually take the
> time to refresh and resend the patch. However, if you can spare some
> time to do it yourself, then this will greatly accelerate the
> inclusion of your patch in buildroot.
>
> If one of your patches falls into category B, you can either accept
> the reasons given, or you may disagree in which case we invite you to
> discuss the matter with us. In this case, please explain why you think
> the patch should still be accepted.
>
> If one of your patches falls into category C, please provide more
> feedback. For some patches, our question to you may be if you are
> still interested in getting this patch in buildroot or not. Other
> patches may be in this category because we don't fully understand its
> purpose (yet). Your feedback will help us in making the right choice.
>
> If one of your patches falls into category D, the buildroot developers
> accept the problem that the patch is addressing, but believe it should
> be fixed in a different way, possibly requiring some changes in the
> buildroot core infrastructure.
>
> We propose a two-week period to give you some time to respond with
> your feedback.
>
> For this cleanup session, here are the patches:
>
> A. To be refreshed and resent to the list
>
> [RFC] uclibc: Don't build shared library if !HAVE_SHARED
> http://patchwork.ozlabs.org/patch/273175/
> Axel Lin
>
> Add pyside + shiboken packages
> http://patchwork.ozlabs.org/patch/275929/
> Thierry Bultel
>
> [v2,1/1] package: remove the trailing slash sign from $(PKG)_SITE variable
> http://patchwork.ozlabs.org/patch/276237/
> Jerzy Grzegorek
>
> [v3,05/11] sunxi-cedarx: bump to newer version, use armel2 binaries, add demo
> http://patchwork.ozlabs.org/patch/278299
> [v3,08/11] libpng12: new package
> http://patchwork.ozlabs.org/patch/278300
> [v3,09/11] libpng: ensure libpng12 is installed before libpng
> http://patchwork.ozlabs.org/patch/278301
> [v3,10/11] glmark2: new package
> http://patchwork.ozlabs.org/patch/278304
> [v3,11/11] mesa3d-demos: new package
> http://patchwork.ozlabs.org/patch/278305
> Spenser Gilliland
>
> B. Rejected
>
> [v2] Standardisation of $(BUILD)/.root name
> http://patchwork.ozlabs.org/patch/265680/
> Jerome Pouiller
> The stamp directory, that this patch is using, is no longer present.
> Either the patch is to be rejected completely, or a new location for
> .root to be found.
>
> infra: display current task as title of the term window
> http://patchwork.ozlabs.org/patch/265214/
> Francois Perrad
> [Reason: as stated in the patch discussion, there is a problem when
> interrupting the build. An alternative has been suggested: update the
> MESSAGEs with a progress indication instead. The patch submitter could
> send a new patch implementing this proposal if he is interested.]
>
> libgcc erroneously built as armv5 for arm920t(armv4t)
> http://patchwork.ozlabs.org/patch/278212/
> Adam Hussein
> Adam: the problem is supposed to be fixed with the following commit:
> d3539dd5: arch: pass cpu option instead of tune option on ARM
> Please try this instead and let us know if the problem remains...
>
>
> C. Unsure, your feedback is requested
>
> directfb-lua: new package
> http://patchwork.ozlabs.org/patch/262971/
> Ezequiel Garcia
> As discussed, Ezequiel is willing to refresh the patch, but it may not
> be needed if no-one is actually using the tool. Up to Ezequiel to
> decide what to do with this patch.
>
> [v2,1/1] u-boot: allow to pass a custom configuration file
> http://patchwork.ozlabs.org/patch/276286/
> Eric Jarrige
> Yann Morin gave the feedback that this patch allows to overwrite
> u-boot sources, rendering the declared license possible invalid.
> Eric: are you still interested in pursuing this patch? If so, I think
> some further discussion on it should be ignited.
>
> [v3,01/11] udev: explicitly include pthreads
> http://patchwork.ozlabs.org/patch/278298
> [v3,02/11] sunxi-mali: add explicit pthread/dl/rt dependencies
> http://patchwork.ozlabs.org/patch/278295
> Spenser Gilliland
> Spenser: the feedback on these patches was that it was unclear why the
> added dependencies could actually fix the problem. If you still
> believe these changes are necessary, could you restart the discussion
> providing more details?
>
> [RESEND] package/Makefile.in: Fix dependency for selecting uclinux as TARGET_OS
> http://patchwork.ozlabs.org/patch/277119/
> Axel Lin
> Axel: do you still think this patch is necessary? Could you restart
> the discussion on this patch? It is unclear to us whether this is the
> right solution.
>
> D. More thorough changes needed instead of the current patch
>
> [2/2] arch/Config.in: Allow ARM to select BR2_BINFMT_FLAT
> http://patchwork.ozlabs.org/patch/272450/
> arch/Config.in: Allow arm7tdmi to select BR2_BINFMT_FLAT
> http://patchwork.ozlabs.org/patch/273066/
> Axel Lin
> This problem should be handled by introducing a split between
> ARCH_HAS_MMU and ARCH_USE_MMU, as discussed in replies on the patches.
> This requires some work in the core infrastructure.
> Axel: you are welcome to propose a patch, or discuss the issue further with us.
> In the mean time, the topic has been added to the buildroot todo list [2].
>
> Thanks all for your feedback,
>
> Best regards,
> Thomas
>
>
> [1] http://patchwork.ozlabs.org/project/buildroot/list/
> [2] http://www.elinux.org/Buildroot#Todo_list

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Eric Jarrige]

Hi Thomas,

>> [v2,1/1] u-boot: allow to pass a custom configuration file
>> http://patchwork.ozlabs.org/patch/276286/
>> Eric Jarrige
>> Yann Morin gave the feedback that this patch allows to overwrite
>> u-boot sources, rendering the declared license possible invalid.

AFAIK this feature cannot overwrite the U-Boot license files and
according to the U-Boot licenses/README - "You can redistribute
U-Boot  and/or modify it under the terms of version 2 of the GNU
General Public License as published by the Free Software
Foundation."
So, it should not be an issue as long as the new config file respects
the terms of the version 2 of the GNU GPL license.

>> Eric: are you still interested in pursuing this patch? If so, I think
>> some further discussion on it should be ignited.


I submitted this patch because I think it is generic enough to support
custom U-Boot configuration file for any board without using a patch
but I can understand I am the only one customizing bootloader for
my boards.
So feel free to reject this patch if there is no interest to manage
U-Boot configuration files within BuildRoot.

Regards,
Eric

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Yann E. MORIN]

On 2014-03-31 10:58 +0200, Eric Jarrige spake thusly:
> Hi Thomas,
> 
> >> [v2,1/1] u-boot: allow to pass a custom configuration file
> >> http://patchwork.ozlabs.org/patch/276286/
> >> Eric Jarrige
> >> Yann Morin gave the feedback that this patch allows to overwrite
> >> u-boot sources, rendering the declared license possible invalid.
> 
> AFAIK this feature cannot overwrite the U-Boot license files and
> according to the U-Boot licenses/README - "You can redistribute
> U-Boot  and/or modify it under the terms of version 2 of the GNU
> General Public License as published by the Free Software
> Foundation."
> So, it should not be an issue as long as the new config file respects
> the terms of the version 2 of the GNU GPL license.

Hmm. There was maybe a bit of misunderstanding in what I said. Lemme
quote it here again:

    ---
    This is likely to overwrite a uboot source file
    with a local file, so we won't be able to generate conpliant
    legal-info when a custom comnfig file is used.
    ---

What I meant was, when running 'make legal-info', we will end up copying
the tarball of the sources, and we will miss this file (since Buildroot
is not recreating the tarballs from the build dir, but just copies what
was downloaded.)

So, this indeed can not overwrite the license file, but the sources in
legal-info will not be the exact sources used to build U-Boot, so the
legal-info will not create a compliant distribution.

That's why I oppposed the change as-is.

> >> Eric: are you still interested in pursuing this patch? If so, I think
> >> some further discussion on it should be ignited.
> 
> I submitted this patch because I think it is generic enough to support
> custom U-Boot configuration file for any board without using a patch
> but I can understand I am the only one customizing bootloader for
> my boards.
> So feel free to reject this patch if there is no interest to manage
> U-Boot configuration files within BuildRoot.

I did not say we did not want to be able to provide a custom config
file. I just said we need to be careful on the impact.

However, I see that it is possible to declare post-legal-info hooks in
packages.

So you could complement your patch with something like:

    UBOOT_CUSTOM_CONFIG = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE))
    ifneq ($(UBOOT_CUSTOM_CONFIG_FILE),)
    define UBOOT_COPY_CUSTOM_CONFIG_FILE
        $(INSTALL) -m 0644 -D $(UBOOT_CUSTOM_CONFIG_FILE) \
                   $(SOMEWHERE)
    endef
    UBOOT_POST_LEGAL_INFO_HOOKS += UBOOT_COPY_CUSTOM_CONFIG_FILE
    endif

I'll leave it to you as an exercise to find what $(SOMEWHERE) should be.
;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Arnout Vandecappelle]

On 31/03/14 19:12, Yann E. MORIN wrote:
> On 2014-03-31 10:58 +0200, Eric Jarrige spake thusly:
>> Hi Thomas,
>>
>>>> [v2,1/1] u-boot: allow to pass a custom configuration file
>>>> http://patchwork.ozlabs.org/patch/276286/
>>>> Eric Jarrige
>>>> Yann Morin gave the feedback that this patch allows to overwrite
>>>> u-boot sources, rendering the declared license possible invalid.
>>
>> AFAIK this feature cannot overwrite the U-Boot license files and
>> according to the U-Boot licenses/README - "You can redistribute
>> U-Boot  and/or modify it under the terms of version 2 of the GNU
>> General Public License as published by the Free Software
>> Foundation."
>> So, it should not be an issue as long as the new config file respects
>> the terms of the version 2 of the GNU GPL license.
> 
> Hmm. There was maybe a bit of misunderstanding in what I said. Lemme
> quote it here again:
> 
>     ---
>     This is likely to overwrite a uboot source file
>     with a local file, so we won't be able to generate conpliant
>     legal-info when a custom comnfig file is used.
>     ---
> 
> What I meant was, when running 'make legal-info', we will end up copying
> the tarball of the sources, and we will miss this file (since Buildroot
> is not recreating the tarballs from the build dir, but just copies what
> was downloaded.)
> 
> So, this indeed can not overwrite the license file, but the sources in
> legal-info will not be the exact sources used to build U-Boot, so the
> legal-info will not create a compliant distribution.

 Note that this is the same for the kernel (although a bit more vague).
One could easily argue that the .config file is part of the
infrastructure needed to build the kernel (if you've ever tried to
reverse engineer a kernel config you will know what I mean). With U-Boot
it's more obvious because the config file is a header file, but the
semantics are really the same.

 That said, this shouldn't be a reason to do the wrong thing in U-Boot.

> 
> That's why I oppposed the change as-is.
> 
>>>> Eric: are you still interested in pursuing this patch? If so, I think
>>>> some further discussion on it should be ignited.
>>
>> I submitted this patch because I think it is generic enough to support
>> custom U-Boot configuration file for any board without using a patch
>> but I can understand I am the only one customizing bootloader for
>> my boards.
>> So feel free to reject this patch if there is no interest to manage
>> U-Boot configuration files within BuildRoot.
> 
> I did not say we did not want to be able to provide a custom config
> file. I just said we need to be careful on the impact.
> 
> However, I see that it is possible to declare post-legal-info hooks in
> packages.
> 
> So you could complement your patch with something like:
> 
>     UBOOT_CUSTOM_CONFIG = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE))
>     ifneq ($(UBOOT_CUSTOM_CONFIG_FILE),)
>     define UBOOT_COPY_CUSTOM_CONFIG_FILE
>         $(INSTALL) -m 0644 -D $(UBOOT_CUSTOM_CONFIG_FILE) \
>                    $(SOMEWHERE)
>     endef
>     UBOOT_POST_LEGAL_INFO_HOOKS += UBOOT_COPY_CUSTOM_CONFIG_FILE
>     endif
> 
> I'll leave it to you as an exercise to find what $(SOMEWHERE) should be.
> ;-)

 Perhaps we should add legal-info infrastructure to support this kind of
thing. Something like

PKG_LICENSE_EXTRA_SOURCE = list of files relative to BR dir



 By the way, since this config.h copying is only useful for changing the
configuration of existing boards, I think this should be explicitly
mentioned in the help text of the option.

 BTW, note that this patch has become more useful since the deprecation
of BR2_TARGET_UBOOT_IPADDR and friends.


 Regards,
 Arnout
> 
> Regards,
> Yann E. MORIN.
> 


-- 
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

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Thomas De Schampheleire]

Arnout, Yann, Luca,

On Fri, Apr 4, 2014 at 6:22 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 31/03/14 19:12, Yann E. MORIN wrote:
>> On 2014-03-31 10:58 +0200, Eric Jarrige spake thusly:
>>> Hi Thomas,
>>>
>>>>> [v2,1/1] u-boot: allow to pass a custom configuration file
>>>>> http://patchwork.ozlabs.org/patch/276286/
>>>>> Eric Jarrige
>>>>> Yann Morin gave the feedback that this patch allows to overwrite
>>>>> u-boot sources, rendering the declared license possible invalid.
>>>
>>> AFAIK this feature cannot overwrite the U-Boot license files and
>>> according to the U-Boot licenses/README - "You can redistribute
>>> U-Boot  and/or modify it under the terms of version 2 of the GNU
>>> General Public License as published by the Free Software
>>> Foundation."
>>> So, it should not be an issue as long as the new config file respects
>>> the terms of the version 2 of the GNU GPL license.
>>
>> Hmm. There was maybe a bit of misunderstanding in what I said. Lemme
>> quote it here again:
>>
>>     ---
>>     This is likely to overwrite a uboot source file
>>     with a local file, so we won't be able to generate conpliant
>>     legal-info when a custom comnfig file is used.
>>     ---
>>
>> What I meant was, when running 'make legal-info', we will end up copying
>> the tarball of the sources, and we will miss this file (since Buildroot
>> is not recreating the tarballs from the build dir, but just copies what
>> was downloaded.)
>>
>> So, this indeed can not overwrite the license file, but the sources in
>> legal-info will not be the exact sources used to build U-Boot, so the
>> legal-info will not create a compliant distribution.
>
>  Note that this is the same for the kernel (although a bit more vague).
> One could easily argue that the .config file is part of the
> infrastructure needed to build the kernel (if you've ever tried to
> reverse engineer a kernel config you will know what I mean). With U-Boot
> it's more obvious because the config file is a header file, but the
> semantics are really the same.
>
>  That said, this shouldn't be a reason to do the wrong thing in U-Boot.
>
>>
>> That's why I oppposed the change as-is.
>>
>>>>> Eric: are you still interested in pursuing this patch? If so, I think
>>>>> some further discussion on it should be ignited.
>>>
>>> I submitted this patch because I think it is generic enough to support
>>> custom U-Boot configuration file for any board without using a patch
>>> but I can understand I am the only one customizing bootloader for
>>> my boards.
>>> So feel free to reject this patch if there is no interest to manage
>>> U-Boot configuration files within BuildRoot.
>>
>> I did not say we did not want to be able to provide a custom config
>> file. I just said we need to be careful on the impact.
>>
>> However, I see that it is possible to declare post-legal-info hooks in
>> packages.
>>
>> So you could complement your patch with something like:
>>
>>     UBOOT_CUSTOM_CONFIG = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE))
>>     ifneq ($(UBOOT_CUSTOM_CONFIG_FILE),)
>>     define UBOOT_COPY_CUSTOM_CONFIG_FILE
>>         $(INSTALL) -m 0644 -D $(UBOOT_CUSTOM_CONFIG_FILE) \
>>                    $(SOMEWHERE)
>>     endef
>>     UBOOT_POST_LEGAL_INFO_HOOKS += UBOOT_COPY_CUSTOM_CONFIG_FILE
>>     endif
>>
>> I'll leave it to you as an exercise to find what $(SOMEWHERE) should be.
>> ;-)
>
>  Perhaps we should add legal-info infrastructure to support this kind of
> thing. Something like
>
> PKG_LICENSE_EXTRA_SOURCE = list of files relative to BR dir
>
>
>
>  By the way, since this config.h copying is only useful for changing the
> configuration of existing boards, I think this should be explicitly
> mentioned in the help text of the option.
>
>  BTW, note that this patch has become more useful since the deprecation
> of BR2_TARGET_UBOOT_IPADDR and friends.
>
>

What should we do with this issue now?

Thanks,
Thomas

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Thomas De Schampheleire]

Hi,

On Thu, Mar 27, 2014 at 9:01 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
[..]
>>
>> C. Unsure, your feedback is requested
[..]
>> [v3,01/11] udev: explicitly include pthreads
>> http://patchwork.ozlabs.org/patch/278298
>> [v3,02/11] sunxi-mali: add explicit pthread/dl/rt dependencies
>> http://patchwork.ozlabs.org/patch/278295
>> Spenser Gilliland
>> Spenser: the feedback on these patches was that it was unclear why the
>> added dependencies could actually fix the problem. If you still
>> believe these changes are necessary, could you restart the discussion
>> providing more details?
>>

No feedback so far. I'm marking these patches as Rejected for now.
Feel free to reopen/resubmit them if you still think this is the
correct fix.


Best regards,
Thomas

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Spenser Gilliland]

Thomas,




On Tue, Apr 29, 2014 at 2:57 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> Hi,
>
> On Thu, Mar 27, 2014 at 9:01 PM, Thomas De Schampheleire
> <patrickdepinguin@gmail.com> wrote:
> [..]
>>>
>>> C. Unsure, your feedback is requested
> [..]
>>> [v3,01/11] udev: explicitly include pthreads
>>> http://patchwork.ozlabs.org/patch/278298
>>> [v3,02/11] sunxi-mali: add explicit pthread/dl/rt dependencies
>>> http://patchwork.ozlabs.org/patch/278295
>>> Spenser Gilliland
>>> Spenser: the feedback on these patches was that it was unclear why the
>>> added dependencies could actually fix the problem. If you still
>>> believe these changes are necessary, could you restart the discussion
>>> providing more details?
>>>
>
> No feedback so far. I'm marking these patches as Rejected for now.
> Feel free to reopen/resubmit them if you still think this is the
> correct fix.

I sent these patches when I was accidentally using an older version of
the Linaro ARM compiler.  The problem only occurred on that compiler
although I can't remember exactly which version it was.  It's fine by
me to mark them as rejected.

>
> Best regards,
> Thomas



-- 
Spenser Gilliland
Computer Engineer
Doctoral Candidate

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Thomas De Schampheleire]

Hi Spenser,

Spenser Gilliland <spenser@gillilanding.com> schreef:
>Thomas,
>
>
>On Tue, Apr 29, 2014 at 2:57 PM, Thomas De Schampheleire
><patrickdepinguin@gmail.com> wrote:
>> Hi,
>>
>> On Thu, Mar 27, 2014 at 9:01 PM, Thomas De Schampheleire
>> <patrickdepinguin@gmail.com> wrote:
>> [..]
>>>>
>>>> C. Unsure, your feedback is requested
>> [..]
>>>> [v3,01/11] udev: explicitly include pthreads
>>>> http://patchwork.ozlabs.org/patch/278298
>>>> [v3,02/11] sunxi-mali: add explicit pthread/dl/rt dependencies
>>>> http://patchwork.ozlabs.org/patch/278295
>>>> Spenser Gilliland
>>>> Spenser: the feedback on these patches was that it was unclear why the
>>>> added dependencies could actually fix the problem. If you still
>>>> believe these changes are necessary, could you restart the discussion
>>>> providing more details?
>>>>
>>
>> No feedback so far. I'm marking these patches as Rejected for now.
>> Feel free to reopen/resubmit them if you still think this is the
>> correct fix.
>
>I sent these patches when I was accidentally using an older version of
>the Linaro ARM compiler.  The problem only occurred on that compiler
>although I can't remember exactly which version it was.  It's fine by
>me to mark them as rejected.

Ok, thanks for the feedback!

Best regards,
Thomas

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Arnout Vandecappelle]

On 29/04/14 21:52, Thomas De Schampheleire wrote:
> Arnout, Yann, Luca,
> 
> On Fri, Apr 4, 2014 at 6:22 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 31/03/14 19:12, Yann E. MORIN wrote:
>>> On 2014-03-31 10:58 +0200, Eric Jarrige spake thusly:
>>>> Hi Thomas,
>>>>
>>>>>> [v2,1/1] u-boot: allow to pass a custom configuration file
>>>>>> http://patchwork.ozlabs.org/patch/276286/
>>>>>> Eric Jarrige
>>>>>> Yann Morin gave the feedback that this patch allows to overwrite
>>>>>> u-boot sources, rendering the declared license possible invalid.
>>>>
>>>> AFAIK this feature cannot overwrite the U-Boot license files and
>>>> according to the U-Boot licenses/README - "You can redistribute
>>>> U-Boot  and/or modify it under the terms of version 2 of the GNU
>>>> General Public License as published by the Free Software
>>>> Foundation."
>>>> So, it should not be an issue as long as the new config file respects
>>>> the terms of the version 2 of the GNU GPL license.
>>>
>>> Hmm. There was maybe a bit of misunderstanding in what I said. Lemme
>>> quote it here again:
>>>
>>>     ---
>>>     This is likely to overwrite a uboot source file
>>>     with a local file, so we won't be able to generate conpliant
>>>     legal-info when a custom comnfig file is used.
>>>     ---
>>>
>>> What I meant was, when running 'make legal-info', we will end up copying
>>> the tarball of the sources, and we will miss this file (since Buildroot
>>> is not recreating the tarballs from the build dir, but just copies what
>>> was downloaded.)
>>>
>>> So, this indeed can not overwrite the license file, but the sources in
>>> legal-info will not be the exact sources used to build U-Boot, so the
>>> legal-info will not create a compliant distribution.
>>
>>  Note that this is the same for the kernel (although a bit more vague).
>> One could easily argue that the .config file is part of the
>> infrastructure needed to build the kernel (if you've ever tried to
>> reverse engineer a kernel config you will know what I mean). With U-Boot
>> it's more obvious because the config file is a header file, but the
>> semantics are really the same.
>>
>>  That said, this shouldn't be a reason to do the wrong thing in U-Boot.
>>
>>>
>>> That's why I oppposed the change as-is.
>>>
>>>>>> Eric: are you still interested in pursuing this patch? If so, I think
>>>>>> some further discussion on it should be ignited.
>>>>
>>>> I submitted this patch because I think it is generic enough to support
>>>> custom U-Boot configuration file for any board without using a patch
>>>> but I can understand I am the only one customizing bootloader for
>>>> my boards.
>>>> So feel free to reject this patch if there is no interest to manage
>>>> U-Boot configuration files within BuildRoot.
>>>
>>> I did not say we did not want to be able to provide a custom config
>>> file. I just said we need to be careful on the impact.
>>>
>>> However, I see that it is possible to declare post-legal-info hooks in
>>> packages.
>>>
>>> So you could complement your patch with something like:
>>>
>>>     UBOOT_CUSTOM_CONFIG = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE))
>>>     ifneq ($(UBOOT_CUSTOM_CONFIG_FILE),)
>>>     define UBOOT_COPY_CUSTOM_CONFIG_FILE
>>>         $(INSTALL) -m 0644 -D $(UBOOT_CUSTOM_CONFIG_FILE) \
>>>                    $(SOMEWHERE)
>>>     endef
>>>     UBOOT_POST_LEGAL_INFO_HOOKS += UBOOT_COPY_CUSTOM_CONFIG_FILE
>>>     endif
>>>
>>> I'll leave it to you as an exercise to find what $(SOMEWHERE) should be.
>>> ;-)
>>
>>  Perhaps we should add legal-info infrastructure to support this kind of
>> thing. Something like
>>
>> PKG_LICENSE_EXTRA_SOURCE = list of files relative to BR dir
>>
>>
>>
>>  By the way, since this config.h copying is only useful for changing the
>> configuration of existing boards, I think this should be explicitly
>> mentioned in the help text of the option.
>>
>>  BTW, note that this patch has become more useful since the deprecation
>> of BR2_TARGET_UBOOT_IPADDR and friends.
>>
>>
> 
> What should we do with this issue now?

 For me:

* the legal-info argument is not a showstopper because it's the same for
many other buildroot features;

* the approach is not great, because it _looks_ like it makes it possible
to create a new board, which is not true;

* the patch is still very useful, and I like it much more than sedding
the config file.

 So for me, this is an A-class. However, I still have some comments on
the patch (see that thread).

 Regards,
 Arnout


-- 
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

[Buildroot] Patchwork cleanup #7: submitter notification (feedback deadline: April 12)

[Luca Ceresoli]

Hi Thomas, Arnout, Yann,

Arnout Vandecappelle wrote:
> On 29/04/14 21:52, Thomas De Schampheleire wrote:
>> Arnout, Yann, Luca,
>>
>> On Fri, Apr 4, 2014 at 6:22 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 31/03/14 19:12, Yann E. MORIN wrote:
>>>> On 2014-03-31 10:58 +0200, Eric Jarrige spake thusly:
>>>>> Hi Thomas,
>>>>>
>>>>>>> [v2,1/1] u-boot: allow to pass a custom configuration file
>>>>>>> http://patchwork.ozlabs.org/patch/276286/
>>>>>>> Eric Jarrige
>>>>>>> Yann Morin gave the feedback that this patch allows to overwrite
>>>>>>> u-boot sources, rendering the declared license possible invalid.
>>>>>
>>>>> AFAIK this feature cannot overwrite the U-Boot license files and
>>>>> according to the U-Boot licenses/README - "You can redistribute
>>>>> U-Boot  and/or modify it under the terms of version 2 of the GNU
>>>>> General Public License as published by the Free Software
>>>>> Foundation."
>>>>> So, it should not be an issue as long as the new config file respects
>>>>> the terms of the version 2 of the GNU GPL license.
>>>>
>>>> Hmm. There was maybe a bit of misunderstanding in what I said. Lemme
>>>> quote it here again:
>>>>
>>>>      ---
>>>>      This is likely to overwrite a uboot source file
>>>>      with a local file, so we won't be able to generate conpliant
>>>>      legal-info when a custom comnfig file is used.
>>>>      ---
>>>>
>>>> What I meant was, when running 'make legal-info', we will end up copying
>>>> the tarball of the sources, and we will miss this file (since Buildroot
>>>> is not recreating the tarballs from the build dir, but just copies what
>>>> was downloaded.)
>>>>
>>>> So, this indeed can not overwrite the license file, but the sources in
>>>> legal-info will not be the exact sources used to build U-Boot, so the
>>>> legal-info will not create a compliant distribution.
>>>
>>>   Note that this is the same for the kernel (although a bit more vague).
>>> One could easily argue that the .config file is part of the
>>> infrastructure needed to build the kernel (if you've ever tried to
>>> reverse engineer a kernel config you will know what I mean). With U-Boot
>>> it's more obvious because the config file is a header file, but the
>>> semantics are really the same.
>>>
>>>   That said, this shouldn't be a reason to do the wrong thing in U-Boot.
>>>
>>>>
>>>> That's why I oppposed the change as-is.
>>>>
>>>>>>> Eric: are you still interested in pursuing this patch? If so, I think
>>>>>>> some further discussion on it should be ignited.
>>>>>
>>>>> I submitted this patch because I think it is generic enough to support
>>>>> custom U-Boot configuration file for any board without using a patch
>>>>> but I can understand I am the only one customizing bootloader for
>>>>> my boards.
>>>>> So feel free to reject this patch if there is no interest to manage
>>>>> U-Boot configuration files within BuildRoot.
>>>>
>>>> I did not say we did not want to be able to provide a custom config
>>>> file. I just said we need to be careful on the impact.
>>>>
>>>> However, I see that it is possible to declare post-legal-info hooks in
>>>> packages.
>>>>
>>>> So you could complement your patch with something like:
>>>>
>>>>      UBOOT_CUSTOM_CONFIG = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE))
>>>>      ifneq ($(UBOOT_CUSTOM_CONFIG_FILE),)
>>>>      define UBOOT_COPY_CUSTOM_CONFIG_FILE
>>>>          $(INSTALL) -m 0644 -D $(UBOOT_CUSTOM_CONFIG_FILE) \
>>>>                     $(SOMEWHERE)
>>>>      endef
>>>>      UBOOT_POST_LEGAL_INFO_HOOKS += UBOOT_COPY_CUSTOM_CONFIG_FILE
>>>>      endif
>>>>
>>>> I'll leave it to you as an exercise to find what $(SOMEWHERE) should be.
>>>> ;-)
>>>
>>>   Perhaps we should add legal-info infrastructure to support this kind of
>>> thing. Something like
>>>
>>> PKG_LICENSE_EXTRA_SOURCE = list of files relative to BR dir
>>>
>>>
>>>
>>>   By the way, since this config.h copying is only useful for changing the
>>> configuration of existing boards, I think this should be explicitly
>>> mentioned in the help text of the option.
>>>
>>>   BTW, note that this patch has become more useful since the deprecation
>>> of BR2_TARGET_UBOOT_IPADDR and friends.
>>>
>>>
>>
>> What should we do with this issue now?
>
>   For me:
>
> * the legal-info argument is not a showstopper because it's the same for
> many other buildroot features;

Indeed. All the patches that are shipped with Buildroot are not
different.

Anyway it would be nice to save the U-Boot custom config file, as well
as all the patches to the enabled packages (which would be a very
welcome feature).

>
> * the approach is not great, because it _looks_ like it makes it possible
> to create a new board, which is not true;

This must be very clearly stated in the kconfig help text.

>
> * the patch is still very useful, and I like it much more than sedding
> the config file.
>
>   So for me, this is an A-class. However, I still have some comments on
> the patch (see that thread).

A-class for me too.

-- 
Luca