Sdk: dotnet-install.sh can't install to path containing spaces

[dagood]

Non-blocking bug, but I noticed this when making an upstream shell script handle spaces properly.

Steps to reproduce

Run scripts/obtain/dotnet-install.sh --install-dir a\ b\ c

Expected behavior

The .NET CLI is installed into a folder in the current directory called "a b c".

Actual behavior

The .NET CLI is installed into a folder in the current directory called "a".

Adding --verbose shows a step where the path is trimmed:

dotnet-install: Calling: resolve_installation_path a b c
dotnet-install: install_root=a

Environment data

OSX bash GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin15)
Ubuntu Trusty bash GNU bash, version 4.3.11(1)-release (x86_64-pc-linux-gnu)

[blackdwarf]

@dagood a good catch. Will take a look.

[blackdwarf]

@dagood what is the version of the shell you are using?

[dagood]

OSX: bash --version: GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin15)

Ubuntu: bash --version: GNU bash, version 4.3.11(1)-release (x86_64-pc-linux-gnu) (the script fails on set -o pipefail when running specifically with sh.)

Added this to the desc.

[benknoble]

Repro'd with

$ bash --version
GNU bash, version 4.4.12(1)-release (x86_64-apple-darwin15.6.0)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

on OSX when trying to build roslyn (see e.g. dotnet/roslyn#22259) and would be happy to submit a patch.

[benknoble]

I believe the problem lies here (though possibly elsewhere)

get_user_install_path() {
    eval $invocation

    if [ ! -z "${DOTNET_INSTALL_DIR:-}" ]; then
        echo $DOTNET_INSTALL_DIR # <-- No quotes
    else
        echo "$HOME/.dotnet"
    fi
    return 0
}

# args:
# install_dir - $1
resolve_installation_path() {
    eval $invocation

    local install_dir=$1
    if [ "$install_dir" = "<auto>" ]; then
        local user_install_path=$(get_user_install_path) # <-- also no quotes
        say_verbose "resolve_installation_path: user_install_path=$user_install_path"
        echo "$user_install_path"
        return 0
    fi

    echo "$install_dir"
    return 0
}

due to a lack of quotes. These are lines 470-496 in cli/scripts/obtain/dotnet-install.sh

[benknoble]

@dagood were I to work on a patch, would you prefer a single, short change to fix this or something more like dotnet/roslyn#22320 where I go through and fix any potential quote problems?

[dagood]

I don't work a whole lot in this repo, not sure. @livarcocc ?

[benknoble]

Quoting the variable expansions and command substitutions in just dotnet-install.sh is ~86 lines of change. Not sure if that counts as a localized change or not.

[benknoble]

Hoping someone can help here... I can't figure out why cp is failing.
The line being executed is something like

cp -R  /tmp/dotnet.F0H6bgo7B/ThirdPartyNotices.txt /mnt/j/workspace/dotnet_cli/master/debug_ubuntu16.04_x64_prtest/.dotnet_stage0/x64/ThirdPartyNotices.txt

which is reporting this error in the tests

cp: target '/mnt/j/workspace/dotnet_cli/master/debug_ubuntu16.04_x64_prtest/.dotnet_stage0/x64/ThirdPartyNotices.txt' is not a directory

However, a simple attempt to repro that (make a directory with some files and an empty directory, then copy the a .txt file over using similar command line) isn't failing. If the directory to copy to doesn't exist, we should get a No such file or directory, which isn't the case here. Thoughts?

[dagood]

The problem is quoting $override_switch. When it's the empty string (rather than -n), cp receives it as another file arg. Try this for a repro in an empty dir:

touch test.txt
mkdir d
cp "" test.txt d/test.txt
# cp: target 'd/test.txt' is not a directory

set -x is very useful for diagnosing this kind of thing. For that cp "" test.txt d/test.txt, it makes the shell echo + cp '' test.txt d/test.txt.

Not quoting there is probably the desired behavior, since it lets you pass in an override like -n -s as well without problems.

[benknoble]

Makes sense. Thanks

[johnbeisner]

@dagood
@benknoble
The https://dot.net/v1/dotnet-install.sh public location has been updated with this fix.