diff mbox series

[v1] devtool: un-/deploy-target: put deploylist into destdir

Message ID 20251017014123.222073-1-johannes.schneider@leica-geosystems.com
State Accepted, archived
Commit 216a4c4a4ee58222127c830ac56126bdbb95308d
Headers show
Series [v1] devtool: un-/deploy-target: put deploylist into destdir | expand

Commit Message

SCHNEIDER Johannes Oct. 17, 2025, 1:41 a.m. UTC
When deploying on devices with a RO root-filesystem, devtool would
fail on writing to the hard-coded "deploylist_path = '/.devtool'"

Since devtool already supports deploying to a different root-prefix
with: hostname[:destdir], we can make use of this guaranteed RW
location to place the deployment-list there.

Add the destdir parameter to the _prepare_remote_script function, to
construct the deploylist_path from it. For the 'undeploy' the same
host:destdir splitting logic is used as in 'deploy'.

Now it is possible to modify and build a recipe 'foo-bar' with
devtool, and have its ./image content deployed through:
$build> devtool deploy foo-bar target:/opt/development-overlay
Or removed again with:
$build> devtool undeploy foo-bar target:/opt/development-overlay

Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
---
 scripts/lib/devtool/deploy.py | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)


base-commit: 58558b97c157469f060bb2ad59a40254fb6181e4

Comments

Adrian Freihofer Oct. 17, 2025, 1:50 p.m. UTC | #1
Hi Johannes

Thank you for the patch. I agree, we should do what you are suggesting. I
will test and think about some details during the next few days.
This is just a quick response written on my smartphone.

Johannes Schneider via lists.openembedded.org <johannes.schneider=
leica-geosystems.com@lists.openembedded.org> schrieb am Fr., 17. Okt. 2025,
03:41:

> When deploying on devices with a RO root-filesystem, devtool would
> fail on writing to the hard-coded "deploylist_path = '/.devtool'"
>
> Since devtool already supports deploying to a different root-prefix
> with: hostname[:destdir], we can make use of this guaranteed RW
> location to place the deployment-list there.
>
> Add the destdir parameter to the _prepare_remote_script function, to
> construct the deploylist_path from it. For the 'undeploy' the same
> host:destdir splitting logic is used as in 'deploy'.
>
> Now it is possible to modify and build a recipe 'foo-bar' with
> devtool, and have its ./image content deployed through:
> $build> devtool deploy foo-bar target:/opt/development-overlay
> Or removed again with:
> $build> devtool undeploy foo-bar target:/opt/development-overlay
>
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com
> >
> ---
>  scripts/lib/devtool/deploy.py | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/lib/devtool/deploy.py b/scripts/lib/devtool/deploy.py
> index b5ca8f2c2f..a98b33c571 100644
> --- a/scripts/lib/devtool/deploy.py
> +++ b/scripts/lib/devtool/deploy.py
> @@ -20,9 +20,9 @@ from devtool import exec_fakeroot_no_d, setup_tinfoil,
> check_workspace_recipe, D
>
>  logger = logging.getLogger('devtool')
>
> -deploylist_path = '/.devtool'
> +deploylist_dirname = '.devtool'
>
> -def _prepare_remote_script(deploy, verbose=False, dryrun=False,
> undeployall=False, nopreserve=False, nocheckspace=False):
> +def _prepare_remote_script(deploy, destdir='/', verbose=False,
> dryrun=False, undeployall=False, nopreserve=False, nocheckspace=False):
>      """
>      Prepare a shell script for running on the target to
>      deploy/undeploy files. We have to be careful what we put in this
> @@ -31,6 +31,7 @@ def _prepare_remote_script(deploy, verbose=False,
> dryrun=False, undeployall=Fals
>      busybox rather than bash with coreutils).
>      """
>      lines = []
> +    deploylist_path = os.path.join(destdir, deploylist_dirname)
>      lines.append('#!/bin/sh')
>      lines.append('set -e')
>      if undeployall:
> @@ -146,7 +147,7 @@ def deploy(args, config, basepath, workspace):
>          except Exception as e:
>              raise DevtoolError('Exception parsing recipe %s: %s' %
>                              (args.recipename, e))
> -
> +
>          srcdir = rd.getVar('D')
>          workdir = rd.getVar('WORKDIR')
>          path = rd.getVar('PATH')
> @@ -244,6 +245,7 @@ def deploy_no_d(srcdir, workdir, path, strip_cmd,
> libdir, base_libdir, max_proce
>          tmpscript = '/tmp/devtool_deploy.sh'
>          tmpfilelist = os.path.join(os.path.dirname(tmpscript),
> 'devtool_deploy.list')
>          shellscript = _prepare_remote_script(deploy=True,
> +                                            destdir=destdir,
>                                              verbose=args.show_status,
>                                              nopreserve=args.no_preserve,
>
>  nocheckspace=args.no_check_space)
> @@ -303,12 +305,19 @@ def undeploy(args, config, basepath, workspace):
>          scp_port = "-P %s" % args.port
>          ssh_port = "-p %s" % args.port
>
> -    args.target = args.target.split(':')[0]
> +    try:
> +        host, destdir = args.target.split(':')
> +    except ValueError:
> +        destdir = '/'
> +    else:
> +        args.target = host
>
Just dropping the directory was probably not the best idea, I agree. But
does it make sense to drop it only in the else case? Is it properly defined
if there is a ValueError?

Or would it be better to not alter the args.target and fix all the code
which uses args.target to handle the directory part properly?

Regards,
Adrian

+    if not destdir.endswith('/'):
> +        destdir += '/'
>
>      tmpdir = tempfile.mkdtemp(prefix='devtool')
>      try:
>          tmpscript = '/tmp/devtool_undeploy.sh'
> -        shellscript = _prepare_remote_script(deploy=False,
> dryrun=args.dry_run, undeployall=args.all)
> +        shellscript = _prepare_remote_script(deploy=False,
> destdir=destdir, dryrun=args.dry_run, undeployall=args.all)
>          # Write out the script to a file
>          with open(os.path.join(tmpdir, os.path.basename(tmpscript)), 'w')
> as f:
>              f.write(shellscript)
>
> base-commit: 58558b97c157469f060bb2ad59a40254fb6181e4
> --
> 2.43.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#224987):
> https://lists.openembedded.org/g/openembedded-core/message/224987
> Mute This Topic: https://lists.openembedded.org/mt/115801291/4454582
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> adrian.freihofer@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
SCHNEIDER Johannes Oct. 24, 2025, 6:42 a.m. UTC | #2
Hoi Adrian,

>
> Hi Johannes
>
> Thank you for the patch. I agree, we should do what you are suggesting. I will test and think about some details during the next few days.

:-)

> This is just a quick response written on my smartphone.
>
> Johannes Schneider via lists.openembedded.org<http://lists.openembedded.org/> <johannes.schneider=leica-geosystems.com@lists.openembedded.org<mailto:leica-geosystems.com@lists.openembedded.org>> schrieb am Fr., 17. Okt. 2025, 03:41:
> When deploying on devices with a RO root-filesystem, devtool would
> fail on writing to the hard-coded "deploylist_path = '/.devtool'"
>
> Since devtool already supports deploying to a different root-prefix
> with: hostname[:destdir], we can make use of this guaranteed RW
> location to place the deployment-list there.
>
> Add the destdir parameter to the _prepare_remote_script function, to
> construct the deploylist_path from it. For the 'undeploy' the same
> host:destdir splitting logic is used as in 'deploy'.
>
> Now it is possible to modify and build a recipe 'foo-bar' with
> devtool, and have its ./image content deployed through:
> $build> devtool deploy foo-bar target:/opt/development-overlay
> Or removed again with:
> $build> devtool undeploy foo-bar target:/opt/development-overlay
>
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com<mailto:johannes.schneider@leica-geosystems.com>>
> ---
>  scripts/lib/devtool/deploy.py | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/lib/devtool/deploy.py b/scripts/lib/devtool/deploy.py
> index b5ca8f2c2f..a98b33c571 100644
> --- a/scripts/lib/devtool/deploy.py
> +++ b/scripts/lib/devtool/deploy.py
> @@ -20,9 +20,9 @@ from devtool import exec_fakeroot_no_d, setup_tinfoil, check_workspace_recipe, D
>
>  logger = logging.getLogger('devtool')
>
> -deploylist_path = '/.devtool'
> +deploylist_dirname = '.devtool'
>
> -def _prepare_remote_script(deploy, verbose=False, dryrun=False, undeployall=False, nopreserve=False, nocheckspace=False):
> +def _prepare_remote_script(deploy, destdir='/', verbose=False, dryrun=False, undeployall=False, nopreserve=False, nocheckspace=False):
>      """
>      Prepare a shell script for running on the target to
>      deploy/undeploy files. We have to be careful what we put in this
> @@ -31,6 +31,7 @@ def _prepare_remote_script(deploy, verbose=False, dryrun=False, undeployall=Fals
>      busybox rather than bash with coreutils).
>      """
>      lines = []
> +    deploylist_path = os.path.join(destdir, deploylist_dirname)
>      lines.append('#!/bin/sh')
>      lines.append('set -e')
>      if undeployall:
> @@ -146,7 +147,7 @@ def deploy(args, config, basepath, workspace):
>          except Exception as e:
>              raise DevtoolError('Exception parsing recipe %s: %s' %
>                              (args.recipename, e))
> -
> +
>          srcdir = rd.getVar('D')
>          workdir = rd.getVar('WORKDIR')
>          path = rd.getVar('PATH')
> @@ -244,6 +245,7 @@ def deploy_no_d(srcdir, workdir, path, strip_cmd, libdir, base_libdir, max_proce
>          tmpscript = '/tmp/devtool_deploy.sh'
>          tmpfilelist = os.path.join(os.path.dirname(tmpscript), 'devtool_deploy.list')
>          shellscript = _prepare_remote_script(deploy=True,
> +                                            destdir=destdir,
>                                              verbose=args.show_status,
>                                              nopreserve=args.no_preserve,
>                                              nocheckspace=args.no_check_space)
> @@ -303,12 +305,19 @@ def undeploy(args, config, basepath, workspace):
>          scp_port = "-P %s" % args.port
>          ssh_port = "-p %s" % args.port
>
> -    args.target = args.target.split(':')[0]
> +    try:
> +        host, destdir = args.target.split(':')
> +    except ValueError:
> +        destdir = '/'
> +    else:
> +        args.target = host
> Just dropping the directory was probably not the best idea, I agree. But does it make sense to drop it only in the else case? Is it properly defined if there is a ValueError?
>
> Or would it be better to not alter the args.target and fix all the code which uses args.target to handle the directory part properly?
>

the try/except/else follows the same pattern that is used up above in 'deploy_no_d'; so your concerns would apply there too

we could refactor both places some more - and raise an error is a user passes a malformed [host[:dest]]?

the current patch tried to be "minimally invasive" :-)

>
> Regards,
> Adrian
>

gruß
Johannes

> +    if not destdir.endswith('/'):
> +        destdir += '/'
>
>      tmpdir = tempfile.mkdtemp(prefix='devtool')
>      try:
>          tmpscript = '/tmp/devtool_undeploy.sh'
> -        shellscript = _prepare_remote_script(deploy=False, dryrun=args.dry_run, undeployall=args.all)
> +        shellscript = _prepare_remote_script(deploy=False, destdir=destdir, dryrun=args.dry_run, undeployall=args.all)
>          # Write out the script to a file
>          with open(os.path.join(tmpdir, os.path.basename(tmpscript)), 'w') as f:
>              f.write(shellscript)
>
> base-commit: 58558b97c157469f060bb2ad59a40254fb6181e4
> --
> 2.43.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#224987): https://lists.openembedded.org/g/openembedded-core/message/224987<https://lists.openembedded.org/g/openembedded-core/message/224987>
> Mute This Topic: https://lists.openembedded.org/mt/115801291/4454582<https://lists.openembedded.org/mt/115801291/4454582>
> Group Owner: openembedded-core+owner@lists.openembedded.org<mailto:openembedded-core%2Bowner@lists.openembedded.org>
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub<https://lists.openembedded.org/g/openembedded-core/unsub> [adrian.freihofer@gmail.com<mailto:adrian.freihofer@gmail.com>]
> -=-=-=-=-=-=-=-=-=-=-=-
>
AdrianF Oct. 24, 2025, 1:56 p.m. UTC | #3
Hoi Johannes

Yes, I also realized that when I looked at the patch on a larger screen 
diff mbox series

Patch

diff --git a/scripts/lib/devtool/deploy.py b/scripts/lib/devtool/deploy.py
index b5ca8f2c2f..a98b33c571 100644
--- a/scripts/lib/devtool/deploy.py
+++ b/scripts/lib/devtool/deploy.py
@@ -20,9 +20,9 @@  from devtool import exec_fakeroot_no_d, setup_tinfoil, check_workspace_recipe, D
 
 logger = logging.getLogger('devtool')
 
-deploylist_path = '/.devtool'
+deploylist_dirname = '.devtool'
 
-def _prepare_remote_script(deploy, verbose=False, dryrun=False, undeployall=False, nopreserve=False, nocheckspace=False):
+def _prepare_remote_script(deploy, destdir='/', verbose=False, dryrun=False, undeployall=False, nopreserve=False, nocheckspace=False):
     """
     Prepare a shell script for running on the target to
     deploy/undeploy files. We have to be careful what we put in this
@@ -31,6 +31,7 @@  def _prepare_remote_script(deploy, verbose=False, dryrun=False, undeployall=Fals
     busybox rather than bash with coreutils).
     """
     lines = []
+    deploylist_path = os.path.join(destdir, deploylist_dirname)
     lines.append('#!/bin/sh')
     lines.append('set -e')
     if undeployall:
@@ -146,7 +147,7 @@  def deploy(args, config, basepath, workspace):
         except Exception as e:
             raise DevtoolError('Exception parsing recipe %s: %s' %
                             (args.recipename, e))
-        
+
         srcdir = rd.getVar('D')
         workdir = rd.getVar('WORKDIR')
         path = rd.getVar('PATH')
@@ -244,6 +245,7 @@  def deploy_no_d(srcdir, workdir, path, strip_cmd, libdir, base_libdir, max_proce
         tmpscript = '/tmp/devtool_deploy.sh'
         tmpfilelist = os.path.join(os.path.dirname(tmpscript), 'devtool_deploy.list')
         shellscript = _prepare_remote_script(deploy=True,
+                                            destdir=destdir,
                                             verbose=args.show_status,
                                             nopreserve=args.no_preserve,
                                             nocheckspace=args.no_check_space)
@@ -303,12 +305,19 @@  def undeploy(args, config, basepath, workspace):
         scp_port = "-P %s" % args.port
         ssh_port = "-p %s" % args.port
 
-    args.target = args.target.split(':')[0]
+    try:
+        host, destdir = args.target.split(':')
+    except ValueError:
+        destdir = '/'
+    else:
+        args.target = host
+    if not destdir.endswith('/'):
+        destdir += '/'
 
     tmpdir = tempfile.mkdtemp(prefix='devtool')
     try:
         tmpscript = '/tmp/devtool_undeploy.sh'
-        shellscript = _prepare_remote_script(deploy=False, dryrun=args.dry_run, undeployall=args.all)
+        shellscript = _prepare_remote_script(deploy=False, destdir=destdir, dryrun=args.dry_run, undeployall=args.all)
         # Write out the script to a file
         with open(os.path.join(tmpdir, os.path.basename(tmpscript)), 'w') as f:
             f.write(shellscript)