client: show correct info about client version #2388
Conversation
On startup, the client prints a message describing itself. This should say the architecture for which the client was compiled, not the architecture of the host on which it's running.
|
Microsoft has ported desktop Windows to ARM64 processors. It runs x86 programs with emulator but with limitations. Apple has been rumoured to be doing the same for years. So "32/64-bit" isn't enough. It needs to include the processor architecture. The way you added code for only a few operating systems means the code either won't compile or fails at runtime on BSDs and others.
The client uses primary platform incorrectly in several places. You should have fixed those too. And finally, you made a change to the VirtualBox message, didn't say anything about it in the commit message and actually broke the message. Now 32-bit client running on 32-bit Windows without VirtualBox tells the user to install 64-bit client. |
|
Concern was expressed yesterday that this might not work on fully 32-bit systems. I've compiled and installed this: I get which seems good enough to fix the presenting problem for v7.9.4 I agree that further development for different architectures etc. would be good for v7.12: there's a TODO in the commit |
|
On the other hand, after removing VirtualBox, I get as @JuhaSointusalo says. That's a blocker. |
|
:Seems correct to me.
A 32-bit client can't tell whether VBox is installed.
A 64-bit client would be able to tell.
It's always been this way.
…On Wed, Mar 21, 2018 at 5:18 AM, RichardHaselgrove ***@***.*** > wrote:
***@***.**** requested changes on this pull request.
Produces false error message and bad advice on 32-bit OS without VBox
installed.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2388 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA8KgeMwrq9d3O0kPrSUUPlgziE7ADglks5tgkUmgaJpZM4SZP6P>
.
|
|
See my first log extract. That build successfully detected the 32-bit VBox installed under the 32-bit version of Windows 7. The hardware (elderly Dell Precision Workstation) is fully 64-bit certified and has virtualisation-enabled Xeon CPUs. I've even run 64-bit VBox apps under the 32-bit OS simply by writing the 32-bit wrapper into an app_info.xml file. It all works, provided you keep the bitness matched. 32-bit BOINC, 32-bit OS, 32-bit VBox is a working combination. Edit - host is ID 2901600 at SETI, if you want to check the details. Connection date of 28 Nov 2006 is accurate, though it was running Windows XP at the time. Hardware (except GPU) and pre-installed business software is unchanged since that date. |
| return " for Android"; | ||
| #endif | ||
| } | ||
|
|
There was a problem hiding this comment.
Do we need a default return type of " for Unknown" or some message in the event that none of the flags _WIN32/_WIN64/APPLE/LINUX_LIKE_SYSTEM/ANDROID is set? I don't know all the cases that things are built and if not having one of these set causes problems elsewhere in the code, but it seems like there should be a default to handle that case.
Note: If the code absolutely requires one of these flags to be set, then I withdraw my comment. However, if the intention is to be able to build the client without one of these flags then a default should exist.
There was a problem hiding this comment.
Additional comment, based on Juha's comments above, the use of HOSTTYPE in the default case would be better than "Unknown"
|
The message "Can't detect VirtualBox because this is a 32-bit version of BOINC; to fix, please install a 64-bit version." is misleading on multiple accounts. It is espicially bad since this now apparently shows up as a notice that there is an error.
BOINC should not imply that an error condition exists when it is possible that there is not an error condition. In 2 out of my 3 items above, there is not an error condition and the user does not need to take any action. A correct message would state something like: It might be better to have a message like: The user has a lot of options to getting it configured properly and pointing them to a help page will be able to provide the range of options that the message log cannot. |
|
Even worse - if the user follows the advice to install a 64-bit version of BOINC on a 32-bit version of windows (which is where the problem arises), they hit this rather clunky error message from Microsoft:
My own way of dealing with a similar problem at SETI was:
Conversely, there is no warning or advice at all if you install a 32-bit version of BOINC on a 64-bit version of Windows. That's the user error which started this whole conversation, and which could not be deduced from the initial line in the current Event Log at startup. Switching to another machine,
Note that the first graphic - the Microsoft error - is itself false. That's from the same machine as yesterday, and the Xeon E5320 processor is fully 64-bit capable. It's the operating system which isn't. |
|
Would it be possible for one of the committers to do a very simple commit / PR to correct the first-line event log message only, please? That caused us considerable confusion when we started this discussion, but the extra matters aren't needed: the old usage of the VBox message was actually correct and didn't need changing. With a quick and simple change, we can get it into v7.10 and keep the progress moving forward. Thanks. |
|
How about if we remove the line about not being able to detect vbox64? |
|
It's accurate for the problem it was designed for: running a 32-bit BOINC on a 64-bit operating system. The inaccuracy is having it appear when BOINC is running on a 32-bit operating system, when the advice to install a 64-bit BOINC can't be followed. I'll re-compile and re-test with the if (!strcmp(get_primary_platform(), "windows_x86_64")) line reinstated. |
|
I've completed tests with lines 260-266 of 5386968 reinstated, in all possible combinations (32 on 32, 64 on 64, and 32 on 64), and all messages appear as I would wish them to. If any change is desired, I'd suggest changing the mixed-mode message to
by adding "running on a 64-bit operating system" and "of BOINC" - to make it clearer which parts are in conflict, and which is easiest to correct. As @TheAspens says, the message appears as a notice, so space isn't a problem. |
|
I think I didn't state my opinion clearly enough above. I think that only one of the two following is the correct thing to do:
You cannot give users a very prominent message stating that something is wrong and that they must taken action when in fact that may not be true. |
|
I should also add that it definitely should not be a notice. |
|
OK, I can live with those observations. :-) The useful.wiki.page.on.boinc.site would be http://boinc.berkeley.edu/wiki/VirtualBox That's an existing part of the User Manual, and I've added the paragraph about avoiding mixed-mode running that's troubling us here. But being the user Wiki, we have to keep it at http until fixed, not https. |
#2436 (almost as requested). |
|
Pleased to note that the useful.wiki.page.on.boinc.site would be https://boinc.berkeley.edu/wiki/VirtualBox The user manual is now accessible via https |
|
The core problem has been resolved in the v7.10 release. Further observations and discussions are active elsewhere. Closing this iteration as redundant. |
On startup, the client prints a message describing itself.
This should say the architecture for which the client was compiled,
not the architecture of the host on which it's running.