fyne-io / tools

Toolchain and helpful commands for building and managing Fyne apps
BSD 3-Clause "New" or "Revised" License
6 stars 6 forks source link

fyne get requires root access #21

Open paboum opened 1 year ago

paboum commented 1 year ago

Checklist

Describe the bug

The fyne get command assumes /usr/local/bin as installation target (hardcoded) and makes no use of GOPATH env var.

$ fyne get fyne.io/apps
2022/11/24 05:08:10 Fyne error:  Failed to create dirrectory
2022/11/24 05:08:10   Cause: mkdir /usr/local/share: permission denied
2022/11/24 05:08:10   At: /usr/go/pkg/mod/fyne.io/fyne/v2@v2.2.4/cmd/fyne/internal/util/file.go:36
failed to copy application binary file: open /usr/local/bin/apps: permission denied

How to reproduce

$ fyne get fyne.io/apps

Screenshots

No response

Example code

    if _, err := os.Stat(filepath.Join("/", "usr", "local")); os.IsNotExist(err) {
        prefixDir = util.EnsureSubDir(util.EnsureSubDir(p.dir, tempDir), "usr")
        local = ""
    } else {
        prefixDir = util.EnsureSubDir(util.EnsureSubDir(util.EnsureSubDir(p.dir, tempDir), "usr"), "local")
    }

    shareDir := util.EnsureSubDir(prefixDir, "share")

    binDir := util.EnsureSubDir(prefixDir, "bin")
    binName := filepath.Join(binDir, filepath.Base(p.exe))
    err := util.CopyExeFile(p.exe, binName)
    if err != nil {
        return fmt.Errorf("failed to copy application binary file: %w", err)
    }

Fyne version

2.2.4

Go compiler version

1.19.2

Operating system

Linux

Operating system version

Gentoo

Additional Information

It would be preferable to respect GOPATH and build apps in $GOPATH/bin, with a possibility of creating symlinks in /usr/local/bin/. Also, note that /usr/local/bin may be writable for regular users on some systems, but it's not a standard behavior: https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard

andydotxyz commented 1 year ago

The behaviour of "fyne get" is to install to the system specific application area - this are graphical app packages not go binaries. On windows this is Program Files and on macOS it is /Applications. This way the apps are findable in standard launchers. Installing to the go binary location would not offer this benefit. I agree that requiring sudo on some systems is undesirable, but I do not know what the alternative is?...

paboum commented 1 year ago

If it respects GOPATH, and GOBIN, one can call it just like go: GOBIN=/usr/local/bin/ go install https://stackoverflow.com/questions/24069664/what-does-go-install-do

andydotxyz commented 1 year ago

Fyne install is not like go install - it is not simply dripping the binary file in. It has metadata and package artefacts depending on OS

Jacalz commented 1 year ago

How about adding a flag like --local that installs into $HOME/.local? We are already doing something similar with the installation Makefile for the packaged Linux/BSD applications: https://github.com/fyne-io/fyne/blob/2c1d0bd8a00beedf283923ad1d91d519b822b858/cmd/fyne/internal/templates/data/Makefile#L30.

andydotxyz commented 1 year ago

Maybe --user as that is a more generally understood term for user space installs?

paboum commented 1 year ago

Maybe --user as that is a more generally understood term for user space installs?

How about best effort, if there is no write access to /usr/local/bin, then try $HOME/.local/bin? Unless, of course, FYNEBIN env var doesn't point to somewhere else.

andydotxyz commented 1 year ago

I don't think that it's a good idea to significantly change location based on permission setup. Have you ever run a command and forgotten you need sudo? In this case it would potentially put the app in an unexpected place and exit with no error.

Jacalz commented 1 year ago

I agree. It is better to default to requiring sudo and then potentially give a suggestion, in the error message, to either elevate permissions (using sudo, doas, etc.) or use --user flag.

paboum commented 1 year ago

What information is still needed then?

andydotxyz commented 1 year ago

It would be good to know if you are happy with with @Jacalz proposed. If so we can proceed.

Jacalz commented 1 year ago

@paboum Why did you close this as not planned? I think it should stay open until the proposed feature has been implemented.

NiceGuyIT commented 1 year ago

I was attempting to install fyneterm when I ran across this issue. There are a few things that can be improved.

  1. Document the need for sudo.
  2. sudo ~/go/bin/fyne get github.com/fyne-io/terminal/cmd/fyneterm builds and installs the application as root leaving behind /root/go/{pkg/src}.
  3. Building as user and prompting for sudo/root access for installation is preferred.
andydotxyz commented 1 year ago

Prompting for sudo would be ideal, however it is not clear how we know whether sudo or su is the setup for a local system - neither is universal. If you have hints and/or interest in creating a PR that would be excellent!

Bluebugs commented 1 year ago

Maybe another approach is to detect the user we are running under and if it isn't root, we do the install as a user (our Unix Makefile support that with the user-install rule). That might be easier.

Jacalz commented 1 year ago

We have already discussed the solutions for this further up in this thread. I believe we already have a good idea for how to solve this

Bluebugs commented 1 year ago

We have already discussed the solutions for this further up in this thread. I believe we already have a good idea for how to solve this

Oops, indeed.

andydotxyz commented 2 months ago

Moving to the tools repo where this development is being picked up.