Allow secondary generators when building with CMake.

[greenc-FNAL]

CMake supports the notion of secondary generators which provide extra information to (e.g.) IDEs over and above that normally provided by the primary generator.

The list of primary generators ('Unix Makefiles' and 'Ninja') supported by Spack has not changed.

Since the secondary generator is irrelevant to a Spack build, it is passed on to CMake without further validation.

[scheibelp]

@chissg

I'm surprised that the first .* is not greedy. Would a regex like

[^-]*(:?-\s+)(.*)

work? Anyhow the unit test confirms that it what you have works, but to explain why I'd have to look up the behavior of python regexes or regexes in general.

Either way, could you also add an explanation or an example in a comment like

# Extract the primary generator from the generator string
# For example, "ninja" from "CodeBlocks - Ninja"

If this logic was isolated to a function, then that wouldn't be necessary, as it would be easy to look up the associated unit test. Since the logic is embedded I think it's worthwhile to add a comment.

[greenc-FNAL]

@scheibelp Thanks for this. The first .* could be greedy: the question is whether you think an embedded - is more likely to be found in the secondary generator name, or the primary. If there's only ever one (as is currently the case with all known values) then there is no difference in behavior. Since the secondary generator (and therefore the separator) is entirely optional though, I wanted to be more specific.
I can certainly pull the extraction out into a function if you think it would improve the clarity.

[scheibelp]

If the rules for the generator name are not specified then this could be fragile. Perhaps it would be safer to check for the names in the generator, like

if 'Ninja' in generator or 'Unix Makefiles' in generator:
  pass
else:
  raise...

Without more info on the constraints on the name (e.g. maybe embedded dashes can't have spaces) I think it might be hard to come up with a good regex.

(I realized the regex I suggested has its own potential pitfalls WRT being greedy)

[greenc-FNAL]

From https://cmake.org/cmake/help/git-master/manual/cmake-generators.7.html:

Some of the CMake Generators listed in the cmake(1) command-line tool --help output may have variants that specify an extra generator for an auxiliary IDE tool. Such generator names have the form - . The following extra generators are known to CMake.

Neither any of the existing defined primary generators, nor any of the secondary ones, have any embedded dashes. Since these names are encoded into CMake, one can only presume that if they come up with a new generator (primary or secondary) that absolutely needs a dash, they'll change the rules.

In the meantime, since as you say we're looking for particular known generators we support, I'd suggest a search rather than a match:

(?: - )?(.*)$

This will check for the (currently) prescribed format in the case of the presence of a secondary generator, followed by something that will either be one of the items we expect, or it won't. If a secondary exists and has an embedded hyphen it won't matter; if there's a secondary generator but they've screwed up the separator we'll match everything and fail to recognize the primary (correct behavior IMO). In the normal case (with or without secondary), we'll pull a primary we can compare against our limited list, and we're insulated against new primaries that we don't support, but whose name is a subset of one we do. Thoughts?

[scheibelp]

This sounds good, and the regex you suggest is clearer (to me at least). I also think it's worth including a short comment giving an example of generator names.

I can certainly pull the extraction out into a function if you think it would improve the clarity.

Even though it's small, that does appeal to me: something like extract_primary_generator.

[greenc-FNAL]

I hope I have addressed these comments with 107df3e. I was not able to turn the regex into a search as we discussed, but I hope things are now clearer nonetheless.

[greenc-FNAL]

@scheibelp Bump?

[scheibelp]

Thanks!