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

switch user.current -> homedir.dir #32

Merged
merged 1 commit into from Dec 16, 2016

Conversation

Projects
None yet
3 participants
@jsloyer
Contributor

jsloyer commented Dec 15, 2016

The call user.Current depends on cgo to compile but when you cross compile on OSX this is not available. Per this github issue there is a library that can get the home dir without using cgo. The library is https://github.com/mitchellh/go-homedir and its licensed under MIT.

@renier

This comment has been minimized.

Contributor

renier commented Dec 15, 2016

@jsloyer I'm on OSX and it works fine. Don't understand the issue.

@jsloyer

This comment has been minimized.

Contributor

jsloyer commented Dec 15, 2016

So this is an issue if you cross compile the softlayer go library

@renier

This comment has been minimized.

Contributor

renier commented Dec 15, 2016

@jsloyer You said cross compile to OSX, so that's the type of binary I'm running, and not experiencing the issue.

Are you using the released binaries? Or are you building it on your own?

@renier

This comment has been minimized.

Contributor

renier commented Dec 15, 2016

@jsloyer Basically, tell me specifically how to reproduce the problem you saw.

@jsloyer

This comment has been minimized.

Contributor

jsloyer commented Dec 15, 2016

So I'm running non OSX and if cross compile a binary that includes this library for Linux is when the issue arises

@renier

This comment has been minimized.

Contributor

renier commented Dec 15, 2016

@jsloyer This is what I did to attempt reproduction:

  1. Created a small snippet of code that uses user.Current():
package main

import (
	"fmt"
	"log"
	"os/user"
)

func main() {
	u, err := user.Current()
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(u.HomeDir)
}
  1. Build it on OSX, with linux as the xcompile target: env GOOS="linux" GOARCH="amd64" go build
  2. Copy the binary to a Linux system and I run it.

The output is what I expected.

@jsloyer

This comment has been minimized.

Contributor

jsloyer commented Dec 16, 2016

This docker image reproduces it, https://hub.docker.com/r/jsloyer/user-current-error/.

The docker host info...

docker version
Client:
 Version:      1.12.4
 API version:  1.24
 Go version:   go1.6.4
 Git commit:   1564f02
 Built:        Mon Dec 12 23:50:16 2016
 OS/Arch:      linux/amd64

Server:
 Version:      1.12.4
 API version:  1.24
 Go version:   go1.6.4
 Git commit:   1564f02
 Built:        Mon Dec 12 23:50:16 2016
 OS/Arch:      linux/amd64

distro info:

~# cat /etc/*release*
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=14.04
DISTRIB_CODENAME=trusty
DISTRIB_DESCRIPTION="Ubuntu 14.04.5 LTS"
NAME="Ubuntu"
VERSION="14.04.5 LTS, Trusty Tahr"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 14.04.5 LTS"
VERSION_ID="14.04"
HOME_URL="http://www.ubuntu.com/"
SUPPORT_URL="http://help.ubuntu.com/"
BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"
@renier

This comment has been minimized.

Contributor

renier commented Dec 16, 2016

@jsloyer Can I see the Dockerfile and how you build it?

@renier

This comment has been minimized.

Contributor

renier commented Dec 16, 2016

@jsloyer I see the issue. Happens when executing the cross-compiled binary in a linux container only, but does not happen in a full-blown VM. I wish I knew what was missing in the container, so that we could just try adding it.

I see two solutions:

  1. You build the binary within a linux container. Avoids cross-compile. This article describes that approach.
  2. We retool the logic in softlayer go to, only if user.Current() fails, try looking at $HOME or $USERPROFILE as done here.

I don't want to add a dependency just to solve this edge case. We have kept the library largely dependency-free and want to keep it that way.

@jsloyer

This comment has been minimized.

Contributor

jsloyer commented Dec 16, 2016

I honestly like #2 better. I update the PR... In number #2 it just skips user.Current and looks at environment variables instead.

@seh

I understand you're intending to remove the dependency on the go-homedir package.

@@ -138,12 +138,13 @@ func New(args ...interface{}) *Session {
envFallback("SOFTLAYER_TIMEOUT", &values[keys["timeout"]])
// Read ~/.softlayer for configuration
u, err := user.Current()
dir, err := homedir.Dir()
fmt.Println(err)

This comment has been minimized.

@seh

seh Dec 16, 2016

  • Did you intend to leave that here?
    At this point, we don't know whether err is nil or not.
@seh

This comment has been minimized.

seh commented Dec 16, 2016

Don't replicate this mistake, though.

@jsloyer jsloyer force-pushed the jsloyer:user-current branch 4 times, most recently from eaa489f to acfd2c4 Dec 16, 2016

@jsloyer jsloyer force-pushed the jsloyer:user-current branch from acfd2c4 to af44563 Dec 16, 2016

@renier renier merged commit a2b747e into softlayer:master Dec 16, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jsloyer jsloyer deleted the jsloyer:user-current branch Dec 16, 2016

@renier

This comment has been minimized.

Contributor

renier commented Mar 3, 2018

@jsloyer Blast from the past, but.. It was never clear whether you had tried building with CGO_ENABLED=0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment