Simplify install.sh (take 2)

[ghost]

You were depending on a non-standard cp flag (-v) and requiring two external utils (sed and tail) to do something really simple.
And then, because some unix utilities print 'a' -> 'b' while others print a -> b (and others print nothing because -v isn't posix) you would check if the output of cp contained the quote or not and remove it. (depending on even more tools like awk).

Forgive me if I'm wrong, but this deserves a serious ''what the hell''
Come on...

[yuyichao]

Please update the same pr instead of opening new one everytime.

[ghost]

Sorry, I'll keep that in mind next time.

[tkelman]

@staticfloat #8024 was 2 years ago, but do you have any recollection why install.sh isn't just using $DEST from the last input argument?

[tkelman]

Nevermind Elliot. @albap see all the Travis failures.

[ghost]

I don't understand why it fails. Can you help?

[staticfloat]

I believe it's because $DEST can be a file or a directory, so the previous (admittedly not great) approach was to look at the output of cp in order to clean the "true" destination address.

If that is the case, a [[ -d "$DEST" ]] check should be enough for you to know whether you need to set DESTFILE=$DEST/$(basename $SRC) or just DESTFILE=$DEST.

[ghost]

@staticfloat
[[ -d "$DEST" ]] is a bashism, no?
One of the reasons we are doing this is to make Julia .sh scripts posix-compliant.
If you want to work on this, I can close this issue.
I thought it'd be as easy as using $DEST, looks a little bit of cruft is indeed needed.

[tkelman]

would chmod -R work or catch too many other files?

[ghost]

Passed Travis CI.
Yay!

[tkelman]

edit: wrong pr, sorry

[staticfloat]

@albap I'd really like to preserve normal install semantics when possible, so we should support installing to a file as well as a directory.

To test for a directly without relying on bashisms, just get rid of one of the square brackets, e.g:

if [ -d "$DEST" ]; then
  DESTFILE="$DEST/$(basename "$SRC")"
else
  DESTFILE="$DEST"
fi

Also, please make sure to always quote filename variables as they may contain spaces. I realize we weren't doing that before, but we probably should be.

[ghost]

@staticfloat
Since you were the author of install.sh and know all little details it need, would you mind making a PR with the fix you mention? Or I can do it myself here if you prefer too...
Let's just try to keep things simple this time, the former install.sh was... ugh. Unnecessarily complicated?

[staticfloat]

the former install.sh was... ugh. Unnecessarily complicated?

Agreed, and I'm glad that you are cleaning it up! I would prefer it if this PR was finished and merged, as I'd like to reward you for your work and attention for detail. Additionally, it's actually helpful for us to have new people helping out, even with small matters like this, so don't get discouraged when we bring up a multitude of small issues that tweak and twist the PR; that means we're paying attention to your work and ensuring that it will pass through the multitude of requirements necessary to get something like this working on lots of platforms.

[ghost]

Thanks for the kind words, I really appreciate it and all the attention given to this!

[tkelman]

@staticfloat satisfied with this version? my ocd would prefer if the indentation were consistent, but functionality-wise it seems to work.

[ghost]

@tkelman I think we shouldn't care about indentation consistency right now. All .sh scripts in the repository have their own indentation and naming style.
For example relative_path.sh and filterArgs.sh. Should we use CamelCase or under_scores?
Some scripts have 2-spaces indentation, some have 4-spaces and some use .
I think when we finish dealing with bashism and get all scripts to be posix-compliant, we can work on naming and indentation consistency across the code-base.
For now I think it's better to focus on getting it working.

[tkelman]

at least within the same short script it makes sense to be internally consistent. I agree that global style tweaks can be done separately.

[ghost]

Is it okay now?
I think we don't have to wait for Travis CI to merge, since I only added 2 spaces and that doesn't change functionality.

[staticfloat]

Thanks for your contribution @albap!