rvm / rvm1-ansible

The official ansible RVM role to install and manage your Ruby versions.
MIT License
271 stars 136 forks source link

Fixes bare variables in conditionals (deprecation warnings with Ansible 2.8+) #204

Closed jljouannic closed 5 years ago

jljouannic commented 5 years ago

This PR is about fixing some warning messages showing up in the console when running the role with ansible ≥ 2.8

pkuczynski commented 5 years ago

@derekgottlieb @lpaulmp what you think?

derekgottlieb commented 5 years ago

Reading through a related discussion here, it sounds like this is the right approach for ansible 2.8+.

pkuczynski commented 5 years ago

Thanks for the confirmation. I will then merge it in.

pkuczynski commented 5 years ago

@jljouannic could you please add a CHANGELOG entry and fix the failing build?

jljouannic commented 5 years ago

Sure, I can add an entry in CHANGELOG.

I have no idea why the travis build crashes. A test named tests_ubuntu16_1 seems to fail:

{
    "changed": true,
    "cmd": "/tmp/rvm-installer.sh stable --path /home/user/.rvm --auto-dotfiles --user-install",
    "delta": "0:00:01.218399",
    "end": "2019-07-11 10:01:45.642142",
    "failed": true,
    "msg": "non-zero return code",
    "rc": 22,
    "start": "2019-07-11 10:01:44.423743",
    "stderr": "curl: (22) The requested URL returned error: 403 Forbidden\ncurl: (22) The requested URL returned error: 410 Gone\ncurl: (22) The requested URL returned error: 404 Not Found\ncurl: (22) The requested URL returned error: 404 Not Found",
    "stderr_lines": [
        "curl: (22) The requested URL returned error: 403 Forbidden",
        "curl: (22) The requested URL returned error: 410 Gone",
        "curl: (22) The requested URL returned error: 404 Not Found",
        "curl: (22) The requested URL returned error: 404 Not Found"
    ],
    "stdout": "Turning on auto dotfiles mode.\nTurning on user install mode.\nDownloading https://github.com/rvm/rvm/archive/.tar.gz\n\nCould not download 'https://github.com/rvm/rvm/archive/.tar.gz'.\n  curl returned status '22'.\n\nDownloading https://bitbucket.org/mpapis/rvm/get/.tar.gz\n\nCould not download 'https://bitbucket.org/mpapis/rvm/get/.tar.gz'.\n  curl returned status '22'.",
    "stdout_lines": [
        "Turning on auto dotfiles mode.",
        "Turning on user install mode.",
        "Downloading https://github.com/rvm/rvm/archive/.tar.gz",
        "",
        "Could not download 'https://github.com/rvm/rvm/archive/.tar.gz'.",
        "  curl returned status '22'.",
        "",
        "Downloading https://bitbucket.org/mpapis/rvm/get/.tar.gz",
        "",
        "Could not download 'https://bitbucket.org/mpapis/rvm/get/.tar.gz'.",
        "  curl returned status '22'."
    ]
}
pkuczynski commented 5 years ago

Ah, this is another issue we are working on with RVM, so we can ignore it for this PR :)

pkuczynski commented 5 years ago

Merged. Thanks!

AwolDes commented 5 years ago

Hey guys

I use this ansible role as apart of my build pipeline, and 4 days ago (when this was released), my builds started to fail. I'm running Ubuntu 16.04, and can see that there is discussion of a test failing for Ubuntu 16.1, could this be related? Attached is an image of the error being raised, and a gist of the formatted payload. I'm not 100% sure this change is related, but it does seem coincidental and wanted to check in.

image

Any help is appreciated, cheers!

Edit: I've just pinned the version to v2.1.2, the the build works, so it seems that this update has had unexpected changes. Let me know if you need anymore info!

pkuczynski commented 5 years ago

@jljouannic can you have a look? Otherwise, I will need to revert this change...

jljouannic commented 5 years ago

I think the problem encountered by @AwolDes and the failing test in build job are the same.

I might be wrong, but I don't think it has anything to do with the fix I proposed in this PR.

@pkuczynski Didn't you mentioned this problem was related to another issue you wre working on with RVM?

pkuczynski commented 5 years ago

That's true. But I could not recognize the same error from his output. @AwolDes I will try to release new rvm today, hopefully this will fix your builds.

jljouannic commented 5 years ago

You're right, the errors are not the same. However in both cases, problem seems related to incomplete URL or path:

pkuczynski commented 5 years ago

Then master contains the fix. Will be released to stable soon.

AwolDes commented 5 years ago

@pkuczynski @jljouannic thanks for getting a fix in for this! Really appreciate it.

pkuczynski commented 5 years ago

I am glad you like it! You might consider a small donation for the team https://opencollective.com/rvm