[conan-center-index] Checks for semantic errors

[prince-chrismc]

It would be nice if there was the addition of two new checks, thought I am not sure where they would go in the hooks.

1. usage of self.requires.add which should not have the last add portion
2. usage of tools.check_min_cppstd should always have a preceding if self.settings.get_safe("compiler.cppstd"):

I am sure there are other "bad practices" which could be added but these two are very common looking at older recipes, next time they get update it will be more complete.

[prince-chrismc]

Seems like #177 helps close this

[prince-chrismc]

It also appears the CCI wiki it a bit out of date.

[Croydon]

What parts of the wiki are outdated?

[uilianries]

1. usage of self.requires.add which should not have the last add portion

It's duplicated with #177

[prince-chrismc]

"Error-Knowledge-Base" ends at # 37 but there's 44 with the addition of #177

https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base

[Croydon]

Probably only 40 is missing https://github.com/conan-io/hooks/blob/b192c590e507b9c3141b5221f46c58ede2531150/hooks/conan-center.py#L45

[SSE4]

@uilianries @prince-chrismc could you give me some additional background, e.g. why does tools.check_min_cppstd require if self.settings.get_safe("compiler.cppstd")? per documentation, it's not required.

[prince-chrismc]

This comment is why I brought it up over here. lots of discussion around it in the PR

[SSE4]

I've been talking with @uilianries about that. the current thing of adding if self.settings.get_safe("compiler.cppstd") bypasses the check for minimal cppstd if cppstd is None. for example, given the GCC 4.9 compiler (all the explanation below is for that particular compiler version), the default C++ standard would be C++98. it means cppstd=None implies -std=c++98 for this particular compiler. besides that, GCC 4.9 compiler still may produce C++11 code, if you specify cppstd=11 explicitly, or pass -std=c++11 by other means (e.g. from the build system). it appears that some projects (don't know the list, but seems to be many of them), unconditionally force -std=c++11 in their build files (Makefiles, CMakeLists, etc.), e.g. by setting CXX_STANDARD 11 or whatever else their build system supports. for such particular projects, we pass cppstd=None (which implies cppstd=98), but it silently changes in build system to -std=c++11, and you still have a package built. if you specify tools.check_min_cppstd(11) in recipe, it will not allow the build with cppstd=None for the reasons above, therefore you will get zero packages generated for the given compiler. that's the downside of the current CI model that it only builds with cppstd=None. recommendation to add if self.settings.get_safe("compiler.cppstd") is temporary workaround to get some packages still generated in the current model of the CI. however, we're currently reconsidering how to handle cppstd in our CI in general, and I'd say there is high chance that everything will change significantly soon. probably, most of recipes will have to change in order to adapt to the new model. the current temporary workaround has some issues, and the risk of producing invalid packages. it means, you're compiling the package with cppstd=None (implies cppstd=98), but it actually silently compiles with -std=c++11, but the package id assigned will still have cppstd=98, which is obviously wrong. given the API headers like:

#if __cplusplus >= 201103L
std::unique_ptr<engine> get_engine();
#else
#error "C++11 is required to use the library
#endif

you will probably be unable to use the library with cpstd=None at all. moreover, you may encounter runtime issues in the cases like:

#if __cplusplus >= 201103L
std::unique_ptr<engine> get_engine();
#else
std::auto_ptr<engine> get_engine(); // fallback to the old-fashioned code
#endif

the cases like above might be rare and some corner cases, but it's still incorrect to mark c++11 packages with c++98 package id (which we're doing for some packages in conan center right now). e.g. if certain C++ project requires C++11 to build, but has C API, you're most likely safe to do such things, but then you probably want to just erase the cppstd completely from the package id, as it doesn't matter at all in such a given case. TL/DR summary: I don't think it's correct to add that check to the hooks. this thing is a least evil, and a temporary workaround to still generate some packages (potentially invalid) with current CI limitations.