-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Support QNX 7.1 build #1758
Support QNX 7.1 build #1758
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
06a47f2
to
c1b6427
Compare
@@ -20,9 +20,13 @@ | |||
|
|||
#if defined(HAS_STRPTIME) && HAS_STRPTIME | |||
#if !defined(_XOPEN_SOURCE) && !defined(__FreeBSD__) && !defined(__OpenBSD__) | |||
#if defined(__QNX__) | |||
#define _XOPEN_SOURCE 600 // Exposes definitions for QNX. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'd need to fix this in the upstream cctz project first. Then it gets copied to absl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. I wasn't aware of the structure (this is from another repo?), that does make the comments in your PR make more sense then. K, thanks, I'll post this there now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I pushed it here: google/cctz#301
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the issue about:
error: pthread_cond_timedwait was not declared in this scope
- pthread_cond_timedwait is defined, but scouring through
platform.h
requiresXOPEN_SOURCE>=600
. This is similar to the fix on [Bug]: compilation of time_zone_format.cc broken under QNX #1490
This simply required the definition of _QNX_SOURCE
which is suppose to be implicit when gnu extensions are enabled (docs) but evidently is not.
This allowed me to close google/cctz#301 though which is nice.
44e7e16
to
2adc40d
Compare
2adc40d
to
aa322de
Compare
Can you share the compiler, compiler version, and full context of the compiler error message? |
Sure, tag
I do get this warning in CMake:
Using |
I don't know how to test this easily, but I believe this fix is simply changing abseil-cpp/absl/flags/internal/flag.h Line 533 in 4447c75
to
|
I gave that a try: ❯ gd c0b9bd08e0 diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h
index a0be31d9..d91eb426 100644
--- a/absl/flags/internal/flag.h
+++ b/absl/flags/internal/flag.h
@@ -530,7 +530,7 @@ struct FlagValue<T, FlagValueStorageKind::kHeapAllocated>
constexpr FlagValue() : FlagMaskedPointerValue(&buffer[0]) {}
bool Get(const SequenceLock&, T& dst) const {
- MaskedPointer ptr_value = value.load(std::memory_order_acquire);
+ MaskedPointer ptr_value(value.load(std::memory_order_acquire));
if (ABSL_PREDICT_TRUE(ptr_value.AllowsUnprotectedRead())) {
::new (static_cast<void*>(&dst)) T(*static_cast<T*>(ptr_value.Ptr())); Got a very similar error. Working with gcc 8.3 isn't fun. :)
|
I ended up making a change similar to your suggestion to add a constructor. Please let us know if it did not work. |
@derekmauro It builds! Thanks |
Support for QNX 7.1. I understand this is not a supported platform
On QNX 7.1 ntoaarch64le, I hit the two build issue:
error: no matching function for call to 'absl::lts_20240722::flags_internal::MaskedPointer::MaskedPointer()'
I've built code using this, in particular the
flat_has_map
, it seems to work really well. Hoping for feedback to get the PR up to merging quality.