Make the install script a little bit safer when being piped

[glenjamin]

• I have read Contribution Guidelines.
• I have checked for similar issues and haven't found anything relevant.
• (ish, but minor) This is not a security issue (which should be reported here: https://circleci.com/security/)

If the connection cut out midway through, the script would partially execute.

Wrapping in a function ensures that nothing will be executed unless the file is fully downloaded

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.41 ⚠️

Comparison is base (bf399b8) 28.00% compared to head (f1cd9e8) 27.59%.

❗ Current head f1cd9e8 differs from pull request most recent head be8c3cc. Consider uploading reports for the commit be8c3cc to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #352      +/-   ##
===========================================
- Coverage    28.00%   27.59%   -0.41%     
===========================================
  Files           42       21      -21     
  Lines         4795     2732    -2063     
===========================================
- Hits          1343      754     -589     
+ Misses        3267     1892    -1375     
+ Partials       185       86      -99     

see 43 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[eric-hu]

Restating how this works to make sure I'm understanding it right.

With the function wrapping everything, trap would exit the install function on an ERR signal from any command within install. Without the the install wrapper, trap would call error after a command like curl raised an ERR signal, but the next line of code would still execute. Does that match what happened before and after?

[glenjamin]

This shouldn't make a difference to trap - I did a local test of making a command fail and it printed the error as before.

The main difference now is that when you run the script as curl <url> | bash, if the download aborts halfway through, the part of the script downloaded so far would still be passed to bash.

Wrapping in a function like this ensures that instead of executing half of the commands, we only define half of a function - nothing will be executed.

[taxonomic-blackfish]

Throwing it out there that you now how nested functions which feels like an anti-pattern. I think should just shuffle this a bit to move error above install and also get consistent indentation so it's obvious the rest belongs in the function. And, a pedantic nit, but I prefer such "do all the things" functions to be called main to really capture intention.

[am-bui]

@glenjamin have you had a chance to consider @taxonomic-blackfish 's suggestions? We'd like to pick this pr back up but would also like to make sure the logic is up to date and applicable.

[christian-stephen]

Perhaps this is platform/shell specific, but this seems to introduce a bug because of a name collision with the install function and the system's install command. When I pipe the script into bash, it loops indefinitely as the install function recursively calls itself:

Starting installation.
Installing CircleCI CLI v0.1.26646
Installing to /usr/local/bin
Starting installation.
Installing CircleCI CLI v0.1.26646
Installing to /usr/local/bin
Starting installation.
Installing CircleCI CLI v0.1.26646
^C⏎