Skip to content

[test] Make xxhash benchmark target name less generic #100172

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BertalanD
Copy link
Member

Downstream users reported that the xxhash target name conflicts with some of their existing build targets. Rename it to bench-xxhash.

See #99634 (comment)

Downstream users reported that the `xxhash` target name conflicts with
some of their existing build targets. Rename it to `bench-xxhash`.

See llvm#99634 (comment)
@hiraditya hiraditya force-pushed the xxhash-bench-name branch from ae73c7d to 0a312a2 Compare July 23, 2024 20:20
@hiraditya
Copy link
Collaborator

LGTM, not sure why the build failed. I have rebased, hopefully the test will pass.

@jayfoad
Copy link
Contributor

jayfoad commented Jul 24, 2024

Thanks. This changes the name of the ninja target to bench-xxhash and also changes the name of the binary to benchmarks/bench-xxhash. I am not much of a cmake expert but I am wondering:

  1. Could this be a generic change that prepends benchmarks- to all benchmark target names, so that ninja benchmarks-xxhash would build benchmarks/xxhash, without changing the name of the executables?
  2. Why do we need a named target to build benchmark executables in the first place? If I want to build it, instead of ninja benchmarks-xxhash I could just type ninja benchmarks/xxhash.

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