Skip to content

make: *** [Makefile:281: ggml-cuda.o] Error 127, and opt-out logging in llama.cpp. #2940

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

Closed
RobertWilliams2 opened this issue Aug 31, 2023 · 7 comments · Fixed by #2911
Closed
Assignees

Comments

@RobertWilliams2
Copy link

RobertWilliams2 commented Aug 31, 2023

Using Linux, latest version of llama.cpp.

make -B LLAMA_CUBLAS=1 LLAMA_DISABLE_LOGS=1
results in

CFLAGS += -DLOG_DISABLE_LOGS
make: CFLAGS: No such file or directory
make: *** [Makefile:281: ggml-cuda.o] Error 127

I suspect the reason why I'm the first one who posted this issue is because few people are aware that since yesterday, llama.cpp has opt-out logging. All text gets stored on users' systems unless explicitly disabled. This is a huge security and privacy concern. Logging should be opt-in only.

@RobertWilliams2 RobertWilliams2 changed the title make: *** [Makefile:281: ggml-cuda.o] Error 127 make: *** [Makefile:281: ggml-cuda.o] Error 127, and opt-out logging in llama.cpp. Aug 31, 2023
@staviq
Copy link
Contributor

staviq commented Aug 31, 2023

Can you try editing Makefile

and moving this (lines 346-349):

ifdef LLAMA_DISABLE_LOGS
	CFLAGS   += -DLOG_DISABLE_LOGS
	CXXFLAGS += -DLOG_DISABLE_LOGS
endif # LLAMA_DISABLE_LOGS

to right below (line 80):

ifdef LLAMA_SERVER_VERBOSE
	CXXFLAGS += -DSERVER_VERBOSE=$(LLAMA_SERVER_VERBOSE)
endif
<---HERE

so it looks like this:

ifdef LLAMA_SERVER_VERBOSE
	CXXFLAGS += -DSERVER_VERBOSE=$(LLAMA_SERVER_VERBOSE)
endif

ifdef LLAMA_DISABLE_LOGS
	CFLAGS   += -DLOG_DISABLE_LOGS
	CXXFLAGS += -DLOG_DISABLE_LOGS
endif # LLAMA_DISABLE_LOGS

@staviq staviq self-assigned this Aug 31, 2023
@RobertWilliams2
Copy link
Author

That fixed the compiling issue.

I still suggest that logging be moved from compile to runtime parameter and be opt-in instead of opt-out.

@staviq
Copy link
Contributor

staviq commented Aug 31, 2023

be opt-in instead of opt-out.

I'm not against that idea, but others would have to agree. Runtime args for disable/enable logs are already there.

@RobertWilliams2
Copy link
Author

RobertWilliams2 commented Aug 31, 2023

Are you referring to

-ld LOGDIR, --logdir LOGDIR
path under which to save YAML logs (no logging if unset)

?

Because it is still saving all text to main.923485746264.log files under llama.cpp\ just by compiling without including LLAMA_DISABLE_LOGS=1 without me using -ld or -logdir parameters.

@ghost
Copy link

ghost commented Aug 31, 2023

still saving all text

Yes, by default it behaves as opt-in, your understanding is correct. As a user, I agree that it should change. At least there's --log-disable for now.

--logdir path is a seperate feature.

@staviq
Copy link
Contributor

staviq commented Aug 31, 2023

Runtime argument for disabling .log generation is --log-disable

Running ./main -h shows that one and couple more.

Please note, .log logs are debug logs, and were meant to make debugging problems easier.

Yaml logs, are a completely different feature with difrent purpose.

@RobertWilliams2
Copy link
Author

RobertWilliams2 commented Aug 31, 2023

I couldn't see the log-related commands of ./main -h because when I ran the command my main was compiled using LLAMA_DISABLE_LOGS=1, which also hides the commands (which makes sense since now the commands are prevented from working during compile, which is a further argument as for why enabling and disabling logs should not be a part of the compilation process.)

Having two separate ways of disabling a feature runs the risk of somewhere down the road their behavior will diverge. If I could disable logging both ways (via compile and adding --log-disable, I would. But as is ./main yells at me with "error: unknown argument: --log-disable"

Basically this is a feature that if you want it you should know you want it, and find out how to enable it which you have to do only once (assuming you have a script file to run main). It's better to keep it off by default and avoid all this mess for people who don't want it, which includes people who aren't even aware logging is taking place.

staviq added a commit to staviq/llama.cpp that referenced this issue Aug 31, 2023
ggerganov pushed a commit that referenced this issue Sep 1, 2023
* fix mingw-like builds

* formatting

* make LOG_COMPAT easier to override and extend

* simplify win detection

* fix for #2940
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 a pull request may close this issue.

2 participants