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

lz4cli: consolidate xxhash to third_party version #854

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

Conversation

fzakaria
Copy link
Contributor

@fzakaria fzakaria commented Jul 8, 2023

Remove the internal xxhash within lz4cli and consolidate to the one true xxhash within third_party.

remove the internal xxhash within lz4cli and consolidate to the
one true xxhash within third_party.
@@ -42,7 +41,8 @@ THIRD_PARTY_LZ4CLI_DIRECTDEPS = \
LIBC_INTRIN \
LIBC_STDIO \
LIBC_LOG \
LIBC_TIME
LIBC_TIME \
THIRD_PARTY_XXHASH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't seem to be picking it up through

o/third_party/gcc/bin/x86_64-linux-musl-ld.bfd: o//third_party/lz4cli/bench.o: in function `BMK_benchMem':
third_party/lz4cli/bench.c:228: undefined reference to `XXH64'
o/third_party/gcc/bin/x86_64-linux-musl-ld.bfd: third_party/lz4cli/bench.c:333: undefined reference to `XXH64'

Copy link
Owner

Choose a reason for hiding this comment

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

You probably need to fix the include foo.mk lines in Makefile so they're topologically ordered again. xxhash.mk should come before lz4cli.mk.

@jart jart force-pushed the master branch 2 times, most recently from faa6285 to 2c4b887 Compare July 25, 2024 08:23
@jart jart force-pushed the master branch 2 times, most recently from 3048620 to 6245732 Compare December 22, 2024 06:13
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.

2 participants