-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/go: can't include C files when building package from named list of files #40570
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
Comments
go build -buildmode=c-archive
go build -buildmode=c-archive
As far as I know, the go tool doesn't currently provide a way to do what you want. Of course, you can do something like this:
|
Usually, this would be solved with build constraints. You could add a line to test2.go like this:
Then build the package with:
Does that work for you? |
@jayconrod for the example above that would technically work, however in my real-world example I would need to be able to tag C files as well since there are a few I would need to exclude along with the Go files. Would adding a similar build-tag comment to the top of a C file and then calling |
Yes. You can run |
So it seems like that solves my problem then. However, the ability to add named C files to the |
go build -buildmode=c-archive
Glad to hear that works for you. I'll leave the issue open. Updated the title to clarify this is not specific to |
go/src/cmd/go/internal/load/pkg.go Lines 2284 to 2301 in f714449
It seems to remove this check will solve the problem. And the buildContext will check the file valid. The function also use by |
Does this patch seem reasonable? Or perhaps it would make more sense if this is only done for specific build types (c-shared or c-archive) https://github.com/nathan-fiscaletti/go/blob/master/src/cmd/go/internal/load/pkg.go#L2284-L2315 The reasoning behind the chosen file extensions is taken from this paragraph of the cgo documentation:
|
After trying to build the source I modified, I found that there is a test specifically for this. Related commit: a16dcc0 |
I've modified the patch, should I just submit this as a PR? |
Change https://golang.org/cl/248685 mentions this issue: |
After thinking about this a bit more and reading through https://golang.org/cl/248685, I don't think we should make this change. There's potential for several bugs and breaking changes to be introduced, and I don't think that changing behavior delivers enough value to be worth the extra complication.
@nathan-fiscaletti Could you explain why this is needed? From the comments above, it sounded like you could build your package with a regular package path rather than a list of files. |
While the method you gave above works perfectly, I feel it's not very intuitive. When I first read about the build constraints, I had incorrectly assumed that they were restricted to Go files and would not work with other files (such as C files) as it didn't explicitly state otherwise. Additionally, I think there is some inconsistency that's brought about due to the fact that calling Overall, I think there's a lack of clarity in the documentation around why this restriction exists and, at least for me, wound up confusing me more than anything. So, while I don't necessarily think that the feature needs to be added to the toolchain, I do think that some clarification in the relevant documentation would go a long way. |
Hmm, I agree the documentation on Build constraints should be clarified. It doesn't explicitly mention any kind of file, so it's easy to assume they only work in .go files.
There's no technical reason this isn't possible. We're mainly optimizing the command line interface for regular packages, and I think allowing other kinds of files would lead to more confusing edge cases for the reasons outlined above. Packages specified as individual files can't be imported, so they tend to be small ad hoc programs and examples: a less common use case, especially when cgo is mixed in. |
Perhaps my viewpoint is biased as I almost exclusively work with CGo. Is there a way to submit a PR that updates the related documentation? I'm still fairly new to contributing to Go, but I'd be happy to update the relevant documentation. |
Sure, if you're interested in updating this, the page I linked is in src/cmd/go/internal/help/helpdoc.go. Make sure to run mkalldocs.sh in src/cmd/go to ensure the package documentation is in sync with the |
I guess I was incorrect. The first line of the Build Constraints documentation states:
That being the case, I updated the |
Change https://golang.org/cl/249657 mentions this issue: |
Timed out in state WaitingForInfo. Closing. (I am just a bot, though. Please speak up if this is a mistake or you have the requested information.) |
What version of Go are you using (
go version
)?What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I seem to have an issue with Go where I'm unable to compile a C library that depends on C files if i want to only include a smaller set of the files.
I've put together this example to hopefully illustrate my issue.
test.c
test.h
test.go
test2.go
When I run the following, it compiles just fine:
However, when I try to build a smaller version that only includes one of the go files, it complains.
If I don't include the
test.c
file, I getmissing symbols
.How can I make smaller builds of my library while, apparently, being unable to include the C files that are required for it to function?
The text was updated successfully, but these errors were encountered: