dlopez-akalam
commented
6 years ago Related original issue on acquia/blt https://github.com/acquia/blt/issues/2610
Closed dlopez-akalam closed 6 years ago
dlopez-akalam
commented
6 years ago Related original issue on acquia/blt https://github.com/acquia/blt/issues/2610
FinBoWa
commented
6 years ago @dlopez-akalam why not install ansible with brew or through system package manager?
If ansible works from the sources only with root then you haven't set the right permissions to the build files.
Personally I'd say that this would be out of scope relating drupal vm as it requires functional ansible, vagrant and virtualbox. And as your ansible wont run without elevated privileges so I'm not sure is this the right place to ask help.
FinBoWa
commented
6 years ago Thought i did a quick snoop trough This is the part that gets the version number:
And this is probably failing:
so I'd presume Gem::Version.new cant handle some of the extra strings that the source build ansible is adding to the version line of ansible --version
But is that something to fix on drupalvms files vagrant rb file is the interesting bit > I'd say no unless ansibles builds start to output some more info on the version part
rsanzante
commented
6 years ago I think this is a Drupal VM bug as long as it's unable to parse the Ansible version. I've created a PR that changes the regexp to ignore part in parenthesis, see https://github.com/geerlingguy/drupal-vm/pull/1726
geerlingguy
commented
6 years ago This is fixed in @rsanzante's PR #1726 - thanks!
(It's currently in Drupal VM dev/master only, I have not yet created a release with this fix).
philipnorton42
commented
6 years ago Sorry, I don't think this has fixed the issue. Or it may have created a different issue? I had some problems setting up a new DrupalVM box this morning from a master branch version of DrupalVM. I eventually tied it down to this change. Reverting the change appeared to fix this issue.
Here is the error I was seeing. It looks like the whole ansible version output is being included as the version number.
There was an error loading a Vagrantfile. The file being loaded
and the error message are shown below. This is usually caused by
a syntax error.
Path: /Users/someone/Sites/project/Vagrantfile
Line number: 0
Message: ArgumentError: Malformed version number string 2.4.3.0
config file = None
configured module search path = [u'/Users/someone/.ansible/plugins/modules',
u'/usr/share/ansible/plugins/modules']
ansible python module location = /Library/Python/2.7/site-packages/ansibleexecutable location = /usr/local/bin/ansible-playbook python version = 2.7.10`
Here is the output of my ansible --version
→ ansible --version
ansible 2.4.3.0
config file = None
configured module search path = [u'/Users/someone/.ansible/plugins/modules',
u'/usr/share/ansible/plugins/modules']
ansible python module location = /Library/Python/2.7/site-packages/ansible
executable location = /usr/local/bin/ansible
python version = 2.7.10 (default, Feb 7 2017, 00:08:15) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)]If you need any more information then let me know.
rsanzante
commented
6 years ago @philipnorton42 What OS are you using?
oxyc
commented
6 years ago I guess it's matching the trailing whitespace:
/^[^\s]+ ([^\(]+).*$/should be
/^[^\s]+ ([^\(\s]+).*$/Maybe worth making the trailing string explicitly lazy as well?
/^[^\s]+ ([^\(\s]+).*?$/Edit: or just remove the trailing string, I don't think there's a reason for it:
/^[^\s]+ ([^\(\s]+)/I asked for the OS because a colleague with MacOS today reported an error with this regular expression.
The problem is the last number, for example, the '0' in 2.4.3.0. In OSX this triggers an error while in Linux it doesn't.
He solved using this regexp:
/^[^\s]+ ((?:\d\.){2}\d).*$/
See it in action in this link: http://rubular.com/r/J4TBshh16b
@oxyc Those expression matches the last number as well so I guess they don't solve the issue.
I'm going to create a new PR.
rsanzante
commented
6 years ago PR done! I can't reopen this issue, should I create a new issue!
Sorry for the first regexp :-1:
jpmc
commented
6 years ago I wanted to chime in I had the exact same error with the same version of Ansible, except on Ubuntu 16.04.2 LTS, Linux 4.10.0-28-generic kernel.
Though I haven't tested the new regex yet, I thought it might be useful to at least confirm the issue is present on more than one OS.
philipnorton42
commented
6 years ago @rsanzante For what it's worth I am running osx Sierra. Thanks for the update though, I will test the regex in the morning (UK time).
FinBoWa
commented
6 years ago Alternative way for matching: https://stackoverflow.com/questions/8955657/regex-pattern-to-extract-version-number-from-string eg.
\d+(\.\d+)+That seems to just match any continous numbers and commas and atleast it matches the number part for something like 5.0.0-patch . Directly that doesn't group like what you have but I thought worth to point out as an alternative.
rsanzante
commented
6 years ago @FinBoWa That expression would match 2.4.1.0 instead of 2.4.1, so the error would still be present. Proposed regexp in https://github.com/geerlingguy/drupal-vm/issues/1725#issuecomment-374749602 would match 5.0.0 from "ansible 5.0.0-alpha", see http://rubular.com/r/J4TBshh16b.
oxyc
commented
6 years ago The old regex before #1726 did parse 2.4.3.0 correctly though?
rsanzante
commented
6 years ago @oxyc, no because it parsed 2.4.1.0 instead of 2.4.1. Semver only has three numbers, not four, see https://semver.org/.
I guess the code that drupal vm depending on OS enforces the three numbers and triggers an error if it receives four numbers.
oxyc
commented
6 years ago But this has been working fine since it was introduced in https://github.com/geerlingguy/drupal-vm/commit/ae4c8031d7ff6f8e362595bbca2acdc7e2594167, https://github.com/geerlingguy/drupal-vm/commit/350851b573f16910b509b49d8a0ffd95dbea703f so I don't think the 4th digit can be the cause.
FinBoWa
commented
6 years ago ansible_version = "2.4.1.0-patch1"
req = Gem::Requirement.new(">2.4")
if req.satisfied_by?(Gem::Version.new(ansible_version))
puts "yes"
else
puts "no"
endThat works but if you put something else there you get a similar error
ansible_version = "2.4.1.0-patch1 thiscausestheerror"
req = Gem::Requirement.new(">2.4")
if req.satisfied_by?(Gem::Version.new(ansible_version))
puts "yes"
else
puts "no"
endSo the amount of should not be the cause of the error and even the -patch sring works . So the rexgp is getting something more than just the actual version, maybe a cr char or something?
rsanzante
commented
6 years ago @oxyc I don't have an answer for that. It strange because old regexp from https://github.com/geerlingguy/drupal-vm/pull/1726 matches the same as regexp from https://github.com/geerlingguy/drupal-vm/commit/410e8788a719777f4f23fd93b46c1b5b5d422091.
Anyway, I think version detection should stick to Semver, just 3 numbers. That's what new PR does (https://github.com/geerlingguy/drupal-vm/pull/1739).
See the three regexs in action
@FinBoWa I don't see any garbage chars in the matches from rubular.com. Maybe their engine is not so similar to Ruby's engine?
FinBoWa
commented
6 years ago I'm just pointing out that you are dropping version information on the wrong premises that the fourth digit was the cause of the issue and at least for me it doesn't seem that way as the ruby Gem::Version.new(ansible_version) is cabable of handling it > somethign else is probably causing the issues.
Anyways: personally im not happy dropping the fourth as its just mere forcing a string to be "semver" if it wasnt that already the string is still valid version with any number of digits.
FinBoWa
commented
6 years ago But im not part of the drupal vm team so these are just my point of view so take them as that way :D
oxyc
commented
6 years ago @philipnorton42 @jpmc which ruby and gem versions do you have?
ruby --version
gem --versionI can confirm I get the same error on macOS with:
drupal-vm-test master*: ruby --version
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin17]
drupal-vm-test master*: gem --version
2.7.4Ensuring the trailing whitespace isn't in the version fixes it for me. I couldn't reproduce this with just a simple trailing whitespace in tests though, maybe it's another space character? In which case \s would cover that.
# Return the ansible version parsed from running the executable path provided.
def ansible_version
/^[^\s]+ ([^\s]+)/.match(`#{ansible_bin} --version`) { |match| return match[1] }
end@rsanzante any chance you and your colleague could test the alternative PR i opened at #1740? If you can confirm it works it's always nicer to be able to lock the full Ansible version :) As I couldn't find the exact cause for the bug I'm still not entirely confident so would appreciate if you could give it a try.
BanovMiroslav
commented
6 years ago I have the same issue as @philipnorton42. Either of patch #1739 or #1740 fixes the issue for me.
Issue Type
Your Environment
Your OS
Full console output
Summary
I get this error when try to start a virtual machine, with ansible installed via git. The problem seens to be related with the fact that drupalvm is not able to process the output of ansible --version command.