Closed vegarsti closed 2 years ago
Thank you for submitting this PR.
It looks reasonable to me.
It would help if you copy some of the information you provided in the PR message into actual commit message. The PR message will get lost when I commit your change. But commit message will stay in git history forever.
I asked @agusterodin to review your change. But if he does not reply, please ping me and I will submit your change.
Thank you.
Alex
How do I easily go about testing this on my machine?
Sorry, I haven't looked at this issue in a long time since I originally got my M1 Mac about 6 months ago.
@alexbrainman I will amend the commit with some more information in the commit message!
@agusterodin You can clone this repo I made and try to run it (go run main.go
). It simply tries to compile with the odbc
library. If you still get a compile error, you can also check if you have anything at /usr/local/opt/unixodbc
, and if not, make a link, like described in the PR messge above.
Will do by end of day
Jeff Rosen
On Sun, Dec 19, 2021, 1:20 PM Vegard Stikbakke @.***> wrote:
@agusterodin https://github.com/agusterodin You can clone this repo I made https://github.com/vegarsti/odbc-repro and try to run it (go run main.go). It simply tries to compile with the odbc library. If you still get a compile error, you can also check if you have anything at /usr/local/opt/unixodbc, and if not, make a link, like described in the PR messge above.
— Reply to this email directly, view it on GitHub https://github.com/alexbrainman/odbc/pull/168#issuecomment-997437501, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOGBS2OJXMZLOSISVOQRADURYO67ANCNFSM5KFWKFYA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID: @.***>
Have added most of the information to the commit message: https://github.com/alexbrainman/odbc/pull/168/commits/44c161daba0000b61b167202358dd84922d8b931
Tried it and it worked!
Have added most of the information to the commit message: 44c161d
Thank you @vegarsti.
Also, please, add this line
Fixes #153
to the end of the same commit message (now that @agusterodin confirmed that this change fixes his issue), and I will submit. We want future code readers be able to find reason why we made this change.
Thank you.
Alex
Or should it be on the first line of the message?
Done! @alexbrainman 9c9a2e6
Thank you.
Or should it be on the first line of the message?
No. You did good.
LGTM.
Alex
You did good.
Haha, great, thanks!
Issue #153 describes the problem: On ARM macs, Homebrew stores binaries in a different place from Intel macs (see docs). cgo is thus unable to find the unixodbc files. If a user with an ARM Mac has created a symbolic link from the standard Homebrew path to where it's installed, this code makes it work on both ARM Macs and Intel Macs. I'm not sure why, but even if the link is created, it doesn't work without this change. My colleague @hawkaa who has an Intel mac can testify; we used this reproduction repo to verify it on each of our machines. (Just doing
go run main.go
in there.)I would be very happy if this was approved @alexbrainman!
To create the symbolic link from the ARM Homebrew path to the Intel Homebrew path, do:
This assumes that
/opt/homebrew/opt/unixodbc
points to the correct version of unixodbc (in my caseunixodbc -> ../Cellar/unixodbc/2.3.9_1
), which Homebrew should have taken care of.