client: show build not OS platform in messages

[JuhaSointusalo]

The client shows what kind of operating system it has detected in startup log message and elsewhere. This is confusing and not helping in troubleshooting when people expect these messages to describe the client itself.

Change the messages to use client's build platform instead.

Startup log message, output of --version and user agent string should be safe to change without breaking anything. The client still uses OS platform in various places but changing these would break stuff:

• Scheduler request XML. Scheduler expects scheduler_request.platform_name to be OS primary platform.
• client_state.xml. Client writes OS primary platform and alt platforms separately to client_state.xml. The client reads primary platform only to see if it has changed. The client doesn't read alt platforms but some other program might and depends on the platforms being as they are now. Not changing this means the client won't be able to detect when its own platform has changed. OS platform changes are still detected.
• get_state GUI RPC. The client writes primary platform to client_state.platform_name and all platforms, including primary, to client_state.platform. BOINC Manager only reads client_state.platform. As such, client_state.platform_name could be changed to client's build platform but some 3rd party manager might depend on it being OS primary platform.
• time_stats_log. The client writes platform info there. Unclear who uses that information.

Closes #2386.

[JuhaSointusalo]

Compared to #2388:

• This PR doesn't bother pretty printing HOSTTYPE in startup log. OS and CPU info is already pretty printed a few lines later.
• Uses correct info in two more places.
• Doesn't change VirtualBox message. As for @TheAspens request, lets get rid of the no checking 64-bit VB in 32-bit client limitation in 7.12.

[JuhaSointusalo]

Well that's a nice one.

▸ Compiling jcapimin.c
** BUILD FAILED **
The following build commands failed:
	CompileC build/boinc.build/Deployment/jpeg.build/Objects-normal/i386/jcapimin.o /Users/travis/build/BOINC/boinc/samples/jpeglib/jcapimin.c normal i386 c com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Is there any way to see why it failed?

[CharlieFenton]

From a visual inspection of your changes, I don't see anything that should affect jcapimin.c. I just now synced with GIT master and built the JPEG library on my Mac. There were no warnings or errors building jcapimin.c for either i386 or x86_64 architecture.

[JuhaSointusalo]

I restarted the Travis build and it failed the same way.

Originally the code read like this:

#ifdef __x86_64__
#define HOSTTYPE "x86_64-apple-darwin"
#else
#error Unknown HOSTTYPE
#endif

The idea I had with the #error was that if someone ever wants to build BOINC on something other than x86_64 the error points to the right file to fix. On a hunch I removed the error case and now the build succeeds.

Build log made it look like the error was in the middle of libjpeg build but jcapimin.c is actually the first file built and it failed for i386 build.

[CharlieFenton]

Build log made it look like the error was in the middle of libjpeg build but jcapimin.c is actually the first file built and it failed for i386 build.

jcapimin.c #includes jpeglib.h which #includes jconfig.h which has:

#ifdef _WIN32
#include "win-config.h"
#elif ( ! defined(__APPLE__))
#include "config.h"
#endif

Since the JPEG library libjpeg.a can be used in project applications, we build it as a universal binary containing both i386 and x86_64 architectures. When Travis-CI attempted the i386 build of the JPEG library, most or all of the source files included config.h, invoking the #error directive.

Moreover, the Xcode build settings for the "boinc' base target has -include ../clientgui/mac/config.h, which is then carried through to the settings for all targets. This means that config.h is included in every source file, so it is probably a bad idea for config.h to have any architecture-specific statements. Since HOSTTYPE is used only in the client, and since client_state.h is #included in every client source file and no others, one option would be to put this code in client_state.h instead of in config.h:

#ifdef __x86_64__
#define HOSTTYPE "x86_64-apple-darwin"
#else
#error Unknown HOSTTYPE
#endif

Another possibility is to take advantage of the fact that BOINC for Mac can be built only on an x86_64 Mac and can be run only on an x86_64 Mac. So this sort of code would also do what you want here:

#ifdef __APPLE__
    get_primary_platform(),
#else
    HOSTTYPE,
#endif

Finally, note that CLIENT_STATE::detect_platforms() in client/cs_platforms.cpp currently has this code:

#elif defined(__APPLE__)

#ifdef __x86_64__
    add_platform("x86_64-apple-darwin");
    add_platform("i686-apple-darwin");
#else
#error Mac client now requires a 64-bit system
#endif

#elif defined(__linux__) && ( defined(__i386__) || defined(__x86_64__) )

so you should also remove this line: add_platform("i686-apple-darwin");

[CharlieFenton]

Correction: I overlooked the ! in

#ifdef _WIN32
#include "win-config.h"
#elif ( ! defined(__APPLE__))
#include "config.h"
#endif

but my explanation for the Travis-CI error and other comments still hold because of the -include ../clientgui/mac/config.h in the the Xcode build settings C and C++ Flags for all targets.

[JuhaSointusalo]

Xcode build settings for the "boinc' base target has -include ../clientgui/mac/config.h, which is then carried through to the settings for all targets.

Thanks. I had looked at Xcode project file to see if / how config.h was included but couldn't figure it out. The file is text all right but not really human readable.

This means that config.h is included in every source file, so it is probably a bad idea for config.h to have any architecture-specific statements.

But that is what config.h is for. Yes, it is used mostly for header and function related stuff but it's not limited to that. You can for example declare types there that depend on bitness of your program and such. Normally you don't see #ifs there but that's because it is generated by configure at build time. It's just that Xcode doesn't run configure so Mac config.h needs to have the #if there.

So this sort of code would also do what you want here:

#ifdef __APPLE__
    get_primary_platform(),
#else
    HOSTTYPE,
#endif

Using get_primary_platform() got us making these changes now. Maybe BOINC now runs only on x86_64 Mac but that doesn't have to be so forever. What if someone ports BOINC to iOS (lets ignore the roadblocks for a moment)? Newer iOS versions are probably 64-bit but that someone might want to build BOINC as 32-bit app to be able to run on older iOS versions. Then BOINC would report itself as being built for arm64 even though it's arm(32) app. So I have to disagree with you on this one too.

CLIENT_STATE::detect_platforms() in client/cs_platforms.cpp

As far as I can tell cs_platforms.cpp is all about what apps the client can run. I think I may have read somewhere that Apple is dropping 32-bit support from OS X. If that is the case the code should probably check at runtime what the OS can run. I don't have Mac and I don't know enough about OS X to make that kind of change so that's something you have to do.

[CharlieFenton]

This means that config.h is included in every source file, so it is probably a bad idea for config.h to have any architecture-specific statements.

But that is what config.h is for. Yes, it is used mostly for header and function related stuff but it's not limited to that.

Right, but my point is that config.h is #included in the library source files, which are built with both i386 and x86_64 architectures, so it can't have any statements which assume only one architecture. But I agree that something like this could be used in config.h:

#ifdef __x86_64__
#define HOSTTYPE "x86_64-apple-darwin"
#else
#define HOSTTYPE "i386-apple-darwin"
#endif

but it is wrong to have this under the #else: #error Unknown HOSTTYPE.

My other point is that because HOSTTYPE is used only by the client, the #define HOSTTYPE "i386-apple-darwin" won't actually get used anywhere. Since HOSTTYPE is used only by the client, it might be better to define it in a client-only header like client_state.h instead of in config.h.

[CharlieFenton]

I just had another idea. If you put this in config.h, it won't cause any errors in build targets that don't use HOSTTYPE, but will cause a HOSTTYPE undefined error in the client if built with an architecture other than x86_64:

#ifdef __x86_64__
#define HOSTTYPE "x86_64-apple-darwin"
#endif

[CharlieFenton]

I just had another idea.

Which of course is exactly what you had already done. Sorry to have added so much "noise" to the discussion of this PR while I processed it in my head. Let me know when you are ready to have this merged, and I will merge it.

[JuhaSointusalo]

Right, but my point is that config.h is #included in the library source files, which are built with both i386 and x86_64 architectures, so it can't have any statements which assume only one architecture.

Yeah, when I wrote the first code I was concentrated on the client and forgot config.h is used for building i386 libs too.

Sorry to have added so much "noise" to the discussion of this PR

No harm done. I learned a bit more about Mac build setup.

Let me know when you are ready to have this merged, and I will merge it.

I think it's ready but since this replaces PR #2388 I don't know if those who commented there should approve or disapprove this PR.

[TheAspens]

I have no objections to this PR and deferring the wxWidgets message discussion to 7.12. It is late in the 7.10 development cycle, so changes for 7.10 should be small and focused which this PR is.

[TheAspens]

I think this has been open long enough for any objections to be voiced. @CharlieFenton - I think you can proceed with the merge since @JuhaSointusalo indicates that it is ready above.

Once you merge it can you either cherry-pick it into the 7.10 release branch or cherry-pick it into #2429?

[CharlieFenton]

I have merged this into master and cherry-picked it into the 7.10 release branch.