Skip to content

implement run command#76

Closed
tosmi wants to merge 2 commits into
containers:masterfrom
tosmi:tosmi/exec
Closed

implement run command#76
tosmi wants to merge 2 commits into
containers:masterfrom
tosmi:tosmi/exec

Conversation

@tosmi

@tosmi tosmi commented Mar 8, 2019

Copy link
Copy Markdown
Contributor

this is my shot at implementing a toolbox run command. it simply runs a command in the toolbox container given.

the use case is that we do not have to enter the container before running various commands. i've various aliases defined using the run command:

alias emacs='toolbox run -c home emacs'
alias vlc='toolbox run -c home vlc'

currently the documentation is missing, i will update the pull request if there is a chance that this gets merged.

i'm not a shell programming expert, so please be patient and excuse any mistakes :-)

thanks for this great tool
toni

edit: fixed typo

@debarshiray debarshiray mentioned this pull request Mar 13, 2019
@debarshiray

debarshiray commented Mar 13, 2019

Copy link
Copy Markdown
Member

Thanks for the patch @tosmi (and @mildred too for the other implementation in #30) !

A run command has been requested by a few people, and I like the idea.

However, I am worried about the toolbox encroaching too much on Flatpak territory. Both of them provide rootless unprivileged containers, and there's significant overlap, but ultimately the toolbox is aimed for hackers to set up a development environment while Flatpak is optimized for non-technical users to run graphical applications. Given that, toolbox run emacs becomes a lot like flatpak run org.gnu.emacs. :)

This is not to say that we don't want to add a run. It's just that I want to get our story straight. :)

@debarshiray

Copy link
Copy Markdown
Member

From #silverblue on Freenode:

15:56 <rishi> otaylor: alexlarsson: Hey! Do you have any thoughts on            
      https://github.com/debarshiray/toolbox/pull/76 ?
15:57 <rishi> In short, people want to be able to do "toolbox run emacs".
15:57 <rishi> And I am worried about encroaching on Flatpak territory.
15:58 <alexlarsson> I don't think its a huge problem.
15:58 <otaylor> rishi: I suspect we need such functionality if we want the      
      toolbox to be a serious tool that people rely on
15:58 <alexlarsson> No 2 tools will be 100% non-overlapping
15:59 <alexlarsson> and i can imagine using this in non-flatpak like way
15:59 <alexlarsson> toolbox run some-service
15:59 <otaylor> rishi: I'd be more worried about adding, say, menu item         
      management so that it looks like 'toolbox run emacs' is a real app
16:00 <alexlarsson> I mean, some people use flatpak for cli stuff
16:00 <rishi> otaylor: alexlarsson: Okay!
16:00 <alexlarsson> which is not quite the point
16:00 <alexlarsson> Still, it works
16:01 <alexlarsson> The main thing is that the design decisions that drive      
      toolbox and flatpak are driven by a particular usecase
16:01 <alexlarsson> Not that they can't be used other ways
16:03 <rishi> Yeah, toolbox is very clearly: "use jhbuild on Silverblue".
16:04 <rishi> walters would say "separate development prefix", but that's       
      about it, I think.

@cgwalters

Copy link
Copy Markdown
Collaborator

This is one direction for https://discussion.fedoraproject.org/t/developing-applications-using-flatpak-packaged-editors-ides/269 right? The use case here is "run my editor from the same context with the other developer tools".

@debarshiray debarshiray left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get a chance to play with it in detail, but it looks sane after a quick glance.

We need to write a man page, though. It would be awesome if you could write one!

Comment thread toolbox Outdated
echo " enter [--container <name>]"
echo " [--release <release>]"
echo " or: toolbox [-v | --verbose]"
echo " run [-c | --container <name>]"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I'd put it after rm to keep them sorted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem, will fix that.

@owtaylor

Copy link
Copy Markdown
Contributor

This is one direction for https://discussion.fedoraproject.org/t/developing-applications-using-flatpak-packaged-editors-ides/269 right? The use case here is "run my editor from the same context with the other developer tools".

it's certainly an approach that works if the IDE is easy to install in the toolbox. Unfortunately, getting Eclipse (big piles of Java libraries) or VisualStudio Code (electron, lots of nodejs) into RPM form is not easy, and Flatpaks are an attractive target for such applications. So we need to figure out "bridge to the toolbox"

@tosmi

tosmi commented Mar 14, 2019

Copy link
Copy Markdown
Contributor Author

thanks for the review. i'll will come up with a manpage asap.

maybe i should read up on flatpack and how stuff works there. toolbox and running everything in a dev container is just a very easy way to migrate my "old" workflow into a container based one.

i'm completely aware that toolbox is more aimed at power users than non-technical users. but it's a great tool for me to get my toolchain running and event tinker around with it. wanna try unstable emacs? create a container, install it, use it via run, if it doesn't work go back to the old container, super easy.

if toolbox is clearly labeled as a tool for "power" users, it hopefully won't get confused with flatpack.

thanks
toni

Comment thread toolbox Outdated
@tosmi

tosmi commented Mar 19, 2019

Copy link
Copy Markdown
Contributor Author

so i've updated my branch:

  • added a manpage (sorry for any typos, it's already late here :-)
  • moved run after rmi
  • rebased to current master

thanks
toni

edit: fixed typos

@tosmi tosmi force-pushed the tosmi/exec branch 3 times, most recently from 2250bbb to 7a70d93 Compare April 12, 2019 19:37
@tosmi

tosmi commented Apr 12, 2019

Copy link
Copy Markdown
Contributor Author

rebased on current master, cleanup of history.

thanks
toni

tosmi added 2 commits April 30, 2019 18:19
enter() now uses run() for running a shell inside the container

@debarshiray debarshiray left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch, @tosmi Playing with this patch led to some simplifications and improvements in #148.

Some comments:

Comment thread doc/toolbox-run.1.md
Comment thread doc/toolbox-run.1.md
Comment thread toolbox
Comment thread toolbox
Comment thread doc/toolbox-run.1.md
Comment thread toolbox
Comment thread toolbox
Comment thread toolbox
Comment thread toolbox
debarshiray referenced this pull request May 6, 2019
This makes 'toolbox enter' similar to 'toolbox run $SHELL'.

The 'run' command is meant to spawn arbitrary binaries present inside
the toolbox container. Therefore it doesn't make sense for it to fall
back to /bin/bash, like it does for 'enter' if $SHELL is absent.

It's expected that users might use 'run' to create ad-hoc *.desktop
files. That's why it neither offers to create nor falls back to an
existing container like 'enter' does, because such interactions can't
happen when used in a *.desktop file. It's also a more advanced command
that new users are less likely to be interested in. Hence, this
shouldn't affect usability.

Some changes by Debarshi Ray.

https://github.com/debarshiray/toolbox/pull/76
@debarshiray debarshiray mentioned this pull request May 6, 2019
@debarshiray

Copy link
Copy Markdown
Member

I took the liberty to rebase against master and made the above changes in PR #149

@debarshiray

Copy link
Copy Markdown
Member

Sorry that it took so long to get it merged, and once again thanks for your contribution!

@debarshiray

Copy link
Copy Markdown
Member

Merged via #149, closing.

@debarshiray debarshiray closed this May 6, 2019
@tosmi

tosmi commented May 6, 2019

Copy link
Copy Markdown
Contributor Author

thank you for taking the time merging this request. i'm happy that i can use the upstream version again.

thanks
toni

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants