Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
@ 2022-02-03 23:54 Giulio Benetti
  2022-02-12 22:56 ` Luca Ceresoli
  2022-02-14  7:57 ` Thomas Petazzoni via buildroot
  0 siblings, 2 replies; 8+ messages in thread
From: Giulio Benetti @ 2022-02-03 23:54 UTC (permalink / raw)
  To: buildroot; +Cc: Giulio Benetti, Thomas De Schampheleire

Often new boards have not been tested with official docker so let's add
instructions to do it.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 docs/manual/adding-board-support.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/docs/manual/adding-board-support.txt b/docs/manual/adding-board-support.txt
index 33ed709535..f5fb3af371 100644
--- a/docs/manual/adding-board-support.txt
+++ b/docs/manual/adding-board-support.txt
@@ -46,3 +46,25 @@ create a directory +board/<manufacturer>+ and a subdirectory
 +board/<manufacturer>/<boardname>+. You can then store your patches
 and configurations in these directories, and reference them from the main
 Buildroot configuration. Refer to xref:customize[] for more details.
+
+Before submitting patches for new boards it would be better to test it
+by building it using .gitlab-ci.yml specified docker. For example at the
+time of this writing the docker is:
+$CI_REGISTRY/buildroot.org/buildroot/base:20220105.2314
+so:
+Pull the docker:
+--------------------
+ $ docker pull registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
+--------------------
+Run the docker:
+--------------------
+ $ sudo docker run -it registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314 /bin/bash
+--------------------
+Inside the docker hint:
+--------------------
+ $ git clone git://git.busybox.net/buildroot
+ $ cd buildroot
+ $ make +<boardname>_defconfig+
+ $ make
+--------------------
+Wait until build finishes and eventually add host dependencies.
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-03 23:54 [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker Giulio Benetti
@ 2022-02-12 22:56 ` Luca Ceresoli
  2022-02-13 10:39   ` Arnout Vandecappelle
  2022-02-14  7:57 ` Thomas Petazzoni via buildroot
  1 sibling, 1 reply; 8+ messages in thread
From: Luca Ceresoli @ 2022-02-12 22:56 UTC (permalink / raw)
  To: buildroot, Giulio Benetti; +Cc: Thomas De Schampheleire

Hi Giulio,

On 04/02/22 00:54, Giulio Benetti wrote:
> Often new boards have not been tested with official docker so let's add
> instructions to do it.

Thank you, I think this is a very useful addition to the documentation!
However I would suggest some changes for it to look more "professional".

> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
>  docs/manual/adding-board-support.txt | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/docs/manual/adding-board-support.txt b/docs/manual/adding-board-support.txt
> index 33ed709535..f5fb3af371 100644
> --- a/docs/manual/adding-board-support.txt
> +++ b/docs/manual/adding-board-support.txt
> @@ -46,3 +46,25 @@ create a directory +board/<manufacturer>+ and a subdirectory
>  +board/<manufacturer>/<boardname>+. You can then store your patches
>  and configurations in these directories, and reference them from the main
>  Buildroot configuration. Refer to xref:customize[] for more details.
> +
> +Before submitting patches for new boards it would be better to test it

"it would be better" -> "it is recommended".

> +by building it using .gitlab-ci.yml specified docker. For example at the

I think this should be reworded in a simpler way: "by building it using
the docker specified in .gitlab-ci.yml".

BTW as I am a docker newbie: is it common to say "the docker"? Or would
"the docker container" be more correct? -- By comparison, I would never
say "using the virtualbox" but rather "using the virtualbox machine".

> +time of this writing the docker is:

Remove the ':' from this line, or you'll have multiple ':' per line,
which looks awkward.

> +$CI_REGISTRY/buildroot.org/buildroot/base:20220105.2314

Hm, this string is already old. There's no sane way to keep docs and
.yml in sync. I wonder whether we should have in the manual a command
line that always use the current string, such as:

DOCKER_IMAGE=$(cat .gitlab-ci.yml | \
               sed -n '/^image/s/^.*CI_REGISTRY/registry.gitlab.com/p')
docker pull $DOCKER_IMAGE
sudo docker run -it  $DOCKER_IMAGE

However I must admit this is not very readable in the docs... :( What
about adding a simple script (utils/run-docker?) that does the trick and
just mention that in the docs?

> +so:

Add an empty line here, so that the output separates from the next line.

> +Pull the docker:
> +--------------------
> + $ docker pull registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314

Missing 'sudo'?

> +--------------------

Add an empty line here. This has no effect on the output but makes
source code more readable.

> +Run the docker:
> +--------------------
> + $ sudo docker run -it registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314 /bin/bash
> +--------------------

As above, add an empty line here.

> +Inside the docker hint:
> +--------------------
> + $ git clone git://git.busybox.net/buildroot
> + $ cd buildroot
> + $ make +<boardname>_defconfig+
> + $ make
> +--------------------

As above, add an empty line here.

> +Wait until build finishes and eventually add host dependencies.

If I understand what you mean here, it should be "and add host
dependencies if needed" ("eventually" is not the english translation of
italian "eventualmente"). If my understanding is correct, I don't find
this sentence very useful: a docker newbie perhaps doesn't know how to
add a host dependency (and maybe not even how to understand that they
are missing one).

I would just remove this line, but if you think it is very important I'd
clarify it, maybe with some examples.

-- 
Luca
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-12 22:56 ` Luca Ceresoli
@ 2022-02-13 10:39   ` Arnout Vandecappelle
  2022-02-13 23:35     ` Giulio Benetti
  0 siblings, 1 reply; 8+ messages in thread
From: Arnout Vandecappelle @ 2022-02-13 10:39 UTC (permalink / raw)
  To: Luca Ceresoli, buildroot, Giulio Benetti
  Cc: Yann E. MORIN, Thomas De Schampheleire



On 12/02/2022 23:56, Luca Ceresoli wrote:
> Hi Giulio,
> 
> On 04/02/22 00:54, Giulio Benetti wrote:
>> Often new boards have not been tested with official docker so let's add
>> instructions to do it.
> 
> Thank you, I think this is a very useful addition to the documentation!
> However I would suggest some changes for it to look more "professional".
> 
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> ---
>>   docs/manual/adding-board-support.txt | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/docs/manual/adding-board-support.txt b/docs/manual/adding-board-support.txt
>> index 33ed709535..f5fb3af371 100644
>> --- a/docs/manual/adding-board-support.txt
>> +++ b/docs/manual/adding-board-support.txt
>> @@ -46,3 +46,25 @@ create a directory +board/<manufacturer>+ and a subdirectory
>>   +board/<manufacturer>/<boardname>+. You can then store your patches
>>   and configurations in these directories, and reference them from the main
>>   Buildroot configuration. Refer to xref:customize[] for more details.
>> +
>> +Before submitting patches for new boards it would be better to test it
> 
> "it would be better" -> "it is recommended".
> 
>> +by building it using .gitlab-ci.yml specified docker. For example at the
> 
> I think this should be reworded in a simpler way: "by building it using
> the docker specified in .gitlab-ci.yml".
> 
> BTW as I am a docker newbie: is it common to say "the docker"? Or would
> "the docker container" be more correct? -- By comparison, I would never
> say "using the virtualbox" but rather "using the virtualbox machine".

  I would say "the container" since you can use it with any container manager 
that follows the OCI spec.

> 
>> +time of this writing the docker is:
> 
> Remove the ':' from this line, or you'll have multiple ':' per line,
> which looks awkward.
> 
>> +$CI_REGISTRY/buildroot.org/buildroot/base:20220105.2314
> 
> Hm, this string is already old.

  Actually, this part of the documentation is already superseded since we now 
have utils/docker-run that does everything.

> There's no sane way to keep docs and
> .yml in sync. I wonder whether we should have in the manual a command
> line that always use the current string, such as:
> 
> DOCKER_IMAGE=$(cat .gitlab-ci.yml | \
>                 sed -n '/^image/s/^.*CI_REGISTRY/registry.gitlab.com/p')
> docker pull $DOCKER_IMAGE
> sudo docker run -it  $DOCKER_IMAGE
> 
> However I must admit this is not very readable in the docs... :( What
> about adding a simple script (utils/run-docker?) that does the trick and
> just mention that in the docs?
> 
>> +so:
> 
> Add an empty line here, so that the output separates from the next line.
> 
>> +Pull the docker:
>> +--------------------
>> + $ docker pull registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
> 
> Missing 'sudo'?

  Docker access is usually managed through the "docker" group rather than sudo.

  And if you use podman as docker replacement, it can even be done in an 
unprivileged container. Not that I tried it, but I think so.

  Oh BTW, the pull is in fact not needed, both podman and docker pull 
automatically when you start a container. That's the reason the container name 
is so convoluted.

> 
>> +--------------------
> 
> Add an empty line here. This has no effect on the output but makes
> source code more readable.
> 
>> +Run the docker:
>> +--------------------
>> + $ sudo docker run -it registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314 /bin/bash
>> +--------------------
> 
> As above, add an empty line here.
> 
>> +Inside the docker hint:
>> +--------------------
>> + $ git clone git://git.busybox.net/buildroot
>> + $ cd buildroot
>> + $ make +<boardname>_defconfig+
>> + $ make
>> +--------------------
> 
> As above, add an empty line here.
> 
>> +Wait until build finishes and eventually add host dependencies.
> 
> If I understand what you mean here, it should be "and add host
> dependencies if needed" ("eventually" is not the english translation of
> italian "eventualmente"). If my understanding is correct, I don't find
> this sentence very useful: a docker newbie perhaps doesn't know how to
> add a host dependency (and maybe not even how to understand that they
> are missing one).
> 
> I would just remove this line, but if you think it is very important I'd
> clarify it, maybe with some examples.

  Yes, I think this is what triggered the addition of this documentation. If you 
have e.g. libopenssl-dev installed on your build host, then you usually won't 
notice in your test builds that a dependency on host-openssl is needed. So test 
builds should be done in a minimal container.

  Unfortunately, the buildroot/base container is not exactly minimal. It's 
really what is meant to be used for running CI tests, not exactly what is needed 
for build tests. Ideally, we'd have

- an absolutely minimal container that can be used for build tests - ideally in 
a couple of variants for different distros;
- a container for CI;
- a more complete container you could use for development, though I can't 
immediately think of extra stuff you'd want in there (but then, I wouldn't use a 
container for development).


  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-13 10:39   ` Arnout Vandecappelle
@ 2022-02-13 23:35     ` Giulio Benetti
  2022-02-14 12:16       ` Luca Ceresoli
  0 siblings, 1 reply; 8+ messages in thread
From: Giulio Benetti @ 2022-02-13 23:35 UTC (permalink / raw)
  To: Arnout Vandecappelle, Luca Ceresoli, buildroot
  Cc: Thomas Petazzoni, Yann E. MORIN, Thomas De Schampheleire

Hi Luca, Arnout, Thomas P.,

On 13/02/22 11:39, Arnout Vandecappelle wrote:
> 
> 
> On 12/02/2022 23:56, Luca Ceresoli wrote:
>> Hi Giulio,
>>
>> On 04/02/22 00:54, Giulio Benetti wrote:
>>> Often new boards have not been tested with official docker so let's add
>>> instructions to do it.
>>
>> Thank you, I think this is a very useful addition to the documentation!
>> However I would suggest some changes for it to look more "professional".

Always welcome!

>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>> ---
>>>    docs/manual/adding-board-support.txt | 22 ++++++++++++++++++++++
>>>    1 file changed, 22 insertions(+)
>>>
>>> diff --git a/docs/manual/adding-board-support.txt b/docs/manual/adding-board-support.txt
>>> index 33ed709535..f5fb3af371 100644
>>> --- a/docs/manual/adding-board-support.txt
>>> +++ b/docs/manual/adding-board-support.txt
>>> @@ -46,3 +46,25 @@ create a directory +board/<manufacturer>+ and a subdirectory
>>>    +board/<manufacturer>/<boardname>+. You can then store your patches
>>>    and configurations in these directories, and reference them from the main
>>>    Buildroot configuration. Refer to xref:customize[] for more details.
>>> +
>>> +Before submitting patches for new boards it would be better to test it
>>
>> "it would be better" -> "it is recommended".

Ok

>>> +by building it using .gitlab-ci.yml specified docker. For example at the
>>
>> I think this should be reworded in a simpler way: "by building it using
>> the docker specified in .gitlab-ci.yml".

Yes

>> BTW as I am a docker newbie: is it common to say "the docker"? Or would
>> "the docker container" be more correct? -- By comparison, I would never
>> say "using the virtualbox" but rather "using the virtualbox machine".
> 
>    I would say "the container" since you can use it with any container manager
> that follows the OCI spec.

Ok

>>
>>> +time of this writing the docker is:
>>
>> Remove the ':' from this line, or you'll have multiple ':' per line,
>> which looks awkward.
>>
>>> +$CI_REGISTRY/buildroot.org/buildroot/base:20220105.2314
>>
>> Hm, this string is already old.

Yes :-/

>    Actually, this part of the documentation is already superseded since we now
> have utils/docker-run that does everything.

Oh, I've missed that, just checked and it eases life not few!
I have a 120 columns command to start it that I copy and paste 
everytime. Because also, what I don't take care of here is the -v flag 
that allows you mount a host folder.

>> There's no sane way to keep docs and
>> .yml in sync. I wonder whether we should have in the manual a command
>> line that always use the current string, such as:
>>
>> DOCKER_IMAGE=$(cat .gitlab-ci.yml | \
>>                  sed -n '/^image/s/^.*CI_REGISTRY/registry.gitlab.com/p')
>> docker pull $DOCKER_IMAGE
>> sudo docker run -it  $DOCKER_IMAGE
>>
>> However I must admit this is not very readable in the docs... :( What
>> about adding a simple script (utils/run-docker?) that does the trick and
>> just mention that in the docs?

utils/docker-run then. Now we know it exists :-)

>>> +so:
>>
>> Add an empty line here, so that the output separates from the next line.

Ok

>>> +Pull the docker:
>>> +--------------------
>>> + $ docker pull registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
>>
>> Missing 'sudo'?
> 
>    Docker access is usually managed through the "docker" group rather than sudo.
> 
>    And if you use podman as docker replacement, it can even be done in an
> unprivileged container. Not that I tried it, but I think so.
> 
>    Oh BTW, the pull is in fact not needed, both podman and docker pull
> automatically when you start a container. That's the reason the container name
> is so convoluted.

All new thing I didn't know!

>>
>>> +--------------------
>>
>> Add an empty line here. This has no effect on the output but makes
>> source code more readable.
>>
>>> +Run the docker:
>>> +--------------------
>>> + $ sudo docker run -it registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314 /bin/bash
>>> +--------------------
>>
>> As above, add an empty line here.

+1

>>> +Inside the docker hint:
>>> +--------------------
>>> + $ git clone git://git.busybox.net/buildroot
>>> + $ cd buildroot
>>> + $ make +<boardname>_defconfig+
>>> + $ make
>>> +--------------------
>>
>> As above, add an empty line here.

+1

>>> +Wait until build finishes and eventually add host dependencies.
>>
>> If I understand what you mean here, it should be "and add host
>> dependencies if needed" ("eventually" is not the english translation of
>> italian "eventualmente").

"...and possibly add host dependencies", right?

>> If my understanding is correct, I don't find
>> this sentence very useful: a docker newbie perhaps doesn't know how to
>> add a host dependency (and maybe not even how to understand that they
>> are missing one).
>>
>> I would just remove this line, but if you think it is very important I'd
>> clarify it, maybe with some examples.
> 
>    Yes, I think this is what triggered the addition of this documentation. If you
> have e.g. libopenssl-dev installed on your build host, then you usually won't
> notice in your test builds that a dependency on host-openssl is needed. So test
> builds should be done in a minimal container.

Yes, it was because of that.

>    Unfortunately, the buildroot/base container is not exactly minimal. It's
> really what is meant to be used for running CI tests, not exactly what is needed
> for build tests. Ideally, we'd have
> 
> - an absolutely minimal container that can be used for build tests - ideally in
> a couple of variants for different distros;

There is a bunch of dockers like that(more or less) here:
https://github.com/aduskett/buildroot-docker-devel
I've also contributed to, and at that time Thomas P. in IRC asked why we 
didn't upstreamed it and I told I would have done like 2/3 years ago and 
I've never done it :-/

And also modifying autobuilder's script to pick random distro and build 
to avoid possible host issues. But it's a bunch of stuff to do.

> - a container for CI;

Do you mean the one we already have but more shrinked?

> - a more complete container you could use for development, though I can't
> immediately think of extra stuff you'd want in there 

Is it really worth it? I mean, I've never seen anybody in IRC(even if 
read few in it) or ML(same) that complain about "I can't have buildroot 
working because I miss host tools". But I've seen recently gitlab-ci 
results that took me like 15-16 hours to fix.

Does Yocto have something like that? And if yes, does someone can give a 
feedback if he really uses it?

>(but then, I wouldn't use a container for development).

Me too, and who would use it? I think nobody, because I don't think a 
newbie is that skilled to use a docker too(or maybe yes), but my first 
try would be using it with my distro and probably same goes for other 
people. But here again, I use Terminator+Midnight Commander as my "IDE", 
so I won't be happy enough with it. Someone else uses real "IDE" and we 
can't add Eclipse or VSCode(I hope), so it will be something that is not 
enough for anybody, thus IMHO useless.

----------------
Going back to this patch:

What I can do with this patch is to rewrite it pointing how to use 
utils/docker-run to check that at least configs/* and board/* patches work.

Another solution to my patch is what Thomas P. pointed in IRC:
"it is probably easier to ask people to use gitlab CI"

But my worry is that lot of people actually fork from github and not 
from gitlab. Who would really do that(both docker and gitlab-CI solutions)?
But also, who would really install docker(if they don't use it) to 
submit a patch for gitlab-CI build failures?

I think that counting the ones who took care about their maintained 
board gives us an idea, very few. BUT for new boards, and I see not few 
of them adding in the last period. It could be a way to force them to 
give a proof of a successfull building with gitlab-CI pipeline log as 
Thomas P. proposed.

So I would modify this patch with instructions to:
- fork Buildroot in gitlab
- trigger gitlab-CI pipeline for a single defconfig

What do you all think?

Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-03 23:54 [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker Giulio Benetti
  2022-02-12 22:56 ` Luca Ceresoli
@ 2022-02-14  7:57 ` Thomas Petazzoni via buildroot
  1 sibling, 0 replies; 8+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-02-14  7:57 UTC (permalink / raw)
  To: Giulio Benetti; +Cc: Thomas De Schampheleire, buildroot

Hello,

On Fri,  4 Feb 2022 00:54:38 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> +Inside the docker hint:
> +--------------------
> + $ git clone git://git.busybox.net/buildroot

As mentioned on IRC, I think this is not very useful: if you want to
test a new defconfig, or changes to a defconfig, the changes to this
defconfig are not yet in the official repo, so simply cloning the
official repo within the Docker container will not allow you to test
your brand new defconfig.

So as they are, those instructions are in fact not very relevant for
the indicated purpose.

Either the instructions include mounting the local Buildroot copy into
the container, or they instead suggest to use Gitlab CI to validate the
defconfig.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-13 23:35     ` Giulio Benetti
@ 2022-02-14 12:16       ` Luca Ceresoli
  2022-02-15  0:49         ` Giulio Benetti
  0 siblings, 1 reply; 8+ messages in thread
From: Luca Ceresoli @ 2022-02-14 12:16 UTC (permalink / raw)
  To: Giulio Benetti, Arnout Vandecappelle, buildroot
  Cc: Thomas Petazzoni, Yann E. MORIN, Thomas De Schampheleire

Hi Giulio,

On 14/02/22 00:35, Giulio Benetti wrote:
> Hi Luca, Arnout, Thomas P.,
> 
> On 13/02/22 11:39, Arnout Vandecappelle wrote:
>>
>>
>> On 12/02/2022 23:56, Luca Ceresoli wrote:
>>> Hi Giulio,
>>>
>>> On 04/02/22 00:54, Giulio Benetti wrote:
>>>> Often new boards have not been tested with official docker so let's add
>>>> instructions to do it.
>>>
>>> Thank you, I think this is a very useful addition to the documentation!
>>> However I would suggest some changes for it to look more "professional".
> 
> Always welcome!
> 
>>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>>> ---
>>>>    docs/manual/adding-board-support.txt | 22 ++++++++++++++++++++++
>>>>    1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/docs/manual/adding-board-support.txt
>>>> b/docs/manual/adding-board-support.txt
>>>> index 33ed709535..f5fb3af371 100644
>>>> --- a/docs/manual/adding-board-support.txt
>>>> +++ b/docs/manual/adding-board-support.txt
>>>> @@ -46,3 +46,25 @@ create a directory +board/<manufacturer>+ and a
>>>> subdirectory
>>>>    +board/<manufacturer>/<boardname>+. You can then store your patches
>>>>    and configurations in these directories, and reference them from
>>>> the main
>>>>    Buildroot configuration. Refer to xref:customize[] for more details.
>>>> +
>>>> +Before submitting patches for new boards it would be better to test it
>>>
>>> "it would be better" -> "it is recommended".
> 
> Ok
> 
>>>> +by building it using .gitlab-ci.yml specified docker. For example
>>>> at the
>>>
>>> I think this should be reworded in a simpler way: "by building it using
>>> the docker specified in .gitlab-ci.yml".
> 
> Yes
> 
>>> BTW as I am a docker newbie: is it common to say "the docker"? Or would
>>> "the docker container" be more correct? -- By comparison, I would never
>>> say "using the virtualbox" but rather "using the virtualbox machine".
>>
>>    I would say "the container" since you can use it with any container
>> manager
>> that follows the OCI spec.
> 
> Ok
> 
>>>
>>>> +time of this writing the docker is:
>>>
>>> Remove the ':' from this line, or you'll have multiple ':' per line,
>>> which looks awkward.
>>>
>>>> +$CI_REGISTRY/buildroot.org/buildroot/base:20220105.2314
>>>
>>> Hm, this string is already old.
> 
> Yes :-/
> 
>>    Actually, this part of the documentation is already superseded
>> since we now
>> have utils/docker-run that does everything.
> 
> Oh, I've missed that, just checked and it eases life not few!
> I have a 120 columns command to start it that I copy and paste
> everytime. Because also, what I don't take care of here is the -v flag
> that allows you mount a host folder.
> 
>>> There's no sane way to keep docs and
>>> .yml in sync. I wonder whether we should have in the manual a command
>>> line that always use the current string, such as:
>>>
>>> DOCKER_IMAGE=$(cat .gitlab-ci.yml | \
>>>                  sed -n
>>> '/^image/s/^.*CI_REGISTRY/registry.gitlab.com/p')
>>> docker pull $DOCKER_IMAGE
>>> sudo docker run -it  $DOCKER_IMAGE
>>>
>>> However I must admit this is not very readable in the docs... :( What
>>> about adding a simple script (utils/run-docker?) that does the trick and
>>> just mention that in the docs?
> 
> utils/docker-run then. Now we know it exists :-)
> 
>>>> +so:
>>>
>>> Add an empty line here, so that the output separates from the next line.
> 
> Ok
> 
>>>> +Pull the docker:
>>>> +--------------------
>>>> + $ docker pull
>>>> registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
>>>
>>> Missing 'sudo'?
>>
>>    Docker access is usually managed through the "docker" group rather
>> than sudo.
>>
>>    And if you use podman as docker replacement, it can even be done in an
>> unprivileged container. Not that I tried it, but I think so.
>>
>>    Oh BTW, the pull is in fact not needed, both podman and docker pull
>> automatically when you start a container. That's the reason the
>> container name
>> is so convoluted.
> 
> All new thing I didn't know!
> 
>>>
>>>> +--------------------
>>>
>>> Add an empty line here. This has no effect on the output but makes
>>> source code more readable.
>>>
>>>> +Run the docker:
>>>> +--------------------
>>>> + $ sudo docker run -it
>>>> registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
>>>> /bin/bash
>>>> +--------------------
>>>
>>> As above, add an empty line here.
> 
> +1
> 
>>>> +Inside the docker hint:
>>>> +--------------------
>>>> + $ git clone git://git.busybox.net/buildroot
>>>> + $ cd buildroot
>>>> + $ make +<boardname>_defconfig+
>>>> + $ make
>>>> +--------------------
>>>
>>> As above, add an empty line here.
> 
> +1
> 
>>>> +Wait until build finishes and eventually add host dependencies.
>>>
>>> If I understand what you mean here, it should be "and add host
>>> dependencies if needed" ("eventually" is not the english translation of
>>> italian "eventualmente").
> 
> "...and possibly add host dependencies", right?

Better.

Even better: "and add any missing host dependencies, if any".

>>> If my understanding is correct, I don't find
>>> this sentence very useful: a docker newbie perhaps doesn't know how to
>>> add a host dependency (and maybe not even how to understand that they
>>> are missing one).
>>>
>>> I would just remove this line, but if you think it is very important I'd
>>> clarify it, maybe with some examples.
>>
>>    Yes, I think this is what triggered the addition of this
>> documentation. If you
>> have e.g. libopenssl-dev installed on your build host, then you
>> usually won't
>> notice in your test builds that a dependency on host-openssl is
>> needed. So test
>> builds should be done in a minimal container.
> 
> Yes, it was because of that.
> 
>>    Unfortunately, the buildroot/base container is not exactly minimal.
>> It's
>> really what is meant to be used for running CI tests, not exactly what
>> is needed
>> for build tests. Ideally, we'd have
>>
>> - an absolutely minimal container that can be used for build tests -
>> ideally in
>> a couple of variants for different distros;
> 
> There is a bunch of dockers like that(more or less) here:
> https://github.com/aduskett/buildroot-docker-devel
> I've also contributed to, and at that time Thomas P. in IRC asked why we
> didn't upstreamed it and I told I would have done like 2/3 years ago and
> I've never done it :-/
> 
> And also modifying autobuilder's script to pick random distro and build
> to avoid possible host issues. But it's a bunch of stuff to do.
> 
>> - a container for CI;
> 
> Do you mean the one we already have but more shrinked?
> 
>> - a more complete container you could use for development, though I can't
>> immediately think of extra stuff you'd want in there 
> 
> Is it really worth it? I mean, I've never seen anybody in IRC(even if
> read few in it) or ML(same) that complain about "I can't have buildroot
> working because I miss host tools". But I've seen recently gitlab-ci
> results that took me like 15-16 hours to fix.
> 
> Does Yocto have something like that? And if yes, does someone can give a
> feedback if he really uses it?

The situation is different with yocto as /usr/bin & co are just not in
the PATH. No host tools are accessible unless explicitly listed in the
HOSTTOOLS variable, so the problem is mostly solved at the root.

>> (but then, I wouldn't use a container for development).
> 
> Me too, and who would use it? I think nobody, because I don't think a
> newbie is that skilled to use a docker too(or maybe yes), but my first
> try would be using it with my distro and probably same goes for other
> people. But here again, I use Terminator+Midnight Commander as my "IDE",
> so I won't be happy enough with it. Someone else uses real "IDE" and we
> can't add Eclipse or VSCode(I hope), so it will be something that is not
> enough for anybody, thus IMHO useless.
> 
> ----------------
> Going back to this patch:
> 
> What I can do with this patch is to rewrite it pointing how to use
> utils/docker-run to check that at least configs/* and board/* patches work.
> 
> Another solution to my patch is what Thomas P. pointed in IRC:
> "it is probably easier to ask people to use gitlab CI"
> 
> But my worry is that lot of people actually fork from github and not
> from gitlab. Who would really do that(both docker and gitlab-CI solutions)?
> But also, who would really install docker(if they don't use it) to
> submit a patch for gitlab-CI build failures?
> 
> I think that counting the ones who took care about their maintained
> board gives us an idea, very few. BUT for new boards, and I see not few
> of them adding in the last period. It could be a way to force them to
> give a proof of a successfull building with gitlab-CI pipeline log as
> Thomas P. proposed.
> 
> So I would modify this patch with instructions to:
> - fork Buildroot in gitlab
> - trigger gitlab-CI pipeline for a single defconfig
> 
> What do you all think?

I think we need simple instructions to ensure that anybody is able to
test their config builds without unnoticed host dependencies, before
they send a patch. I don't care whether it's docker or gitlab or
whatever. But it must be something documented and easy to do in a few
commands.

-- 
Luca
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-14 12:16       ` Luca Ceresoli
@ 2022-02-15  0:49         ` Giulio Benetti
  2022-02-15  2:17           ` Giulio Benetti
  0 siblings, 1 reply; 8+ messages in thread
From: Giulio Benetti @ 2022-02-15  0:49 UTC (permalink / raw)
  To: Luca Ceresoli, Arnout Vandecappelle, buildroot
  Cc: Thomas Petazzoni, Yann E. MORIN, Thomas De Schampheleire

Hi Luca,

On 14/02/22 13:16, Luca Ceresoli wrote:
> Hi Giulio,
> 
> On 14/02/22 00:35, Giulio Benetti wrote:
>> Hi Luca, Arnout, Thomas P.,
>>
>> On 13/02/22 11:39, Arnout Vandecappelle wrote:
>>>
>>>
>>> On 12/02/2022 23:56, Luca Ceresoli wrote:
>>>> Hi Giulio,
>>>>
>>>> On 04/02/22 00:54, Giulio Benetti wrote:
>>>>> Often new boards have not been tested with official docker so let's add
>>>>> instructions to do it.
>>>>
>>>> Thank you, I think this is a very useful addition to the documentation!
>>>> However I would suggest some changes for it to look more "professional".
>>
>> Always welcome!
>>
>>>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>>>> ---
>>>>>     docs/manual/adding-board-support.txt | 22 ++++++++++++++++++++++
>>>>>     1 file changed, 22 insertions(+)
>>>>>
>>>>> diff --git a/docs/manual/adding-board-support.txt
>>>>> b/docs/manual/adding-board-support.txt
>>>>> index 33ed709535..f5fb3af371 100644
>>>>> --- a/docs/manual/adding-board-support.txt
>>>>> +++ b/docs/manual/adding-board-support.txt
>>>>> @@ -46,3 +46,25 @@ create a directory +board/<manufacturer>+ and a
>>>>> subdirectory
>>>>>     +board/<manufacturer>/<boardname>+. You can then store your patches
>>>>>     and configurations in these directories, and reference them from
>>>>> the main
>>>>>     Buildroot configuration. Refer to xref:customize[] for more details.
>>>>> +
>>>>> +Before submitting patches for new boards it would be better to test it
>>>>
>>>> "it would be better" -> "it is recommended".
>>
>> Ok
>>
>>>>> +by building it using .gitlab-ci.yml specified docker. For example
>>>>> at the
>>>>
>>>> I think this should be reworded in a simpler way: "by building it using
>>>> the docker specified in .gitlab-ci.yml".
>>
>> Yes
>>
>>>> BTW as I am a docker newbie: is it common to say "the docker"? Or would
>>>> "the docker container" be more correct? -- By comparison, I would never
>>>> say "using the virtualbox" but rather "using the virtualbox machine".
>>>
>>>     I would say "the container" since you can use it with any container
>>> manager
>>> that follows the OCI spec.
>>
>> Ok
>>
>>>>
>>>>> +time of this writing the docker is:
>>>>
>>>> Remove the ':' from this line, or you'll have multiple ':' per line,
>>>> which looks awkward.
>>>>
>>>>> +$CI_REGISTRY/buildroot.org/buildroot/base:20220105.2314
>>>>
>>>> Hm, this string is already old.
>>
>> Yes :-/
>>
>>>     Actually, this part of the documentation is already superseded
>>> since we now
>>> have utils/docker-run that does everything.
>>
>> Oh, I've missed that, just checked and it eases life not few!
>> I have a 120 columns command to start it that I copy and paste
>> everytime. Because also, what I don't take care of here is the -v flag
>> that allows you mount a host folder.
>>
>>>> There's no sane way to keep docs and
>>>> .yml in sync. I wonder whether we should have in the manual a command
>>>> line that always use the current string, such as:
>>>>
>>>> DOCKER_IMAGE=$(cat .gitlab-ci.yml | \
>>>>                   sed -n
>>>> '/^image/s/^.*CI_REGISTRY/registry.gitlab.com/p')
>>>> docker pull $DOCKER_IMAGE
>>>> sudo docker run -it  $DOCKER_IMAGE
>>>>
>>>> However I must admit this is not very readable in the docs... :( What
>>>> about adding a simple script (utils/run-docker?) that does the trick and
>>>> just mention that in the docs?
>>
>> utils/docker-run then. Now we know it exists :-)
>>
>>>>> +so:
>>>>
>>>> Add an empty line here, so that the output separates from the next line.
>>
>> Ok
>>
>>>>> +Pull the docker:
>>>>> +--------------------
>>>>> + $ docker pull
>>>>> registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
>>>>
>>>> Missing 'sudo'?
>>>
>>>     Docker access is usually managed through the "docker" group rather
>>> than sudo.
>>>
>>>     And if you use podman as docker replacement, it can even be done in an
>>> unprivileged container. Not that I tried it, but I think so.
>>>
>>>     Oh BTW, the pull is in fact not needed, both podman and docker pull
>>> automatically when you start a container. That's the reason the
>>> container name
>>> is so convoluted.
>>
>> All new thing I didn't know!
>>
>>>>
>>>>> +--------------------
>>>>
>>>> Add an empty line here. This has no effect on the output but makes
>>>> source code more readable.
>>>>
>>>>> +Run the docker:
>>>>> +--------------------
>>>>> + $ sudo docker run -it
>>>>> registry.gitlab.com/buildroot.org/buildroot/base:20220105.2314
>>>>> /bin/bash
>>>>> +--------------------
>>>>
>>>> As above, add an empty line here.
>>
>> +1
>>
>>>>> +Inside the docker hint:
>>>>> +--------------------
>>>>> + $ git clone git://git.busybox.net/buildroot
>>>>> + $ cd buildroot
>>>>> + $ make +<boardname>_defconfig+
>>>>> + $ make
>>>>> +--------------------
>>>>
>>>> As above, add an empty line here.
>>
>> +1
>>
>>>>> +Wait until build finishes and eventually add host dependencies.
>>>>
>>>> If I understand what you mean here, it should be "and add host
>>>> dependencies if needed" ("eventually" is not the english translation of
>>>> italian "eventualmente").
>>
>> "...and possibly add host dependencies", right?
> 
> Better.
> 
> Even better: "and add any missing host dependencies, if any".

Ah yes, thank you!

>>>> If my understanding is correct, I don't find
>>>> this sentence very useful: a docker newbie perhaps doesn't know how to
>>>> add a host dependency (and maybe not even how to understand that they
>>>> are missing one).
>>>>
>>>> I would just remove this line, but if you think it is very important I'd
>>>> clarify it, maybe with some examples.
>>>
>>>     Yes, I think this is what triggered the addition of this
>>> documentation. If you
>>> have e.g. libopenssl-dev installed on your build host, then you
>>> usually won't
>>> notice in your test builds that a dependency on host-openssl is
>>> needed. So test
>>> builds should be done in a minimal container.
>>
>> Yes, it was because of that.
>>
>>>     Unfortunately, the buildroot/base container is not exactly minimal.
>>> It's
>>> really what is meant to be used for running CI tests, not exactly what
>>> is needed
>>> for build tests. Ideally, we'd have
>>>
>>> - an absolutely minimal container that can be used for build tests -
>>> ideally in
>>> a couple of variants for different distros;
>>
>> There is a bunch of dockers like that(more or less) here:
>> https://github.com/aduskett/buildroot-docker-devel
>> I've also contributed to, and at that time Thomas P. in IRC asked why we
>> didn't upstreamed it and I told I would have done like 2/3 years ago and
>> I've never done it :-/
>>
>> And also modifying autobuilder's script to pick random distro and build
>> to avoid possible host issues. But it's a bunch of stuff to do.
>>
>>> - a container for CI;
>>
>> Do you mean the one we already have but more shrinked?
>>
>>> - a more complete container you could use for development, though I can't
>>> immediately think of extra stuff you'd want in there
>>
>> Is it really worth it? I mean, I've never seen anybody in IRC(even if
>> read few in it) or ML(same) that complain about "I can't have buildroot
>> working because I miss host tools". But I've seen recently gitlab-ci
>> results that took me like 15-16 hours to fix.
>>
>> Does Yocto have something like that? And if yes, does someone can give a
>> feedback if he really uses it?
> 
> The situation is different with yocto as /usr/bin & co are just not in
> the PATH. No host tools are accessible unless explicitly listed in the
> HOSTTOOLS variable, so the problem is mostly solved at the root.

Ah ok, thank for the explanation!

>>> (but then, I wouldn't use a container for development).
>>
>> Me too, and who would use it? I think nobody, because I don't think a
>> newbie is that skilled to use a docker too(or maybe yes), but my first
>> try would be using it with my distro and probably same goes for other
>> people. But here again, I use Terminator+Midnight Commander as my "IDE",
>> so I won't be happy enough with it. Someone else uses real "IDE" and we
>> can't add Eclipse or VSCode(I hope), so it will be something that is not
>> enough for anybody, thus IMHO useless.
>>
>> ----------------
>> Going back to this patch:
>>
>> What I can do with this patch is to rewrite it pointing how to use
>> utils/docker-run to check that at least configs/* and board/* patches work.
>>
>> Another solution to my patch is what Thomas P. pointed in IRC:
>> "it is probably easier to ask people to use gitlab CI"
>>
>> But my worry is that lot of people actually fork from github and not
>> from gitlab. Who would really do that(both docker and gitlab-CI solutions)?
>> But also, who would really install docker(if they don't use it) to
>> submit a patch for gitlab-CI build failures?
>>
>> I think that counting the ones who took care about their maintained
>> board gives us an idea, very few. BUT for new boards, and I see not few
>> of them adding in the last period. It could be a way to force them to
>> give a proof of a successfull building with gitlab-CI pipeline log as
>> Thomas P. proposed.
>>
>> So I would modify this patch with instructions to:
>> - fork Buildroot in gitlab
>> - trigger gitlab-CI pipeline for a single defconfig
>>
>> What do you all think?
> 
> I think we need simple instructions to ensure that anybody is able to
> test their config builds without unnoticed host dependencies, before
> they send a patch. I don't care whether it's docker or gitlab or
> whatever. But it must be something documented and easy to do in a few
> commands.
> 

At this point, once this patch [1] is upstreamed I will modify this 
patch and point to utils/docker-run.

Thank you!

Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker
  2022-02-15  0:49         ` Giulio Benetti
@ 2022-02-15  2:17           ` Giulio Benetti
  0 siblings, 0 replies; 8+ messages in thread
From: Giulio Benetti @ 2022-02-15  2:17 UTC (permalink / raw)
  To: Luca Ceresoli, Arnout Vandecappelle, buildroot
  Cc: Thomas De Schampheleire, Yann E. MORIN, Thomas Petazzoni

On 15/02/22 01:49, Giulio Benetti wrote:
>> I think we need simple instructions to ensure that anybody is able to
>> test their config builds without unnoticed host dependencies, before
>> they send a patch. I don't care whether it's docker or gitlab or
>> whatever. But it must be something documented and easy to do in a few
>> commands.
>>
> 
> At this point, once this patch [1] is upstreamed I will modify this
> patch and point to utils/docker-run.

[1]: 
https://lists.buildroot.org/pipermail/buildroot/2022-February/637017.html
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-02-15  2:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-03 23:54 [Buildroot] [PATCH] manual: board support: add instructions to test defconfig in the official docker Giulio Benetti
2022-02-12 22:56 ` Luca Ceresoli
2022-02-13 10:39   ` Arnout Vandecappelle
2022-02-13 23:35     ` Giulio Benetti
2022-02-14 12:16       ` Luca Ceresoli
2022-02-15  0:49         ` Giulio Benetti
2022-02-15  2:17           ` Giulio Benetti
2022-02-14  7:57 ` Thomas Petazzoni via buildroot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox