Add cpanm option --with-recommends #9555

ZephOne · 2025-01-08T15:28:55Z

SUMMARY

This PR aims to add the use of the option --with-recommands to cpanm module

minor_changes:
  - cpanm - enable usage of option --with-recommands (https://github.com/ansible-collections/community.general/issues/9554).

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

cpanm

ansibullbot · 2025-01-08T15:39:07Z

cc @fcuny @russoz
click here for bot help

russoz · 2025-01-08T19:12:08Z

Hi @ZephOne please update the main branch, then rebase your PR branch with it to fix the Alpine tests

felixfontein

Thanks for your contribution!

You don't need to rebase BTW, adding another commit also resolves the CI problems.

changelogs/fragments/9554-add-cpanm-option_with-recommands.yml

plugins/modules/cpanm.py

Fix ansible-collections#9554

ZephOne · 2025-01-08T20:53:35Z

Hi, thanks for your quick answer.
Yet I rebased.
I've written a bad commit message for the second commit that include your suggestion (they were good suggests). I assume that you would fixup that last commit, if you want me to do it on my branch, let me know.

russoz

hi @ZephOne thanks for your contribution!

The spelling needs fixing.

That being said, a couple of general comments:

since you are doing --with-recommends, I would suggest you expand that and make the exact same thing for --with-suggests, which seem to be a distinct categorie of dependencies for cpanm.
quoting from cpanm docs:

There's also --without-recommend and --without-suggests to override the default decision made earlier in PERL_CPANM_OPT. I made a suggestion that allows both forms to be selected by the user - docs will need an update for that as well.

plugins/modules/cpanm.py

changelogs/fragments/9554-add-cpanm-option_with-recommands.yml

plugins/modules/cpanm.py

changelogs/fragments/9554-add-cpanm-option_with-recommands.yml

plugins/modules/cpanm.py

russoz · 2025-01-11T11:34:07Z

plugins/modules/cpanm.py

+      - Installs dependencies declared as recommends per META spec.
+      - When these dependencies fail to install, cpanm continues the installation, since they are just recommendation.


We should be telling the user about the 3 options they have. Maybe something like:

Suggested change

- Installs dependencies declared as recommends per META spec.

- When these dependencies fail to install, cpanm continues the installation, since they are just recommendation.

- If V(true), installs dependencies declared as recommends per META spec.

- If V(false), it ensures the dependencies declared as recommends are not installed, overriding any decision made earlier in E(PERL_CPANM_OPT).

- If parameter is not set, C(cpanm) will use its existing defaults.

- When these dependencies fail to install, cpanm continues the installation, since they are just recommendation.

Yep, I agree. Explicit is better. I'll just use your suggestion.
By the way what's the keyword for the doc about those doc functions : V(true), V(false), C(cpanm) and E(PERL_CPANM_OPT) ?

Found it: https://docs.ansible.com/ansible/devel/dev_guide/developing_modules_documenting.html#semantic-markup-within-module-documentation

russoz · 2025-01-11T11:53:06Z

Merci for the changes :-) the docs need just one more adjustment, IMO.

And sorry, I should have picked it up earlier: you need to update line 273 in your code, adding the new args to the command runner, otherwise it will not pass them to the command line. See more in https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_cmdrunner.html

One other thing - not mandatory, but recommended - is to add test cases to tests/unit/plugins/modules/test_cpanm.yaml for these new parameters. The case of line 273 would have been picked up by this. ;-)

ZephOne · 2025-01-11T20:16:52Z

One other thing - not mandatory, but recommended - is to add test cases to tests/unit/plugins/modules/test_cpanm.yaml for these new parameters. The case of line 273 would have been picked up by this. ;-)

Currently I cannot see any test in tests/unit/plugins/modules/test_cpanm.yaml. I will check the other modules plugins for a start.

ZephOne · 2025-01-11T20:21:37Z

My bad I must look at tests/unit/plugins/modules/test_cpanm.yaml and not at tests/unit/plugins/modules/test_cpanm.py.
Indeed there are tests 😄
I'll try to add something

ZephOne · 2025-01-11T23:23:38Z

@russoz I added the tests.
Where can I find the doc concerning the link between test_cpanm.py and test_cpanm.yml, in fact the link between yml file and py file for testing.

russoz · 2025-01-12T00:37:49Z

@russoz I added the tests. Where can I find the doc concerning the link between test_cpanm.py and test_cpanm.yml, in fact the link between yml file and py file for testing.

Awesome, will take a look in a moment. As for the docs right they are.......... in my brain. I wrote that helper code, and it is in my to-do list to write a developer guide within the collection docs (I have a couple of other such guides there). It's in the list for 2025 :-)

russoz

LGTM

felixfontein

One small adjustment to the changelog, then it looks good from my side as well:

changelogs/fragments/9554-add-cpanm-option_with-recommends-and-suggests.yml

recommands -> recommends

felixfontein · 2025-01-12T10:50:44Z

LGTM, will merge in a few days if nobody objects.

ZephOne force-pushed the add-cpanm-options-with-recommands branch from 93a6439 to ed02509 Compare January 8, 2025 15:29

ansibullbot added feature This issue/PR relates to a feature request module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) labels Jan 8, 2025

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-10 Automatically create a backport for the stable-10 branch labels Jan 8, 2025

felixfontein reviewed Jan 8, 2025

View reviewed changes

changelogs/fragments/9554-add-cpanm-option_with-recommands.yml Outdated Show resolved Hide resolved

plugins/modules/cpanm.py Outdated Show resolved Hide resolved

plugins/modules/cpanm.py Outdated Show resolved Hide resolved

ZephOne added 2 commits January 8, 2025 21:41

Add cpanm option --with-recommands

94526df

Fix ansible-collections#9554

With accepted suggestions

0b4ccb4

ZephOne force-pushed the add-cpanm-options-with-recommands branch from ed02509 to 0b4ccb4 Compare January 8, 2025 20:50

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jan 8, 2025

russoz reviewed Jan 10, 2025

View reviewed changes

ZephOne changed the title ~~Add cpanm option --with-recommands~~ Add cpanm option --with-recommends Jan 10, 2025

ZephOne force-pushed the add-cpanm-options-with-recommands branch from e8736fe to 818cfbf Compare January 10, 2025 13:38

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 10, 2025

ZephOne force-pushed the add-cpanm-options-with-recommands branch from 818cfbf to 94206b0 Compare January 10, 2025 14:33

ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 10, 2025

russoz reviewed Jan 11, 2025

View reviewed changes

Use install_recommendations for cpanm option --with-recommends

1349fd9

ZephOne force-pushed the add-cpanm-options-with-recommands branch from 94206b0 to fd76584 Compare January 11, 2025 21:41

ansibullbot added tests tests unit tests/unit needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 11, 2025

ZephOne force-pushed the add-cpanm-options-with-recommands branch from cb9d15d to f8c145c Compare January 11, 2025 23:17

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jan 11, 2025

russoz approved these changes Jan 12, 2025

View reviewed changes

felixfontein reviewed Jan 12, 2025

View reviewed changes

changelogs/fragments/9554-add-cpanm-option_with-recommends-and-suggests.yml Outdated Show resolved Hide resolved

ZephOne added 4 commits January 12, 2025 11:42

Fix typo in changelogs/fragments/9554

64fddee

recommands -> recommends

Doc for options users have for recommands and suggests dependencies

56d37fc

Add new args to the command runner.

24ed756

Add test for cpanm --with-recommends

0245115

ZephOne force-pushed the add-cpanm-options-with-recommands branch from f8c145c to 0245115 Compare January 12, 2025 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cpanm option --with-recommends #9555

Add cpanm option --with-recommends #9555

ZephOne commented Jan 8, 2025

ansibullbot commented Jan 8, 2025

russoz commented Jan 8, 2025

felixfontein left a comment

ZephOne commented Jan 8, 2025

russoz left a comment •

edited

Loading

This comment was marked as outdated.

russoz Jan 11, 2025

ZephOne Jan 11, 2025

ZephOne Jan 11, 2025 •

edited

Loading

russoz commented Jan 11, 2025

ZephOne commented Jan 11, 2025

ZephOne commented Jan 11, 2025 •

edited

Loading

ZephOne commented Jan 11, 2025 •

edited

Loading

russoz commented Jan 12, 2025

russoz left a comment

felixfontein left a comment

felixfontein commented Jan 12, 2025

		- Installs dependencies declared as recommends per META spec.
		- When these dependencies fail to install, cpanm continues the installation, since they are just recommendation.

Add cpanm option --with-recommends #9555

Are you sure you want to change the base?

Add cpanm option --with-recommends #9555

Conversation

ZephOne commented Jan 8, 2025

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibullbot commented Jan 8, 2025

russoz commented Jan 8, 2025

felixfontein left a comment

Choose a reason for hiding this comment

ZephOne commented Jan 8, 2025

russoz left a comment • edited Loading

Choose a reason for hiding this comment

This comment was marked as outdated.

russoz Jan 11, 2025

Choose a reason for hiding this comment

ZephOne Jan 11, 2025

Choose a reason for hiding this comment

ZephOne Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

russoz commented Jan 11, 2025

ZephOne commented Jan 11, 2025

ZephOne commented Jan 11, 2025 • edited Loading

ZephOne commented Jan 11, 2025 • edited Loading

russoz commented Jan 12, 2025

russoz left a comment

Choose a reason for hiding this comment

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein commented Jan 12, 2025

russoz left a comment •

edited

Loading

ZephOne Jan 11, 2025 •

edited

Loading

ZephOne commented Jan 11, 2025 •

edited

Loading

ZephOne commented Jan 11, 2025 •

edited

Loading