Skip to content

fix: ld link test-tokenizer-0 error #1042

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

Merged
merged 4 commits into from
Apr 21, 2023
Merged

Conversation

fumiama
Copy link
Contributor

@fumiama fumiama commented Apr 18, 2023

cmake3 --build . --config Release
[  5%] Built target ggml
[ 16%] Built target llama
[ 22%] Linking CXX executable ../bin/test-tokenizer-0
../libllama.a(ggml.c.o):在函数‘ggml_graph_compute’中:
ggml.c:(.text+0xf2db):对‘pthread_create’未定义的引用
ggml.c:(.text+0xf9d4):对‘pthread_join’未定义的引用
collect2: error: ld returned 1 exit status
gmake[2]: *** [bin/test-tokenizer-0] 错误 1
gmake[1]: *** [tests/CMakeFiles/test-tokenizer-0.dir/all] 错误 2
gmake: *** [all] 错误 2

```
cmake3 --build . --config Release
[  5%] Built target ggml
[ 16%] Built target llama
[ 22%] Linking CXX executable ../bin/test-tokenizer-0
../libllama.a(ggml.c.o):在函数‘ggml_graph_compute’中:
ggml.c:(.text+0xf2db):对‘pthread_create’未定义的引用
ggml.c:(.text+0xf9d4):对‘pthread_join’未定义的引用
collect2: error: ld returned 1 exit status
gmake[2]: *** [bin/test-tokenizer-0] 错误 1
gmake[1]: *** [tests/CMakeFiles/test-tokenizer-0.dir/all] 错误 2
gmake: *** [all] 错误 2
```
Copy link
Collaborator

@Green-Sky Green-Sky left a comment

Choose a reason for hiding this comment

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

  1. Don't link to pthread directly. Use cmake Threads::Threads package instead.
  2. This should not be necessarily in the first place since the ggml target gets linked with Threads::Threads already.

It might be that https://github.com/ggerganov/llama.cpp/blob/42747220b4cac548b6e3059b66b3e960b517cfa4/CMakeLists.txt#L273-L279
ggml being an OBJECT library and the dependencies being private causes issues.
Can you test with making the ggml target STATIC instead and or making the dependencies PUBLIC instead?

@Green-Sky Green-Sky added need more info The OP should provide more details about the issue build Compilation issues labels Apr 18, 2023
@fumiama
Copy link
Contributor Author

fumiama commented Apr 18, 2023

Well, I simply removed the PRIVATE in line 279 and it works fine.

Copy link
Collaborator

@Green-Sky Green-Sky left a comment

Choose a reason for hiding this comment

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

change PRIVATE to PUBLIC instead of dropping it completely. fits more the style of the rest of the project and is generally better code.

@ggerganov seems like OBJECT libraries having private dependencies does not make sense, since they don't have a linking step. Or it just breaks with certain cmake versions?

@fumiama
Copy link
Contributor Author

fumiama commented Apr 18, 2023

change PRIVATE to PUBLIC instead of dropping it completely. fits more the style of the rest of the project and is generally better code.

Thanks. I don't know much about cmake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Compilation issues need more info The OP should provide more details about the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants