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.

Sign up for GitHub

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

Jump to bottom

CMake: warn if the user did not set a build type #8162

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

CMake: warn if the user did not set a build type #8162

wants to merge 1 commit into from

Conversation

LebedevRI
Copy link
Contributor

I did not get a warning for -GNinja,
and the resulting failure is bizarre:

$ ninja Halide_Python
[10/1842] Generating .__init__.py.stamp
FAILED: python_bindings/src/halide/.__init__.py.stamp /repositories/halide/build-halide/python_bindings/src/halide/.__init__.py.stamp
cd /repositories/halide/build-halide/python_bindings/src/halide && /usr/bin/cmake -E copy /repositories/halide/python_bindings/src/halide/__init__.py /halide/__init__.py1111 && /usr/bin/cmake -E touch /repositories/halide/build-halide/python_bindings/src/halide/.__init__.py.stamp
Error copying file "/repositories/halide/python_bindings/src/halide/__init__.py" to "/halide/__init__.py".
[43/1842] Building CXX object _deps/flatbuffers-build/CMakeFiles/flatbuffers.dir/src/idl_parser.cpp.o
ninja: build stopped: subcommand failed.

(TARGET_FILE_DIR is somehow missing CMAKE_CURRENT_BINARY_DIR prefix)

@alexreinking
Copy link
Member

This is not a valid patch since CMAKE_BUILD_TYPE has no effect on multi-config generators (Ninja is single-config) unless we have mistakenly introduced such a behavior. A valid patch fixes that behavior.

If you weren't getting a warning you might have set CMAKE_BUILD_TYPE to the empty string?

Most likely we need to fix a different build rule that unintentionally depends on it.

@LebedevRI
Copy link
Contributor Author

Huh, this is quite weird, but somehow CMAKE_BUILD_TYPE is indeed an empty string by default,
without me doing anything.

@LebedevRI
CMake: actually warn if the user did not set a build type
a46be40 
I did not get a warning for `-GNinja`,
and the resulting failure is bizarre:
```
$ ninja Halide_Python
[10/1842] Generating .__init__.py.stamp
FAILED: python_bindings/src/halide/.__init__.py.stamp /repositories/halide/build-halide/python_bindings/src/halide/.__init__.py.stamp
cd /repositories/halide/build-halide/python_bindings/src/halide && /usr/bin/cmake -E copy /repositories/halide/python_bindings/src/halide/__init__.py /halide/__init__.py1111 && /usr/bin/cmake -E touch /repositories/halide/build-halide/python_bindings/src/halide/.__init__.py.stamp
Error copying file "/repositories/halide/python_bindings/src/halide/__init__.py" to "/halide/__init__.py".
[43/1842] Building CXX object _deps/flatbuffers-build/CMakeFiles/flatbuffers.dir/src/idl_parser.cpp.o
ninja: build stopped: subcommand failed.
```
(`TARGET_FILE_DIR` is somehow missing `CMAKE_CURRENT_BINARY_DIR` prefix)
@LebedevRI
Copy link
Contributor Author

Ok, right, the issue is DEFINED part...

@LebedevRI
Copy link
Contributor Author

unless we have mistakenly introduced such a behavior. A valid patch fixes that behavior.

FWIW, halide$ grep -r CMAKE_BUILD_TYPE does not really show it being used anywhere,
so i'm not really sure that it is the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexreinking alexreinking Awaiting requested review from alexreinking

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LebedevRI @alexreinking