test: reshim of plugins installed by path

[alexezio]

Summary

change sed delimiter character from / to |

to prevent reshim local path error(sed delimiter character conflict)

Fixes: #1286

Other Information

maybe we could use a better delimiter character for this scenario?

[jthegedus]

Can you add a test case for regression testing?

[alexezio]

Can you add a test case for regression testing?

Hi, I add a test case for testing, could you please review the change?

[Stratus3D]

The | pipe character is permitted in file names so this fix may not be sufficient for all cases.

[alexezio]

don't allowed filename special character:

• Windows (FAT32, NTFS): Any Unicode except NUL, \, /, :, *, ?, ",<,>, |. Also, no space character at the start or end, and no period in the end.
• Mac(HFS, HFS+): Any valid Unicode except : or /
• Linux(ext[2-4]): Any byte except NUL or /

so I think | may not be the best character in this scenario, but I can't find the better one~, and most of us won't use | as a part of filename

[jthegedus]

We started validating plugin names in #1010, perhaps we should be validating dir names before we create/modify them in asdf so we can be more confident about which special char we use here.

I also think once we decide which to use we should use it consistently across the project (a follow up PR seems best)

[Stratus3D]

What if we used Bash substring replacement for this? The Bash expression can be parameterized. (see https://tldp.org/LDP/abs/html/string-manipulation.html and scroll down to the Substring Replacement header).

Something like this (I haven't had a chance to test it):

line=$(...)
match='exec '
replacement="# asdf-plugin: ${plugin_name} ${version}\\"$'\n''exec "
updated_line=${line/$match/$repl}

[alexezio]

@Stratus3D @jthegedus
Hi guys:
Maybe I have found a better way to solve this problem.
Insert a new line before the last line rather than regex and replace the previous line with multi-lines.
the code below:

sed -i.bak -e '$i\'$'\n'"# asdf-plugin: ${plugin_name} ${version}" "$shim_path"

$i\'$'\n' means inserting a new line before the last line. this flag works on both Linux and macOS. so we insert the plugin info to the new line without regex.
We could avoid the conflict problem with sed delimiter character by using this approach.
could you guys review this change? thx.

[alexezio]

another question need help~
I pass the test suite case in my MacBook, but can't be passed here, is there any approach to solve this issue?

[Stratus3D]

another question need help~ I pass the test suite case in my MacBook, but can't be passed here, is there any approach to solve this issue?

This can be challenging. It looks like the new test is failing in Github actions because one of the commands it runs exited with a non-zero exit status. I typically echo the $output of the command before asserting the exit code is 0 when debugging issues like that. Usually $output holds the answer to the issue, and BATS will take whatever you print and render it to the test output so it will be visible in the runner output. Hope this helps!

[alexezio]

can anyone review the new change?

[jthegedus]

@alexezio please review the failed status checks

[alexezio]

@jthegedus
hello, jthegedus:
I fix the unit-test issue, could you please approve for auto-test again?

[alexezio]

It seems like all unit-test are passed, so could this pr be approved?

[jthegedus]

@alexezio Sorry for my absence and delay here. Can you address the Shellcheck issue?

In lib/commands/reshim.bash line 93:
sed -i.bak -e '$i\'$'\n'"# asdf-plugin: ${plugin_name} ${version}" "$shim_path"
^-- SC1003 (info): Want to escape a single quote? echo 'This is how it'\''s done'.

For more information:
https://www.shellcheck.net/wiki/SC1003 -- Want to escape a single quote? ec...

[alexezio]

@jthegedus hi, sorry for my absence. I check the shellcheck error, and I think that is not a issue.
The backslash \ after $i means that insert the text after the backslash to the last line without needed a /pattern/ for the last line. so it can't be quoted. the $ symol that surrended with single quote string represent the end of old lines.
And I try to solve the shell format, but it doesn't work. So could you pelease review this?

[jthegedus]

From @dylan-chong from - #1311 (comment)

@alexezio This means that the change to the sed pattern in your PR #1287 is not longer required. But it would still be great to merge the test that you wrote in your PR. I have checked and that test passes in this branch, but I don't want to steal your contribution so I'll let you commit that :)

Let's:

• remove the changes here to lib/commands/reshim.bash
• rebase
• resolve conflicts for test/

and get this test case in and close out this PR.

[jthegedus]

Updated this to just be a new test case as per #1287 (comment)

Thanks 🙇

[Stratus3D]

Thanks for writing the test for this @alexezio ! And thanks for getting this merged @jthegedus !