Implement a YAML linter hook

[ericLemanissier]

fixes #308

[ericLemanissier]

example of output: https://github.com/ericLemanissier/conan-center-index/pull/5/checks?check_run_id=3820599048

[danimtb]

the linter can be very useful. The main downside is that we don't have a way to install external dependencies for hooks...

Anyway, if this is going to check the conandata.yml only I'd probably change the name to explicit this or make the hook more generic to check all YAML files exported alongside the recipe.

Another question: Should this be part of the conan-center hook or better keep it as a different hook for wider use rather than ConanCenter only?

[ericLemanissier]

the linter can be very useful. The main downside is that we don't have a way to install external dependencies for hooks...

I just added some code that made it explicit what has to be done if yamllint is not installed.

Another question: Should this be part of the conan-center hook or better keep it as a different hook for wider use rather than ConanCenter only?

As it has an external requirement, I think we cannot include it in conan-center hooks. That said we can still run the hook in Conan-center-index CI, as demonstrated in ericLemanissier/cocorepo#5

Anyway, if this is going to check the conandata.yml only I'd probably change the name to explicit this or make the hook more generic to check all YAML files exported alongside the recipe.

ok, I guess that would make the check a post_export instead of pre_export

[ericLemanissier]

@danimtb it's ready for review

[uilianries]

@ericLemanissier Is yaml lint recursive? If yes, I would suggest looking one level above to check config.yml too

[ericLemanissier]

How do we know If it is cci's structure ? We could end up scanning the full disk.

EDIT: I'll simply scan ../config.yml if it exists

[uilianries]

EDIT: I'll simply scan ../config.yml if it exists

Smart!

[uilianries]

Well, it will be funny once merged. Not in Conan Center by default, but still has a big potential:

[HOOK - yaml_linter.py] pre_export(): Lint yaml '/home/uilian/Development/conan/conan-center-index/recipes/zlib/1.2.11/conanfile.py'
[HOOK - yaml_linter.py] pre_export(): /home/uilian/Development/conan/conan-center-index/recipes/zlib/1.2.11/conandata.yml:4:10: [warning] wrong indentation: expected 6 but found 9 (indentation)
[HOOK - yaml_linter.py] pre_export(): /home/uilian/Development/conan/conan-center-index/recipes/zlib/1.2.11/conandata.yml:5:10: [warning] wrong indentation: expected 6 but found 9 (indentation)
[HOOK - yaml_linter.py] pre_export(): /home/uilian/Development/conan/conan-center-index/recipes/zlib/1.2.11/conandata.yml:6:8: [warning] wrong indentation: expected 4 but found 7 (indentation)
[HOOK - yaml_linter.py] pre_export(): YAML Linter detected '0' errors

[ericLemanissier]

These are warnings which can be disabled if you prefer. That said, this hook is not part of conan-center hooks, so it will never be reported by c3i. This is why I intend to make it run in a dedicated github action,: nobody will look at the log, except if the status is failed.

[jgsogo]

We can add more hooks other that conan-center to CCI (in fact I think some of those check should be extracted from conan-center into their own hook). And we will, probably this will be one of them.

Besides that, I have some concerns with this idiom (not first time here):

try:
    import yamllint
except ImportError as e:
    sys.stderr.write("Install yamllint to use 'yaml_linter' hook: 'pip install yamllint'")
    sys.exit(1)

I think Conan client will exit with the sys.exit, it is unconditional, no exception to catch. I would prefer just an error: "Hook XXX cannot be loaded because of ...". I'm not sure if this is something already working if we let the ImportError to propagate, or it's better right now to capture the exception and noop if the hook cannot run:

try:
   import yamllint
   
   def pre_export(...):
      ...
      
except ImportError as e:
    sys.stderr.write("...")

Maybe, for some other plugins it makes sense to raise and stop Conan execution, but for most of them an error while loading the hook should be enough. wdyt?

[ericLemanissier]

I agree !

[jgsogo]

Feel free to add an entry to README.md

[uilianries]

Great job, thank you!