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 clip build on windows + clang #6934

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

Conversation

dhiltgen
Copy link
Contributor

Building on windows with clang errors without common.h included before log.h

2024-04-26T00:23:04.3278937Z C:/a/ollama/ollama/llm/llama.cpp/examples/llava/../../common\log.h:467:5: error: expected ')'
2024-04-26T00:23:04.3281132Z     LOG("01 Hello World to nobody, because logs are disabled!\n");
2024-04-26T00:23:04.3281887Z     ^
2024-04-26T00:23:04.3282880Z C:/a/ollama/ollama/llm/llama.cpp/examples/llava/../../common\log.h:300:27: note: expanded from macro 'LOG'
2024-04-26T00:23:04.3284337Z     #define LOG(str, ...) LOG_IMPL("%s" str, "", ##__VA_ARGS__, "")

Building on windows with clang errors without common.h included
before log.h
@cebtenzzre
Copy link
Collaborator

cebtenzzre commented Apr 28, 2024

It seems like this is related to LOG_NO_FILE_LINE_FUNCTION. Defining it in common.h as-is is definitely wrong, this is just a workaround. I can imagine a few correct fixes.

Ways that would make it mandatory:

  • Define LOG_NO_FILE_LINE_FUNCTION in log.h
  • Make log.h a private header that can only be included directly by common.h
  • Remove the code that is used when you don't define LOG_NO_FILE_LINE_FUNCTION

Ways that would make it opt-in, and require fixing the clang-cl implementation in log.h:

  • Define LOG_NO_FILE_LINE_FUNCTION at the top of every .cpp source that uses logging
  • Define LOG_NO_FILE_LINE_FUNCTION in the build scripts via -D

@mofosyne mofosyne added bugfix fixes an issue or bug review complexity : low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix labels May 9, 2024
@mofosyne mofosyne marked this pull request as draft May 9, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes an issue or bug review complexity : low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants