-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: GLIBCXX version detection bug in check-requirements-linux.sh (issue #204186) #204635
Conversation
- Existing detection scripts simply use the `awk` command to record the version number in the `libstdc++.so` filename, - but in some specific versions of glibc++, the detailed version number is not indicated in the filename, - so we need to use a script to read the current version of GLIBCXX in the environment to see if it meets the expectations. Co-authored-by: chengy-sysu <939416532@qq.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, some follow-ups
- Can you provide the output of
bash -x check-requirements-linux.sh
without the changes from this PR. strings
will not available by default on many cases so we would need a fallback
Since some Linux distributions do not come with GNU binutils pre-installed, the `strings` command does not fit on all platforms, so we use `grep` and `sed` instead of `strings`. Co-authored-by: chengy-sysu <939416532@qq.com>
|
Co-authored-by: chengy-sysu <939416532@qq.com>
Sorry the change still uses |
Since some Linux distributions do not come with GNU binutils pre-installed, the `strings` command does not fit on all platforms, so we use `grep` and `sed` instead of `strings`. Co-authored-by: chengy-sysu <939416532@qq.com>
Apologies for the fact that due to the changes I copied from vim, I didn't succeed in pasting them into the online editor when editing, now the current version no longer requires I tested this using a real Linux SSH connection to the server - in fact, it was a workstation with an Ubuntu 22.04 system. I was not involved in the installation and maintenance of the environment, so you may want to contact the author of issue #204186 if you need a reproducible container environment. |
Issue #204186 is different from the issue you are describing, there the script resolved to proper files
But based on the ldd output in #204186 (comment) it is very likely the author has multiple toolchain installations, so we would need to understand why that is the case and resolve that. If this PR is trying to address #204186 then let's wait for #204186 (comment). Also the script has got many changes since the issue was reported so I would also like to try the latest insiders on the setup. |
Okay, so it looks like the issue is different from what I'm experiencing. Going back to my environment, when I run
Run So the reason of the problem is that under the current system, instead of soft linking Skimmed GNU C++ Library Documentation. I didn't find any indication that Therefore, I prefer to use the internal contents of |
Then, another problem is that the script looks for the |
Thanks for the details, quick clarification have you tested the latest version of the script from vscode/resources/server/bin/helpers/check-requirements-linux.sh Lines 50 to 61 in cccd228
|
What does |
The result of
|
Thanks, I am good with this change given the context. Let me test with a couple of different setups before merging. |
There are conflicts in this PR, can you please rebase and resolve them. |
I used the following section from the latest version for my tests vscode/resources/server/bin/helpers/check-requirements-linux.sh Lines 50 to 91 in cccd228
For analysis purposes, I have included two The result obtained by running it is:
The new version has a new problem: it incorrectly reads and adopts the i386 toolchain library. |
Co-authored-by: chengy-sysu <939416532@qq.com>
Solved it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I add a small change to skip the new check on alpine since it will not work there. Please check if the script continues to work for your setup.
Yes It work's well. |
@microsoft-github-policy-service agree |
Thanks for the review and the conversation, I've updated the current PR to based on the latest Look forward to the next collaboration! |
awk
command to record the version number in thelibstdc++.so
filename,GLIBCXX_3.4.25
but fails version checking because the filename is simplylibstdc++.so.6
,awk
command to filter file names, we use a script to simulate thestrings /usr/lib/x86_64-linux-gnu/libstdc++.so.6 | grep GLIBCXX
operation, which reads the highest-supported version number of GLIBCXX, and determines whether the existing environment supports the minimum VSCode requirements. instead of simply filtering filenames with theawk
command.