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 cmake tests #1478

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Fix cmake tests #1478

wants to merge 4 commits into from

Conversation

m-fila
Copy link
Contributor

@m-fila m-fila commented Jan 10, 2024

Some of the tests built with cmake were failing while the same test built with autotools were passing

  • Fixed naming of cmake tests to easily correlate with autotools tests.
  • Fixed broken paths to test executables.
  • Fixed defining project version patch - tcmalloc_unittest and tcmalloc_debug_unittest

Related issues: #1422, #1477

@alk
Copy link
Contributor

alk commented Jan 12, 2024

Thanks for the patches. Looks fine. But somehow github is telling me this is "draft". Not sure what this means and whether it is intentional or not. Just let me know if you want me to merge those.

@m-fila
Copy link
Contributor Author

m-fila commented Jan 12, 2024

Thanks. This is a bit of a work in progress as I hope to add a few more fixes for the remaining tests relatively soon.

@alk
Copy link
Contributor

alk commented Jan 13, 2024

BTW, please consider this thought: I don't think it is good idea for us to try and to 100% match autotools-based features.

For example, I.e. am seeing that cmake lacks proper support for libtool-like feature of being able to build both libtcmalloc.so and libtcmalloc.a. Instead we do have the ugly libtcmalloc_static.a thingy. Perhaps we simply let users choose which one option to build via BUILD_SHARED_LIBS option.

And same likely applies elsewhere (we do try in autotools make file to test "with asserts" config, which we don't have to duplicate in cmake etc).

I.e. just give people simplest way to do cmake-way of building (most or all of) gperftools. Perhaps because they love cmake, and/or perphaps because cmake has IDE integrations. etc.

So feel free to reach out if you want to discuss direction.

@alk
Copy link
Contributor

alk commented Feb 7, 2024

BTW have a look at a02ac30

I intend cmake bits to get into similar simplified shape (actually even more). We're currently way too complicated there.

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.

None yet

2 participants