mviereck / x11docker

Run GUI applications and desktops in docker and podman containers. Focus on security.
MIT License
5.62k stars 378 forks source link

Remove dependency on zip #115

Closed eine closed 5 years ago

eine commented 5 years ago

Currently unzip is required in order to install x11docker:

# curl -fsSL https://raw.githubusercontent.com/mviereck/x11docker/master/x11docker | bash -s -- --update-master
main: line 5149: ip: command not found
main: line 5150: ip: command not found
x11docker note: Failed to check for sshd. ps -p not supported.

main: line 5181: ps: command not found
x11docker note: Your terminal seems to be not POSIX compliant.
  Command 'logname' does not return a value.
  Consider to use another terminal emulator.
  Fallback: Will try to check $SUDO_USER and $PKEXEC_UID.

x11docker note: Will use $(id -un) = root as host user.

x11docker WARNING: Running as user root.
  Maybe $(logname) did not provide an unprivileged user.
  Please use option --hostuser=USER to specify an unprivileged user.
  Otherwise, new X server runs as root, and container user will be root.

Failed to connect to bus: No such file or directory
x11docker note: Current installed version: x11docker GNU bash, version 5.0.0(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

x11docker ERROR: Can not unpack archive. Please install 'unzip'.

  Type 'x11docker --help' for usage information
  Debug options: '--verbose' (full log) or '--debug' (log excerpt).
  Logfile will be: /root/.cache/x11docker/x11docker.log
  Please report issues at https://github.com/mviereck/x11docker

The check is done in https://github.com/mviereck/x11docker/blob/master/x11docker#L1084, and unzip is used in https://github.com/mviereck/x11docker/blob/master/x11docker#L1100. In between, the repo is downloaded in zip format.

There are two issues here:

mviereck commented 5 years ago

I just want to note that this is a good suggestion and I still have it in mind. Thank you!

I am still hesitating because it is more work than it looks on the first glance. It changes an essential functionality that must work reliable in any case. I often have hassle with common linux commands that behave different on different systems. So I have to check out the behaviour of tar on many linux systems first before implementing it in the update routine.

eine commented 5 years ago

I am still hesitating because it is more work than it looks on the first glance. It changes an essential functionality that must work reliable in any case.

Is it 'more work' because many things need to be changed or just because of the effects it might have?

I often have hassle with common linux commands that behave different on different systems. So I have to check out the behaviour of tar on many linux systems first before implementing it in the update routine.

Did you find any particular issue with tar? I found the following to be quite portable:

mkdir $TARGET_PATH
curl -fsSL https://codeload.github.com/mviereck/x11docker/tar.gz/master | tar xzf - --strip-components=1 -C $TARGET_PATH
mviereck commented 5 years ago

Is it 'more work' because many things need to be changed or just because of the effects it might have?

The installer() function is rather short and simple. The work is to check its functionality on several different linux systems.

If an option like e.g. --clipboard would fail somewhere, it is a minor issue and does not affect core x11docker functionality.

If --update fails, it breaks x11docker entirely. So I hesitate to touch it.

Did you find any particular issue with tar?

I rarely used tar at all. I just found that I can't rely on the same syntax or same result of many common linux commands on several systems. I even found one POSIX command that does not work everywhere (logname).

I could create a bunch of different containers based on as much different systems as possible and check tar in them. Some day.

mviereck commented 5 years ago

I have included tar support now. I did some tests on some systems, and its syntax has been the same on all of them. The download pathes are changed to codeload.github.com.

Thanks for your suggestion!

eine commented 5 years ago

I just tested it and it works great. Thanks!

mviereck commented 5 years ago

Thanks for the feedback!