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.

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

Fixes #1449 - avoid shell tricks, brace expansion, GOFLAGS in docker … #1460

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

StevenACoffman
Copy link
Contributor

@StevenACoffman StevenACoffman commented Mar 5, 2021

For universal portability, the build_dist.sh script uses posix shell, which the build_docker.sh script adopted after testing. Brace expansion is not supported in the posix shell, and if more tags were ever added, was a brittle solution anyway.

I verified this works by running the script to build an image, and then running the latest built image:

./build_docker.sh
docker run -it $(docker images | awk '{print $3}' | awk 'NR==2') /usr/local/bin/tailscale version

and this produces the following output:

1.5.189
  tailscale commit: 937a46d12672c8afe127e43b7da2ea4222ffc4ec
  go version: go1.16

Signed-off-by: Steve Coffman steve@khanacademy.org

build_docker.sh Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@bradfitz
Copy link
Member

bradfitz commented Mar 5, 2021

Can you git push -f a new single squashed commit in our existing style? (same as Go style: https://golang.org/wiki/CommitMessage)

Like:

build_docker.sh, Dockerfile: fix bug with shell quoting

Fixes #1449

Fixes tailscale#1449

Signed-off-by: Steve Coffman <steve@khanacademy.org>
@StevenACoffman
Copy link
Contributor Author

Ah, I see, but leave the DCO. Sorry for the noise.

Another reason to avoid GOFLAGS here for now is because you cannot currently have multiple values for flags other than -tags without the last one "winning".

@StevenACoffman
Copy link
Contributor Author

Also, not sure why the Windows / test fails, as this shouldn't affect it, so I'm guessing it's flaky or was introduced by something before this.

@bradfitz bradfitz merged commit 0d0fad4 into tailscale:main Mar 5, 2021
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.

2 participants