Multiline volume decleration support for distrobox-assemble

[abulusky]

Like additional_packages can be decleared in multiple line, it can't be done for volumes. For reference, running distrobox assemble dry run on the example.ini file:

[example]
additional_packages="package1"
additional_packages="package2"
additional_packages="package3"
hostname=debian
image=docker.io/library/debian:latest
entry=false
home=$HOME/.distros/debian
volume="$HOME/path1:/path1 $HOME/path2:/path2"
volume="$HOME/path3:/path3"
volume="$HOME/path4:/path4"

Current version gives:

$ distrobox assemble create -d --file example.ini 
 - Creating example...
/home/abulusky/.local/bin/distrobox-create --yes --name example --image docker.io/library/debian:latest --no-entry --home /home/abulusky/.distros/debian --hostname debian --additional-packages " package1  package2  package3" --volume "/home/abulusky/path1:/path1 /home/abulusky/path2:/path2" --volume  --volume /home/abulusky/path3:/path3 --volume  --volume /home/abulusky/path4:/path4

This update intends to fix this by taking them all as a arg of --volume command

After modification:

$ distrobox assemble create -d --file example.ini 
 - Creating example...
/home/abulusky/.local/bin/distrobox-create --yes --name example --image docker.io/library/debian:latest --no-entry --home /home/abulusky/.distros/debian --hostname debian --additional-packages " package1  package2  package3" --volume " /home/abulusky/path1:/path1 /home/abulusky/path2:/path2  /home/abulusky/path3:/path3  /home/abulusky/path4:/path4"

Please Note : There is a line in documentation which needs mention of the version no. from which the feature will be available

[softmoth]

I don't think this is necessary. As of distrobox: 1.7.2.1, your example produces (with --dry-run, and all on one line)

/usr/bin/distrobox-create --yes --name example --image docker.io/library/debian:latest \
  --no-entry --home /home/trs/.distros/debian --hostname debian \
  --additional-packages "   package1 package2 package3" \
  --volume "  /home/trs/path1:/path1 /home/trs/path2:/path2" \
  --volume /home/trs/path3:/path3 \
  --volume /home/trs/path4:/path4

And running that with distrobox-create --dry-run ... produces exactly the same output as if all of the --volume options are consolidated to a single --volume "/p/p1:/p1 /p/p2:/p2 /p/p3:/p3 /p/p4:/p4" option.

And neither the existing code nor your version handles paths with spaces in them properly, so I don't think this patch is needed anymore.

[abulusky]

@softmoth I suggest you try making a distrobox instead of dry run. The build fails.

testing.ini file:

[testing]
image=quay.io/toolbx-images/debian-toolbox:12
hostname=testing
replace=true
home=/tmp/home/testing
volume="/tmp/1:/1 /tmp/2:/2"
volume="/tmp/3:/3"
volume="/tmp/4:/4"

Shell interaction:

abulusky@machine:~ $ distrobox --version 
distrobox: 1.7.2.1
abulusky@machine:~ $ cd /tmp/
abulusky@machine:/tmp $ mkdir -p 1 2 3 4 home/testing
abulusky@machine:/tmp $ distrobox assemble create --file testing.ini -d
 - Deleting testing...
 - Creating testing...
/home/abulusky/.local/bin/distrobox-create --yes --name testing --image quay.io/toolbx-images/machinean-toolbox:12 --home /tmp/home/testing --hostname testing --volume "/tmp/1:/1 /tmp/2:/2" --volume  --volume /tmp/3:/3 --volume  --volume /tmp/4:/4
abulusky@machine:/tmp $ distrobox assemble create --file testing.ini
 - Deleting testing...
Error: no such container testing
Error: no such container testing
Cannot find container testing.
 - Creating testing...
Creating '/tmp/4:/4' using image quay.io/toolbx-images/machinean-toolbox:12        Error: invalid container path "--volume", must be an absolute path
 [ ERR ]
abulusky@machine:/tmp $ ./distrobox-assemble-modified create --file testing.ini -d
 - Deleting testing...
 - Creating testing...
./distrobox-assemble-modified: 328: ./distrobox-list: not found
./distrobox-create --yes --name testing --image quay.io/toolbx-images/machinean-toolbox:12 --home /tmp/home/testing --hostname testing --volume " /tmp/1:/1 /tmp/2:/2  /tmp/3:/3  /tmp/4:/4"
abulusky@machine:/tmp $ distrobox-create --yes --name testing --image quay.io/toolbx-images/machinean-toolbox:12 --home /tmp/home/testing --hostname testing --volume " /tmp/1:/1 /tmp/2:/2  /tmp/3:/3  /tmp/4:/4"
Creating 'testing' using image quay.io/toolbx-images/machinean-toolbox:12      [ OK ]
Distrobox 'testing' successfully created.
To enter, run:

distrobox enter testing

abulusky@machine:/tmp $ distrobox enter testing
Starting container...                            [ OK ]
Installing basic packages...                     [ OK ]
Setting up devpts mounts...                      [ OK ]
Setting up read-only mounts...                   [ OK ]
Setting up read-write mounts...                  [ OK ]
Setting up host's sockets integration...         [ OK ]
Integrating host's themes, icons, fonts...       [ OK ]
Setting up distrobox profile...                  [ OK ]
Setting up sudo...                               [ OK ]
Setting up user's group list...                  [ OK ]
Setting up existing user...                      [ OK ]
Ensuring user's access...                        [ OK ]
Setting up skel...                               [ OK ]

Container Setup Complete!
abulusky@testing:/tmp$ exit
logout
abulusky@machine:/tmp $ 

[softmoth]

It's strange, regardelss of --dry-run or not, I'm not getting the extraneous --volume options (I'm running distrobox 1.7.2.1 on SuSE).

However, I did find this related issue that has a simpler fix for the problem I think: #1514 (comment) . I prefer it slightly as it addresses the root issue of empty volume names, and it keeps the sanitize_variable call isolated to the $vol variable.

I guess it's not quite right to mention in docs that the multi-line feature is new, since it's clearly the intended behavior in the code. I guess it's best to mention as a bug fix in the Changelog (which affects some host operating systems and not others, maybe, since it works for me but not for you?).

[abulusky]

Oh! I see...

Indeed fixing the mentioned code would be a better solution. I'm using podman, on Debain testing system if that helps ...

[abulusky]

Implemented the suggested solution on #1514 (comment) . Code working fine. Do I create new PR?

[softmoth]

I'm just a user like you, not a maintainer of distrobox. That said, I think it's fine to just force-push to the same branch on this PR.

[89luca89]

Implemented the suggested solution on #1514 (comment) . Code working fine. Do I create new PR?

you can do the fix in this PR

[89luca89]

We can remove this

[89luca89]

We can just implement this: #1514 (comment)

[89luca89]

Fixed in cbf9f88 as part of similar fixes for other multilines