Skip to content
This repository was archived by the owner on Jan 26, 2020. It is now read-only.

add cgo support#3

Merged
mikedanese merged 1 commit intomasterfrom
cgo-support
Nov 23, 2016
Merged

add cgo support#3
mikedanese merged 1 commit intomasterfrom
cgo-support

Conversation

@mikedanese
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown
Collaborator

@ixdy ixdy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically LG. just a few questions.

Comment thread gazel.go Outdated
cgoDeps := v.extractDeps(pkg.TestImports)
cgoRule := newRule("cgo_library", cgoPname, map[string]bzl.Expr{
"srcs": cgoSrcs,
"clinkopts": asExpr([]string{"-ldl", "-lz", "-lm", "-lpthread", "-ldl"}),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is -ldl twice intentional?

where did you get this list from?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. I ripped it off of what I saw the go compiler adding by default.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol.

wonder what happens if we just leave this off entirely.

Comment thread gazel.go Outdated
})
rules = append(rules, cgoRule)
if len(cgoDeps.List) != 0 {
cgoRule.SetAttr("deps", cgoDeps)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess i didn't like the empty deps = [] blocks but I don't really care actually. Removed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :)

Comment thread gazel.go

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)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :\

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikedanese mikedanese merged commit 391f709 into master Nov 23, 2016
@mikedanese mikedanese deleted the cgo-support branch November 23, 2016 00:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants