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

Jump to bottom

cmd/go: can't handle relative TMPDIR #23264

Closed
cherti opened this issue Dec 27, 2017 · 8 comments
Closed

cmd/go: can't handle relative TMPDIR #23264

cherti opened this issue Dec 27, 2017 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@cherti
Copy link

cherti commented Dec 27, 2017

It seems like go build is not able to handle non-absolute TMPDIR and fails to build instead.

go version

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/tmp/gopath"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/tmppath/go-build325593383=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

#!/bin/bash

# prepare testsetup
workdir=$(mktemp -d)
echo "Working in $workdir"
demofile=$workdir/gopath/src/a/demo.go
GOPATH=$workdir/gopath
mkdir -p /$workdir/gopath/src/a/
mkdir -p /$workdir/tmpfoo


cat > $demofile <<EOF
package main

import "fmt"

func main() {
	fmt.Println("Hello World")
}
EOF


# starting the tests

cd $workdir
echo 'This will succeed (1)'
go build a  # this will succeed
echo 'Yep it did (1)'

TMPDIR=$workdir/tmpfoo
echo 'This will succeed (2)'
go build a  # this will succeed
echo 'Yep it did (2)'

cd $workdir
TMPDIR=tmpfoo
echo 'This will fail (3)'
go build a  # this will fail, although the directory is present
echo 'Yep it did (3)'

What did you expect to see?

go build (3) in script above succeed as well as (1) and (2)

What did you see instead?

go build (3) in script above failing.
@ianlancetaylor ianlancetaylor changed the title build: can't handle relative TMPDIR cmd/go: can't handle relative TMPDIR Dec 28, 2017
@ianlancetaylor
Copy link
Contributor

I'm not sure this is something we want to worry about supporting. It seems simple enough to not do that. Do most Unix tools support a relative TMPDIR?

@Merovius
Copy link
Contributor

I think if this is not supported, we should at least provide a more useful error message (the current error message is something along the lines of could not create $WORK/src/a/a.a: No such file or directory which took us a while to figure out).

@davecheney
Copy link
Contributor

davecheney commented Dec 28, 2017 via email

@gopherbot
Copy link

Change https://golang.org/cl/85596 mentions this issue: cmd/go: Resolve TMPDIR to an absolute path

@Merovius
Copy link
Contributor

I started adding this, but it doesn't reproduce anymore on tip, presumably because of the build cache changes. Some go commands still fail, if TMPDIR does not exist, even if they shouldn't use it at all (like go version) or if they seemingly don't actually need/use it (go build doesn't seem to actually write anything there anymore). There might still be places where this problem persists, so we might still add the check.

I think supporting a relative TMPDIR in the go tool would be as simple as erroring out: We can just call filepath.Abs on what os.TempDir returns to get an absolute path and use that. So I sent a CL to that effect.

I tried some of the POSIX interfaces to create temporary files and they all seem to do the same thing as Go in that regard: Use it just like /tmp and return a relative path. Meaning Go is consistent here with POSIX and that any C tool with a similar case (calling chdir(1), potentially in a child process). We could also consider doing that in os.TempDir directly, eliminating this problem in all Go code, but I'm not sure it's actually always the right thing to do.

@gopherbot
Copy link

Change https://golang.org/cl/91075 mentions this issue: runtime: allocated uninitialized in gobytes

@gopherbot
Copy link

Change https://golang.org/cl/103236 mentions this issue: cmd/go: reject relative tmpdir

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 28, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 28, 2018
@gopherbot
Copy link

Change https://golang.org/cl/123876 mentions this issue: cmd/go: handle relative temp dir

gopherbot pushed a commit that referenced this issue Jul 16, 2018
@rsc
cmd/go: handle relative temp dir
c2ba0ca 
Most programs seem to accept a relative temp dir, as weird as that might be.

Also, the meaning of relative is a little more fluid on Windows:
TMP=\temp is relative (to the current drive) but will work well enough.

Also, Windows GetTempPath automatically converts a relative
%TMP% into an absolute path, so we'd be imposing different
behavior for GOTMPDIR vs TMP.

It seems easier and more consistent to just impose the obvious
meaning than to add an error we can only implement some of
the time.

Originally got here because "cmd/go:" should be"go:" in error message,
but the error message is gone now.

Fixes #23264.

Change-Id: I3c3fb801cbd5e652364f1f62bb3881e9317e3581
Reviewed-on: https://go-review.googlesource.com/123876
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jul 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@davecheney @Merovius @ianlancetaylor @cherti @gopherbot