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

feat(ci): add an option to fail on compile warning #3952

Merged

Conversation

ananta
Copy link
Contributor

@ananta ananta commented Nov 5, 2023

Add optional flags to the build system[make and CMake] to fail upon compile warnings. These flags are disabled by default so that during the development this does not interfere.

fix #3899

.github/workflows/build.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@cebtenzzre
Copy link
Collaborator

I think you are cherry-picking from master when you should be merging from it or rebasing on it.

@cebtenzzre
Copy link
Collaborator

There's no need to close the PR, just fix whatever you need to on your end (e.g. making a new branch, putting your changes on it without the extra commits, then checking out this branch again and running git reset --keep new-branch) and then force-push.

@ananta ananta force-pushed the add-option-to-fail-on-compile-warning branch from adb41e5 to a1d7cca Compare November 5, 2023 23:27
ggml.c Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
ggml.c Outdated Show resolved Hide resolved
@ananta ananta force-pushed the add-option-to-fail-on-compile-warning branch from cd78f4b to b39ae69 Compare November 27, 2023 05:27
CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Owner

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

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

Will merge this after the CI is green

@ggerganov ggerganov merged commit 6e4e973 into ggerganov:master Feb 17, 2024
53 of 54 checks passed
@slaren
Copy link
Collaborator

slaren commented Feb 18, 2024

I don't think this works correctly with GNU systems, make prints this:

expr: syntax error: unexpected argument ‘070100’
expr: syntax error: unexpected argument ‘080100’

@slaren
Copy link
Collaborator

slaren commented Feb 18, 2024

It looks like CC_VER is not set. Originally the PR had this code to set it, but it was removed at some point:

ifeq '' '$(findstring clang,$(shell $(CC) --version))'
	CC_IS_GCC=1
	CC_VER := $(shell $(CC) -dumpfullversion -dumpversion | awk -F. '{ printf("%02d%02d%02d", $$1, $$2, $$3) }')
else
	CC_IS_CLANG=1
	ifeq '' '$(findstring Apple LLVM,$(shell $(CC) --version))'
		CC_IS_LLVM_CLANG=1
	else
		CC_IS_APPLE_CLANG=1
	endif
	CC_VER := $(shell $(CC) --version | sed -n 's/^.* version \([0-9.]*\).*$$/\1/p' \
				| awk -F. '{ printf("%02d%02d%02d", $$1, $$2, $$3) }')
endif

cebtenzzre added a commit that referenced this pull request Feb 18, 2024
cebtenzzre added a commit that referenced this pull request Feb 18, 2024
* build : pass all warning flags to nvcc via -Xcompiler
* make : fix apparent mis-merge from #3952
* make : fix incorrect GF_CC_VER for CUDA host compiler
jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Mar 13, 2024
* feat(ci): add an option to fail on compile warning

* Update CMakeLists.txt

* minor : fix compile warnings

ggml-ci

* ggml : fix unreachable code warnings

ggml-ci

* ci : disable fatal warnings for windows, ios and tvos

* ggml : fix strncpy warning

* ci : disable fatal warnings for MPI build

* ci : add fatal warnings to ggml-ci

ggml-ci

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Mar 13, 2024
* build : pass all warning flags to nvcc via -Xcompiler
* make : fix apparent mis-merge from ggerganov#3952
* make : fix incorrect GF_CC_VER for CUDA host compiler
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
* feat(ci): add an option to fail on compile warning

* Update CMakeLists.txt

* minor : fix compile warnings

ggml-ci

* ggml : fix unreachable code warnings

ggml-ci

* ci : disable fatal warnings for windows, ios and tvos

* ggml : fix strncpy warning

* ci : disable fatal warnings for MPI build

* ci : add fatal warnings to ggml-ci

ggml-ci

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
* build : pass all warning flags to nvcc via -Xcompiler
* make : fix apparent mis-merge from ggerganov#3952
* make : fix incorrect GF_CC_VER for CUDA host compiler
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.

ci : add an option to fail on compile warning
5 participants