Make roles generation compatible with ansible-lint autofix feature

[shatakshiiii]

SUMMARY

While generating Ansible role using the ansible-galaxy role init command, it creates a default meta/main.yml file. If we later run ansible-lint --fix against that role, it changes the comment indentation for the galaxy_tags section.

All the required fields for meta/main.yml to pass ansible-lint are in the default file created by ansible-galaxy role init and its indention should be suitable for ansible-lint --fix.

Hence, this PR tries to make the generated meta/main.yml file compatible with ansible-lint --fix.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

• galaxy

Related: #82394, Fixes: ansible/ansible-lint#3941

[cidrblock]

Thank you @shatakshiiii.

Regarding #82394, while I agree it is not necessarily a bug, we certainly have an opportunity here to improve the developer experience.

[nitzmahone]

I don't really have a problem with this particular change, but it does make me nervous that ansible-lint may be delegating the rule implementation that's driving it to a non-spec YAML comment roundtripping implementation that's subject to change (further discussion here).

[cidrblock]

at this point in time, the dep packages used to reformat yaml can't really change. But I agree, it is a conversation we should keep having.

[nitzmahone]

Since it sounds like the YAML formatting requirement is limited to "whatever ruamel says", and I know from experience that's pretty difficult to change everywhere, we should probably just merge this one for internal consistency.

[nitzmahone]

(assuming we merge this, we should probably also backport to all upstream/downstream-supported branches as well)

[sivel]

It looks like these lines should also be corrected? It doesn't affect anything because there is nothing after, but I'm guessing the formatting gets changed here too. If we're going to correct it, we should correct all places that get reformatted.

[shatakshiiii]

@sivel, Thanks for the review. I double-checked the behavior of comments in "dependencies" section. It does not get reformatted while using ansible-lint --fix.

[webknjaz]

Is the formatting arbitrary, then? It doesn't feel right accepting a change that introduces such an inconsistency.

[s-hertel]

@shatakshiiii From my testing, if something follows the comment it is also unindented. I think it should look visually-consistent, since the one-off change by the linter a bit unfriendly to usability. Another change ansible-lint --fix makes is add --- to the top of all the YAML files, why is the indentation changing but not that?

[nitzmahone]

The underlying problem with all of this is that lint is not really formatting-aware, but reformatting sometimes occurs as a side-effect of round-tripping the document through ruamel (which invents its own rules to tie YAML comments to YAML blocks and indents them to match). IIRC its general rule is that a comment on a non-content line is tied to the next content element below it and indented to match. The comment on this line has no content element below it (since it's EOF), so it's just left alone.

Changing these to follow a general rule of "comments always above content" should make that behavior more consistent and predictable- I think that's preferable to, eg, adding bogus empty block lists that have to be removed, just to keep the "below the element" comment formatting.

The doc-start marker is another problem case- IIRC ruamel doesn't have a "re-emit if present" knob for that, so it's either always going to remove it or always going to add it, so if lint's not shutting that off, then yeah, the template should also include that.

Longer-term, I'm still not sure lint's going to be able to get away without having some standard wrappers or customization around ruamel's round-trip reformatting.

[cidrblock]

For consistency, having the explanation about a given key above that key, as several are, sounds reasonable to me.

[s-hertel]

This could use a changelog fragment https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to.

[s-hertel]

I think the original template was more intuitive to actually fill out. Adding an item inline with a comment is exactly how I'd normally comment a yaml list:

galaxy_info:
  tags:
    # comment is inline with items
    - tag

Now the only ansible-lint --fix-compat way to add a tag with one line is:

galaxy_info:
  tags:
  # comment indentation is different than items
    - tag

Maybe we could move the comment on top of the keys they describe since they're no longer at the same indentation as items?

[webknjaz]

I think the original template was more intuitive to actually fill out. Adding an item inline with a comment is exactly how I'd normally comment a yaml list:

galaxy_info:
  tags:
    # comment is inline with items
    - tag

Now the only ansible-lint --fix-compat way to add a tag with one line is:

galaxy_info:
  tags:
  # comment indentation is different than items
    - tag

I also find this weird. The yamllint rule is supposed to enforce same-level indents: https://yamllint.rtfd.io/en/stable/rules.html#module-yamllint.rules.comments_indentation.

Honestly, I'd like it better if the skeleton used multi-line list items rather than []. Of course, injecting a dummy tag is silly, but maybe it could be empty, forcing the user to actually fill it out?

[bcoca]

Against this change, as you can see in discussions this leads us to follow the linting, not actual parser correctness.

Aside from changing a lot more often and arbitrarily, the linting is an opinion, which is far from universal, as seen in the back and forth above.

[cidrblock]

Against this change, as you can see in discussions this leads us to follow the linting, not actual parser correctness.

Aside from changing a lot more often and arbitrarily, the linting is an opinion, which is far from universal, as seen in the back and forth above.

While all true, there is a commitment that ansible-lint reformats ansible files for ansible users. Currently that is done by a RT through ruamel, with some tuning of ruamel's behaviour.

Given these are simply formating changes and will lead to a more consistent output and experience for ansible users, I am still in favor of moving forward with this.