Skip to content

Add build tags to support both x86 and ARM compilation on macOS#1069

Merged
rittneje merged 5 commits intomattn:masterfrom
Spaider:x86-arm-compilation-macOS
Feb 23, 2023
Merged

Add build tags to support both x86 and ARM compilation on macOS#1069
rittneje merged 5 commits intomattn:masterfrom
Spaider:x86-arm-compilation-macOS

Conversation

@Spaider
Copy link
Copy Markdown
Contributor

@Spaider Spaider commented Jul 28, 2022

Fix for #1068

Changes

  • Introduce arm64 and amd64 build tags
  • Appropriate changes in documentation

README.md Outdated
To compile for macOS X on x86:

```bash
go build --tags "darwin,amd64"
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.

These instructions were actually totally wrong. It should be using GOOS and GOARCH for this. The same applies for Windows and Linux. (Also the flag is -tags not --tags.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm not following where's my fault here. I've added architecture flags exactly in a way it was for OS selection: by explicitly define target OS/arch. I'm a roocky in cgo and not sure how to re-write this to make it refer to GOOS and GOARCH env vars: #cgo flag does not have if-then-else notion.

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.

The original instructions (on master) were incorrect.

If you are doing a native compile, it is just go build.
If you are doing a cross compile, it is GOOS=<target-os> GOARCH=<target-arch> CGO_ENABLED=1 CC=<c-cross-compiler> go build
For example: GOOS=darwin GOARCH=amd64 CGO_ENABLED=1 CC=o64-clang go build

Separately from that, if you are trying to include optional features via build tags, in modern versions of Go you provide a comma-separated list via -tags.

-tags tag,list
a comma-separated list of build tags to consider satisfied during the
build. For more information about build tags, see the description of
build constraints in the documentation for the go/build package.
(Earlier versions of Go used a space-separated list, and that form
is deprecated but still recognized.)

For example: go build -tags sqlite_stat4,secure_delete

Note that all of this is separate from the #cgo directive changes you made. (Those changes are correct.)

Copy link
Copy Markdown
Contributor Author

@Spaider Spaider Jul 31, 2022

Choose a reason for hiding this comment

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

Thanks for an explanation, now it makes a perfect sense. Looks like the problem of both native and cross-compilcation can be resolved only by external tool. For example, shell script which will set CGO_CFLAGS and CGO_LDFLAGS based on target GOOS and GOARCH. But this is beyond the scope of my PR.

@rittneje
Copy link
Copy Markdown
Collaborator

@mattn Do you know if GitHub has any M1/M2 build agents we could add to the workflow? https://github.com/mattn/go-sqlite3/blob/master/.github/workflows/go.yaml

@mattn
Copy link
Copy Markdown
Owner

mattn commented Jul 31, 2022

No I don't.

@Spaider
Copy link
Copy Markdown
Contributor Author

Spaider commented Jul 31, 2022

@mattn Do you know if GitHub has any M1/M2 build agents

Issue is still open. Looks like it's in plans but not there yet.

#cgo darwin,amd64 CFLAGS: -I/usr/local/opt/icu4c/include
#cgo darwin,amd64 LDFLAGS: -L/usr/local/opt/icu4c/lib
#cgo darwin,arm64 CFLAGS: -I/opt/homebrew/opt/icu4c/include
#cgo darwin,arm64 LDFLAGS: -L/opt/homebrew/opt/icu4c/lib
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm wondering why this is different for them.

-I/usr/local/opt/icu4c/include -I/opt/homebrew/opt/icu4c/include
-L/usr/local/opt/icu4c/lib -L/opt/homebrew/opt/icu4c/lib

This is not good?

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.

@mattn Not sure I understand your question, but Homebrew installs to a different location on arm64 (i.e, M1/M2) than on amd64 (i.e., Intel), as mentioned in the docs. https://docs.brew.sh/Installation

We could also choose to include both directories for macOS regardless of GOARCH, and I suppose that would also work. (Although I think we should put /opt/homebrew first in that case.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I just hit this issue on a M1 Mac and these locations are correct. This PR along with the build tags go build -tags "libsqlite3 darwin arm64" allowed me to build against the sqlite version installed via homebrew.

@zzet
Copy link
Copy Markdown

zzet commented Oct 20, 2022

Hey folks! I'm wondering if any progress there?

@Spaider
Copy link
Copy Markdown
Contributor Author

Spaider commented Oct 24, 2022

I've resolved the conflict in README.md.

@ARolek
Copy link
Copy Markdown

ARolek commented Feb 22, 2023

Just checking in. It looks like this PR was approved, but never merged. Is there anything blocking the merge at this time?

@rittneje rittneje merged commit 85a15a7 into mattn:master Feb 23, 2023
meiyang1123 pushed a commit to meiyang1123/go-sqlite3 that referenced this pull request Aug 28, 2024
…n#1069)

* Add build tags to support both x86 and ARM compilation on macOS

* Documentation fix: command line for build under macOS

* Global replace 'macOS X' -> 'macOS'. Fix typo in -tags cmd line param

* `README.md`: fix all `--tags` -> `-tags`

---------

Co-authored-by: Denis Dmitriev <dmitriev@itspartner.net>
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.

5 participants