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

cmd/go: applying swig's %go_import directive to non-builtin packages is not supported #18343

Open
hzarnani opened this issue Dec 16, 2016 · 3 comments
Milestone

Comments

@hzarnani
Copy link

hzarnani commented Dec 16, 2016

One of swig's useful language-specific directives for the Go language is %go_import. It allows defining typemaps and wrappers (with %insert(go_wrapper)), among others, using symbols defined in a user-defined package found under $GOPATH/src. However, the go build system only allows %go_import of builtin packages. User-defined packages are not accessible in the temporary workspace which go creates to build a package that uses swig interface files.

The following, while not really interfacing with any C/C++ library, demonstrates this problem. It's using go1.7 on darwin/amd64. But this problem is seen on all platforms.

The first attempt at making and executing the main package at swig-sandbox/import relies on the go build system's support for swig. It fails. The second attempt is made after invoking swig manually (removing *.swigcxx to prevent the go build system to also try to invoke swig). It succeeds, obviously.

bash-3.2$ [ $GOPATH/src/swig-sandbox/import == $PWD ] && echo 'We are at swig-sandbox/import in Go sources directory'
We are at swig-sandbox/import in Go sources directory
bash-3.2$ ls -R
.:
main.i  main.swigcxx  mytime  printNextYear.go

./mytime:
consts.go
bash-3.2$ head *.{i,swigcxx,go} */*.go
==> main.i <==
%go_import("fmt", c "swig-sandbox/import/mytime")

%insert(go_wrapper) %{
  func myprint(a ...interface{}) {
    fmt.Print("[myprint] ")
    fmt.Println(a...)
  }
 func nextYear() uint {
   return c.Year + 1
 }

==> main.swigcxx <==
%include "main.i"

==> printNextYear.go <==
package main

func main() {
	myprint("Next year is:", nextYear())
}
==> mytime/consts.go <==
package consts

const Year uint = 2016
bash-3.2$ go build -o main && ./main
# swig-sandbox/import
/tmp/go-build726259823/swig-sandbox/import/_obj/main.go:40: can't find import: "swig-sandbox/import/mytime"
bash-3.2$ rm main.swigcxx 
bash-3.2$ swig -go -cgo -intgosize 64 -module main main.i
bash-3.2$ go build -o main && ./main
[myprint] Next year is: 2017
bash-3.2$

As can be seen, the workaround is to not use *.swigcxx files and instead do a two-step compilation -- explicitly invoke swig and then use go build. But it's desirable to have the go build system to call swig. Note also that %go_import of "fmt" was fine. It was the %go_import of "swig-sandbox/import/mytime", a user-defined package, that was problematic.

@hzarnani
Copy link
Author

hzarnani commented Dec 16, 2016

A more tangible example showing why %go_import is useful follows. (This is partly for clarification and partly in response to a comment received on issues #18339 where the utility of the use case was questioned.)

Consider two C++ libraries, one whose declarations are exported in the header foo.hpp and another in bar.hpp. Consider further that bar.hpp uses symbols from foo.hpp. In the following example, foo.hpp defines an enum called state and bar.hpp defines a predicate IsUp on that enum. It's desirable to wrap each of these libraries in a separate go package and be able to import them in another package and have the go bindings work in a similar way to how they would be used in a C/C++ program (see the imports in go/test/main.go and the call to b.IsUp).

The first version of the bar package simply wraps bar.hpp which results in the enum to become a type different than the same enum wrapped in the foo package (bar.State vs. foo.State), and as such, the program fails to compile in type-checking the call to b.IsUp.

The second version of the bar package maps the enum to the State type defined in the foo package, allowing the program to type-check correctly. However, this can only be done by explicitly invoking swig and not relying on the go build system's support for swig.

If one wants to avoid the two-step build, one has to wrap both foo.hpp and bar.hpp in a single package, say foo_and_bar, with a swig file that %includes both headers. But that's also undesirable because a package that only needs bindings of symbols from one header would have to import bindings from the other header too. Good programmers really need %go_import to achieve proper abstractions and modularity in their code, and therefore support for it in the go build system is highly desired.

bash-3.2$ BUGREPT=swig-sandbox/bug-reports/go-swig/go_import
bash-3.2$ cd $GOPATH/src/$BUGREPT
bash-3.2$ find -E cpp -type f -iregex '.*.(h|hpp|cpp)' -print0 | xargs -0 head
==> cpp/include/bar.hpp <==
#pragma once

#include "foo.hpp"

static inline bool isUp(enum state s) {
  return(s == UP);
}
==> cpp/include/foo.hpp <==
#pragma once

enum state { UP, DN };
bash-3.2$ find -L -E go -type f -iregex '.*.(go|swigcxx)' -print0 | xargs -0 head -n20
==> go/bar/bar.swigcxx <==
%{
#include "bar.hpp"
%}
%include "bar.hpp"

==> go/bar/package.go <==
package bar

// #cgo CXXFLAGS: -I${SRCDIR}/../../cpp/include
import "C"
==> go/foo/foo.swigcxx <==
%{
#include "foo.hpp"
%}
%include "foo.hpp"

==> go/foo/package.go <==
package foo

// #cgo CXXFLAGS: -I${SRCDIR}/../../cpp/include
import "C"
==> go/test/main.go <==
package main

import (
	"log"
	b "swig-sandbox/bug-reports/go-swig/go_import/go/bar"
	f "swig-sandbox/bug-reports/go-swig/go_import/go/foo"
)

func main() {
	if !b.IsUp(f.UP) {
		log.Fatal("f.UP is not up!")
	}
	log.Println("A-OK")
}
bash-3.2$ go install $BUGREPT/go/test
# swig-sandbox/bug-reports/go-swig/go_import/go/test
go/test/main.go:10: cannot use foo.UP (type foo.State) as type bar.State in argument to bar.IsUp
bash-3.2$ head go/bar/bar.swigcxx
%{
#include "bar.hpp"
%}
%go_import(f "swig-sandbox/bug-reports/go-swig/go_import/go/foo")
%typemap(gotype) enum state "f.State"
%include "bar.hpp"
bash-3.2$ go install $BUGREPT/go/test
# swig-sandbox/bug-reports/go-swig/go_import/go/bar
/var/folders/m6/8y6rbg4s2td16l0z7f1pm1_c0000gn/T/go-build810351302/swig-sandbox/bug-reports/go-swig/go_import/go/bar/_obj/bar.go:40: can't find import: "swig-sandbox/bug-reports/go-swig/go_import/go/foo"
bash-3.2$ cd go/bar
bash-3.2$ ls -l
total 24
-rw-r--r--  1 hormozzarnani  staff  148 Dec 15 21:27 bar.i
lrwxr-xr-x  1 hormozzarnani  staff    5 Dec 15 21:01 bar.swigcxx -> bar.i
-rw-r--r--  1 hormozzarnani  staff   72 Dec 15 20:14 package.go
bash-3.2$ rm bar.swigcxx 
bash-3.2$ swig -go -cgo -intgosize 64 -c++ -I../../cpp/include -module bar bar.i
bash-3.2$ ls -l
total 40
-rw-r--r--  1 hormozzarnani  staff  1534 Dec 15 21:29 bar.go
-rw-r--r--  1 hormozzarnani  staff   148 Dec 15 21:27 bar.i
-rw-r--r--  1 hormozzarnani  staff  6972 Dec 15 21:29 bar_wrap.cxx
-rw-r--r--  1 hormozzarnani  staff    72 Dec 15 20:14 package.go
bash-3.2$ tail bar.go
}

func IsUp(arg1 f.State) (_swig_ret bool) {
	var swig_r bool
	_swig_i_0 := arg1
	swig_r = (bool)(C._wrap_isUp_bar_7f645dbcfef3df15(C.swig_intgo(_swig_i_0)))
	return swig_r
}
bash-3.2$ go install $BUGREPT/go/test
bash-3.2$ $GOPATH/bin/test
2016/12/15 21:30:53 A-OK
bash-3.2$

@ianlancetaylor ianlancetaylor changed the title Applying swig's %go_import directive to non-builtin packages is not supported cmd/go: applying swig's %go_import directive to non-builtin packages is not supported Dec 16, 2016
@ianlancetaylor
Copy link
Contributor

I agree it would be nice to fix this problem.

Although I haven't tested it, I think that another workaround would be to run go install swig-sandbox/import/mytime before trying to build swig-sandbox/import. That is, the problem is not that the package can not be found. The problem is that the go tool does not see the dependency between the packages, because the go tool only looks at .go files to determine import dependencies.

Yet another workaround would be to modify some .go file in the directory to say

import _ "sandbox/import/mytime"

I'm not sure how to actually fix this problem, because a proper fix would require the go tool to parse the .swig file. It seems to me that fully correct parsing of a .swig file requires implementing the C preprocessor and requires parsing some C or C++ code to distinguish the SWIG directives from other code in the file. SWIG uses a yacc parser with the comment "This file is a bit of a mess and probably needs to be rewritten at some point. Beware." I don't really foresee the go tool incorporating its own version of the SWIG parser.

One plausible approach might be to add an option to SWIG to essentially dump the list of go_imports. Then the go tool could invoke SWIG with that option and use that to get the list of dependencies. I'm not sure whether or not that is a good idea, since it means that go get would behave differently on different systems based on whether SWIG was installed or not.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Dec 16, 2016
@hzarnani
Copy link
Author

hzarnani commented Dec 16, 2016

Putting import _ "user/def/pkg" in some go file is a good workaround. Thanks.

I'm sure there are reasons for go build working this way, but I think the go build system should first run swig to generate go files and then proceed to the normal build as if swig isn't present. That way, a package could simply contain an interface file and still build (but today the build fails with the "no buildable Go source file" error)

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

No branches or pull requests

2 participants