check offline/insiders VSIX#5341
check offline/insiders VSIX#5341
Conversation
…-cpptools into elrashed/5119_2
| const files: string[] = await readDir(path_to_check); | ||
| installedPlatform = "win32"; | ||
| if (files !== undefined) { | ||
| (files).forEach((element: string) => { |
There was a problem hiding this comment.
it looks like you are relying on checking to see which mono runtime we have installed to figure out the installed platform. Is that the best way to do this? Is the uname method not working? Also, once you have the installedPlatform set, you should exit instead of looping through all the files in the folder.
There was a problem hiding this comment.
Is the bug to do with the language service or with the debugger?
There was a problem hiding this comment.
This is not related to debugger. uname works, but we're trying to determine which "offline vsix" was installed and if it matches the OS is was installed on. We need to look at something in the extension's files to determine this. I think another alternative would be to embed a simple text file in the extension as part of our build process. We already inject lock.txt when we build offline vsix'es. We could potentially add content to lock.txt (currently it's empty) and just read it to see if it matches the OS. If lock.txt is empty (like for online vsix), we wouldn't do the check.
There was a problem hiding this comment.
This should be a bug on both. If a user decides to download the wrong offline installer it wont execute on their machine.
uname will determine the current machine but does not determine the offline vsix.
I am not a fan of this solution since its assuming the debugger will always use a mono runtime with specific extensions to determine the offline package type.
There was a problem hiding this comment.
You could just check the LLVM/clang-format or clang-format.exe or clang-format.darwin.
There was a problem hiding this comment.
A possible solution for the future is to add a dummy file in the VSIX folder that indicates the VSIX platform's info. For now, a unified solution for win/mac/linux32/linux64 is to check this folder. We can replace this with the name of the dummy file later on.
There was a problem hiding this comment.
We wouldn't need to add a new dummy file. We could just add contents to install.lock and you can read the file. But since we don't support 32-bit linux anymore, we can just look at clang-format as was discussed earlier today..
There was a problem hiding this comment.
In the new approach, we check the LLVM/bin folder.
|
The original issue mentions:
Can we not catch where the file not found exception happens and then show the user the correct download dialog? |
Add check to ensure offline/insiders VSIX is installed on the correct platform
add Github download link to the correct platform
bugfix: #5119