Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/build/cmd/gomote: gomote run arguments are subtle and tedious #53957

Closed
bcmills opened this issue Jul 19, 2022 · 7 comments
Closed

x/build/cmd/gomote: gomote run arguments are subtle and tedious #53957

bcmills opened this issue Jul 19, 2022 · 7 comments
Assignees
@bcmills
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Jul 19, 2022

What version of Go are you using (go version)?

~$ go version -m $(which gomote) 
/usr/local/google/home/bcmills/bin/gomote: devel go1.19-b9c4d94fdb Fri Jun 24 18:54:27 2022 +0000
        path    golang.org/x/build/cmd/gomote
        mod     golang.org/x/build      v0.0.0-20220623213349-3ceb9f4e34a9      h1:UXzH6j5xqqdwolS/8sIboizJIjD1FchifnbxqrO4wro=
        dep     cloud.google.com/go/compute     v1.3.0  h1:mPL/MzDDYHsh5tHRS9mhmhWlcgClCrCa6ApQCU6wnHI=
        dep     github.com/aws/aws-sdk-go       v1.30.15        h1:Sd8QDVzzE8Sl+xNccmdj0HwMrFowv6uVUx9tGsCE1ZE=
        dep     github.com/golang/groupcache    v0.0.0-20200121045136-8c9f03a8e57e      h1:1r7pUrabqp18hOBcwBwiTsbnFeTZHV9eER/QT5JVZxY=
        dep     github.com/golang/protobuf      v1.5.2  h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw=
        dep     github.com/googleapis/gax-go/v2 v2.1.1  h1:dp3bWCh+PPO1zjRRiCSczJav13sBvG4UhNyVTa1KqdU=
        dep     github.com/jmespath/go-jmespath v0.4.0  h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
        dep     go.opencensus.io        v0.23.0 h1:gqCw0LfLxScz8irSi8exQc7fyQ0fKQU/qnC/X8+V/1M=
        dep     golang.org/x/net        v0.0.0-20220127200216-cd36cc0744dd      h1:O7DYs+zxREGLKzKoMQrtrEacpb0ZVXA5rIwylE2Xchk=
        dep     golang.org/x/oauth2     v0.0.0-20211104180415-d3ed0bb246c8      h1:RerP+noqYHUQ8CMRcPlC2nvTa4dcBIjegkuWdcUDuqg=
        dep     golang.org/x/sync       v0.0.0-20210220032951-036812b2e83c      h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
        dep     golang.org/x/sys        v0.0.0-20220209214540-3681064d5158      h1:rm+CHSpPEEW2IsXUib1ThaHIjuBVZjxNgSKmBLFfD4c=
        dep     golang.org/x/text       v0.3.7  h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
        dep     golang.org/x/time       v0.0.0-20210723032227-1f47c861a9ac      h1:7zkz7BUtwNFFqcowJ+RIgu2MaV/MapERkDIy+mwPyjs=
        dep     google.golang.org/api   v0.70.0 h1:67zQnAE0T2rB0A3CwLSas0K+SbVzSxP+zTLkQLexeiw=
        dep     google.golang.org/genproto      v0.0.0-20220222213610-43724f9ea8cf      h1:SVYXkUz2yZS9FWb2Gm8ivSlbNQzL2Z/NpPKE3RG2jWk=
        dep     google.golang.org/grpc  v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg=
        dep     google.golang.org/protobuf      v1.27.1 h1:SnqbnDw1V7RiZcXPx5MEeqPv2s79L9i7BJUlG/+RurQ=
        dep     gopkg.in/inf.v0 v0.9.1  h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
        build   -compiler=gc
        build   CGO_ENABLED=1
        build   CGO_CFLAGS=
        build   CGO_CPPFLAGS=
        build   CGO_CXXFLAGS=
        build   CGO_LDFLAGS=
        build   GOARCH=amd64
        build   GOOS=linux
        build   GOAMD64=v1

What did you do?

$ gomote create linux-amd64
$ gomote run user-bcmills-linux-amd64-0 pwd

$ gomote create windows-amd64-2012
$ gomote run user-bcmills-windows-amd64-2012-0 dir

What did you expect to see?

The output of the pwd or dir command.

What did you see instead?

On Linux, commands typically only work if they are given by absolute path, even if the command would be available in the $PATH normally available on the builder (compare #32430):

~$ gomote run user-bcmills-linux-amd64-0 pwd
Error running run: fork/exec /workdir/pwd: no such file or directory

~$ gomote run user-bcmills-linux-amd64-0 /bin/pwd
/workdir

On Windows, commands seem to require not only the absolute path, but also the magic -system flag, and the failure mode without it is incredibly misleading:

~$ gomote run user-bcmills-windows-amd64-2012-0 dir
Error running run: exec: "C:\\workdir\\dir": file does not exist

~$ gomote run user-bcmills-windows-amd64-2012-0 cmd /c dir
Error running run: exec: "C:\\workdir\\cmd": file does not exist

~$ gomote run user-bcmills-windows-amd64-2012-0 'C:\windows\system32\cmd.exe' /c dir
Error running run: Error trying to execute C:\windows\system32\cmd.exe: buildlet: HTTP status 400 Bad Request: requires 'cmd' parameter

~$ gomote run -system user-bcmills-windows-amd64-2012-0 'C:\windows\system32\cmd.exe' /c dir
 Volume in drive C has no label.
 Volume Serial Number is C8EA-22A6

 Directory of C:\workdir

07/02/2018  05:51 PM    <DIR>          .
07/02/2018  05:51 PM    <DIR>          ..
               0 File(s)              0 bytes
               2 Dir(s)  19,789,328,384 bytes free
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Jul 19, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jul 19, 2022
@bcmills
Copy link
Contributor Author

bcmills commented Jul 19, 2022

(CC @dmitshur @mknyszek)

@mknyszek
Copy link
Contributor

Thanks for filing this, this is on my radar and I plan to resolve it.

@mknyszek mknyszek self-assigned this Jul 19, 2022
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 19, 2022
@bcmills
Copy link
Contributor Author

bcmills commented Jul 19, 2022

I think https://go.dev/cl/367896 will be necessary (but perhaps not sufficient) to get to a state that works more like the native $PATH search. I'm going to get that reviewed and merged, and you're welcome to follow up with further improvements on top of it. 🙂

@gopherbot
Copy link

Change https://go.dev/cl/367896 mentions this issue: buildlet,cmd/coordinator: clean up and document buildlet path invariants

@bcmills bcmills self-assigned this Jul 19, 2022
@mknyszek
Copy link
Contributor

Thanks @bcmills! I suspect your CL is indeed sufficient. I could've sworn I saw an empty PATH on a builder before, but executing /usr/bin/env on a Linux instance does indeed reveal that there is a baseline PATH set.

gopherbot pushed a commit to golang/build that referenced this issue Jul 20, 2022
buildlet,cmd/coordinator: clean up and document buildlet path invariants
7b01ecc 
For golang/go#53957.
Updates golang/go#33598.

Change-Id: Ie2c47dc0dfd82614b70c983f11462b70bd0703d2
Reviewed-on: https://go-review.googlesource.com/c/build/+/367896
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
@dmitshur
Copy link
Contributor

Tested with the new coordinator and buildlet versions, and it works as expected:

$ gomote v2 create linux-amd64
dmitshur-linux-amd64-0
$ gomote v2 run dmitshur-linux-amd64-0 pwd     
/bin
gomote legacy
$ gomote create linux-amd64
user-dmitshur-linux-amd64-0
$ gomote run user-dmitshur-linux-amd64-0 pwd
/bin

@bcmills
Copy link
Contributor Author

bcmills commented Jul 25, 2022

Seems to be fixed, then? (Closing, but please reopen if this still reproduces.)

@bcmills bcmills closed this as completed Jul 25, 2022
@golang golang locked and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bcmills bcmills

Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @bcmills @gopherbot