Make the cgo tool do more linking work #159

Merged
carlosmn merged 1 commits from cgo-directives into master 2014-12-30 04:09:54 -06:00
carlosmn commented 2014-12-25 12:59:54 -06:00 (Migrated from github.com)

The cgo directives let us do a lot more than I previously thought, so we
can use this to make the building process of git2go go through the go
tool directly rather than via the script.

libgit2 still needs to be built manually, so we do still require make,
but only for building libgit2. Once that's build, any modifications to
git2go can be built with

go build -tags static

Leaving out the static tag will make git2go try to link against a
system-provided version of libgit2 via the normal pkg-config channels.

The cgo directives let us do a lot more than I previously thought, so we can use this to make the building process of git2go go through the go tool directly rather than via the script. libgit2 still needs to be built manually, so we do still require make, but only for building libgit2. Once that's build, any modifications to git2go can be built with ``` go build -tags static ``` Leaving out the static tag will make git2go try to link against a system-provided version of libgit2 via the normal pkg-config channels.
dmitshur commented 2014-12-31 18:25:34 -06:00 (Migrated from github.com)

It's a shame that didn't work out. Is there any workaround?

It's a shame that didn't work out. Is there any workaround?
slimsag commented 2015-01-03 02:17:50 -06:00 (Migrated from github.com)

I only had time to dig in a little bit here. What the problem actually was wasn't clear to me from the discussion that took place. From what I see (with this change) any package main program fails to properly link against libgit2.

This is because when building the binary the go tool pulls the linker flags from the package's (git2go's) source code. The primary problem here is that you can't link against a library using a relative filepath, which is what this PR does.

When building source in the git2go directory (go build), it links fine because the working directory makes the relative path a valid one, but, when building a main binary the working directory is not git2go's source directory.

Ultimately, there isn't anything (easy) you can do to work around this right now. However Go 1.5 (just barely missed 1.4) does feature a change (131758183f) allowing for CGO lines like this:

#cgo LDFLAGS: -L${SRCDIR}/vendor/libgit2/build -lgit2

Which when building the main binary (outside the package's directory) would have ${SRCDIR} expanded to the package's source directory (e.g. /home/stephen/godev/src/github.com/libgit2/git2go).

I think that would fix this issue.

@shurcooL the same issue doesn't apply with go-gl/glfw3 because we include the C sources directly and are not linking to a static library (.a file).

I only had time to dig in a little bit here. What the problem _actually was_ wasn't clear to me from the discussion that took place. From what I see (with this change) any `package main` program fails to properly link against `libgit2`. This is because when building the binary the go tool pulls the linker flags from the package's (git2go's) source code. The primary problem here is that you can't link against a library using a relative filepath, which is what this PR does. When building source in the `git2go` directory (`go build`), it links fine because the working directory makes the relative path a valid one, but, when building a main binary the working directory is not git2go's source directory. _Ultimately, there isn't anything (easy) you can do to work around this right now._ However Go 1.5 (just barely missed 1.4) does feature a change (https://github.com/golang/go/commit/131758183f7dc2610af489da3a7fcc4d30c6bc48) allowing for CGO lines like this: ``` #cgo LDFLAGS: -L${SRCDIR}/vendor/libgit2/build -lgit2 ``` Which when building the main binary (outside the package's directory) would have `${SRCDIR}` expanded to the package's source directory (e.g. `/home/stephen/godev/src/github.com/libgit2/git2go`). I think that would fix this issue. @shurcooL the same issue doesn't apply with go-gl/glfw3 because we include the C sources directly and are not linking to a static library (`.a` file).
dmitshur commented 2015-01-03 02:50:24 -06:00 (Migrated from github.com)

Thanks for looking into this thoroughly @slimsag.

Thanks for looking into this thoroughly @slimsag.
carlosmn commented 2015-01-04 06:28:37 -06:00 (Migrated from github.com)

I don't know that the addition of ${SRCDIR} is going to fix it. git2go already includes a statically-buit version of libgit2 in it. If a program which uses git2go is then also made to include the statically-built version of libgit2, it's not clear that it won't be included twice, which would still fail to compile.

git2go is the only thing that depends on libgit2 being included in it, and it's the only one which should. This is why we now have to have the script with the env variables. Anything depending on libgit2 should not be getting any of the flags we used to include libgit2 in git2go.

The only clear way that this is going to work is dynamic linking, but that won't work for following the master branch. When v0.22 releases, I'll create a branch for git2go so you can use it with a libgit2 on the system via gopkg.in or something similar.

I don't know that the addition of `${SRCDIR}` is going to fix it. git2go already includes a statically-buit version of libgit2 in it. If a program which uses git2go is then also made to include the statically-built version of libgit2, it's not clear that it won't be included twice, which would still fail to compile. git2go is the only thing that depends on libgit2 being included in it, and it's the only one which should. This is why we now have to have the script with the env variables. Anything depending on libgit2 should not be getting any of the flags we used to include libgit2 in git2go. The only clear way that this is going to work is dynamic linking, but that won't work for following the master branch. When v0.22 releases, I'll create a branch for git2go so you can use it with a libgit2 on the system via gopkg.in or something similar.
slimsag commented 2015-01-04 07:30:34 -06:00 (Migrated from github.com)

Just to be clear, the issue we are talking about is when running go test -c in github.com/carlosmn/go.gitfs, correct?

# testmain
/var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_file':
/home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:50: undefined reference to `git_blame_file'
/var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_get_hunk_byindex':
/home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:74: undefined reference to `git_blame_get_hunk_byindex'
/var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_get_hunk_byline':
/home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:89: undefined reference to `git_blame_get_hunk_byline'
/var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_get_hunk_count':
/home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:103: undefined reference to `git_blame_get_hunk_count'

...

If you build git2go using go build -x in the package directory, you'll get a log of all commands that go build executes while running. You will find that Go's build process does not repack libgit2.a into the final static library that is your package git2go.a, seen by the final pack command in the log:

pack r $WORK/github.com/libgit2/git2go.a $WORK/github.com/libgit2/git2go/_obj/_cgo_import.6 $WORK/github.com/libgit2/git2go/_obj/_cgo_defun.6 $WORK/github.com/libgit2/git2go/_obj/_all.o # internal

git2go already includes a statically-buit version of libgit2 in it.

Go package's aren't built as libraries in that sense. libgit2.a is not combined with your git2go package, but rather libgit2.a is linked when a package main program is built.

If a program which uses git2go is then also made to include the statically-built version of libgit2, it's not clear that it won't be included twice, which would still fail to compile.

That is not what the ${SRCDIR} variable expansion would do: it would simply allow the Go linker (which runs when a main program binary -- not package/library -- is built) to find the libgit2.a static library and link against it properly.

It also helps to examine the output of go test -c -ldflags="-v" which shows the final host linker command used to build the resulting binary:

host link: gcc -m64 -gdwarf-2 -o $WORK/github.com/carlosmn/go.gitfs/_test/go.gitfs.test -rdynamic /var/tmp/go-link-OiUJ43/000000.o /var/tmp/go-link-OiUJ43/000001.o /var/tmp/go-link-OiUJ43/000002.o /var/tmp/go-link-OiUJ43/go.o -g -O2 -lgit2 -Lvendor/libgit2/build -lrt -lgit2 -lssl -lcrypto -ldl -lz -g -O2 -lpthread -g -O2

When the linker runs inside cd $WORK/github.com/carlosmn/go.gitfs/_test (see -v flag) -- those relative paths are no longer valid.

If you replace your #cgo LDFLAGS line with, for example on my machine what is an absolute path to your lib directory:

#cgo LDFLAGS: -lgit2 -L/home/stephen/Desktop/godev/src/github.com/libgit2/git2go/vendor/libgit2/build

go test -c builds without error and the resulting binary is statically linked.

Obviously, using an absolute path isn't viable because it's specific to my system, this is the part that ${SRCDIR} fixes.

Hope I've been of some help here,
Cheers

Just to be clear, the issue we are talking about is when running `go test -c` in `github.com/carlosmn/go.gitfs`, correct? ``` # testmain /var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_file': /home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:50: undefined reference to `git_blame_file' /var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_get_hunk_byindex': /home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:74: undefined reference to `git_blame_get_hunk_byindex' /var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_get_hunk_byline': /home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:89: undefined reference to `git_blame_get_hunk_byline' /var/tmp/go-link-YtWosH/000000.o: In function `_cgo_f8b7b3879ba7_Cfunc_git_blame_get_hunk_count': /home/stephen/Desktop/godev/src/github.com/libgit2/git2go/blame.go:103: undefined reference to `git_blame_get_hunk_count' ... ``` If you build git2go using `go build -x` in the package directory, you'll get a log of all commands that go build executes while running. You will find that Go's build process does not repack `libgit2.a` into the final static library that is your package `git2go.a`, seen by the final `pack` command in the log: ``` pack r $WORK/github.com/libgit2/git2go.a $WORK/github.com/libgit2/git2go/_obj/_cgo_import.6 $WORK/github.com/libgit2/git2go/_obj/_cgo_defun.6 $WORK/github.com/libgit2/git2go/_obj/_all.o # internal ``` > git2go already includes a statically-buit version of libgit2 in it. Go package's aren't built as _libraries_ in that sense. `libgit2.a` is not combined with your `git2go` package, but rather `libgit2.a` is linked when a `package main` program is built. > If a program which uses git2go is then also made to include the statically-built version of libgit2, it's not clear that it won't be included twice, which would still fail to compile. That is not what the `${SRCDIR}` variable expansion would do: it would simply allow the Go linker (which runs when a _main program binary_ -- not package/library -- is built) to find the `libgit2.a` static library and link against it properly. It also helps to examine the output of `go test -c -ldflags="-v"` which shows the final host linker command used to build the resulting binary: ``` host link: gcc -m64 -gdwarf-2 -o $WORK/github.com/carlosmn/go.gitfs/_test/go.gitfs.test -rdynamic /var/tmp/go-link-OiUJ43/000000.o /var/tmp/go-link-OiUJ43/000001.o /var/tmp/go-link-OiUJ43/000002.o /var/tmp/go-link-OiUJ43/go.o -g -O2 -lgit2 -Lvendor/libgit2/build -lrt -lgit2 -lssl -lcrypto -ldl -lz -g -O2 -lpthread -g -O2 ``` When the linker runs inside `cd $WORK/github.com/carlosmn/go.gitfs/_test` (see `-v` flag) -- those relative paths are no longer valid. If you replace your `#cgo LDFLAGS` line with, for example on my machine what is an absolute path to your lib directory: ``` #cgo LDFLAGS: -lgit2 -L/home/stephen/Desktop/godev/src/github.com/libgit2/git2go/vendor/libgit2/build ``` `go test -c` builds without error and the resulting binary is statically linked. Obviously, using an absolute path isn't viable because it's specific to my system, this is the part that `${SRCDIR}` fixes. Hope I've been of some help here, Cheers
carlosmn commented 2015-01-04 10:07:52 -06:00 (Migrated from github.com)

We can have absolute paths right now by passing an absolute path to CMake and letting it do the install phase. But that's just a part of it.

When git2go gets installed, we know the libgit2.a version inside git2go agrees with the usage. This then lands in pkg/.../git2go.a which you can then use. During the build phase of whatever program uses git2go, there is no guarantee that the libgit2.a in the source portion of git2go has anything to do with what git2go was built with, and it doesn't have to because that's not the archive in the compiled portion of the tree.

We can have absolute paths right now by passing an absolute path to CMake and letting it do the install phase. But that's just a part of it. When git2go gets installed, we know the libgit2.a version inside git2go agrees with the usage. This then lands in `pkg/.../git2go.a` which you can then use. During the build phase of whatever program uses git2go, there is no guarantee that the libgit2.a in the source portion of git2go has anything to do with what git2go was built with, and it doesn't have to because that's not the archive in the compiled portion of the tree.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: jcarr/git2go#159
No description provided.