gokrazy / tools

this repository contains the gok CLI tool of gokrazy
https://gokrazy.org
BSD 3-Clause "New" or "Revised" License
50 stars 28 forks source link

/tmp/gokrazy-bins-... is not an ELF binary! read /tmp/gokrazy-bins-...: is a directory #71

Closed oliverpool closed 3 months ago

oliverpool commented 3 months ago

After not upgrading my gokrazy for a year (very bad practice), I finally took the time to attempt an upgrade.

I stumbled upon the following error when running gok update:

(...)
[done] in 5.24s                        

2024/07/02 21:07:40 /tmp/gokrazy-bins-2652291663 is not an ELF binary! read /tmp/gokrazy-bins-2652291663: is a directory (perhaps running into https://github.com/golang/go/issues/53804?)

The mention of a directory looked highly suspicious (and unrelated to the linked issued). After some debugging (i.e. print statements and panics), it appears that the Target field is missing from the go list -json: https://github.com/gokrazy/tools/blob/41f11fbe3a53695e8ad701b98ec105d4b88084b6/packer/gotool.go#L366

In my case this likely happens because I set a custom GOBIN in the GOENV file (the env var GOVIN gets set to an empty value by Env(), but this is not enough to overwrite the value written inside my ~/.config/go/env file).

As a rough workaround, I also reset GOENV=invalid-value in Env() and it seems to be working (however it discards all my other settings, like cache location & co.)

Has anyone an idea on how to solve this in a nicer way? (if not, the empty Target field should be detected earlier and trigger an earlier failure).

oliverpool commented 3 months ago

The Go command uses os.Getenv, whereas it should call os.LookupEnv to differentiate between an unset var and an empty var:

https://github.com/golang/go/blob/09aeb6e33ab426eff4676a3baf694d5a3019e9fc/src/cmd/go/internal/cfg/cfg.go#L388

And the Target seems to be cleared when GOBIN is not empty: https://github.com/golang/go/blob/09aeb6e33ab426eff4676a3baf694d5a3019e9fc/src/cmd/go/internal/load/pkg.go#L1785

oliverpool commented 3 months ago

Looking at the code of gok, Target is "only" used to determine the executable name. Since gokrazy only allows import paths (and not pointing to individual go files), only the Package.exeFromImportPath logic should be ported: take the path.Basename, except if it is a `vXX'.

I might craft a PR, if no better solution is suggested in the coming days.

stapelberg commented 3 months ago

Thanks for the investigation.

For context, we switched away from using GOBIN in https://github.com/gokrazy/tools/issues/2, so likely your setup broke with that commit or shortly after.

I was trying to reproduce the issue, but when I run GOBIN=/some/path go list -json github.com/gokrazy/gokrazy/cmd/... in my checkout of https://github.com/gokrazy/gokrazy, I do get filled-in Target fields:

GOBIN=/some/path go list -json github.com/gokrazy/gokrazy/cmd/randomd
{
    "Dir": "/home/michael/go/src/github.com/gokrazy/gokrazy/cmd/randomd",
    "ImportPath": "github.com/gokrazy/gokrazy/cmd/randomd",
    "Name": "main",
    "Doc": "Binary randomd carries entropy across restarts.",
    "Target": "/some/path/randomd",
    "Root": "/home/michael/go/src/github.com/gokrazy/gokrazy",
    "Module": {
        "Path": "github.com/gokrazy/gokrazy",
        "Main": true,
        "Dir": "/home/michael/go/src/github.com/gokrazy/gokrazy",
        "GoMod": "/home/michael/go/src/github.com/gokrazy/gokrazy/go.mod",
        "GoVersion": "1.21"
    },
    "Match": [
        "github.com/gokrazy/gokrazy/cmd/randomd"
    ],
    "DefaultGODEBUG": "httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1",
    "Stale": true,
    "StaleReason": "build ID mismatch",
    "GoFiles": [
        "randomd.go"
    ],
    "Imports": [
        "github.com/gokrazy/gokrazy/internal/teelogger",
        "golang.org/x/sys/unix",
        "io",
        "io/ioutil",
        "os",
        "os/signal",
        "syscall",
        "time",
        "unsafe"
    ],
    "Deps": [
        "bytes",
        […]
        "unsafe"
    ]
}

So there probably the triggering condition needs more than just GOBIN set?

We can probably synthesize the path when missing as you suggest, but it would be good to fully understand the issue first.

oliverpool commented 3 months ago

I do get filled-in Target fields

Because GOARCH was not overridden. I get a missing Target field with (which is present when GOBIN is completely unset):

> GOARCH=arm GOBIN=/some/path go list -json github.com/gokrazy/gokrazy/cmd/randomd
stapelberg commented 3 months ago

Ah, yes, setting GOARCH to some other architecture than the host is the trigger.

Do you want to send a PR to synthesize the binary name when absent? Thanks.