switch from include to import_tasks

[danfoster]

When using this module on ansible 2.16.0 I get:

ERROR! [DEPRECATED]: ansible.builtin.include has been removed. Use include_tasks or import_tasks instead. This feature was removed from ansible-core in a release after 2023-05-16. Please update your playbooks.

Following https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_reuse.html#imports-static-reuse , updating it to import_tasks seems to be the correct solution.

[morgangraphics]

Thanks for taking the time to participate in the project, I very much appreciate the effort. Unfortunately, this PR isn't quite good enough as it stands now. By doing it the way you've proposed, you've essentially eliminated all backwards compatibility for this role. This would be a breaking change, and breaking changes require a bit of a grace period to allow for people to upgrade their code and processes.

A better approach which would allow for newer installations and still support legacy installations would be something like:

---
- include: nvm.yml
  when: "ansible_version.full is version_compare('2.16.0', '<')"

- import_tasks: nvm.yml
  when: "ansible_version.full is version_compare('2.16.0', '>=')"

[danfoster]

Thanks for the feedback.

My reason for not including the conditionals as you suggested is that ansible has handled the deprication/removal of import over a 6 year lifecycle and it felt an unneeded overhead to continue to support old installs of ansible. Specifically:

• import_tasks was added in ansible 2.4, released in August 2017 . import was marked as deprecated at the same time.
• import was finally removed from ansible in 2.16, released in November 2023

Are you supporting versions of ansible older that 2.4?

[morgangraphics]

Unfortunately, neither Ansible (or RedHat) provide any information (other than number of downloads) as to what platforms or versions of Ansible, Python etc. this role is running in. I don't feel strongly enough about collecting that data either, for a number of reasons.

While Ansible was good at broadcasting deprecation notices as an upstream repo, I have been less vocal about it from this repo. If I am an upstream repo for anyone else (again, I have no idea if this is the case or not), then the proposal you provided would be a breaking change.

My approach is to support stuff until it no longer makes sense. In this particular case, the amount of work to support both import declarations is pretty minimal. It would allow me to broadcast a breaking change and phase out the old stuff at some future date.

[chandrangreat]

Hi

I too faced this issue and I just changed to import_tasks. I believe the points in this conversation are all valid.

What if we remove the use of import/include statement completely and put the contents of nvm.yml file inside main.yml file? This would also enable backward compatibility?

[morgangraphics]

So, I've finally had some time to sit down and look into this particular issue further. Again, thanks for taking the time to chime in and offer a solution.

I'm not going to accept this PR, not because you did anything wrong, Your PR has had me wrestling with how to handle maintaining multiple versions of Ansible in a single branch/role. It was something I've been avoiding for some time now.

The solution I've come up with is to make an ansible-role-nvm-legacy branch that maintains the 1.5.X semver and supports Ansible versions below 2.16.0. Only security updates and NVM version changes will be made here from here on out.

I am also creating a new 2.0 branch which will update the role to the "new" ansible.builtin collection methodology and support Ansible versions greater than or equal to 2.16.0 and integrate some other work I've been meaning to add to this role. The 1.5.3 Release notes in the new ansible-role-nvm-legacy branch mention you as the catalyst for this change. Thanks again and stay tuned for some updates.