search

clMathLibraries / clBLAS

a software library containing BLAS functions written in OpenCL
Apache License 2.0
839 stars 240 forks source link

don't install build dir with sources #69

Closed glehmann closed 9 years ago

glehmann commented 9 years ago

many cmake users are creating the build dir inside the source dir

kknox commented 9 years ago

Can you describe what REGEX REPLACE "([.[{()\\*+?|^$])" "\\\\\\1" does in english? That looks like you are modifying paths, but did you test with windows style paths?.

glehmann commented 9 years ago

this regex replaces all the characters that may have a special meaning in the path when read as a regular expression, so the path can be used with REGEX ... EXCLUDE to exclude the build path.

OK, this is not exactly obvious, and probably deserve a comment :-) And CMake makes it even harder, by forcing to use many \\ to make it work properly.

I haven't tested the PR on windows, but the regex should work fine on that OS because the paths are / separated on windows as well with CMake. Plus I'm already using it in another project (vera++) that builds fine on that OS.

TimmyLiu commented 9 years ago

I think the users should not build binary within the source directory. I am going to close this one without merging.

glehmann commented 9 years ago

Then you should make it explicit by generating a error during the configuration. Not allowing it is OK, but again this is a very common practice, so it should be clear that this practice is not allowed for clBLAS.

TimmyLiu commented 9 years ago

Thanks! That's a good advice actually. Do you want to reopen or make another pull request preventing user from building in the source directory? Or I can do it whenever I have a chance.

pavanky commented 9 years ago

If there are no problems that explicitly prohibit having the build directory in that place, I think the PR should be merged as opposed to erroring out.