Fix CMake build - missing hashtablez_sampler build #238
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Newly added file `hashtablez_sampler.cc` is not being built by the cmake build and hence linking fails.
derekmauro
reviewed
Dec 22, 2018
| @@ -22,6 +22,7 @@ absl_cc_library( | |||
| container | |||
| SRCS | |||
| "internal/raw_hash_set.cc" | |||
| "internal/hashtablez_sampler.cc" | |||
There was a problem hiding this comment.
See the comment a few lines up on lines 17-19. This target is deprecated and doesn't do anything anyway. I'd rather just remove this target.
|
Yeah, I saw that but it's still the recommended way per the documentation
and I don't think one can expect people to read every single cmake file.
Personally, I also prefer having a single target but that's just me so I
would understand removing it but then the docs should be updated first.
I can do that as well though, if you want.
…On Sat, 22 Dec 2018, 20:28 Derek Mauro ***@***.*** wrote:
***@***.**** commented on this pull request.
------------------------------
In absl/container/CMakeLists.txt
<#238 (comment)>:
> @@ -22,6 +22,7 @@ absl_cc_library(
container
SRCS
"internal/raw_hash_set.cc"
+ "internal/hashtablez_sampler.cc"
See the comment a few lines up on lines 17-19. This target is deprecated
and doesn't do anything anyway. I'd rather just remove this target.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#238 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACxSkzANnrqgGjMx9qUgWX7AFQs9tGfBks5u7ofKgaJpZM4ZfLTM>
.
|
|
Which documentation are you referring to? |
|
… On Sat, 22 Dec 2018, 21:41 Derek Mauro ***@***.*** wrote:
Which documentation are you referring to?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#238 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACxSkwQm8V8QR9d_UCB6y19ZTlFiu5p-ks5u7pkLgaJpZM4ZfLTM>
.
|
|
That one is also missing `absl::hash` I think.
On Sat, 22 Dec 2018, 21:44 Stephan Dollberg <stephan.dollberg@gmail.com
wrote:
… This one https://github.com/abseil/abseil-cpp/blob/master/CMake/README.md
On Sat, 22 Dec 2018, 21:41 Derek Mauro ***@***.*** wrote:
> Which documentation are you referring to?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#238 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ACxSkwQm8V8QR9d_UCB6y19ZTlFiu5p-ks5u7pkLgaJpZM4ZfLTM>
> .
>
|
|
We are doing to have to fix that documentation. We are planning on generating the CMakeLists from Bazel Build files so there is going to be a one-to-one correspondence between targets soon. |
|
Ok, just to be clear, you want me to do that now and remove the |
|
I'll consult with the team, but I think we are probably going to remove it. This probably won't be merged until Wednesday at the earliest due to the holidays. |
4s5t2os41n
added a commit
to 4s5t2os41n/abseil-cpp
that referenced
this pull request
Jul 4, 2024
-- ffe1bf0e5f98c77cf4193f24ae9ce94d16a72c6e by Alex Strelnikov <strel@google.com>: Remove accidental duplication of conanfile.py. PiperOrigin-RevId: 226926125 -- daf639ddd32c57d1c5ab99b26a9b15107f47ce16 by Derek Mauro <dmauro@google.com>: Fix the CMake build for absl::container. This target is deprecated and will be removed in the future. Fixes abseil/abseil-cpp#238 PiperOrigin-RevId: 226921798 -- b8ab2bb9081c266ced1d966c86d5b19af6b5b3ef by Abseil Team <absl-team@google.com>: Cleanup: Fix some ClangTidy warnings. PiperOrigin-RevId: 226678127 -- 8cdc95316fc8baba00073c38a444c089ed2d5f5e by Abseil Team <absl-team@google.com>: Cleanup: Fix some ClangTidy warnings. PiperOrigin-RevId: 226567814 GitOrigin-RevId: ffe1bf0e5f98c77cf4193f24ae9ce94d16a72c6e Change-Id: Idfe30b8b3229082eb7db4bfa928d3257be7dce1a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Newly added file
hashtablez_sampler.ccis not being built by the cmakebuild and hence linking fails.