Adapt options when built as sub-project

[ClausKlein]

skipp tests and example build, install only

[d-frey]

Shouldn't the test be based on https://stackoverflow.com/questions/25199677/how-to-detect-if-current-scope-has-a-parent-in-cmake? This should prevent problems on Windows due to paths being case-insensitive.

Also, AFAIK, the variable should be prefixed with PEGTL_ to avoid conflicts when used as subproject. And I'd like to avoid the word "master", as we will switch our main branch's name from "master" to "main" when GitHub finished adapting the infrastructure. How about PEGTL_HAS_PARENT or PEGTL_IS_MAIN_PROJECT?

Removing the arguments from endforeach() and endfunction() will not work with older versions of CMake, they were only made optional in CMake 3.14.

I'm unsure about the exact purpose of the part you labelled TODO, it was added by @pleroux0. Paul, is this still required in modern CMake? What exactly does it do and how would it interact with the proposed PR?

[ClausKlein]

see https://discourse.cmake.org/t/top-project-master-project-vocabulary/412/3

Im not sure about endforeach() and endfunction(). As far as I know, it is not needed sine 3.0.

[d-frey]

For endforeach() I'm comparing https://cmake.org/cmake/help/v3.13/command/endforeach.html with https://cmake.org/cmake/help/v3.14/command/endforeach.html, likewise for endfunction().

The link you posted tells me that PARENT_DIRECTORY is the way to go. Are you reading it differently?

[ClausKlein]

https://discourse.cmake.org/t/top-project-master-project-vocabulary/412/5

If have already seen 2 mechanisms to detect that:

1. compare CMAKE_CURRENT_SOURCE_DIR and CMAKE_SOURCE_DIR
2. compare CMAKE_PROJECT_NAME and PROJECT_NAME

I always use option 1. The second option technically isn’t guaranteed, since a project name isn’t required to be unique.
[/quote]

[d-frey]

I've seen that. But that is only a part of what is written there. And quite frankly, just quoting back some part doesn't really explain your view.

You are leaving out the third option (the one I was talking about). That third option was added by Brad King, a core developer of CMake. The /5 comment doesn't really show any problem with it, just that the author had some problem which could very well be caused by something else as the author himself explains.

Also, in the link to SO that I posted above, option 3 is the accepted answer, the person that wrote it has a gold badge for CMake on SO.

And finally, I told you that your solution (option 1) could lead to problems on Windows as you need to compare case-insensitive. See also https://stackoverflow.com/a/42716588/2073257

[ClausKlein]

And finally, I told you that your solution (option 1) could lead to problems on Windows as you need to compare case-insensitive. See also https://stackoverflow.com/a/42716588/2073257

That is simply not true. On OSX, the filesystem is also not case sensitive. I never had problems with this.
If it were a Problem, the cmake would have problems too, then it create for every source dir a binary dir. In case of 2 dirs only different in case, there would be an Error!

It seems, you dit not see the main point of my patch: the different behaviour in case of a subproject.
You will use PEGTL as a submodule in json. but in this case, you have to change the options manually.

No one should make his project in this way.

[d-frey]

I did see that. Isn't the commit which closes this doing just that?

[ClausKlein]

FYI https://discourse.cmake.org/t/needs-endforeach-a-lable-with-older-cmake-version/2442

[d-frey]

Thanks, I removed the arguments now. FWIW, they should update the documentation.

[d-frey]

It seems option 1 (what you suggested) actually works, while option 3 (the one I found and used) doesn't. I do not understand why option 3 does not work, but it is what it is. The problem seems to show up when I try and use the updated PEGTL in taoJSON. So, despite the arguments made on SO, let us hope that the case-sensitivity is not a problem anymore. I found some folks saying it was related to CMake converting some paths to lower-case, but this is (hopefully) fixed by now? We'll see.