Skip to content
This repository was archived by the owner on Nov 30, 2023. It is now read-only.

Conversation

andreiborisov
Copy link
Contributor

Fixes #1501

@eitsupi
Copy link
Contributor

eitsupi commented Jun 14, 2022

Does this change fix #1501?
The log show that the fish installation was not successful?

@eitsupi
Copy link
Contributor

eitsupi commented Jun 14, 2022

I think the fish installation was successful because I see Installing Fisher... in the log.
What is failing is the installation of Fisher.

echo "Installing Fisher..."
fish -c 'curl -sL https://raw.githubusercontent.com/jorgebucaran/fisher/main/functions/fisher.fish | source && fisher install jorgebucaran/fisher'
if [ "${USERNAME}" != "root" ]; then
sudo -u $USERNAME fish -c 'curl -sL https://raw.githubusercontent.com/jorgebucaran/fisher/main/functions/fisher.fish | source && fisher install jorgebucaran/fisher'

image

The error came from this line.
https://github.com/jorgebucaran/fisher/blob/cf4c6cc561b6b495fc8ac0c3b31482f89dd08d49/functions/fisher.fish#L197

@andreiborisov
Copy link
Contributor Author

andreiborisov commented Jun 14, 2022

@eitsupi the issues was check_packages fish not working properly by failing to run apt-get update and as a result the script installing ancient fish version from the system package list. It has a slightly different syntax, hence fisher fails to work properly since it only supports fish v3 syntax and greater.

tldr: it should fix it.

@eitsupi
Copy link
Contributor

eitsupi commented Jun 14, 2022

That makes sense. Thank you for working on it!

This change should be revert in this PR, I think.
https://github.com/microsoft/vscode-dev-containers/pull/1500/files#diff-b4fcdb4d2b5b4dd46c896cb5bfa0a42e633340c263aa0238d71dce261bbd5c36R112

And version check e.x. fish -v in this script maybe useful for debug?

@andreiborisov
Copy link
Contributor Author

andreiborisov commented Jun 14, 2022

@eitsupi good suggestion! Added version check🙌

@eitsupi
Copy link
Contributor

eitsupi commented Jun 15, 2022

@andreiborisov Thanks!
I don't think the test is currently running, could you uncomment the following and re-enable the test?
https://github.com/microsoft/vscode-dev-containers/pull/1500/files#diff-b4fcdb4d2b5b4dd46c896cb5bfa0a42e633340c263aa0238d71dce261bbd5c36R112

@andreiborisov
Copy link
Contributor Author

@joshspicer seem to be good for the merge🙌🏻

@joshspicer
Copy link
Member

Thank you guys for your continued support! 🚀

@joshspicer joshspicer merged commit 8f60eff into microsoft:main Jun 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'fish' script "(line 197): 'case' builtin not inside of switch block"
3 participants