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: darwin linking undefined reference #4337

Merged

Conversation

pdelewski
Copy link
Contributor

Linking fails on darwin due to undefined references

ld: Undefined symbols:
  _CFRelease, referenced from:
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
      _$LT$iana_time_zone..platform..system_time_zone..SystemTimeZone$u20$as$u20$core..ops..drop..Drop$GT$::drop::hd8c2e5ebcdd4805e in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
  _CFStringGetBytes, referenced from:
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
  _CFStringGetCStringPtr, referenced from:
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
  _CFStringGetLength, referenced from:
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
  _CFTimeZoneCopySystem, referenced from:
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
  _CFTimeZoneGetName, referenced from:
      iana_time_zone::platform::get_timezone_inner::h7a8f0a0ab162d89a in libprqlc_c.a[106](iana_time_zone-ee5bd39b68211751.iana_time_zone.9869ef72d5904e9b-cgu.0.rcgu.o)
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [build] Error 1

Copy link
Member

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

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

Thanks, some minor comments.

prqlc/bindings/prqlc-c/examples/minimal-cpp/Makefile Outdated Show resolved Hide resolved
prqlc/bindings/prqlc-c/examples/minimal-cpp/Makefile Outdated Show resolved Hide resolved
prqlc/bindings/prqlc-c/examples/minimal-c/Makefile Outdated Show resolved Hide resolved
prqlc/bindings/prqlc-c/examples/minimal-c/Makefile Outdated Show resolved Hide resolved
@max-sixty
Copy link
Member

Thanks @pdelewski !

(tbh I don't know this well, I will merge given @eitsupi has reviewed; @eitsupi hope that's OK)

@max-sixty max-sixty merged commit b07f755 into PRQL:main Mar 17, 2024
38 checks passed
@eitsupi
Copy link
Member

eitsupi commented Mar 17, 2024

I was just pointing out a simple formatting issue, not sure about the validity of any other changes.

In particular, the fact that the GNU Make extension was brought in here could be a very significant change, I think.

@max-sixty
Copy link
Member

OK. I don't think the original version was written with much confidence. So I generally think it's reasonable to merge and folks can raise objections if something becomes worse...

(but open-minded ofc)

@pdelewski
Copy link
Contributor Author

@eitsupi What GNU Make extension do you mind?

@eitsupi
Copy link
Member

eitsupi commented Mar 18, 2024

I personally don't use this file so it doesn't bother me, but I thought the person who created it might have. If not, I think it's fine.

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.

3 participants