yarn: Fix state=latest not working with global=true #5829

sargunv · 2023-01-13T05:08:02Z

SUMMARY

Fixes #5712.

Updates _exec to support running Yarn commands that wouldn't ordinarily be available with yarn global by setting the working directory to the path from yarn global dir instead.
Updates list_outdated to use this new functionality when invoking yarn outdated.
Updates list_outdated to check for actual errors in stderr output instead of failing on the presence of anything in stderr. This is b/c the Yarn global package.json has no license field, causing a warning to be logged to stderr when invoking yarn outdated in that directory.

Through testing, found a couple more related bugs to fix (see comments below):

Replaces the hardcoded DEFAULT_GLOBAL_INSTALLATION_PATH with code to check the output of yarn global dir, as the default path may be different when the use is root, or when the user has set YARN_GLOBAL_FOLDER or the corresponding config flag.
Updates list to function correctly for packages without binaries (regular libraries) when global=true by invoking yarn list without global, the same way I did for yarn outdated above.

And finally, added tests for all the above cases. Previously, there were no tests for global=true at all.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

yarn

ADDITIONAL INFORMATION

Before:

After:

ansibullbot · 2023-01-13T05:10:41Z

cc @chrishoffman @verkaufer
click here for bot help

sargunv · 2023-01-13T05:22:58Z

plugins/modules/yarn.py

+            if self.globally and unsupported_with_global:
+                _rc, out, _err = self.module.run_command(self.executable + ['global', 'dir'], check_rc=check_rc)
+                path = out.strip()


This does mean a global=true param will override the path param, but that was previously true anyway due to this line:

if self.path and not self.globally:

russoz · 2023-01-16T08:26:33Z

Hi @sargunv
Thank you for the contribution! Please add a changelog fragment.

Also, if you could please add a testcase in the integration tests, so future modifications in this module take that change into account.

sargunv · 2023-01-16T21:26:08Z

There were previously no yarn global tests, so I added test cases to global install, upgrade, and remove a package.

The test for upgrade (state=latest, what this PR fixes) is passing, but the newly added test for remove (state=absent) is failing. I'll have to look into how to get these tests running locally to investigate. Normal usage of global=true with state=absent does work locally.

EDIT: wasn't able to get the tests running locally at all b/c I'm on a distro w/o systemd. I'll just add only the tests relevant to this PR's scope instead of a full set of yarn global tests.

sargunv · 2023-01-16T22:36:58Z

Got the tests working locally (turns out WSL can enable systemd) and found the issue with the yarn global remove test I had attempted to add.

The yarn module assumes a certain yarn global dir but this assumption is incorrect when running as the root user (as the tests do). When root, the default yarn global dir appears to be /usr/local/share/.config/yarn/global/, not /root/.config/yarn/global/

community.general/plugins/modules/yarn.py

Line 166 in 44172dd

DEFAULT_GLOBAL_INSTALLATION_PATH = os.path.expanduser('~/.config/yarn/global')

And regardless, hardcoding this path isn't correct as the global dir can be modified by the user with the Yarn config or by environment variable (YARN GLOBAL FOLDER).

russoz · 2023-01-16T22:53:19Z

The magic of tests, we find problems we did not imagine we had 😁.

Thanks!

…nary

sargunv · 2023-01-16T23:32:11Z

Fixed that, and while testing that fix I found yet another little bug 🙃

The list() function uses yarn list or yarn global list depending on whether global=true, but yarn global list only lists packages with binaries, not all globally installed packages. So, cases like name=left-pad state=absent global=true do not work; the step thinks left-pad is already absent.

I updated that one to use the same unsupported_with_global param I added for the bug this PR was initially addressing, so list() will always use the regular yarn list. I also removed the bit of code that handled the different output format of yarn global list, and added tests for this case too.

codecov · 2023-01-17T08:27:13Z

Codecov Report

Merging #5829 (44172dd) into main (58eb495) will decrease coverage by 0.01%.
The diff coverage is 41.66%.

❗ Current head 44172dd differs from pull request most recent head 74f8dfa. Consider uploading reports for the commit 74f8dfa to get more accurate results

@@            Coverage Diff             @@
##             main    #5829      +/-   ##
==========================================
- Coverage   45.54%   45.54%   -0.01%     
==========================================
  Files        1005     1005              
  Lines       97706    97713       +7     
  Branches    17808    17811       +3     
==========================================
+ Hits        44505    44507       +2     
- Misses      51144    51149       +5     
  Partials     2057     2057

Flag	Coverage Δ
integration	`71.28% <80.00%> (-0.12%)`	⬇️
sanity	`21.43% <8.33%> (-0.01%)`	⬇️
stub	`0.00% <0.00%> (ø)`
units	`67.86% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
plugins/doc_fragments/ldap.py	`100.00% <ø> (ø)`
plugins/doc_fragments/rackspace.py	`100.00% <ø> (ø)`
plugins/modules/consul.py	`20.28% <0.00%> (-0.49%)`	⬇️
plugins/modules/terraform.py	`55.17% <50.00%> (ø)`
plugins/doc_fragments/attributes.py	`100.00% <100.00%> (ø)`
plugins/modules/apache2_module.py	`78.75% <100.00%> (+0.26%)`	⬆️
plugins/modules/xml.py	`80.15% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

changelogs/fragments/5829-fix-yarn-global.yml

Co-authored-by: Felix Fontein <[email protected]>

sargunv · 2023-02-11T18:53:24Z

Hi, is there anything else I need to do to make this PR ready to merge?

russoz

Hi @sargunv it looks good to me. Only one small issue with the changelog fragment.

changelogs/fragments/5829-fix-yarn-global.yml

Co-authored-by: Alexei Znamensky <[email protected]>

sargunv · 2023-02-12T06:46:31Z

Fixed!

russoz

LGTM

patchback · 2023-02-13T20:55:00Z

Backport to stable-5: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 4c4ef80 on top of patchback/backports/stable-5/4c4ef80ca9518792bc4e0551864a7929254a27a0/pr-5829

Backporting merged PR #5829 into main

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/ansible-collections/community.general.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/stable-5/4c4ef80ca9518792bc4e0551864a7929254a27a0/pr-5829 upstream/stable-5

Now, cherry-pick PR yarn: Fix state=latest not working with global=true #5829 contents into that branch:
```
$ git cherry-pick -x 4c4ef80ca9518792bc4e0551864a7929254a27a0
```
If it'll yell at you with something like fatal: Commit 4c4ef80ca9518792bc4e0551864a7929254a27a0 is a merge but no -m option was given., add -m 1 as follows intead:
```
$ git cherry-pick -m1 -x 4c4ef80ca9518792bc4e0551864a7929254a27a0
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR yarn: Fix state=latest not working with global=true #5829 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/stable-5/4c4ef80ca9518792bc4e0551864a7929254a27a0/pr-5829

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-02-13T20:55:07Z

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/4c4ef80ca9518792bc4e0551864a7929254a27a0/pr-5829

Backported as #5992

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* Yarn module: fix state=latest not working with global=true * fix whitespace * add changelog fragment * add integration test cases * add only tests for this PR (install+upgrade) * fix assuming default global dir * fix list() not working when global=true and name a package with no binary * remove ignores * whitespace * Update changelogs/fragments/5829-fix-yarn-global.yml Co-authored-by: Felix Fontein <[email protected]> * Update changelogs/fragments/5829-fix-yarn-global.yml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 4c4ef80)

felixfontein · 2023-02-13T20:55:19Z

@sargunv thanks for fixing this!
@russoz thanks for reviewing!

…king with global=true (#5992) yarn: Fix state=latest not working with global=true (#5829) * Yarn module: fix state=latest not working with global=true * fix whitespace * add changelog fragment * add integration test cases * add only tests for this PR (install+upgrade) * fix assuming default global dir * fix list() not working when global=true and name a package with no binary * remove ignores * whitespace * Update changelogs/fragments/5829-fix-yarn-global.yml Co-authored-by: Felix Fontein <[email protected]> * Update changelogs/fragments/5829-fix-yarn-global.yml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 4c4ef80) Co-authored-by: Sargun Vohra <[email protected]>

…ons#5829) * Yarn module: fix state=latest not working with global=true * fix whitespace * add changelog fragment * add integration test cases * add only tests for this PR (install+upgrade) * fix assuming default global dir * fix list() not working when global=true and name a package with no binary * remove ignores * whitespace * Update changelogs/fragments/5829-fix-yarn-global.yml Co-authored-by: Felix Fontein <[email protected]> * Update changelogs/fragments/5829-fix-yarn-global.yml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Felix Fontein <[email protected]> Co-authored-by: Alexei Znamensky <[email protected]>

Yarn module: fix state=latest not working with global=true

24ea4b9

sargunv changed the title ~~Yarn module: fix state=latest not working with global=true~~ Fix state=latest not working with global=true in yarn module Jan 13, 2023

ansibullbot added bug This issue/PR relates to a bug language module module packaging plugins plugin (any type) labels Jan 13, 2023

fix whitespace

be1b4d1

sargunv commented Jan 13, 2023

View reviewed changes

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Jan 13, 2023

russoz changed the title ~~Fix state=latest not working with global=true in yarn module~~ yarn: Fix state=latest not working with global=true Jan 16, 2023

add changelog fragment

4fa4a0a

ansibullbot added integration tests/integration tests tests labels Jan 16, 2023

sargunv force-pushed the fix-yarn-latest branch from 507a647 to 56d8a49 Compare January 16, 2023 21:14

add integration test cases

5d162de

sargunv force-pushed the fix-yarn-latest branch from 56d8a49 to 5d162de Compare January 16, 2023 21:19

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jan 16, 2023

add only tests for this PR (install+upgrade)

78ae045

sargunv force-pushed the fix-yarn-latest branch from e9c51e5 to 78ae045 Compare January 16, 2023 21:53

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

sargunv added 2 commits January 16, 2023 15:21

fix assuming default global dir

2886d4a

fix list() not working when global=true and name a package with no bi…

b2fd87f

…nary

sargunv added 2 commits January 16, 2023 15:32

remove ignores

120dd0a

whitespace

c21f1a4

felixfontein reviewed Jan 17, 2023

View reviewed changes

changelogs/fragments/5829-fix-yarn-global.yml Outdated Show resolved Hide resolved

Update changelogs/fragments/5829-fix-yarn-global.yml

76755fc

Co-authored-by: Felix Fontein <[email protected]>

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Jan 29, 2023

russoz reviewed Feb 12, 2023

View reviewed changes

changelogs/fragments/5829-fix-yarn-global.yml Outdated Show resolved Hide resolved

Update changelogs/fragments/5829-fix-yarn-global.yml

74f8dfa

Co-authored-by: Alexei Znamensky <[email protected]>

ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label Feb 12, 2023

russoz approved these changes Feb 12, 2023

View reviewed changes

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 13, 2023

felixfontein merged commit 4c4ef80 into ansible-collections:main Feb 13, 2023

patchback bot mentioned this pull request Feb 13, 2023

[PR #5829/4c4ef80c backport][stable-6] yarn: Fix state=latest not working with global=true #5992

Merged

sargunv deleted the fix-yarn-latest branch February 13, 2023 21:24

This was referenced Mar 3, 2023

yarn: #5829 causes "global dir" and "global: GTAGS not found." error ansible/ansible#80133

Closed

yarn: #5829 causes "global dir" and "global: GTAGS not found." error #6132

Closed

sargunv mentioned this pull request Mar 3, 2023

Fix Yarn global not working without explicit executable path #6138

Merged

patchback bot mentioned this pull request Mar 4, 2023

[PR #6138/3d67f518 backport][stable-6] Fix Yarn global not working without explicit executable path #6141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

yarn: Fix state=latest not working with global=true #5829

yarn: Fix state=latest not working with global=true #5829

sargunv commented Jan 13, 2023 •

edited

Loading

ansibullbot commented Jan 13, 2023

sargunv Jan 13, 2023 •

edited

Loading

russoz commented Jan 16, 2023

sargunv commented Jan 16, 2023 •

edited

Loading

sargunv commented Jan 16, 2023 •

edited

Loading

russoz commented Jan 16, 2023

sargunv commented Jan 16, 2023 •

edited

Loading

codecov bot commented Jan 17, 2023 •

edited

Loading

sargunv commented Feb 11, 2023

russoz left a comment

sargunv commented Feb 12, 2023

russoz left a comment

patchback bot commented Feb 13, 2023 •

edited

Loading

patchback bot commented Feb 13, 2023 •

edited

Loading

felixfontein commented Feb 13, 2023

yarn: Fix state=latest not working with global=true #5829

yarn: Fix state=latest not working with global=true #5829

Conversation

sargunv commented Jan 13, 2023 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Jan 13, 2023

sargunv Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

russoz commented Jan 16, 2023

sargunv commented Jan 16, 2023 • edited Loading

sargunv commented Jan 16, 2023 • edited Loading

russoz commented Jan 16, 2023

sargunv commented Jan 16, 2023 • edited Loading

codecov bot commented Jan 17, 2023 • edited Loading

Codecov Report

sargunv commented Feb 11, 2023

russoz left a comment

Choose a reason for hiding this comment

sargunv commented Feb 12, 2023

russoz left a comment

Choose a reason for hiding this comment

patchback bot commented Feb 13, 2023 • edited Loading

Backport to stable-5: 💔 cherry-picking failed — conflicts found

Backporting merged PR #5829 into main

patchback bot commented Feb 13, 2023 • edited Loading

Backport to stable-6: 💚 backport PR created

felixfontein commented Feb 13, 2023

sargunv commented Jan 13, 2023 •

edited

Loading

sargunv Jan 13, 2023 •

edited

Loading

sargunv commented Jan 16, 2023 •

edited

Loading

sargunv commented Jan 16, 2023 •

edited

Loading

sargunv commented Jan 16, 2023 •

edited

Loading

codecov bot commented Jan 17, 2023 •

edited

Loading

patchback bot commented Feb 13, 2023 •

edited

Loading

patchback bot commented Feb 13, 2023 •

edited

Loading