Use noninteractive mode and update sha to latest master install version#11
Use noninteractive mode and update sha to latest master install version#11iloveitaly wants to merge 1 commit into
Conversation
| path: "{{ linuxbrew_prefix_user }}/bin/brew" | ||
| register: linuxbrew_user_st | ||
| become: false | ||
| become: true |
There was a problem hiding this comment.
I'm not sure if this change should be included, but it was required for me
There was a problem hiding this comment.
I think this doesn't make sense. linuxbrew_prefix_user is gathered based on the SSH user (e.g., "ubuntu"). So it makes sense to check for the existence of that path with the same user (e.g., "ubuntu").
There was a problem hiding this comment.
This one is checking if the installation exists in the current user home, so is purposedly set to false.
| linuxbrew_install_url: "https://raw.githubusercontent.com/Homebrew/install/master/install.sh" | ||
| linuxbrew_install_checksum: "sha256:45c12bcd7765986674142230fc0860f5274903adbe37d582e5537771a1bae0b8" | ||
| linuxbrew_install_checksum: "sha256:dff8f0c0ecf3f9bd288385e0750a74f25d0c540078f4c4382492c8d0337a45f9" |
There was a problem hiding this comment.
I'm not sure it makes sense to check for the checksum with a link to a file that keeps changing.
Either we use that generic URL and don't check the checksum, or we use an URL like this:
linuxbrew_install_url: "https://raw.githubusercontent.com/Homebrew/install/bcc0e9c0ac3fa957d711c6b8e05060940a47b162/install.sh"
linuxbrew_install_checksum: "sha256:41d5eb515a76b43e192259fde18e6cd683ea8b6a3d7873bcfc5065ab5b12235c"
There was a problem hiding this comment.
Removing the default checksum and checking it only if defined might be more convenient.
One could still override the URL and add checksum if they needed the added security.
|
|
||
| - name: Install Linuxbrew | ||
| command: sh -c "{{ linuxbrew_install_tmp }}/install.sh" | ||
| command: sh -c "NONINTERACTIVE=1 {{ linuxbrew_install_tmp }}/install.sh" |
|
@huyz want to submit changes/GH suggestions for some of the comments? Then I can merge them into the branch. |
|
Until @markosamuli shows that he's got time to merge in all the PRs and make a new release, I'm just going to work on my own fork, cleaning up a lot of different issues. For example, I don't want to favor pinning the installer script (as could be misinterpreted above); I'm giving users a choice of also just using a generic URL and not doing a checksum. |
|
I've removed checksum from the role in #12 |
|
@iloveitaly I've refactored the role a bit in release v2 so these changes need some updates. If you rebase against the |
|
@markosamuli Thanks for the update. I just had a chance to re-use this role. The other part from this PR that's still needed is to pass |
iloveitaly
force-pushed
the
noninteractive
branch
from
9f6d868 to
88936f0
Compare
|
updated branch to just make the NONINTERACTIVE=1 change! |
|
@huyz friendly reminder here! |
|
@iloveitaly Did you mean to ping @markosamuli ? |
|
Current change should be merged IMO as just tested and |
4 participants
No description provided.