[Buildroot] Patchwork cleanup #9: submitter notification

[Buildroot] Patchwork cleanup #9: submitter notification

[Thomas De Schampheleire]

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:

[1/1] Option to copy Linaro gconv libs to target
Stanislav Vasic <svlasic@gmail.com>
http://patchwork.ozlabs.org/patch/288022

This patch has been marked as Superseded (by someone else, presumably
Yann E. Morin), as a new patch covering the same problem has been
introduced: http://patchwork.ozlabs.org/patch/357067/. The discussion
is ongoing and hopefully we'll soon have a solution to the problem.


[RFC] core: Download all package sources
Clayton Shotwell <clshotwe@rockwellcollins.com>
http://patchwork.ozlabs.org/patch/288224

B. Rejected for now, as per feedback of Clayton.


perf: libelf is required to compile perf
Andi Shyti <andi@etezian.org>
http://patchwork.ozlabs.org/patch/288534

B reject. Manually selecting libelf if you're using older kernel
versions works fine.


[1/1] add support for congatec conga-qmx6
Fabien Lahoudere (ECASINTERS) <fabien.lahoudere@openwide.fr>
http://patchwork.ozlabs.org/patch/288951

A accept (to be refreshed)


gcc: use generic infrastructure for patches
Arnout Vandecappelle <arnout@mind.be>
http://patchwork.ozlabs.org/patch/289052

C unsure. In the 'triaging proposal' Arnout gave more feedback, also
requesting feedback from ThomasP and Peter. No feedback received after
that...


[1/2] util-linux: nommu: Add patch to use vfork in nommu arch.
Sonic Zhang <sonic.adi@gmail.com>
http://patchwork.ozlabs.org/patch/290234

[v2,1/3] e2fsprogs: nommu: Add patch to use vfork in nommu arch.
Sonic Zhang <sonic.adi@gmail.com>
http://patchwork.ozlabs.org/patch/291514

All two patches: C unsure: the first patch has received several
comments but does not seem to have a follow-up version. What to do
with all these Blackfin vfork patches is also unclear to me: should we
wait for upstream to accept them or not?


audiofile: needs dynamic library
Simon Dawson <spdawson@gmail.com>
http://patchwork.ozlabs.org/patch/290552

C unsure: the autobuild problem this patch is supposed to fixed is
still present (http://autobuild.buildroot.org/?reason=audiofile-0.3.6)
but the correctness of the patch has been under discussion. Gustavo
acknowledged the problem. I marked the patch as delegated to him.


[v2,2/3] e2fsprogs: flat: Add patch to append uuid link flag after the
blkid link flags when checking the blkid library in the FLAT binary
mode.
Sonic Zhang <sonic.adi@gmail.com>
http://patchwork.ozlabs.org/patch/291515

B reject: according to me this patch is superseded by commit
http://git.buildroot.org/buildroot/commit/?id=a8da3cd61aafd3e2fd44b87725fcc14b60b93be8.
ThomasP: could you confirm?


[v2] util-linux: flat: Add patch to skip creating dynamic libraries
Sonic Zhang <sonic.adi@gmail.com>
http://patchwork.ozlabs.org/patch/291522

C unsure, is this still relevant? Is the solution correct? I couldn't
answer it...


[2/2] mpg123: use code optimized for ARM NEON SIMD engine if available
Sven Neumann <neumann@teufel.de>
http://patchwork.ozlabs.org/patch/293605

Now superseded by commit 1cf2c6ea93d3bd855df7c9883d3882034f0568fa.

Thanks,
Thomas

[Buildroot] Patchwork cleanup #9: submitter notification

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Sun, 8 Jun 2014 22:13:56 +0200, Thomas De Schampheleire wrote:

> [1/1] Option to copy Linaro gconv libs to target
> Stanislav Vasic <svlasic@gmail.com>
> http://patchwork.ozlabs.org/patch/288022
> 
> This patch has been marked as Superseded (by someone else, presumably
> Yann E. Morin), as a new patch covering the same problem has been
> introduced: http://patchwork.ozlabs.org/patch/357067/. The discussion
> is ongoing and hopefully we'll soon have a solution to the problem.

Correct, we discussed it with Yann on IRC yesterday, and it is indeed
superseded by a more generic solution proposed by Yann, and that I
commented yesterday.

> [RFC] core: Download all package sources
> Clayton Shotwell <clshotwe@rockwellcollins.com>
> http://patchwork.ozlabs.org/patch/288224
> 
> B. Rejected for now, as per feedback of Clayton.

ACK. Did you mark is as rejected?

> perf: libelf is required to compile perf
> Andi Shyti <andi@etezian.org>
> http://patchwork.ozlabs.org/patch/288534
> 
> B reject. Manually selecting libelf if you're using older kernel
> versions works fine.

ACK. Did you mark it as rejected?

> [1/1] add support for congatec conga-qmx6
> Fabien Lahoudere (ECASINTERS) <fabien.lahoudere@openwide.fr>
> http://patchwork.ozlabs.org/patch/288951
> 
> A accept (to be refreshed)

Be careful: there are some more recent versions of this patch available
in patchwork, submitted by other persons.

> gcc: use generic infrastructure for patches
> Arnout Vandecappelle <arnout@mind.be>
> http://patchwork.ozlabs.org/patch/289052
> 
> C unsure. In the 'triaging proposal' Arnout gave more feedback, also
> requesting feedback from ThomasP and Peter. No feedback received after
> that...

Need to have a look. I think I had some objections to the patch, which
I shared in the review process.

> [1/2] util-linux: nommu: Add patch to use vfork in nommu arch.
> Sonic Zhang <sonic.adi@gmail.com>
> http://patchwork.ozlabs.org/patch/290234
> 
> [v2,1/3] e2fsprogs: nommu: Add patch to use vfork in nommu arch.
> Sonic Zhang <sonic.adi@gmail.com>
> http://patchwork.ozlabs.org/patch/291514
> 
> All two patches: C unsure: the first patch has received several
> comments but does not seem to have a follow-up version. What to do
> with all these Blackfin vfork patches is also unclear to me: should we
> wait for upstream to accept them or not?

My problem with those noMMU patches is that they seem to simply do
s/fork/vfork/ and s/exit/_exit/ without any real testing other than 'it
builds'. fork() and vfork() are not interchangeable, and therefore we
want to see a justification for each replacement of the reasons why it
works. I think for now, we've solved the problem by marking the
relevant parts of util-linux as 'depends on BR2_USE_MMU' and ditto for
e2fsprogs.

Therefore, I would indeed suggest to ask Sonic to work with the
upstream folks of util-linux and e2fsprogs to get the problems fixed.

> audiofile: needs dynamic library
> Simon Dawson <spdawson@gmail.com>
> http://patchwork.ozlabs.org/patch/290552
> 
> C unsure: the autobuild problem this patch is supposed to fixed is
> still present (http://autobuild.buildroot.org/?reason=audiofile-0.3.6)
> but the correctness of the patch has been under discussion. Gustavo
> acknowledged the problem. I marked the patch as delegated to him.

Discussed with Yann yesterday: we should reject. The problem is not
related to audiofile directly, other packages are affected as well. We
see it on audiofile because it starts with 'a' so tends to be at the
beginning of the build. Gustavo's patches are more likely to resolve
the problem correctly for all packages.

> [v2,2/3] e2fsprogs: flat: Add patch to append uuid link flag after the
> blkid link flags when checking the blkid library in the FLAT binary
> mode.
> Sonic Zhang <sonic.adi@gmail.com>
> http://patchwork.ozlabs.org/patch/291515
> 
> B reject: according to me this patch is superseded by commit
> http://git.buildroot.org/buildroot/commit/?id=a8da3cd61aafd3e2fd44b87725fcc14b60b93be8.
> ThomasP: could you confirm?

Well, it is surely not the same fix. Sonic patches were making possible
to build libblkid and e2fsprogs on noMMU platforms, while my patch
simply disallowed them. But see the explanation above: we're
unconvinced by the apparently blind replacements of fork() by vfork(),
and Sonic never came back with new iterations and proper justification.
So I'd say Reject.

> [v2] util-linux: flat: Add patch to skip creating dynamic libraries
> Sonic Zhang <sonic.adi@gmail.com>
> http://patchwork.ozlabs.org/patch/291522
> 
> C unsure, is this still relevant? Is the solution correct? I couldn't
> answer it...

This patch touches the build of three libraries: libmount, libblkid and
libuuid. libmound and libblkid are now disabled on noMMU platforms
(since the patch you pointed above). And libuuid builds just fine on
Blackfin FLAT (I just tested again to be sure).

> [2/2] mpg123: use code optimized for ARM NEON SIMD engine if available
> Sven Neumann <neumann@teufel.de>
> http://patchwork.ozlabs.org/patch/293605
> 
> Now superseded by commit 1cf2c6ea93d3bd855df7c9883d3882034f0568fa.

Correct. Did you mark Sven's patch as superseded?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] Patchwork cleanup #9: submitter notification

[Thomas De Schampheleire]

Hi Thomas,

On Mon, Jun 9, 2014 at 10:44 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Sun, 8 Jun 2014 22:13:56 +0200, Thomas De Schampheleire wrote:
>
>> [1/1] Option to copy Linaro gconv libs to target
>> Stanislav Vasic <svlasic@gmail.com>
>> http://patchwork.ozlabs.org/patch/288022
>>
>> This patch has been marked as Superseded (by someone else, presumably
>> Yann E. Morin), as a new patch covering the same problem has been
>> introduced: http://patchwork.ozlabs.org/patch/357067/. The discussion
>> is ongoing and hopefully we'll soon have a solution to the problem.
>
> Correct, we discussed it with Yann on IRC yesterday, and it is indeed
> superseded by a more generic solution proposed by Yann, and that I
> commented yesterday.
>
>> [RFC] core: Download all package sources
>> Clayton Shotwell <clshotwe@rockwellcollins.com>
>> http://patchwork.ozlabs.org/patch/288224
>>
>> B. Rejected for now, as per feedback of Clayton.
>
> ACK. Did you mark is as rejected?

yes

>
>> perf: libelf is required to compile perf
>> Andi Shyti <andi@etezian.org>
>> http://patchwork.ozlabs.org/patch/288534
>>
>> B reject. Manually selecting libelf if you're using older kernel
>> versions works fine.
>
> ACK. Did you mark it as rejected?

yes

>
>> [1/1] add support for congatec conga-qmx6
>> Fabien Lahoudere (ECASINTERS) <fabien.lahoudere@openwide.fr>
>> http://patchwork.ozlabs.org/patch/288951
>>
>> A accept (to be refreshed)
>
> Be careful: there are some more recent versions of this patch available
> in patchwork, submitted by other persons.

Thanks for mentioning this. Stephan Hoffmann did indeed send a similar patch:
http://patchwork.ozlabs.org/patch/355196/
As the e-mail explicitly references Fabien's patch, stating that it is
an improvement in the eyes of the submitter, I'm marking Fabien's
patch as Superseded.

Fabien: if you believe the mentioned patch of Stephan is not ok for
some reason, please comment on that patch.

>
>> gcc: use generic infrastructure for patches
>> Arnout Vandecappelle <arnout@mind.be>
>> http://patchwork.ozlabs.org/patch/289052
>>
>> C unsure. In the 'triaging proposal' Arnout gave more feedback, also
>> requesting feedback from ThomasP and Peter. No feedback received after
>> that...
>
> Need to have a look. I think I had some objections to the patch, which
> I shared in the review process.
>
>> [1/2] util-linux: nommu: Add patch to use vfork in nommu arch.
>> Sonic Zhang <sonic.adi@gmail.com>
>> http://patchwork.ozlabs.org/patch/290234
>>
>> [v2,1/3] e2fsprogs: nommu: Add patch to use vfork in nommu arch.
>> Sonic Zhang <sonic.adi@gmail.com>
>> http://patchwork.ozlabs.org/patch/291514
>>
>> All two patches: C unsure: the first patch has received several
>> comments but does not seem to have a follow-up version. What to do
>> with all these Blackfin vfork patches is also unclear to me: should we
>> wait for upstream to accept them or not?
>
> My problem with those noMMU patches is that they seem to simply do
> s/fork/vfork/ and s/exit/_exit/ without any real testing other than 'it
> builds'. fork() and vfork() are not interchangeable, and therefore we
> want to see a justification for each replacement of the reasons why it
> works. I think for now, we've solved the problem by marking the
> relevant parts of util-linux as 'depends on BR2_USE_MMU' and ditto for
> e2fsprogs.
>
> Therefore, I would indeed suggest to ask Sonic to work with the
> upstream folks of util-linux and e2fsprogs to get the problems fixed.

Agreed.
Sonic: please try to work with upstream to get proper support for noMMU, thanks!
In the mean time I'm marking them as Rejected in patchwork.

>
>> audiofile: needs dynamic library
>> Simon Dawson <spdawson@gmail.com>
>> http://patchwork.ozlabs.org/patch/290552
>>
>> C unsure: the autobuild problem this patch is supposed to fixed is
>> still present (http://autobuild.buildroot.org/?reason=audiofile-0.3.6)
>> but the correctness of the patch has been under discussion. Gustavo
>> acknowledged the problem. I marked the patch as delegated to him.
>
> Discussed with Yann yesterday: we should reject. The problem is not
> related to audiofile directly, other packages are affected as well. We
> see it on audiofile because it starts with 'a' so tends to be at the
> beginning of the build. Gustavo's patches are more likely to resolve
> the problem correctly for all packages.

Ok, marked as such.

>
>> [v2,2/3] e2fsprogs: flat: Add patch to append uuid link flag after the
>> blkid link flags when checking the blkid library in the FLAT binary
>> mode.
>> Sonic Zhang <sonic.adi@gmail.com>
>> http://patchwork.ozlabs.org/patch/291515
>>
>> B reject: according to me this patch is superseded by commit
>> http://git.buildroot.org/buildroot/commit/?id=a8da3cd61aafd3e2fd44b87725fcc14b60b93be8.
>> ThomasP: could you confirm?
>
> Well, it is surely not the same fix. Sonic patches were making possible
> to build libblkid and e2fsprogs on noMMU platforms, while my patch
> simply disallowed them. But see the explanation above: we're
> unconvinced by the apparently blind replacements of fork() by vfork(),
> and Sonic never came back with new iterations and proper justification.
> So I'd say Reject.

Ok, done.

>
>> [v2] util-linux: flat: Add patch to skip creating dynamic libraries
>> Sonic Zhang <sonic.adi@gmail.com>
>> http://patchwork.ozlabs.org/patch/291522
>>
>> C unsure, is this still relevant? Is the solution correct? I couldn't
>> answer it...
>
> This patch touches the build of three libraries: libmount, libblkid and
> libuuid. libmound and libblkid are now disabled on noMMU platforms
> (since the patch you pointed above). And libuuid builds just fine on
> Blackfin FLAT (I just tested again to be sure).

Ok, so I'm marking this as rejected.

>
>> [2/2] mpg123: use code optimized for ARM NEON SIMD engine if available
>> Sven Neumann <neumann@teufel.de>
>> http://patchwork.ozlabs.org/patch/293605
>>
>> Now superseded by commit 1cf2c6ea93d3bd855df7c9883d3882034f0568fa.
>
> Correct. Did you mark Sven's patch as superseded?

Yes I did.

Thanks for the feedback!
Thomas