public inbox for docs@lists.yoctoproject.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] tools/build-docs-container: improve concurrent safety
@ 2025-10-09 10:23 Quentin Schulz
  2025-10-09 10:23 ` [PATCH v2 1/3] tools/build-docs-container: guarantee the image to run matches the just-built image Quentin Schulz
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Quentin Schulz @ 2025-10-09 10:23 UTC (permalink / raw)
  To: docs; +Cc: Quentin Schulz

Currently, it is possible to run the script from two different trees and
ending up running the wrong image due to the non-uniqueness of our tags.

Indeed, one could simply run the script in parallel for the same distro.
Script runtime A will build the image A and tag it X, Script runtime B
will build the image B and tag it X, Script runtime B will run the tag X
(image B), Script runtime A will run the tag X (image B). Note that this
problem exists whether we are building from the same tree concurrently
or from different yocto-docs trees concurrently.
This fixes the issue by using the iid-file generated by podman build
containing the sha of the just-built image as "image" argument to
podman run, guaranteeing we are using the image we just built.

This also now forces builds triggered by tools/build-docs-container to
build in separate directories. This helps comparing the output between
different runs as well as making sure we aren't reusing the build
directory from another run by mistake. The downside is that it can
clutter your local tree and make clean is not able to clean previous
runs because it cannot know which build directory you picked in earlier
runs.

set_versions.py still modifies the tree so this probably means we aren't
concurrent-safe from the same tree because of that. But we should now be
safe running tools/build-docs-container from separate trees.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Changes in v2:
- made image_id_file a global variable so the trap can use the variable,
- added patch to customize BUILDDIR,
- added patch to set BUILDDIR uniquely per run of
  tools/build-docs-container,
- Link to v1: https://lore.kernel.org/r/20251009-iid-file-v1-1-d9ca8563c88c@cherry.de

---
Quentin Schulz (3):
      tools/build-docs-container: guarantee the image to run matches the just-built image
      Makefile: allow to specify build directory
      tools/build-docs-container: build in separate directory for each distro

 documentation/Makefile                   |  2 +-
 documentation/tools/build-docs-container | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)
---
base-commit: 09c7800333b17b21e50d2a089a3ae1b123697243
change-id: 20251009-iid-file-f276784e0425

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>



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

* [PATCH v2 1/3] tools/build-docs-container: guarantee the image to run matches the just-built image
  2025-10-09 10:23 [PATCH v2 0/3] tools/build-docs-container: improve concurrent safety Quentin Schulz
@ 2025-10-09 10:23 ` Quentin Schulz
  2025-10-09 10:24 ` [PATCH v2 2/3] Makefile: allow to specify build directory Quentin Schulz
  2025-10-09 10:24 ` [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro Quentin Schulz
  2 siblings, 0 replies; 7+ messages in thread
From: Quentin Schulz @ 2025-10-09 10:23 UTC (permalink / raw)
  To: docs; +Cc: Quentin Schulz

From: Quentin Schulz <quentin.schulz@cherry.de>

We aren't that interested in tags actually, the only thing it brings is
a belief that we are going to run exactly what we just built. The issue
is that this is incorrect.

Indeed, one could simply run the script in parallel for the same image.
Script runtime A will build the image A and tag it X, Script runtime B
will build the image B and tag it X, Script runtime B will run the tag X
(image B), Script runtime A will run the tag X (image B). Note that this
problem exists whether we are building from the same tree concurrently
or from different yocto-docs trees concurrently.
One way to fix this could be to introduce random numbers in the tag so
that it's always unique, but we would be flooding the system with
useless tags.

Instead, we can use the sha of the generated image and run that sha
directly. If it's the same across rebuilds, it'll stay the same. If it's
different, the sha will be different and thus we are safe from
concurrent use.

The only downside is that we cannot infer from the image sha the
underlying distro we're testing.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 documentation/tools/build-docs-container | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
index 70e05f295..831d357fb 100755
--- a/documentation/tools/build-docs-container
+++ b/documentation/tools/build-docs-container
@@ -64,10 +64,6 @@ main ()
 
   OCI=$(which "$CONTAINERCMD")
 
-  # docker build doesn't accept 2 colons, so "sanitize" the name
-  local sanitized_dockername
-  sanitized_dockername=$(echo "$image" | tr ':.' '-')
-
   local version
   version=$(echo "$image" | awk -F: '{print $NF}')
 
@@ -139,8 +135,13 @@ main ()
       ;;
   esac
 
+  local image_sha
+  image_id_file=$(mktemp)
+  # Don't clutter tmpfs on fails
+  trap 'rm -f "$image_id_file"' EXIT
+
   $OCI build \
-    --tag "yocto-docs-$sanitized_dockername:latest" \
+    --iidfile "$image_id_file" \
     --build-arg ARG_FROM="docker.io/$image" \
     --build-arg DOCS="$docs" \
     --build-arg DOCS_PDF="$docs_pdf" \
@@ -148,6 +149,9 @@ main ()
     --file "$SCRIPT_DIR/$containerfile" \
     "$SH_DIR/"
 
+  image_sha="$(< "$image_id_file")"
+  rm "$image_id_file"
+
   local -a args_run=(
     --rm
     --interactive
@@ -171,7 +175,7 @@ main ()
 
   $OCI run \
     "${args_run[@]}" \
-    "yocto-docs-$sanitized_dockername" \
+    "$image_sha" \
     "$@"
 }
 

-- 
2.51.0



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

* [PATCH v2 2/3] Makefile: allow to specify build directory
  2025-10-09 10:23 [PATCH v2 0/3] tools/build-docs-container: improve concurrent safety Quentin Schulz
  2025-10-09 10:23 ` [PATCH v2 1/3] tools/build-docs-container: guarantee the image to run matches the just-built image Quentin Schulz
@ 2025-10-09 10:24 ` Quentin Schulz
  2025-10-09 10:24 ` [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro Quentin Schulz
  2 siblings, 0 replies; 7+ messages in thread
From: Quentin Schulz @ 2025-10-09 10:24 UTC (permalink / raw)
  To: docs; +Cc: Quentin Schulz

From: Quentin Schulz <quentin.schulz@cherry.de>

This can be useful to compare the output of two runs or separate the
build directory of 2+ distro builds from tools/build-docs-container.

Note that make clean cannot remove BUILDDIR from previous runs if they
differ as it wouldn't be able to know what it was named in the past.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/documentation/Makefile b/documentation/Makefile
index bade78fe8..94d19e350 100644
--- a/documentation/Makefile
+++ b/documentation/Makefile
@@ -11,7 +11,7 @@ SOURCEDIR      = .
 VALEDOCS       ?= $(SOURCEDIR)
 SPHINXLINTDOCS ?= $(SOURCEDIR)
 IMAGEDIRS      = */svg
-BUILDDIR       = _build
+BUILDDIR       ?= _build
 DESTDIR        = final
 SVG2PNG        = rsvg-convert
 SVG2PDF        = rsvg-convert

-- 
2.51.0



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

* [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro
  2025-10-09 10:23 [PATCH v2 0/3] tools/build-docs-container: improve concurrent safety Quentin Schulz
  2025-10-09 10:23 ` [PATCH v2 1/3] tools/build-docs-container: guarantee the image to run matches the just-built image Quentin Schulz
  2025-10-09 10:24 ` [PATCH v2 2/3] Makefile: allow to specify build directory Quentin Schulz
@ 2025-10-09 10:24 ` Quentin Schulz
  2025-10-14  9:27   ` [docs] " Antonin Godard
  2 siblings, 1 reply; 7+ messages in thread
From: Quentin Schulz @ 2025-10-09 10:24 UTC (permalink / raw)
  To: docs; +Cc: Quentin Schulz

From: Quentin Schulz <quentin.schulz@cherry.de>

This allows to compare build output from different distros if necessary
and make it easier to make sure we start building from a clean slate.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 documentation/tools/build-docs-container | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
index 831d357fb..47320e182 100755
--- a/documentation/tools/build-docs-container
+++ b/documentation/tools/build-docs-container
@@ -60,6 +60,7 @@ main ()
   fi
 
   local image="$1"
+  local orig_image=$image
   shift
 
   OCI=$(which "$CONTAINERCMD")
@@ -159,6 +160,7 @@ main ()
     --volume="$DOCS_DIR:/docs:rw"
     --workdir=/docs
     --security-opt label=disable
+    --env BUILDDIR="_build-$orig_image-$image_sha"
   )
 
   if [ "$OCI" = "docker" ]; then

-- 
2.51.0



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

* Re: [docs] [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro
  2025-10-09 10:24 ` [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro Quentin Schulz
@ 2025-10-14  9:27   ` Antonin Godard
  2025-10-15 11:37     ` Quentin Schulz
  0 siblings, 1 reply; 7+ messages in thread
From: Antonin Godard @ 2025-10-14  9:27 UTC (permalink / raw)
  To: Quentin Schulz, docs; +Cc: Quentin Schulz

On Thu Oct 9, 2025 at 12:24 PM CEST, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
>
> This allows to compare build output from different distros if necessary
> and make it easier to make sure we start building from a clean slate.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  documentation/tools/build-docs-container | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
> index 831d357fb..47320e182 100755
> --- a/documentation/tools/build-docs-container
> +++ b/documentation/tools/build-docs-container
> @@ -60,6 +60,7 @@ main ()
>    fi
>  
>    local image="$1"
> +  local orig_image=$image
>    shift
>  
>    OCI=$(which "$CONTAINERCMD")
> @@ -159,6 +160,7 @@ main ()
>      --volume="$DOCS_DIR:/docs:rw"
>      --workdir=/docs
>      --security-opt label=disable
> +    --env BUILDDIR="_build-$orig_image-$image_sha"

I think the idea is good for improving concurrency, however I think for
development it would be nice to keep _build the default. What about having
the following default behavior:

- Build in "_build-$orig_image-$image_sha"
- If "_build" does not exist _or_ is a symlink, create or replace a symlink to
  "_build-$orig_image-$image_sha" from the previous step, named "_build"

    _build -> _build-$orig_image-$image_sha"

- If "_build" exists and is a directory, leave it untouched as it may be
  originating from another "regular" build.

Then:

- `make clean` removes "_build" and "_build-$orig_image-$image_sha" (when
  "_build-$orig_image-$image_sha" exists and _build is a symlink to it)

- We could maybe also have a "cleanall" (or name alike) that removes all the
  _build* it finds, to clean everything up.

With this, additional runs with different SHAs will just stack next to this,
re-creating the symlink only when it is not a directory. This way, I know that
for any previous run of build-docs-container, I can open the "_build" symlink
to get the latest output. Of course I need to be careful when running concurrent
builds, but that is another use-case, a bit different from day-to-day
development IMO.

What do you think?

Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



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

* Re: [docs] [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro
  2025-10-14  9:27   ` [docs] " Antonin Godard
@ 2025-10-15 11:37     ` Quentin Schulz
  2025-12-22 12:47       ` Antonin Godard
  0 siblings, 1 reply; 7+ messages in thread
From: Quentin Schulz @ 2025-10-15 11:37 UTC (permalink / raw)
  To: Antonin Godard, Quentin Schulz, docs

Hi Antonin,

On 10/14/25 11:27 AM, Antonin Godard wrote:
> On Thu Oct 9, 2025 at 12:24 PM CEST, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> This allows to compare build output from different distros if necessary
>> and make it easier to make sure we start building from a clean slate.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>> ---
>>   documentation/tools/build-docs-container | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
>> index 831d357fb..47320e182 100755
>> --- a/documentation/tools/build-docs-container
>> +++ b/documentation/tools/build-docs-container
>> @@ -60,6 +60,7 @@ main ()
>>     fi
>>   
>>     local image="$1"
>> +  local orig_image=$image
>>     shift
>>   
>>     OCI=$(which "$CONTAINERCMD")
>> @@ -159,6 +160,7 @@ main ()
>>       --volume="$DOCS_DIR:/docs:rw"
>>       --workdir=/docs
>>       --security-opt label=disable
>> +    --env BUILDDIR="_build-$orig_image-$image_sha"
> 
> I think the idea is good for improving concurrency, however I think for
> development it would be nice to keep _build the default. What about having
> the following default behavior:
> 
> - Build in "_build-$orig_image-$image_sha"
> - If "_build" does not exist _or_ is a symlink, create or replace a symlink to
>    "_build-$orig_image-$image_sha" from the previous step, named "_build"
> 
>      _build -> _build-$orig_image-$image_sha"
> 
> - If "_build" exists and is a directory, leave it untouched as it may be
>    originating from another "regular" build.
> 
> Then:
> 
> - `make clean` removes "_build" and "_build-$orig_image-$image_sha" (when
>    "_build-$orig_image-$image_sha" exists and _build is a symlink to it)
> 
> - We could maybe also have a "cleanall" (or name alike) that removes all the
>    _build* it finds, to clean everything up.
> 

To make sure the make clean still removes the old _build directories, I 
think we could simply build in a subdirectory of _build, e.g.

BUILDDIR=_build/$orig_image-$image_sha

then make clean should work just fine.

> With this, additional runs with different SHAs will just stack next to this,
> re-creating the symlink only when it is not a directory. This way, I know that
> for any previous run of build-docs-container, I can open the "_build" symlink
> to get the latest output. Of course I need to be careful when running concurrent
> builds, but that is another use-case, a bit different from day-to-day
> development IMO.
> 

Mmmmm...

So I think we could make this concurrent-safe by using an flock on some 
file?

e.g. surround any Make target that modifies the source directory with an 
flock, something like shown here: https://unix.stackexchange.com/a/382916

And actually... I think we already have some issue with concurrency 
without calling make multiple times from the same tree.

I believe when you run make all, make will run make html, make epub and 
make latexpdf in parallel which will all call set_versions.py which 
modifies the source tree. Same would happen if passing multiple targets 
to make (e.g. make latexpdf html). I don't think the f.write() and 
w.write() in set_versions.py are concurrent-safe.

> What do you think?
> 

With the flock above, we could simply add the creation of a 
_build/latest symlink which points to the last make target that ran. 
Maybe we want a _build/latest/html for the last make html target, 
otherwise imagine you run make all once and then make html one more time 
for another distro, then _build/latest will now point to _build/distroB/ 
which didn't generate the html part). But that could also mean we should 
split the build directories for latexpdf, html and epub so this works... 
but that means not being able to reuse the sphinx "cache" (I believe 
there's one to do incremental builds? maybe it's also used for different 
output types?).

I think we (I) may be overengineering this a tiny bit :)

I think we probably should add an flock at least for the writing of the 
files by set_versions.py if that proves to be an issue (testing needs to 
be done :) )

Cheers,
Quentin


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

* Re: [docs] [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro
  2025-10-15 11:37     ` Quentin Schulz
@ 2025-12-22 12:47       ` Antonin Godard
  0 siblings, 0 replies; 7+ messages in thread
From: Antonin Godard @ 2025-12-22 12:47 UTC (permalink / raw)
  To: quentin.schulz, Quentin Schulz, docs

Hi,

On Wed Oct 15, 2025 at 1:37 PM CEST, Quentin Schulz via lists.yoctoproject.org wrote:
[...]
>> With this, additional runs with different SHAs will just stack next to this,
>> re-creating the symlink only when it is not a directory. This way, I know that
>> for any previous run of build-docs-container, I can open the "_build" symlink
>> to get the latest output. Of course I need to be careful when running concurrent
>> builds, but that is another use-case, a bit different from day-to-day
>> development IMO.
>> 
>
> Mmmmm...
>
> So I think we could make this concurrent-safe by using an flock on some 
> file?
>
> e.g. surround any Make target that modifies the source directory with an 
> flock, something like shown here: https://unix.stackexchange.com/a/382916
>
> And actually... I think we already have some issue with concurrency 
> without calling make multiple times from the same tree.
>
> I believe when you run make all, make will run make html, make epub and 
> make latexpdf in parallel which will all call set_versions.py which 
> modifies the source tree. Same would happen if passing multiple targets 
> to make (e.g. make latexpdf html). I don't think the f.write() and 
> w.write() in set_versions.py are concurrent-safe.
>
>> What do you think?
>> 
>
> With the flock above, we could simply add the creation of a 
> _build/latest symlink which points to the last make target that ran. 
> Maybe we want a _build/latest/html for the last make html target, 
> otherwise imagine you run make all once and then make html one more time 
> for another distro, then _build/latest will now point to _build/distroB/ 
> which didn't generate the html part). But that could also mean we should 
> split the build directories for latexpdf, html and epub so this works... 
> but that means not being able to reuse the sphinx "cache" (I believe 
> there's one to do incremental builds? maybe it's also used for different 
> output types?).
>
> I think we (I) may be overengineering this a tiny bit :)
>
> I think we probably should add an flock at least for the writing of the 
> files by set_versions.py if that proves to be an issue (testing needs to 
> be done :) )

Sorry for the delay answering to this, I was caught up with the Whinlatter
release and never had the time to properly look at this.

I've sent a respin of this with what we were mentioning here, copied you to it:
https://lore.kernel.org/r/20251222-concurrent-safety-v1-0-e3d86e44cd38@bootlin.com

Thanks,
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



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

end of thread, other threads:[~2025-12-22 12:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09 10:23 [PATCH v2 0/3] tools/build-docs-container: improve concurrent safety Quentin Schulz
2025-10-09 10:23 ` [PATCH v2 1/3] tools/build-docs-container: guarantee the image to run matches the just-built image Quentin Schulz
2025-10-09 10:24 ` [PATCH v2 2/3] Makefile: allow to specify build directory Quentin Schulz
2025-10-09 10:24 ` [PATCH v2 3/3] tools/build-docs-container: build in separate directory for each distro Quentin Schulz
2025-10-14  9:27   ` [docs] " Antonin Godard
2025-10-15 11:37     ` Quentin Schulz
2025-12-22 12:47       ` Antonin Godard

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