[cmake-developers] cmake -E capabilities

[Tobias Hunger]

Sorry for the late reply... we had a beta release and a new patch
level release out this week and I had to fix some bugs in both:-)

On Sun, Jul 3, 2016 at 1:30 PM, Stephen Kelly <steveire at gmail.com> wrote:
>> "KDevelop3",
>
> <off topic> This generator should probably be removed/hidden by now. It has
> confused users of more-recent KDevelop versions.

Yes, that should be hidden. I'll add that in the next update.

>> What do you think? What else should we report here?
>
> It looks like a good start. The intention of the output is to satisfy needs
> that consumers like you have, and I guess it does have what you need.

Obviously, or I would not propose it:-)

The hope is to get feedback from other potential users. But so far no
luck on that front:-/

>> Compared to the bug report mentioned above the fields "multiconfig" and
>> "recursive" are missing. I could not figure out how to get that
>> information:-/
>
> When I made the example in the issue tracker, 'recursive' meant 'you can cd
> to a directory and run the CMAKE_MAKE_PROGRAM there'. At the time, it was
> possible to do that when using the Makefiles generator, but not the Ninja
> generator.
>
> I think that capability has since been added for the Ninja generator, but I
> don't know if it is possible with the Xcode and VS generators.
>
> Would that be a useful thing to expose here in your view?

I think that information would be useful at some point. But it is not
needed before a project is opened.

>> I would also welcome some code review on the patch.
>
> 1) For consistency you should change
>
>  GetRegisteredGenerators
>
> to
>
>  this->GetRegisteredGenerators
>
> whether that is a preferred style is orthogonal to the fact that it's
> consistently used in cmake code.

Oh, sorry. I still find that horrible to read, so I only sprinkle
this-> over the code shortly before sending it in to review.

> 2) CMake has to build with toolchains which do not provide
> std::unordered_map. See uses of CMake_HAVE_CXX_UNORDERED_MAP for existing
> code which uses either std::unordered_map or std::unordered_map. (Yes there
> is room for improvement there, but such improvement is orthogonal to your
> branch).

Oh, too bad.

> 3) Similarly, CMake has to build with compilers which do not support
> cxx_range_for or cxx_auto_type. For the cmServer implementation that may or
> may not be the case, but within the cmake class, that's the way it is.

I'll fix that.

> 4) It seems that the
>
>  this->Generators
>
> member of the cmake class has the names of all generators (without 'extra'
> generators). It seems unfortunate to not use that container and instead
> parse the names out of the names from the GetRegisteredGenerators call by
> splitting on the ' - '. It leads to hard coded magic expressions like
> 'separator + 3', so it should be avoided if possible.

Yes, that is ugly.

> Is there another way of determining the extra generators supported by a
> given generator and avoiding parsing a string which we just generated? Can
>
>   const std::vector<std::string>& supportedGlobalGenerators =
>     extraGenerator->GetSupportedGlobalGenerators();
>
> be used somehow? Can you first loop over this->Generators to get the
> 'normal' generators, then loop over the extra generators, call that method
> to match things up and output the result?

Doesn't this require me to instanciate the generators? Won't that  be
potentially expensive?

> Or would it make sense to refactor the container members in the cmake class
> themselves to make this information more easily available for this use-case?
>
> 5) You use the term 'appendix' for the version, but 'suffix' is the more-
> commonly used name for that concept.

I'll change that.

Best Regards,
Tobias