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

Fix the Dockerfile's environment for CGO #69

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Fix the Dockerfile's environment for CGO #69

merged 1 commit into from
Apr 29, 2024

Conversation

kanielrkirby
Copy link
Contributor

@kanielrkirby kanielrkirby commented Apr 25, 2024

I'm not sure if there were things that used to be done implicitly, or what exactly has prompted this issue, but I've found that a fresh build and run of the binary with Docker results in an environment that doesn't have gcc or alpine-sdk installed, and requires a couple build flags passed in, namely CGO_ENABLED=1 and CGO_CFLAGS="-D_LARGEFILE64_SOURCE". The last flag can be ignored if we update go-sqlite3, which seems like the right move.

All in all, I can say for sure that adding those packages and flags into the Dockerfile seems to solve all related issues completely. If you have any insights, please do let me know.

A slight alternative would be to continue with the update to sqlite3, but pass in CGO_ENABLED into the Makefile's build section, but this seems more well-scoped to the issue at hand. Let me know if you'd like me to change that though.

Decided to open a PR as a form of discussion and potential solutions, rather than just an issue.

Changes

go get -u github.com/mattn/go-sqlite3

Dockerfile:

FROM golang:alpine
WORKDIR /app
-RUN apk add make
+RUN apk add make alpine-sdk gcc
COPY . .
+ENV CGO_ENABLED=1
RUN make build
CMD ["/app/nom", "--config-path", "docker-config.yml"]

@guyfedwards
Copy link
Owner

Seems like this wasn't updated when the switch to sqlite happened, I don't use nom through docker so hadn't noticed, thanks for the fix.

Can you remove config.yml and nom.db from the changes, then I'll merge.

@kanielrkirby
Copy link
Contributor Author

Absolutely, sorry about that. I'm horrible with seeing those changes, I really just need to run difftool to be sure of what I'm actually commiting lol.

@kanielrkirby
Copy link
Contributor Author

I think (?) I fixed it. I don't know why that happened, I remember explicitly not adding those.

@guyfedwards guyfedwards merged commit 1d5afa3 into guyfedwards:master Apr 29, 2024
2 checks passed
@guyfedwards
Copy link
Owner

Great, thanks @kanielrkirby

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

Successfully merging this pull request may close these issues.

2 participants