fix: ensure shims get created when data dir has spaces

[mook-as]

Summary

If ASDF_DATA_DIR has a space in the path (by default, this means asdf itself was installed to a path with a space), then the shims will not be created correctly in bash. This PR fixes the issue.

Other Information

I have only checked up to the actual executable (in my case, node) running — I do not yet know if there are other issues related to spaces in the path.

I'm using extra variables for the lists (which contain newlines) because POSIX shell wouldn't support while read … done; while < <(command) (and is caught by the tests).

[Stratus3D]

Thanks for the PR @mook-as ! Thank you for catching this, it's definitely something we need to get fixed. Do you think you'd be able to write a basic unit tests for these two functions to prevent this bug from being introduced again in the future?

[mook-as]

Thanks for the fast review!

I've made changes to make it match existing code better, as well changed the tests to just default to having a space in ASDF_DIR to catch any issues. This does involve changing a lot of the tests to quote things properly; sorry that the PR is so invasive.

I couldn't figure out a nice way to fix shim_exec.bats, though, so I've opted that one out of having spaces for now. This is because it tests having a tool version of path:…, but it's unclear from the documentation what should happen if there's a space in the path, given that it's a file with space-delimited fields.

[jthegedus]

This does involve changing a lot of the tests to quote things properly; sorry that the PR is so invasive.

Is there a way to add specific test cases for the code paths you modified to support the space, leaving the large existing set of tests for without a space in place?

[mook-as]

Okay, I've changed things so that having spaces in the test is opt in (per test setup), and only do it for the reshim tests. Would that be better? I still fixed all the test within the one file, but it's not quite as big (and I can start doing follow up PRs slowly until the whole thing is covered).

[jthegedus]

Okay, I've changed things so that having spaces in the test is opt in (per test setup), and only do it for the reshim tests. Would that be better?

Yes, I think so. We don't need to have every single asdf test cover both cases, just as long as the modified codepaths in

lib/commands/command-reshim.bash 
lib/utils.bash 

are covered with the existing cases and your new cases.

[Stratus3D]

This does involve changing a lot of the tests to quote things properly; sorry that the PR is so invasive.

Is there a way to add specific test cases for the code paths you modified to support the space, leaving the large existing set of tests for without a space in place?

I had the exact same thought as @jthegedus.

Checking the updated code now.

[jthegedus]

@mook-as are you happy with this to be merged?

[mook-as]

@jthegedus I am; I can't figure out what's wrong with the WSL tests, though.

[jthegedus]

They do not work. We do not expect them to either. It is my fault they are there. I will be removing those test cases shortly. We do want to test in other envs like wsl2 eventually.