Fix update path logic

[jonnyz32]

Changes

Ensure the user has /usr/bin, and /QOpenSys/usr/bin on their PATH

Ensure that /QOpenSys/pkgs/bin is on the path and occurs before /usr/bin and /QOpenSys/usr/bin. This was not guaranteed before as if /usr/bin or /QOpenSys/usr/bin was not on path, the code would think the order of the path variables was wrong, even though the problem was a missing path requirement

When rewriting the users bashrc, with a new PATH, append /QOpenSys/usr/bin and /usr/bin AFTER the original PATH, to make sure they exist, but do not overwrite the priority of other existing paths.

[worksofliam]

@jonnyz32 Can you please provide a sample .profile that we can use to test this?

[jonnyz32]

@worksofliam How about this?

export PATH="/QOpenSys/pkgs/bin:/opt/homebrew/opt/libpq/bin:/Users/zakjonat/Downloads:/usr/local/mysql/bin:/Users/zakjonat/.nvm/versions/node/v20.0.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/usr/local/share/dotnet:/home/AURORAJPN2/.dotnet/tools:/usr/local/go/bin:/opt/podman/bin:/opt/homebrew/opt/libpq/bin:/Users/zakjonat/Downloads:/usr/local/mysql/bin:/Users/zakjonat/.nvm/versions/node/v20.0.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin"

[worksofliam]

@jonnyz32 Any chance you can fix the conflict here?

[jonnyz32]

@worksofliam Done

[worksofliam]

@jonnyz32

Had some deliberations, and we think it better that, instead of ignoring this entry, that really it might be required by Code for IBM i. We actually use some executables in that directory in Code for IBM i, and we don't qualify the paths, so we are kind of assuming it might be on there.

Therefore, it might be a risk if we merge this and have things stop working. It is recommended to append /usr/bin to your list so other items are picked up first.

[jonnyz32]

@worksofliam I have made some changes to my pr to now ensure the user has these required paths, instead of ignoring it. Let me know what you think.

[worksofliam]

@jonnyz32 Thanks - it's quite a change from the original PR requirement. Any chance you can update the PR body to reflect what the PRs intention is?

[jonnyz32]

@worksofliam Done

[jonnyz32]

@worksofliam I intended /QOpenSys/usr/bin:/usr/bin to come after $PATH as I didn't want it to overwrite the priority on their original path. I am wondering why the $PATH variable wasn't expanded in the bashrc though, or is it supposed to be like that?

[worksofliam]

@jonnyz32 Yes, we don't expand the $PATH variable, we just place it in like that so we don't affect the users own PATH.

https://github.com/codefori/vscode-ibmi/pull/1977/files#diff-af91aaa8436f73c4457121b35da47e46d53459b47f044b7dbdb78ea785cb1824R783

[jonnyz32]

@worksofliam Is there anything you want changed still?

[worksofliam]

@jonnyz32 Nope. If what I shared is what you expected, this can probably be merged.

Thanks!

[jonnyz32]

@jonnyz32 One issue found during review:

When /QOpenSys/usr/bin or /usr/bin is not found in PATH, the warning dialog appears as expected. But when the user accepts to have .bashrc updated (because it already exists), none of the two directories are appended to the export PATH line...?

Seems like you didn't change the code for updating an existing .bashrc file, since the code still only mentions /QOpenSys/pkgs/bin?

Good catch. Just fixed this issue. Ran a small test to confirm. .bashrc before

-bash-5.2$ cat ~/.bashrc
export PS1="\u@\h \W:\$ "
export PATH=$PATH:/QOpenSys/pkgs/bin

.bashrc after

-bash-5.2$ cat ~/.bashrc
export PS1="\u@\h \W:\$ "
export PATH=/QOpenSys/pkgs/bin:$PATH:/QOpenSys/usr/bin:/usr/bin

[jonnyz32]

@jonnyz32 Only a small error left to fix... 😉 ...see comment in code below...

Another issue is that the code will add both /usr/bin and /QOpenSys/usr/bin to the PATH if one of them is missing, resulting in duplicate PATH entries... but this is acceptable, since it is allowed in the shell...

Thanks, I committed your suggestion :)