guyfedwards / nom

RSS reader for the terminal
GNU General Public License v3.0
372 stars 25 forks source link

Fix the Dockerfile's environment for CGO #69

Closed kanielrkirby closed 6 months ago

kanielrkirby commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

guyfedwards commented 6 months ago

Great, thanks @kanielrkirby