| 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 |
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] > -=-=-=-=-=-=-=-=-=-=-=- > >
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>] > -=-=-=-=-=-=-=-=-=-=-=- >
Hoi Johannes Yes, I also realized that when I looked at the patch on a larger screen
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)
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