fix(go/adbc/driver): inject version to built Go drivers #2916
Conversation
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
@davidhcoe Can you check this out and test it with your workflows to ensure it works outside of my testing?
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
What happens if you build outside of CMake and outside of Git/the Makefile? (I don't think we ever do this...)
yes, I will try it out tomorrow. |
|
The version comes back as Is the |
|
Oy, looks like this is gonna be more difficult than I thought because of the tag aliases. Quick question: did you build with cmake? Or the makefile in the pkg directory? |
|
I run make <name>.dll in the pkg directory.
…________________________________
From: Matt Topol ***@***.***>
Sent: Tuesday, June 3, 2025 2:23:05 PM
To: apache/arrow-adbc ***@***.***>
Cc: David Coe ***@***.***>; Mention ***@***.***>
Subject: Re: [apache/arrow-adbc] fix(go/adbc/driver): inject version to built Go drivers (PR #2916)
[https://avatars.githubusercontent.com/u/555095?s=20&v=4]zeroshade left a comment (apache/arrow-adbc#2916)<#2916 (comment)>
Oy, looks like this is gonna be more difficult than I thought because of the tag aliases.
Quick question: did you build with cmake? Or the makefile in the pkg directory?
—
Reply to this email directly, view it on GitHub<#2916 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADFTVNM765FHKDW3GLATBTD3BXRYTAVCNFSM6AAAAAB6OATSOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMZWGYZDANZVG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@davidhcoe Can you try the updated version I just pushed?
Yea, it indicates that when it was built Go's build info had |
It shows me the version is |
|
@davidhcoe are you working on a fork or on a clone of the main repo? Can you make sure you pull all the tags down locally? |
|
Maybe I am missing something here. I deleted then re-cloned the repo. I ran |
|
That's REALLY weird. For the heck of it, can you try building with cmake and let me know if it picks up the version correctly? |
zeroshade
force-pushed
the
static-version-handling
branch
from
c5bb683 to
6b25f11
Compare
|
I get |
|
from building with the |
|
Yes, that is how I am building it and yes, that works for me.
…________________________________
From: Matt Topol ***@***.***>
Sent: Wednesday, June 18, 2025 4:37:59 PM
To: apache/arrow-adbc ***@***.***>
Cc: David Coe ***@***.***>; Mention ***@***.***>
Subject: Re: [apache/arrow-adbc] fix(go/adbc/driver): inject version to built Go drivers (PR #2916)
[https://avatars.githubusercontent.com/u/555095?s=20&v=4]zeroshade left a comment (apache/arrow-adbc#2916)<#2916 (comment)>
from building with the Makefile in pkg? That makes sense given that it's pulling the version from the git tags. Is that sufficient for you? Does it need to be different?
—
Reply to this email directly, view it on GitHub<#2916 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADFTVNM55YMXGPMZEAK7YAT3EHE2PAVCNFSM6AAAAAB6OATSOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSOBVGYZDANJSHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I gave it a test and the drivers built by CI report version The C++ drivers need to have the same thing done ideally but that can be done separately |
|
Ah, it's because the CMake version is getting injected in. I'm guessing David Coe is building off of a fork of v1.6.0 or has local changes - then it would combine the |
|
Pushed a small fix to the one failing test. Then I'll merge. |
|
I filed #3000 for PostgreSQL/SQLite to also have this fix |
|
I was doing make <x>.dll from Matt’s branch.
…________________________________
From: David Li ***@***.***>
Sent: Wednesday, June 18, 2025 11:49:20 PM
To: apache/arrow-adbc ***@***.***>
Cc: David Coe ***@***.***>; Mention ***@***.***>
Subject: Re: [apache/arrow-adbc] fix(go/adbc/driver): inject version to built Go drivers (PR #2916)
[https://avatars.githubusercontent.com/u/327919?s=20&v=4]lidavidm left a comment (apache/arrow-adbc#2916)<#2916 (comment)>
Ah, it's because the CMake version is getting injected in. I'm guessing David Coe is building off of a fork of v1.6.0 or has local changes - then it would combine the v1.6.0 in CMakeLists.txt with -dev since the worktree is apparently dirty.
—
Reply to this email directly, view it on GitHub<#2916 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADFTVNIHKXUTVTK737FP4FL3EI6NBAVCNFSM6AAAAAB6OATSOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSOBWGU4TIMJUGU>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Ah ok. But 1.6.0 can come from the tags. I was just slightly confused at first why 1.7.0 (which isn't yet tagged) came in and had to review the CMakeLists config |
3 participants
expanding the solution from #2889 We can make sure that we inject the proper version string into the internal/driverbase handling. This should populate the GetInfo correctly with the driver version via vcs information.