fix: `reshim` did not rewrite executable path

[dylan-chong]

Summary

Fixes reshimming not working by always recreating the file from scratch. We grab the existing plugin versions from the existing shim file and squish those into a brand new shim file. This solves the problem of updating asdf from homebrew causing the executable path to be wrong, and not fixable by asdf reshim

Regenerating the shim file from scratch is much simpler, no additional tests required like https://github.com/asdf-vm/asdf/pull/1287.

Fixes: https://github.com/asdf-vm/asdf/issues/1115 Fixes: #1231 (dupe) Fixes: #1286 Makes change in #1287 not needed, although the test is still useful

TLDR for comment section

Read from this comment down https://github.com/asdf-vm/asdf/pull/1311#issuecomment-1200301398

Other Information

[alexezio]

hello, @dylan-chong : What if the execution have more than one shim path?
if we re-generate the reshim file, the shim file will only contains one shim path other than all of their. This will cause issue for origin shim path. could you please run the bats for verify the results?

[dylan-chong]

Hey

@alexezio you're right it is breaking some tests. Why was it implemented as using regex to accumulate data in a file? That gives me the heebie jeebies. Why not take a list of Also why does the shim have to list the versions of the plugin?

If we could simplify the API such that we remove all shim files and regenerate them all (so asdf reshim takes no arguments) then that would simplify things massively. There would be only one code path:

1. delete all existing shims
2. generate shims for all versions of each executable for each plugin

There's no need to keep files around and do complicated regexes on them if we can regenerate them from scratch every time (which is what you have to do when upgrading asdf with homebrew anyway). Unless, there's something I'm missing?

Happy to have a go at implementing this idea if you'd like ^. Think it'd be a good improvement to make

[alexezio]

hey dylan @dylan-chong : In my opinion, asdf is stateless, so the shim file have also saved the program version state. If we regenrate them from scrach, We have no where to fetch version state except the file itself. although regex is annoying, It can modify the version state without break them(by increment). I am a newbee for asdf, Maybe there is a better approach to reshim. but for now, I vote for regex... thanks~

[dylan-chong]

@alexezio but you can delete the shim files manually and regenerate the files from scratch? Does this not put the version comments in the file? Is it not possible to just list all versions of the plugin?

if you would prefer to keep the regex I could have a go at replacing everything except the existing version list using the regex. This would fix the issue in the description.

[dylan-chong]

Cc @Stratus3D @jthegedus which option would you prefer:

1. simplify the reshim API and have the command re-generate all files from scratch [for any given plug-in]. Would need a way to be able to list all versions of the plugin supported by the executable
2. Replace sed with grep so in the existing function we grab out the existing versions and copy them over into a freshly generated file, which is written over the top. This is less error prone since we can use simple grep patterns and generate most of the file from scratch

IMO 1 is better long term , but requires more work now.

[dylan-chong]

The easiest option was to implement no 2 (see above), which I have committed to this branch.

@Stratus3D @jthegedus This is ready to merge

@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 :)

[jthegedus]

I am not sure why the tests didn't run here, but merging master so they can. We have some banned command usage which I believe this solution uses, so might need an update.

[jthegedus]

@Stratus3D looking at our open PRs we have 3 which attempt to resolve this issue. Are you able to check this as it seems to me to be the best option. I am concerned about performance, but here correctness is preferred, we can come back and fix perf.

[Stratus3D]

@Stratus3D looking at our open PRs we have 3 which attempt to resolve this issue. Are you able to check this as it seems to me to be the best option. I am concerned about performance, but here correctness is preferred, we can come back and fix perf.

I completely agree. Let's try to get this shipped. I can re-assess performance later if necessary. Thanks @jthegedus !

[jthegedus]

Let's see how it goes 😀