community.general
community.general copied to clipboard
ansible-collections
gem: Fixes #3779 installing updated gem using gem_source
SUMMARY
When specifying the gem_source, Ansible currently only checks to see if a gem with the same module is installed. If it is, the task returns OK even though the version of the file specified by the gem_source is not installed.
Modify modules/packaging/language/gem.py so when gem_source is specified, the gem_version will be set to the version
specified in the filename of the gem given by gem_source. This will cause Ansible to correctly install the specified version of the gem when it is missing, even when another version is installed.
This fixes #3779, though in a manner different then the author of that issue suggested.
ISSUE TYPE
- Bugfix Pull Request
COMPONENT NAME
gem
The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:
plugins/modules/packaging/language/gem.py:138:0: traceback: ImportError: No module named pathlib
@giner @vladimir-mencl-eresearch since you contributed to this module in the past, I would be glad if you could take a look at this PR.
Hi,
Thanks for bringing me into the discussion @felixfontein - and thanks for the contribution @cvoltz .
The patch looks straighforward: the main concern I have is whether it is OK to get the gem version from the name of the directory used as the local source. I can't tell if the directory name could be arbitrary ... and whether it would be possible to get the actual version from within the directory via some form of introspection (such as extracting it out of the gemspec file). Sorry, I'm getting out of my depth here, so these are thoughts I have not been able to confirm, just asking them aloud.
I see the regex expects the directory name to exactly follow the format <gem_name>-<version> - but the example given for gem_source in the module documentation does not follow this format (/path/to/gems/rake-1.0.gem).
So up to a Ruby expect to say whether the directory name used as the install source can be reliably used to tell the gem version - and whether the format can be relied upon. (If so, at the very least the example in the documentation should be fixed).
Hope this helps.
Cheers, Vlad
You said directory but I assume you actually meant filename ? The patch uses os.path.basename to get the filename without path so when it extracts the version, it is doing it only from the filename.
Details on requirements for gems are documented here. It is not documented but the build tools used to create a gem name the gem file with the version so introspection isn't needed to get the version. If one wanted too, however, it is possible to get the version information from the spec file within the gem file. Given that the version is always available in the filename, I opted to get it from there.
With regards to the example in the module, the author just made that number up. Rake has used the major, minor, bug fix version format since it was initially released as 0.2.3. There was never a 1.0 version of rake. Rake went from 0.9.3 to 10.0.0. See the changelog for the rationale. If you want, I can update the patch to fix the version provided in the example.
An unaddressed issue in the existing code is that the module assumes gem versions contain match \d+\.+\d+\.\d+ for the version (see get_rubygems_version function) while the reality is that the version can be longer then that. For example, pre-release gems usually add the suffix .pre to the version and follow that with the pass of the pre-release so a gem could have a version like 1.2.3.pre.4. Additionally, some developers append a build version so some versions will look like 1.2.3.5000.
I consider adding support for those extended versions (such as used by semantic versioning) as a separate feature request from this bug fix which was intended only to address the bug that the gem is not updated (irrespective of the installed version) when gem_source is specified. Given that, I followed the same version regex as the existing code so the patch fixes the versioning bug but doesn't address the existing limitation on supported versions. The vast majority of gems do use the major, minor, bug fix version format so in practice, the limitation is not a significant issue but it would be nice if it was fixed at some point in the future.
Hi @cvoltz ,
Thanks for the reply and the clarifications.
I did actually mean directory ... because I've never worked with manually downloaded gem files and always saw gems unpacked into a directory - so I assumed a locally installed gem unpacked into a directory would be passed to gem_source. Oops, I stand corrected - I see it would be a downloaded gem file.
So I see I should think of it same as if someone downloaded an RPM or DEB file. Under reasonable assumptions, the version in the filename would match "real" version "inside the file".
But this assumption would not hold in all cases. There could be use cases where the version gets dropped from the filename - e.g., someone stores the file as mypackage-lastest.gem.
More importantly, behaviour like this would have been tolerated up to now, but this change would cause the module to break on such gem_source file names.
Also, if someone had a pre-build version in the local source, the parsing expression would get the "trimmed" base version ... and would cause misbehaviour.
Overall, when processing the file, package managers (gem, dpkg, yum) would go by the version inside ... and I think this module should do the same as well.
The introspection should be trivial - though at the performance cost of invoking a remote command:
$ gem specification rake-13.0.6.gem version
--- !ruby/object:Gem::Version
version: 13.0.6
How would you feel about getting the package version via gem specification (returning YAML) ?
Cheers, Vlad
PS: As for the regular expression, I agree the change should be done in a separate PR. But, the two instances of the regexp are for very different use cases. The one used in your change has to deal with very arbitrary package versions (incl. pre-release and so) - but, if we go with introspection instead, can be discarded.
The one used in get_rubygems_version has to deal with just the version of the gem command - which can be expected to be a proper release. (And is only used once to determine whether its a version supporting --norc)
Yes, thinking about a gem like an RPM is a better way to think about it.
If you create a gem using the gem tools or you want to host it on https://rubygems.org/, the filename would have to follow the convention so I question the validity of your example but if switching to introspection will resolve your concern, I'll just do that. I'll update the example while I'm at it.
I'm a Ruby programmer, not a Python programmer. Looking at the existing code, it looks like I can use module.run_command to run an arbitrary command so something like this should work?
cmd = "gem specification"
cmd.extend(module.params['gem_source'], 'version')
(rc, out, err) = module.run_command(cmd, check_rc=True)
Then I just have to parse (just grab the second field of the second line) the out to get the version.
Hi @cvoltz ,
Yes, from what I've seen in Ansible modules, I believe this is the way to run a remote command.
Note that the output you get from gem specification is in YAML format, so ideally you'd parse it as YAML, expect a dict and get the version key from the dict.
Cheers, Vlad
This has to be slightly modified to work (in the first and second lines:
cmd = ['gem', 'specification'] cmd.extend([module.params['gem_source'], 'version']) (rc, out, err) = module.run_command(cmd, check_rc=True)
It has been a while but I do plan to update the PR with requested changes. Thought I would let you know before the ticket gets auto-closed as stale.
Sorry, just haven't had a chance to get back to it. Might be able to get back to it this month.
Please note that in #5461 the collection repository was restructured to remove the directory tree in plugins/modules/, and the corresponding tree in tests/unit/plugins/modules/. Your PR modifies files in this directory structure, and unfortunately now has some conflicts, potentially because of this. Please rebase with the current main branch to remove the conflicts.
@cvoltz This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.
@cvoltz You have not responded to information requests in this pullrequest so we will assume it no longer affects you. If you are still interested in this, please create a new pullrequest with the requested information.