add cgo support#3
Conversation
ixdy
left a comment
There was a problem hiding this comment.
basically LG. just a few questions.
| cgoDeps := v.extractDeps(pkg.TestImports) | ||
| cgoRule := newRule("cgo_library", cgoPname, map[string]bzl.Expr{ | ||
| "srcs": cgoSrcs, | ||
| "clinkopts": asExpr([]string{"-ldl", "-lz", "-lm", "-lpthread", "-ldl"}), |
There was a problem hiding this comment.
is -ldl twice intentional?
where did you get this list from?
There was a problem hiding this comment.
no. I ripped it off of what I saw the go compiler adding by default.
There was a problem hiding this comment.
lol.
wonder what happens if we just leave this off entirely.
| }) | ||
| rules = append(rules, cgoRule) | ||
| if len(cgoDeps.List) != 0 { | ||
| cgoRule.SetAttr("deps", cgoDeps) |
There was a problem hiding this comment.
minor nit: it's not clear why you did this after adding cgoRule to rules, rather than before. I know it makes no difference either way.
There was a problem hiding this comment.
I guess i didn't like the empty deps = [] blocks but I don't really care actually. Removed.
There was a problem hiding this comment.
well, I meant move the if block before rules = append(rules, cgoRule), just so the flow was "create rule, then append", but this is fine too.
as I said, super minor nit. :)
|
|
||
| var attrs Attrs = make(Attrs) | ||
| srcs := asExpr(merge(pkg.GoFiles, pkg.SFiles)).(*bzl.ListExpr) | ||
| cgoSrcs := asExpr(merge(pkg.CgoFiles, pkg.CFiles, pkg.CXXFiles, pkg.HFiles)).(*bzl.ListExpr) |
There was a problem hiding this comment.
are we supposed to include header files too in the srcs rule?
cc_library keeps them separate, but there's no such option for cgo_library. :\
There was a problem hiding this comment.
For whatever reason, I think this is how they implemented it. https://github.com/bazelbuild/rules_go#cgo_library
9dccd4f to
9fa4534
Compare
No description provided.