From 7dc014474de0c2d83a3cd314acd9dc0882622299 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Aug 2018 10:48:29 -0400 Subject: [PATCH 01/14] DRTVWR-447: Attempt to post BugSplat metadata with Mac crash reports. Introduce CrashMetadata, an LLSingleton in llappviewermacosx.cpp, declared in llappviewermacosx-for-objc.h and accessed by the various BugsplatStartupManagerDelegate override methods. CrashMetadata is populated by reading the previous (presumably crashed) run's static_debug_info.log file. This replaces the previous getOldLogFilePathname(), getFatalMessage() and getAgentFullname() functions. To extend that suite for additional metadata, not only would we have to keep adding new free functions, but we'd have to keep rereading the static_debug_info.log file. Override the new applicationKeyForBugsplatStartupManager, defaultUserNameForBugsplatStartupManager, defaultUserEmailForBugsplatStartupManager methods to extract relevant fields from CrashMetadata. Change applicationLogForBugsplatStartupManager and attachmentForBugsplatStartupManager to do the same. Enhance llviewerregion.cpp to update the static_debug_info.log file every time we enter a new region. --- indra/newview/llappdelegate-objc.mm | 43 +++++++++++++++---- indra/newview/llappviewermacosx-for-objc.h | 21 ++++++++-- indra/newview/llappviewermacosx.cpp | 49 ++++++++++++++++++---- indra/newview/llviewerregion.cpp | 19 +++++++++ 4 files changed, 111 insertions(+), 21 deletions(-) diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 82e49540a4..ba697d0f77 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -199,10 +199,34 @@ - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { infos("Reached applicationLogForBugsplatStartupManager"); - // Apparently this override method only contributes the User Description - // field of BugSplat's All Crashes table. Despite the method name, it - // would seem to be a bad place to try to stuff all of SecondLife.log. - return [NSString stringWithCString:getFatalMessage().c_str() + // This strangely-named override method contributes the User Description + // metadata field. + return [NSString stringWithCString:CrashMetadata_instance().fatalMessage.c_str() + encoding:NSUTF8StringEncoding]; +} + +- (NSString *)applicationKeyForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager signal:(NSString *)signal exceptionName:(NSString *)exceptionName exceptionReason:(NSString *)exceptionReason { + // TODO: exceptionName, exceptionReason + + // Windows sends location within region as well, but that's because + // BugSplat for Windows intercepts crashes during the same run, and that + // information can be queried once. On the Mac, any metadata we have is + // written (and rewritten) to the static_debug_info.log file that we read + // at the start of the next viewer run. It seems ridiculously expensive to + // rewrite that file on every frame in which the avatar moves. + return [NSString stringWithCString:CrashMetadata_instance().regionName.c_str() + encoding:NSUTF8StringEncoding]; +} + +- (NSString *)defaultUserNameForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { + return [NSString stringWithCString:CrashMetadata_instance().agentFullname.c_str() + encoding:NSUTF8StringEncoding]; +} + +- (NSString *)defaultUserEmailForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { + // Use the email field for OS version, just as we do on Windows, until + // BugSplat provides more metadata fields. + return [NSString stringWithCString:CrashMetadata_instance().OSInfo.c_str() encoding:NSUTF8StringEncoding]; } @@ -212,11 +236,12 @@ } - (BugsplatAttachment *)attachmentForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - // We get the *old* log file pathname (for SecondLife.old) because it's on - // the run *following* the crash that BugsplatStartupManager notices that - // the previous run crashed and calls this override. By that time, we've - // already renamed SecondLife.log to SecondLife.old. - std::string logfile = getOldLogFilePathname(); + std::string logfile = CrashMetadata_instance().logFilePathname; + // Still to do: + // userSettingsPathname + // staticDebugPathname + // but the BugsplatMac version 1.0.5 BugsplatStartupManagerDelegate API + // doesn't yet provide a way to attach more than one file. NSString *ns_logfile = [NSString stringWithCString:logfile.c_str() encoding:NSUTF8StringEncoding]; NSData *data = [NSData dataWithContentsOfFile:ns_logfile]; diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index ac85d7e8c3..79da453cbe 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -29,9 +29,24 @@ void handleUrl(const char* url_utf8); bool pumpMainLoop(); void handleQuit(); void cleanupViewer(); -std::string getOldLogFilePathname(); -std::string getFatalMessage(); -std::string getAgentFullname(); void infos(const std::string& message); +// This struct is malleable; it only serves as a way to convey a number of +// fields from llappviewermacosx.cpp's CrashMetadata_instance() function to the +// consuming functions in llappdelegate-objc.mm. As long as both those sources +// are compiled with this same header, the content and order of CrashMetadata +// can change as needed. +struct CrashMetadata +{ + std::string logFilePathname; + std::string userSettingsPathname; + std::string staticDebugPathname; + std::string OSInfo; + std::string agentFullname; + std::string regionName; + std::string fatalMessage; +}; + +CrashMetadata& CrashMetadata_instance(); + #endif /* ! defined(LL_LLAPPVIEWERMACOSX_FOR_OBJC_H) */ diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index c3a3c3284a..7f7284a796 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -39,6 +39,7 @@ #include "llappviewermacosx-for-objc.h" #include "llwindowmacosx-objc.h" #include "llcommandlineparser.h" +#include "llsdserialize.h" #include "llviewernetwork.h" #include "llviewercontrol.h" @@ -53,6 +54,7 @@ #endif #include #include +#include #include "lldir.h" #include @@ -150,19 +152,48 @@ void cleanupViewer() gViewerAppPtr = NULL; } -std::string getOldLogFilePathname() +// The BugsplatMac API is structured as a number of different method +// overrides, each returning a different piece of metadata. But since we +// obtain such metadata by opening and parsing a file, it seems ridiculous to +// reopen and reparse it for every individual string desired. What we want is +// to open and parse the file once, retaining the data for subsequent +// requests. That's why this is an LLSingleton. +// Another approach would be to provide a function that simply returns +// CrashMetadata, storing the struct in LLAppDelegate, but nat doesn't know +// enough Objective-C++ to code that. We'd still have to detect which of the +// method overrides is called first so that the results are order-insensitive. +class CrashMetadataSingleton: public CrashMetadata, public LLSingleton { - return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.old"); + LLSINGLETON(CrashMetadataSingleton); +}; + +// Populate the fields of our public base-class struct. +CrashMetadataSingleton::CrashMetadataSingleton() +{ + // Note: we depend on being able to read the static_debug_info.log file + // from the *previous* run before we overwrite it with the new one for + // *this* run. LLAppViewer initialization must happen in the Right Order. + staticDebugPathname = *gViewerAppPtr->getStaticDebugFile(); + std::ifstream static_file(staticDebugPathname); + LLSD info; + if (static_file.is_open() && + LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) + { + logFilePathname = info["SLLog"].asString(); + userSettingsPathname = info["SettingsFilename"].asString(); + OSInfo = info["OSInfo"].asString(); + agentFullname = info["LoginName"].asString(); + // Translate underscores back to spaces + LLStringUtil::replaceChar(agentFullname, '_', ' '); + regionName = info["CurrentRegion"].asString(); + fatalMessage = info["FatalMessage"].asString(); + } } -std::string getFatalMessage() +// Avoid having to compile all of our LLSingleton machinery in Objective-C++. +CrashMetadata& CrashMetadata_instance() { - return LLError::getFatalMessage(); -} - -std::string getAgentFullname() -{ - return gAgentAvatarp? gAgentAvatarp->getFullname() : std::string(); + return CrashMetadataSingleton::instance(); } void infos(const std::string& message) diff --git a/indra/newview/llviewerregion.cpp b/indra/newview/llviewerregion.cpp index b759c2a3ab..ca452fc766 100644 --- a/indra/newview/llviewerregion.cpp +++ b/indra/newview/llviewerregion.cpp @@ -44,6 +44,7 @@ #include "llagent.h" #include "llagentcamera.h" +#include "llappviewer.h" #include "llavatarrenderinfoaccountant.h" #include "llcallingcard.h" #include "llcommandhandler.h" @@ -104,6 +105,18 @@ typedef std::map CapabilityMap; static void log_capabilities(const CapabilityMap &capmap); +namespace +{ + +void newRegionEntry(LLViewerRegion& region) +{ + LL_INFOS("LLViewerRegion") << "Entering region [" << region.getName() << "]" << LL_ENDL; + gDebugInfo["CurrentRegion"] = region.getName(); + LLAppViewer::instance()->writeDebugInfo(); +} + +} // anonymous namespace + // support for secondlife:///app/region/{REGION} SLapps // N.B. this is defined to work exactly like the classic secondlife://{REGION} // However, the later syntax cannot support spaces in the region name because @@ -249,6 +262,9 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle) return; // this error condition is not recoverable. } + // record that we just entered a new region + newRegionEntry(*regionp); + // After a few attempts, continue login. But keep trying to get the caps: if (mSeedCapAttempts >= mSeedCapMaxAttemptsBeforeLogin && STATE_SEED_GRANTED_WAIT == LLStartUp::getStartupState()) @@ -369,6 +385,9 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro(U64 regionHandle) break; // this error condition is not recoverable. } + // record that we just entered a new region + newRegionEntry(*regionp); + LLSD capabilityNames = LLSD::emptyArray(); buildCapabilityNames(capabilityNames); From 787053ffeb70f4e3d7ade36290ad7e75f1146b74 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Aug 2018 13:26:19 -0400 Subject: [PATCH 02/14] DRTVWR-447: Add logging to BugsplatMac override methods. --- indra/newview/llappdelegate-objc.mm | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index ba697d0f77..66bcf58961 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -198,10 +198,11 @@ - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - infos("Reached applicationLogForBugsplatStartupManager"); + std::string fatalMessage(CrashMetadata_instance().fatalMessage); + infos("applicationLogForBugsplatStartupManager -> '" + fatalMessage + "'"); // This strangely-named override method contributes the User Description // metadata field. - return [NSString stringWithCString:CrashMetadata_instance().fatalMessage.c_str() + return [NSString stringWithCString:fatalMessage.c_str() encoding:NSUTF8StringEncoding]; } @@ -214,25 +215,31 @@ // written (and rewritten) to the static_debug_info.log file that we read // at the start of the next viewer run. It seems ridiculously expensive to // rewrite that file on every frame in which the avatar moves. - return [NSString stringWithCString:CrashMetadata_instance().regionName.c_str() + std::string regionName(CrashMetadata_instance().regionName); + infos("applicationKeyForBugsplatStartupManager -> '" + regionName + "'"); + return [NSString stringWithCString:regionName.c_str() encoding:NSUTF8StringEncoding]; } - (NSString *)defaultUserNameForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - return [NSString stringWithCString:CrashMetadata_instance().agentFullname.c_str() + std::string agentFullname(CrashMetadata_instance().agentFullname); + infos("defaultUserNameForBugsplatStartupManager -> '" + agentFullname + "'"); + return [NSString stringWithCString:agentFullname.c_str() encoding:NSUTF8StringEncoding]; } - (NSString *)defaultUserEmailForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { // Use the email field for OS version, just as we do on Windows, until // BugSplat provides more metadata fields. - return [NSString stringWithCString:CrashMetadata_instance().OSInfo.c_str() + std::string OSInfo(CrashMetadata_instance().OSInfo); + infos("defaultUserEmailForBugsplatStartupManager -> '" + OSInfo + "'"); + return [NSString stringWithCString:OSInfo.c_str() encoding:NSUTF8StringEncoding]; } - (void)bugsplatStartupManagerWillSendCrashReport:(BugsplatStartupManager *)bugsplatStartupManager { - infos("Reached bugsplatStartupManagerWillSendCrashReport"); + infos("bugsplatStartupManagerWillSendCrashReport"); } - (BugsplatAttachment *)attachmentForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { @@ -252,7 +259,7 @@ [[BugsplatAttachment alloc] initWithFilename:@"SecondLife.log" attachmentData:data contentType:@"text/plain"]; - infos("attachmentForBugsplatStartupManager: attaching " + logfile); + infos("attachmentForBugsplatStartupManager attaching " + logfile); return attachment; } From 8d4e6b6df0d21e18a24c8e1d9f6008c2092a24c5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Aug 2018 16:16:26 -0400 Subject: [PATCH 03/14] DRTVWR-447: Additional logging getting metadata for previous run --- indra/newview/llappviewer.cpp | 7 ++---- indra/newview/llappviewermacosx.cpp | 33 ++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 3e25b395c4..d324a82bf8 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3051,14 +3051,11 @@ void LLAppViewer::writeDebugInfo(bool isStatic) ? getStaticDebugFile() : getDynamicDebugFile() ); - LL_INFOS() << "Opening debug file " << *debug_filename << LL_ENDL; - llofstream out_file(debug_filename->c_str()); + LL_INFOS() << "Writing debug file " << *debug_filename << LL_ENDL; + llofstream out_file(debug_filename->c_str()); isStatic ? LLSDSerialize::toPrettyXML(gDebugInfo, out_file) : LLSDSerialize::toPrettyXML(gDebugInfo["Dynamic"], out_file); - - - out_file.close(); } LLSD LLAppViewer::getViewerInfo() const diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 7f7284a796..77a16f7307 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -165,6 +165,14 @@ void cleanupViewer() class CrashMetadataSingleton: public CrashMetadata, public LLSingleton { LLSINGLETON(CrashMetadataSingleton); + + // convenience method to log each metadata field retrieved by constructor + std::string get_metadata(const LLSD& info, const LLSD::String& key) const + { + std::string data(info[key].asString()); + LL_INFOS() << " " << key << "='" << data << "'" << LL_ENDL; + return data; + } }; // Populate the fields of our public base-class struct. @@ -176,17 +184,26 @@ CrashMetadataSingleton::CrashMetadataSingleton() staticDebugPathname = *gViewerAppPtr->getStaticDebugFile(); std::ifstream static_file(staticDebugPathname); LLSD info; - if (static_file.is_open() && - LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) + if (! static_file.is_open()) { - logFilePathname = info["SLLog"].asString(); - userSettingsPathname = info["SettingsFilename"].asString(); - OSInfo = info["OSInfo"].asString(); - agentFullname = info["LoginName"].asString(); + LL_INFOS() << "Can't open '" << staticDebugPathname + << "'; no metadata about previous run" << LL_ENDL; + } + else if (! LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) + { + LL_INFOS() << "Can't parse '" << staticDebugPathname + << "'; no metadata about previous run" << LL_ENDL; + } + { + LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL; + logFilePathname = get_metadata(info, "SLLog"); + userSettingsPathname = get_metadata(info, "SettingsFilename"); + OSInfo = get_metadata(info, "OSInfo"); + agentFullname = get_metadata(info, "LoginName"); // Translate underscores back to spaces LLStringUtil::replaceChar(agentFullname, '_', ' '); - regionName = info["CurrentRegion"].asString(); - fatalMessage = info["FatalMessage"].asString(); + regionName = get_metadata(info, "CurrentRegion"); + fatalMessage = get_metadata(info, "FatalMessage"); } } From afbf243f1a049bdeec6410bcc57350b00d0da169 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 23 Aug 2018 10:27:31 -0400 Subject: [PATCH 04/14] DRTVWR-447: Update to bugsplat build 518982 --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index e88fd04940..ba8d874420 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -244,9 +244,9 @@ archive hash - f01caa9aeb4c19f679e60609e6095335 + 40a9258f96f2f800489aeb7dee366654 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23285/178722/bugsplat-1.0.5.518876-darwin64-518876.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23472/180173/bugsplat-1.0.6.518982-darwin64-518982.tar.bz2 name darwin64 @@ -256,9 +256,9 @@ archive hash - 17f0e6d2fce03ae48ac02ccaebe48f61 + 735dc6d92fa973e08d280efaa381c452 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23286/178729/bugsplat-3.6.0.4.518876-windows-518876.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23474/180182/bugsplat-3.6.0.4.518982-windows-518982.tar.bz2 name windows @@ -268,16 +268,16 @@ archive hash - e963a42106ed0fc8b87974cfce040714 + 028c13cc0437c0881ad38b4ac685f794 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23284/178715/bugsplat-3.6.0.4.518876-windows64-518876.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23473/180187/bugsplat-3.6.0.4.518982-windows64-518982.tar.bz2 name windows64 version - 1.0.5.518876 + 1.0.6.518982 chardet From e674f11757ab55c5ca7aab4cb1c8e059fa98f466 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 23 Aug 2018 12:31:54 -0400 Subject: [PATCH 05/14] DRTVWR-447: Add (some) metadata to Mac crash reports. This required reordering certain operations during Mac viewer startup. Split llappviewermacosx.cpp's initViewer() function into constructViewer() (which instantiates LLAppViewerMacOSX) and initViewer() (which calls LLAppViewerMacOSX::init()). llappdelegate-objc.mm's applicationDidFinishLaunching override now calls [BugsplatStartupManager start] between constructViewer() and initViewer(): we want constructViewer() to have set up the logging subsystem so we can log the actions of BugsplatStartupManagerDelegate override methods, but otherwise we want BugsplatStartupManager in place as early as possible to catch any early crashes. Besides, initViewer() ends up overwriting the static_debug_info.log on which we depend for the *previous* run's crash metadata. Move the code that initializes the pathname of the static_debug_info.log file from LLAppViewerMacOSX::init() to the LLAppViewerMacOSX() constructor, since BugsplatStartupManagerDelegate override methods need to read (the previous run's) file. Add code to applicationLogForBugsplatStartupManager override to set new BugsplatMac 1.0.6 properties userName and userEmail. Don't log empty fields from static_debug_info.log if we couldn't read it. --- indra/newview/llappdelegate-objc.mm | 55 +++++++++++++++++----- indra/newview/llappviewer.cpp | 29 +++++++----- indra/newview/llappviewermacosx-for-objc.h | 1 + indra/newview/llappviewermacosx.cpp | 17 ++++--- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 66bcf58961..f55304f30b 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -54,6 +54,25 @@ - (void) applicationDidFinishLaunching:(NSNotification *)notification { + // Call constructViewer() first so our logging subsystem is in place. This + // risks missing crashes in the LLAppViewerMacOSX constructor, but for + // present purposes it's more important to get the startup sequence + // properly logged. + // Someday I would like to modify the logging system so that calls before + // it's initialized are cached in a std::ostringstream and then, once it's + // initialized, "played back" into whatever handlers have been set up. + constructViewer(); + +#if defined(LL_BUGSPLAT) + // Engage BugsplatStartupManager *before* calling initViewer() to handle + // any crashes during initialization. + // https://www.bugsplat.com/docs/platforms/os-x#initialization + [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES; + [BugsplatStartupManager sharedManager].askUserDetails = NO; + [BugsplatStartupManager sharedManager].delegate = self; + [[BugsplatStartupManager sharedManager] start]; +#endif + frameTimer = nil; [self languageUpdated]; @@ -71,14 +90,6 @@ [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(languageUpdated) name:@"NSTextInputContextKeyboardSelectionDidChangeNotification" object:nil]; // [[NSAppleEventManager sharedAppleEventManager] setEventHandler:self andSelector:@selector(handleGetURLEvent:withReplyEvent:) forEventClass:kInternetEventClass andEventID:kAEGetURL]; - -#if defined(LL_BUGSPLAT) - // https://www.bugsplat.com/docs/platforms/os-x#initialization - [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES; - [BugsplatStartupManager sharedManager].askUserDetails = NO; - [BugsplatStartupManager sharedManager].delegate = self; - [[BugsplatStartupManager sharedManager] start]; -#endif } - (void) handleGetURLEvent:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEventDescriptor *)replyEvent { @@ -198,11 +209,29 @@ - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - std::string fatalMessage(CrashMetadata_instance().fatalMessage); - infos("applicationLogForBugsplatStartupManager -> '" + fatalMessage + "'"); - // This strangely-named override method contributes the User Description - // metadata field. - return [NSString stringWithCString:fatalMessage.c_str() + CrashMetadata& meta(CrashMetadata_instance()); + // As of BugsplatMac 1.0.6, userName and userEmail properties are now + // exposed by the BugsplatStartupManager. Set them here, since the + // defaultUserNameForBugsplatStartupManager and + // defaultUserEmailForBugsplatStartupManager methods are called later, for + // the *current* run, rather than for the previous crashed run whose crash + // report we are about to send. + infos("applicationLogForBugsplatStartupManager setting userName = '" + + meta.agentFullname + '"'); + bugsplatStartupManager.userName = + [NSString stringWithCString:meta.agentFullname.c_str() + encoding:NSUTF8StringEncoding]; + // Use the email field for OS version, just as we do on Windows, until + // BugSplat provides more metadata fields. + infos("applicationLogForBugsplatStartupManager setting userEmail = '" + + meta.OSInfo + '"'); + bugsplatStartupManager.userEmail = + [NSString stringWithCString:meta.OSInfo.c_str() + encoding:NSUTF8StringEncoding]; + // This strangely-named override method's return value contributes the + // User Description metadata field. + infos("applicationLogForBugsplatStartupManager -> '" + meta.fatalMessage + "'"); + return [NSString stringWithCString:meta.fatalMessage.c_str() encoding:NSUTF8StringEncoding]; } diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index d324a82bf8..846b937a4e 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -707,6 +707,22 @@ LLAppViewer::LLAppViewer() // LLLoginInstance::instance().setPlatformInfo(gPlatform, LLOSInfo::instance().getOSVersionString(), LLOSInfo::instance().getOSStringSimple()); + + // Under some circumstances we want to read the static_debug_info.log file + // from the previous viewer run between this constructor call and the + // init() call, which will overwrite the static_debug_info.log file for + // THIS run. So setDebugFileNames() early. +#if LL_BUGSPLAT + // MAINT-8917: don't create a dump directory just for the + // static_debug_info.log file + std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, ""); +#else // ! LL_BUGSPLAT + // write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues. + std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, ""); +#endif // ! LL_BUGSPLAT + mDumpPath = logdir; + setMiniDumpDir(logdir); + setDebugFileNames(logdir); } LLAppViewer::~LLAppViewer() @@ -781,19 +797,6 @@ bool LLAppViewer::init() initMaxHeapSize() ; LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize")); -#if LL_BUGSPLAT - // MAINT-8917: don't create a dump directory just for the - // static_debug_info.log file - std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, ""); -#else // ! LL_BUGSPLAT - // write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues. - std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, ""); -#endif // ! LL_BUGSPLAT - mDumpPath = logdir; - setMiniDumpDir(logdir); - logdir += gDirUtilp->getDirDelimiter(); - setDebugFileNames(logdir); - // Although initLoggingAndGetLastDuration() is the right place to mess with // setFatalFunction(), we can't query gSavedSettings until after diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index 79da453cbe..37e8a3917a 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -24,6 +24,7 @@ #include +void constructViewer(); bool initViewer(); void handleUrl(const char* url_utf8); bool pumpMainLoop(); diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 77a16f7307..81f04744f8 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -86,7 +86,7 @@ static void exceptionTerminateHandler() gOldTerminateHandler(); // call old terminate() handler } -bool initViewer() +void constructViewer() { // Set the working dir to /Contents/Resources if (chdir(gDirUtilp->getAppRODataDir().c_str()) == -1) @@ -102,18 +102,20 @@ bool initViewer() gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler); gViewerAppPtr->setErrorHandler(LLAppViewer::handleViewerCrash); +} - +bool initViewer() +{ bool ok = gViewerAppPtr->init(); if(!ok) { LL_WARNS() << "Application init failed." << LL_ENDL; } - else if (!gHandleSLURL.empty()) - { - dispatchUrl(gHandleSLURL); - gHandleSLURL = ""; - } + else if (!gHandleSLURL.empty()) + { + dispatchUrl(gHandleSLURL); + gHandleSLURL = ""; + } return ok; } @@ -194,6 +196,7 @@ CrashMetadataSingleton::CrashMetadataSingleton() LL_INFOS() << "Can't parse '" << staticDebugPathname << "'; no metadata about previous run" << LL_ENDL; } + else { LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL; logFilePathname = get_metadata(info, "SLLog"); From c2178bb6ac139d47eb2bfdf9e85811a6f02810ed Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 24 Aug 2018 09:56:56 -0400 Subject: [PATCH 06/14] DRTVWR-447: Introduce explicit CMake BUGSPLAT_DB variable. Define the CMake cache variable, with empty string as its default. Make build.sh pass the BUGSPLAT_DB environment variable as a CMake command-line variable assignment. Change CMake 'if (DEFINED ENV{BUGSPLAT_DB})' to plain 'if (BUGSPLAT_DB)'. Make CMake pass new --bugsplat switch to every one of SIX different invocations of viewer_manifest.py. Give llmanifest.main() function an argument to allow supplementing the base set of command-line switches with additional application-specific switches. In viewer_manifest.py, define new --bugsplat command-line switch and pass to llmanifest.main(). Instead of consulting os.environ['BUGSPLAT_DB'], consult self.args['bugsplat']. --- build.sh | 1 + indra/cmake/Copy3rdPartyLibs.cmake | 4 +- indra/cmake/Variables.cmake | 1 + indra/cmake/bugsplat.cmake | 9 ++-- indra/lib/python/indra/util/llmanifest.py | 56 +++++++++++++---------- indra/llcommon/CMakeLists.txt | 4 +- indra/newview/CMakeLists.txt | 52 +++++++++++---------- indra/newview/viewer_manifest.py | 45 +++++++++--------- 8 files changed, 94 insertions(+), 78 deletions(-) diff --git a/build.sh b/build.sh index d531c0b046..c38bb6fff4 100755 --- a/build.sh +++ b/build.sh @@ -122,6 +122,7 @@ pre_build() -DPACKAGE:BOOL=ON \ -DHAVOK:BOOL="$HAVOK" \ -DRELEASE_CRASH_REPORTING:BOOL="$RELEASE_CRASH_REPORTING" \ + -DBUGSPLAT_DB:STRING="${BUGSPLAT_DB:-}" \ -DVIEWER_CHANNEL:STRING="${viewer_channel}" \ -DGRID:STRING="\"$viewer_grid\"" \ -DTEMPLATE_VERIFIER_OPTIONS:STRING="$template_verifier_options" $template_verifier_master_url \ diff --git a/indra/cmake/Copy3rdPartyLibs.cmake b/indra/cmake/Copy3rdPartyLibs.cmake index c9519b0e1d..dde53835fb 100644 --- a/indra/cmake/Copy3rdPartyLibs.cmake +++ b/indra/cmake/Copy3rdPartyLibs.cmake @@ -51,7 +51,7 @@ if(WINDOWS) # Filenames are different for 32/64 bit BugSplat file and we don't # have any control over them so need to branch. - if (DEFINED ENV{BUGSPLAT_DB}) + if (BUGSPLAT_DB) if(ADDRESS_SIZE EQUAL 32) set(release_files ${release_files} BugSplat.dll) set(release_files ${release_files} BugSplatRc.dll) @@ -61,7 +61,7 @@ if(WINDOWS) set(release_files ${release_files} BugSplatRc64.dll) set(release_files ${release_files} BsSndRpt64.exe) endif(ADDRESS_SIZE EQUAL 32) - endif (DEFINED ENV{BUGSPLAT_DB}) + endif (BUGSPLAT_DB) if (FMODEX) diff --git a/indra/cmake/Variables.cmake b/indra/cmake/Variables.cmake index b913d6398e..e49de0a79b 100644 --- a/indra/cmake/Variables.cmake +++ b/indra/cmake/Variables.cmake @@ -33,6 +33,7 @@ set(INTEGRATION_TESTS_PREFIX) set(LL_TESTS ON CACHE BOOL "Build and run unit and integration tests (disable for build timing runs to reduce variation") set(INCREMENTAL_LINK OFF CACHE BOOL "Use incremental linking on win32 builds (enable for faster links on some machines)") set(ENABLE_MEDIA_PLUGINS ON CACHE BOOL "Turn off building media plugins if they are imported by third-party library mechanism") +set(BUGSPLAT_DB "" CACHE STRING "BugSplat database name, if BugSplat crash reporting is desired") if(LIBS_CLOSED_DIR) file(TO_CMAKE_PATH "${LIBS_CLOSED_DIR}" LIBS_CLOSED_DIR) diff --git a/indra/cmake/bugsplat.cmake b/indra/cmake/bugsplat.cmake index eb5808b1fb..59644b73ce 100644 --- a/indra/cmake/bugsplat.cmake +++ b/indra/cmake/bugsplat.cmake @@ -1,7 +1,6 @@ -# BugSplat is engaged by setting environment variable BUGSPLAT_DB to the -# target BugSplat database name prior to running CMake (and during autobuild -# build). -if (DEFINED ENV{BUGSPLAT_DB}) +# BugSplat is engaged by setting BUGSPLAT_DB to the target BugSplat database +# name. +if (BUGSPLAT_DB) if (USESYSTEMLIBS) message(STATUS "Looking for system BugSplat") set(BUGSPLAT_FIND_QUIETLY ON) @@ -23,4 +22,4 @@ if (DEFINED ENV{BUGSPLAT_DB}) endif (WINDOWS) set(BUGSPLAT_INCLUDE_DIR ${LIBS_PREBUILT_DIR}/include/bugsplat) endif (USESYSTEMLIBS) -endif (DEFINED ENV{BUGSPLAT_DB}) +endif (BUGSPLAT_DB) diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index 611f72269e..c4dcad51b7 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -33,13 +33,14 @@ import filecmp import fnmatch import getopt import glob +import itertools +import operator import os import re import shutil +import subprocess import sys import tarfile -import errno -import subprocess class ManifestError(RuntimeError): """Use an exception more specific than generic Python RuntimeError""" @@ -90,7 +91,7 @@ DEFAULT_SRCTREE = os.path.dirname(sys.argv[0]) CHANNEL_VENDOR_BASE = 'Second Life' RELEASE_CHANNEL = CHANNEL_VENDOR_BASE + ' Release' -ARGUMENTS=[ +BASE_ARGUMENTS=[ dict(name='actions', description="""This argument specifies the actions that are to be taken when the script is run. The meaningful actions are currently: @@ -108,8 +109,19 @@ ARGUMENTS=[ Example use: %(name)s --arch=i686 On Linux this would try to use Linux_i686Manifest.""", default=""), + dict(name='artwork', description='Artwork directory.', default=DEFAULT_SRCTREE), dict(name='build', description='Build directory.', default=DEFAULT_SRCTREE), dict(name='buildtype', description='Build type (i.e. Debug, Release, RelWithDebInfo).', default=None), + dict(name='bundleid', + description="""The Mac OS X Bundle identifier.""", + default="com.secondlife.indra.viewer"), + dict(name='channel', + description="""The channel to use for updates, packaging, settings name, etc.""", + default='CHANNEL UNSET'), + dict(name='channel_suffix', + description="""Addition to the channel for packaging and channel value, + but not application name (used internally)""", + default=None), dict(name='configuration', description="""The build configuration used.""", default="Release"), @@ -117,12 +129,6 @@ ARGUMENTS=[ dict(name='grid', description="""Which grid the client will try to connect to.""", default=None), - dict(name='channel', - description="""The channel to use for updates, packaging, settings name, etc.""", - default='CHANNEL UNSET'), - dict(name='channel_suffix', - description="""Addition to the channel for packaging and channel value, but not application name (used internally)""", - default=None), dict(name='installer_name', description=""" The name of the file that the installer should be packaged up into. Only used on Linux at the moment.""", @@ -134,10 +140,14 @@ ARGUMENTS=[ description="""The current platform, to be used for looking up which manifest class to run.""", default=get_default_platform), + dict(name='signature', + description="""This specifies an identity to sign the viewer with, if any. + If no value is supplied, the default signature will be used, if any. Currently + only used on Mac OS X.""", + default=None), dict(name='source', description='Source directory.', default=DEFAULT_SRCTREE), - dict(name='artwork', description='Artwork directory.', default=DEFAULT_SRCTREE), dict(name='touch', description="""File to touch when action is finished. Touch file will contain the name of the final package in a form suitable @@ -145,23 +155,15 @@ ARGUMENTS=[ default=None), dict(name='versionfile', description="""The name of a file containing the full version number."""), - dict(name='bundleid', - description="""The Mac OS X Bundle identifier.""", - default="com.secondlife.indra.viewer"), - dict(name='signature', - description="""This specifies an identity to sign the viewer with, if any. - If no value is supplied, the default signature will be used, if any. Currently - only used on Mac OS X.""", - default=None) ] -def usage(srctree=""): +def usage(arguments, srctree=""): nd = {'name':sys.argv[0]} print """Usage: %(name)s [options] [destdir] Options: """ % nd - for arg in ARGUMENTS: + for arg in arguments: default = arg['default'] if hasattr(default, '__call__'): default = "(computed value) \"" + str(default(srctree)) + '"' @@ -172,11 +174,15 @@ def usage(srctree=""): default, arg['description'] % nd) -def main(): -## import itertools +def main(extra=[]): ## print ' '.join((("'%s'" % item) if ' ' in item else item) ## for item in itertools.chain([sys.executable], sys.argv)) - option_names = [arg['name'] + '=' for arg in ARGUMENTS] + # Supplement our default command-line switches with any desired by + # application-specific caller. + arguments = list(itertools.chain(BASE_ARGUMENTS, extra)) + # Alphabetize them by option name in case we display usage. + arguments.sort(key=operator.itemgetter('name')) + option_names = [arg['name'] + '=' for arg in arguments] option_names.append('help') options, remainder = getopt.getopt(sys.argv[1:], "", option_names) @@ -199,11 +205,11 @@ def main(): # early out for help if 'help' in args: # *TODO: it is a huge hack to pass around the srctree like this - usage(args['source']) + usage(arguments, srctree=args['source']) return # defaults - for arg in ARGUMENTS: + for arg in arguments: if arg['name'] not in args: default = arg['default'] if hasattr(default, '__call__'): diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 8977acb873..42ad56f1b0 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -255,10 +255,10 @@ set(llcommon_HEADER_FILES set_source_files_properties(${llcommon_HEADER_FILES} PROPERTIES HEADER_FILE_ONLY TRUE) -if (DEFINED ENV{BUGSPLAT_DB}) +if (BUGSPLAT_DB) set_source_files_properties(llapp.cpp PROPERTIES COMPILE_DEFINITIONS "LL_BUGSPLAT") -endif (DEFINED ENV{BUGSPLAT_DB}) +endif (BUGSPLAT_DB) list(APPEND llcommon_SOURCE_FILES ${llcommon_HEADER_FILES}) diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 31c4c02d99..64a8e15c4b 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -8,9 +8,9 @@ include(00-Common) include(Linking) include(Boost) -if (DEFINED ENV{BUGSPLAT_DB}) +if (BUGSPLAT_DB) include(bugsplat) -endif (DEFINED ENV{BUGSPLAT_DB}) +endif (BUGSPLAT_DB) include(BuildPackagesInfo) include(BuildVersion) include(CMakeCopyIfDifferent) @@ -99,11 +99,11 @@ include_directories( ${CMAKE_CURRENT_SOURCE_DIR} ) -if (DEFINED ENV{BUGSPLAT_DB}) +if (BUGSPLAT_DB) include_directories( ${BUGSPLAT_INCLUDE_DIR} ) -endif (DEFINED ENV{BUGSPLAT_DB}) +endif (BUGSPLAT_DB) include_directories(SYSTEM ${LLCOMMON_SYSTEM_INCLUDE_DIRS} @@ -1390,11 +1390,11 @@ if (DARWIN) ${COREAUDIO_LIBRARY} ) - if (DEFINED ENV{BUGSPLAT_DB}) + if (BUGSPLAT_DB) list(APPEND viewer_LIBRARIES ${BUGSPLAT_LIBRARIES} ) - endif (DEFINED ENV{BUGSPLAT_DB}) + endif (BUGSPLAT_DB) # Add resource files to the project. set(viewer_RESOURCE_FILES @@ -1729,10 +1729,10 @@ if (SDL_FOUND) ) endif (SDL_FOUND) -if (DEFINED ENV{BUGSPLAT_DB}) +if (BUGSPLAT_DB) set_property(TARGET ${VIEWER_BINARY_NAME} PROPERTY COMPILE_DEFINITIONS "LL_BUGSPLAT") -endif (DEFINED ENV{BUGSPLAT_DB}) +endif (BUGSPLAT_DB) # add package files file(GLOB EVENT_HOST_SCRIPT_GLOB_LIST @@ -1841,15 +1841,16 @@ if (WINDOWS) --actions=copy --arch=${ARCH} --artwork=${ARTWORK_DIR} + "--bugsplat=${BUGSPLAT_DB}" --build=${CMAKE_CURRENT_BINARY_DIR} --buildtype=${CMAKE_BUILD_TYPE} + "--channel=${VIEWER_CHANNEL}" --configuration=${CMAKE_CFG_INTDIR} --dest=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR} --grid=${GRID} - "--channel=${VIEWER_CHANNEL}" - --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt --source=${CMAKE_CURRENT_SOURCE_DIR} --touch=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/copy_touched.bat + --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py stage_third_party_libs @@ -1892,15 +1893,16 @@ if (WINDOWS) ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py --arch=${ARCH} --artwork=${ARTWORK_DIR} + "--bugsplat=${BUGSPLAT_DB}" --build=${CMAKE_CURRENT_BINARY_DIR} --buildtype=${CMAKE_BUILD_TYPE} "--channel=${VIEWER_CHANNEL}" - --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt --configuration=${CMAKE_CFG_INTDIR} --dest=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR} --grid=${GRID} --source=${CMAKE_CURRENT_SOURCE_DIR} --touch=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/touched.bat + --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt DEPENDS ${VIEWER_BINARY_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py @@ -2007,11 +2009,11 @@ target_link_libraries(${VIEWER_BINARY_NAME} ${LLAPPEARANCE_LIBRARIES} ) -if (DEFINED ENV{BUGSPLAT_DB}) +if (BUGSPLAT_DB) target_link_libraries(${VIEWER_BINARY_NAME} ${BUGSPLAT_LIBRARIES} ) -endif (DEFINED ENV{BUGSPLAT_DB}) +endif (BUGSPLAT_DB) set(ARTWORK_DIR ${CMAKE_CURRENT_SOURCE_DIR} CACHE PATH "Path to artwork files.") @@ -2036,15 +2038,16 @@ if (LINUX) ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py --arch=${ARCH} --artwork=${ARTWORK_DIR} + "--bugsplat=${BUGSPLAT_DB}" --build=${CMAKE_CURRENT_BINARY_DIR} --buildtype=${CMAKE_BUILD_TYPE} "--channel=${VIEWER_CHANNEL}" - --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt --configuration=${CMAKE_CFG_INTDIR} --dest=${CMAKE_CURRENT_BINARY_DIR}/packaged --grid=${GRID} --source=${CMAKE_CURRENT_SOURCE_DIR} --touch=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/.${product}.touched + --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py ${COPY_INPUT_DEPENDENCIES} @@ -2058,17 +2061,18 @@ if (LINUX) COMMAND ${PYTHON_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py - --arch=${ARCH} --actions=copy + --arch=${ARCH} --artwork=${ARTWORK_DIR} + "--bugsplat=${BUGSPLAT_DB}" --build=${CMAKE_CURRENT_BINARY_DIR} --buildtype=${CMAKE_BUILD_TYPE} + "--channel=${VIEWER_CHANNEL}" --configuration=${CMAKE_CFG_INTDIR} --dest=${CMAKE_CURRENT_BINARY_DIR}/packaged --grid=${GRID} - "--channel=${VIEWER_CHANNEL}" - --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt --source=${CMAKE_CURRENT_SOURCE_DIR} + --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py ${COPY_INPUT_DEPENDENCIES} @@ -2137,15 +2141,16 @@ if (DARWIN) --actions=copy --arch=${ARCH} --artwork=${ARTWORK_DIR} + "--bugsplat=${BUGSPLAT_DB}" --build=${CMAKE_CURRENT_BINARY_DIR} --buildtype=${CMAKE_BUILD_TYPE} + --bundleid=${MACOSX_BUNDLE_GUI_IDENTIFIER} + "--channel=${VIEWER_CHANNEL}" --configuration=${CMAKE_CFG_INTDIR} --dest=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app --grid=${GRID} - "--channel=${VIEWER_CHANNEL}" - --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt - --bundleid=${MACOSX_BUNDLE_GUI_IDENTIFIER} --source=${CMAKE_CURRENT_SOURCE_DIR} + --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt DEPENDS ${VIEWER_BINARY_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py @@ -2170,15 +2175,16 @@ if (DARWIN) ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py --arch=${ARCH} --artwork=${ARTWORK_DIR} + "--bugsplat=${BUGSPLAT_DB}" --build=${CMAKE_CURRENT_BINARY_DIR} --buildtype=${CMAKE_BUILD_TYPE} + "--channel=${VIEWER_CHANNEL}" --configuration=${CMAKE_CFG_INTDIR} --dest=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app --grid=${GRID} - "--channel=${VIEWER_CHANNEL}" - --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt --source=${CMAKE_CURRENT_SOURCE_DIR} --touch=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/.${product}.touched + --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt ${SIGNING_SETTING} DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py @@ -2190,7 +2196,7 @@ if (INSTALL) include(${CMAKE_CURRENT_SOURCE_DIR}/ViewerInstall.cmake) endif (INSTALL) -if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND NOT DEFINED ENV{BUGSPLAT_DB}) +if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND NOT BUGSPLAT_DB) set(SYMBOL_SEARCH_DIRS "") # Note that the path to VIEWER_SYMBOL_FILE must match that in ../../build.sh if (WINDOWS) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index ea0b3625be..e5f0575e86 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -188,11 +188,10 @@ class ViewerManifest(LLManifest): "Address Size":self.address_size, "Update Service":"https://update.secondlife.com/update", } - try: - build_data_dict["BugSplat DB"] = os.environ["BUGSPLAT_DB"] - except KeyError: - # skip the assignment if there's no BUGSPLAT_DB variable - pass + # Only store this if it's both present and non-empty + bugsplat_db = self.args.get('bugsplat') + if bugsplat_db: + build_data_dict["BugSplat DB"] = bugsplat_db build_data_dict = self.finish_build_data_dict(build_data_dict) with open(os.path.join(os.pardir,'build_data.json'), 'w') as build_data_handle: json.dump(build_data_dict,build_data_handle) @@ -589,14 +588,15 @@ class WindowsManifest(ViewerManifest): self.path("libhunspell.dll") # BugSplat - if(self.address_size == 64): - self.path("BsSndRpt64.exe") - self.path("BugSplat64.dll") - self.path("BugSplatRc64.dll") - else: - self.path("BsSndRpt.exe") - self.path("BugSplat.dll") - self.path("BugSplatRc.dll") + if self.args.get('bugsplat'): + if(self.address_size == 64): + self.path("BsSndRpt64.exe") + self.path("BugSplat64.dll") + self.path("BugSplatRc64.dll") + else: + self.path("BsSndRpt.exe") + self.path("BugSplat.dll") + self.path("BugSplatRc.dll") # For google-perftools tcmalloc allocator. try: @@ -1038,7 +1038,7 @@ open "%s" --args "$@" if ("package" in self.args['actions'] or "unpacked" in self.args['actions']): # only if we're engaging BugSplat - if "BUGSPLAT_DB" in os.environ: + if self.args.get('bugsplat'): # Create a symbol archive BEFORE stripping the # binary. self.run_command(['dsymutil', exepath]) @@ -1085,13 +1085,11 @@ open "%s" --args "$@" # runs the executable, instead of launching the app) Info["CFBundleExecutable"] = exename Info["CFBundleIconFile"] = viewer_icon - try: + bugsplat_db = self.args.get('bugsplat') + if bugsplat_db: # https://www.bugsplat.com/docs/platforms/os-x#configuration Info["BugsplatServerURL"] = \ - "https://{BUGSPLAT_DB}.bugsplatsoftware.com/".format(**os.environ) - except KeyError: - # skip the assignment if there's no BUGSPLAT_DB variable - pass + "https://{}.bugsplatsoftware.com/".format(bugsplat_db) self.put_in_file( plistlib.writePlistToString(Info), os.path.basename(Info_plist), @@ -1104,7 +1102,8 @@ open "%s" --args "$@" # Remember where we parked this car. CEF_framework = self.dst_path_of(CEF_framework) - self.path2basename(relpkgdir, "BugsplatMac.framework") + if self.args.get('bugsplat'): + self.path2basename(relpkgdir, "BugsplatMac.framework") with self.prefix(dst="Resources"): # defer cross-platform file copies until we're in the right @@ -1727,4 +1726,8 @@ class Linux_x86_64_Manifest(LinuxManifest): ################################################################ if __name__ == "__main__": - main() + extra_arguments = [ + dict(name='bugsplat', description="""BugSplat database to which to post crashes, + if BugSplat crash reporting is desired""", default=''), + ] + main(extra=extra_arguments) From 3f7c75b8a075a5cd5765b1791a58f5d8e2b164dd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 27 Aug 2018 13:55:50 -0400 Subject: [PATCH 07/14] SL-957: Explicitly pass VIEWER_SYMBOL_FILE from build.sh into CMake instead of relying on both indra/newview/CMakeLists.txt and build.sh generating the same file pathname. Make build.sh set VIEWER_SYMBOL_FILE (instead of symbolfile) in pre_build, and pass it to autobuild configure via -D switch. Then the uploads stanza can just use VIEWER_SYMBOL_FILE instead of performing its platform-sensitive case statement right there. Introduce VIEWER_SYMBOL_FILE CMake cache variable, default empty string. Make indra/newview/CMakeLists.txt generate_breakpad_symbols logic conditional on VIEWER_SYMBOL_FILE being non-empty, as well as everything else. Eliminate local set(VIEWER_SYMBOL_FILE) directives. --- build.sh | 34 +++++++++++++++++++--------------- indra/cmake/Variables.cmake | 1 + indra/newview/CMakeLists.txt | 7 ++----- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/build.sh b/build.sh index c38bb6fff4..976228cdf1 100755 --- a/build.sh +++ b/build.sh @@ -103,6 +103,23 @@ pre_build() "-DSIGNING_IDENTITY:STRING=Developer ID Application: Linden Research, Inc.") fi + if [ "${RELEASE_CRASH_REPORTING:-}" != "OFF" ] + then + case "$arch" in + CYGWIN) + symplat="windows" + ;; + Darwin) + symplat="darwin" + ;; + Linux) + symplat="linux" + ;; + esac + # This name is consumed by indra/newview/CMakeLists.txt + VIEWER_SYMBOL_FILE="$build_dir/newview/$variant/secondlife-symbols-$symplat-${AUTOBUILD_ADDRSIZE}.tar.bz2" + fi + # don't spew credentials into build log bugsplat_sh="$build_secrets_checkout/bugsplat/bugsplat.sh" set +x @@ -122,6 +139,7 @@ pre_build() -DPACKAGE:BOOL=ON \ -DHAVOK:BOOL="$HAVOK" \ -DRELEASE_CRASH_REPORTING:BOOL="$RELEASE_CRASH_REPORTING" \ + -DVIEWER_SYMBOL_FILE:STRING="${VIEWER_SYMBOL_FILE:-}" \ -DBUGSPLAT_DB:STRING="${BUGSPLAT_DB:-}" \ -DVIEWER_CHANNEL:STRING="${viewer_channel}" \ -DGRID:STRING="\"$viewer_grid\"" \ @@ -245,7 +263,6 @@ initialize_version # provided by buildscripts build.sh; sets version id # Now run the build succeeded=true -build_processes= last_built_variant= for variant in $variants do @@ -253,7 +270,6 @@ do last_built_variant="$variant" build_dir=`build_dir_$arch $variant` - build_dir_stubs="$build_dir/win_setup/$variant" begin_section "Initialize $variant Build Directory" rm -rf "$build_dir" @@ -432,19 +448,7 @@ then -a -z "${BUGSPLAT_DB:-}" ] then # Upload crash reporter file - # These names must match the set of VIEWER_SYMBOL_FILE in indra/newview/CMakeLists.txt - case "$arch" in - CYGWIN) - symbolfile="$build_dir/newview/Release/secondlife-symbols-windows-${AUTOBUILD_ADDRSIZE}.tar.bz2" - ;; - Darwin) - symbolfile="$build_dir/newview/Release/secondlife-symbols-darwin-${AUTOBUILD_ADDRSIZE}.tar.bz2" - ;; - Linux) - symbolfile="$build_dir/newview/Release/secondlife-symbols-linux-${AUTOBUILD_ADDRSIZE}.tar.bz2" - ;; - esac - python_cmd "$helpers/codeticket.py" addoutput "Symbolfile" "$symbolfile" \ + python_cmd "$helpers/codeticket.py" addoutput "Symbolfile" "$VIEWER_SYMBOL_FILE" \ || fatal "Upload of symbolfile failed" fi diff --git a/indra/cmake/Variables.cmake b/indra/cmake/Variables.cmake index e49de0a79b..2b54cd4155 100644 --- a/indra/cmake/Variables.cmake +++ b/indra/cmake/Variables.cmake @@ -33,6 +33,7 @@ set(INTEGRATION_TESTS_PREFIX) set(LL_TESTS ON CACHE BOOL "Build and run unit and integration tests (disable for build timing runs to reduce variation") set(INCREMENTAL_LINK OFF CACHE BOOL "Use incremental linking on win32 builds (enable for faster links on some machines)") set(ENABLE_MEDIA_PLUGINS ON CACHE BOOL "Turn off building media plugins if they are imported by third-party library mechanism") +set(VIEWER_SYMBOL_FILE "" CACHE STRING "Name of tarball into which to place symbol files") set(BUGSPLAT_DB "" CACHE STRING "BugSplat database name, if BugSplat crash reporting is desired") if(LIBS_CLOSED_DIR) diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 64a8e15c4b..28fa56b54b 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2196,12 +2196,11 @@ if (INSTALL) include(${CMAKE_CURRENT_SOURCE_DIR}/ViewerInstall.cmake) endif (INSTALL) -if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND NOT BUGSPLAT_DB) +# Note that the conventional VIEWER_SYMBOL_FILE is set by ../../build.sh +if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND NOT BUGSPLAT_DB AND VIEWER_SYMBOL_FILE) set(SYMBOL_SEARCH_DIRS "") - # Note that the path to VIEWER_SYMBOL_FILE must match that in ../../build.sh if (WINDOWS) list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}") - set(VIEWER_SYMBOL_FILE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/secondlife-symbols-windows-$ENV{AUTOBUILD_ADDRSIZE}.tar.bz2") # slplugin.exe failing symbols dump - need to debug, might have to do with updated version of google breakpad # set(VIEWER_EXE_GLOBS "${VIEWER_BINARY_NAME}${CMAKE_EXECUTABLE_SUFFIX} slplugin.exe") set(VIEWER_EXE_GLOBS "${VIEWER_BINARY_NAME}${CMAKE_EXECUTABLE_SUFFIX}") @@ -2214,14 +2213,12 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND NOT list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/llplugin/slplugin/${CMAKE_CFG_INTDIR}") list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/mac_crash_logger/${CMAKE_CFG_INTDIR}") list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/media_plugins/gstreamer010/${CMAKE_CFG_INTDIR}") - set(VIEWER_SYMBOL_FILE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/secondlife-symbols-darwin-$ENV{AUTOBUILD_ADDRSIZE}.tar.bz2") set(VIEWER_EXE_GLOBS "'${product}' SLPlugin mac-crash-logger") set(VIEWER_EXE_GLOBS "'${product}' mac-crash-logger") set(VIEWER_LIB_GLOB "*.dylib") endif (DARWIN) if (LINUX) list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/packaged") - set(VIEWER_SYMBOL_FILE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/secondlife-symbols-linux-$ENV{AUTOBUILD_ADDRSIZE}.tar.bz2") set(VIEWER_EXE_GLOBS "do-not-directly-run-secondlife-bin SLPlugin") set(VIEWER_EXE_GLOBS "do-not-directly-run-secondlife-bin") set(VIEWER_LIB_GLOB "*${CMAKE_SHARED_MODULE_SUFFIX}*") From b6e22902f350be6cff5b4d941b0fb9d5b9ac890a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 27 Aug 2018 16:15:31 -0400 Subject: [PATCH 08/14] SL-824: Update to bugsplat build 519074 --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index ba8d874420..905ce411bd 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -244,9 +244,9 @@ archive hash - 40a9258f96f2f800489aeb7dee366654 + ebb14356ae840d79906f742f6f3f695f url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23472/180173/bugsplat-1.0.6.518982-darwin64-518982.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23627/181379/bugsplat-1.0.6.519074-darwin64-519074.tar.bz2 name darwin64 @@ -256,9 +256,9 @@ archive hash - 735dc6d92fa973e08d280efaa381c452 + 61ece5855ca6bb68e6e24e45b16f34d6 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23474/180182/bugsplat-3.6.0.4.518982-windows-518982.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23630/181394/bugsplat-3.6.0.4.519074-windows-519074.tar.bz2 name windows @@ -268,16 +268,16 @@ archive hash - 028c13cc0437c0881ad38b4ac685f794 + e9c62bde423c548a8248a91af5897e29 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23473/180187/bugsplat-3.6.0.4.518982-windows64-518982.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23629/181387/bugsplat-3.6.0.4.519074-windows64-519074.tar.bz2 name windows64 version - 1.0.6.518982 + 1.0.6.519074 chardet From aaa57c67d617e7761f459f7bdc3dd882ca069b12 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 28 Aug 2018 11:48:52 -0400 Subject: [PATCH 09/14] SL-824: Update to bugsplat build 519106 --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index 905ce411bd..806c51205c 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -244,9 +244,9 @@ archive hash - ebb14356ae840d79906f742f6f3f695f + eb7357084addd54e9f19f66ea74720cd url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23627/181379/bugsplat-1.0.6.519074-darwin64-519074.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23677/181687/bugsplat-1.0.6.519106-darwin64-519106.tar.bz2 name darwin64 @@ -256,9 +256,9 @@ archive hash - 61ece5855ca6bb68e6e24e45b16f34d6 + b9a8dc85dfe365d8f5f621635b6f8e48 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23630/181394/bugsplat-3.6.0.4.519074-windows-519074.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23679/181701/bugsplat-3.6.0.4.519106-windows-519106.tar.bz2 name windows @@ -268,16 +268,16 @@ archive hash - e9c62bde423c548a8248a91af5897e29 + f3c0670c4ada369320bdcc4ffcd6a158 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23629/181387/bugsplat-3.6.0.4.519074-windows64-519074.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23678/181694/bugsplat-3.6.0.4.519106-windows64-519106.tar.bz2 name windows64 version - 1.0.6.519074 + 1.0.6.519106 chardet From 76f75d8068fabc94f2a9d80054436e945859d8d3 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 28 Aug 2018 16:55:25 -0400 Subject: [PATCH 10/14] SL-824: Update to bugsplat build 519145 --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index 806c51205c..b08475baa1 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -244,9 +244,9 @@ archive hash - eb7357084addd54e9f19f66ea74720cd + 7a7bd828233e8a2b0e9c022f6219e6e7 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23677/181687/bugsplat-1.0.6.519106-darwin64-519106.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23730/182106/bugsplat-1.0.6.519145-darwin64-519145.tar.bz2 name darwin64 @@ -256,9 +256,9 @@ archive hash - b9a8dc85dfe365d8f5f621635b6f8e48 + a3938332a11215e6909d67d1b9be5259 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23679/181701/bugsplat-3.6.0.4.519106-windows-519106.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23732/182120/bugsplat-3.6.0.4.519145-windows-519145.tar.bz2 name windows @@ -268,16 +268,16 @@ archive hash - f3c0670c4ada369320bdcc4ffcd6a158 + 453d624d87a80779f59cfb1880613d90 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23678/181694/bugsplat-3.6.0.4.519106-windows64-519106.tar.bz2 + http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/23731/182115/bugsplat-3.6.0.4.519145-windows64-519145.tar.bz2 name windows64 version - 1.0.6.519106 + 1.0.6.519145 chardet From 5ff160f72e8f4eab7a74491a7b848348267a180a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 Aug 2018 12:12:37 -0400 Subject: [PATCH 11/14] SL-957: Generate the expected symbols tarball even with BugSplat. This is a separate step from generating and posting BugSplat symbols, since BugSplat needs the executable along with the symbols, and we don't need to consume that space in a symbols tarball. Move Mac BugSplat symbol generation logic to CMake land, the same general area where Breakpad symbols are generated. Add stanzas to pack up the usual tarball for Windows and Mac. Remove the build.sh test that suppressed uploading the symbols tarball for BugSplat builds. --- build.sh | 4 +- indra/newview/CMakeLists.txt | 190 ++++++++++++++++++++++--------- indra/newview/viewer_manifest.py | 35 +----- 3 files changed, 137 insertions(+), 92 deletions(-) diff --git a/build.sh b/build.sh index 976228cdf1..d6a6d7aca8 100755 --- a/build.sh +++ b/build.sh @@ -443,9 +443,7 @@ then if [ "$last_built_variant" = "Release" ] then # nat 2016-12-22: without RELEASE_CRASH_REPORTING, we have no symbol file. - # Likewise, BUGSPLAT_DB suppresses generating the symbol file. - if [ "${RELEASE_CRASH_REPORTING:-}" != "OFF" \ - -a -z "${BUGSPLAT_DB:-}" ] + if [ "${RELEASE_CRASH_REPORTING:-}" != "OFF" ] then # Upload crash reporter file python_cmd "$helpers/codeticket.py" addoutput "Symbolfile" "$VIEWER_SYMBOL_FILE" \ diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 28fa56b54b..717c835031 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2128,9 +2128,14 @@ if (DARWIN) "${CMAKE_CURRENT_SOURCE_DIR}/Info-SecondLife.plist" ) + set(VIEWER_APP_BUNDLE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app") + set(VIEWER_APP_EXECUTABLE "${VIEWER_APP_BUNDLE}/Contents/MacOS/${product}") + set(VIEWER_APP_DSYM "${VIEWER_APP_EXECUTABLE}.dSYM") + set(VIEWER_APP_XCARCHIVE "${VIEWER_APP_BUNDLE}/../${product}.xcarchive.zip") + configure_file( "${CMAKE_CURRENT_SOURCE_DIR}/Info-SecondLife.plist" - "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app/Contents/Info.plist" + "${VIEWER_APP_BUNDLE}/Contents/Info.plist" ) add_custom_command( @@ -2147,7 +2152,7 @@ if (DARWIN) --bundleid=${MACOSX_BUNDLE_GUI_IDENTIFIER} "--channel=${VIEWER_CHANNEL}" --configuration=${CMAKE_CFG_INTDIR} - --dest=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app + --dest=${VIEWER_APP_BUNDLE} --grid=${GRID} --source=${CMAKE_CURRENT_SOURCE_DIR} --versionfile=${CMAKE_CURRENT_BINARY_DIR}/viewer_version.txt @@ -2180,7 +2185,7 @@ if (DARWIN) --buildtype=${CMAKE_BUILD_TYPE} "--channel=${VIEWER_CHANNEL}" --configuration=${CMAKE_CFG_INTDIR} - --dest=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app + --dest=${VIEWER_APP_BUNDLE} --grid=${GRID} --source=${CMAKE_CURRENT_SOURCE_DIR} --touch=${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/.${product}.touched @@ -2197,60 +2202,135 @@ if (INSTALL) endif (INSTALL) # Note that the conventional VIEWER_SYMBOL_FILE is set by ../../build.sh -if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND NOT BUGSPLAT_DB AND VIEWER_SYMBOL_FILE) - set(SYMBOL_SEARCH_DIRS "") - if (WINDOWS) - list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}") - # slplugin.exe failing symbols dump - need to debug, might have to do with updated version of google breakpad - # set(VIEWER_EXE_GLOBS "${VIEWER_BINARY_NAME}${CMAKE_EXECUTABLE_SUFFIX} slplugin.exe") - set(VIEWER_EXE_GLOBS "${VIEWER_BINARY_NAME}${CMAKE_EXECUTABLE_SUFFIX}") - set(VIEWER_LIB_GLOB "*${CMAKE_SHARED_MODULE_SUFFIX}") - set(VIEWER_COPY_MANIFEST copy_w_viewer_manifest) - endif (WINDOWS) - if (DARWIN) - list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}") - # *TODO: Generate these search dirs in the cmake files related to each binary. - list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/llplugin/slplugin/${CMAKE_CFG_INTDIR}") - list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/mac_crash_logger/${CMAKE_CFG_INTDIR}") - list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/media_plugins/gstreamer010/${CMAKE_CFG_INTDIR}") - set(VIEWER_EXE_GLOBS "'${product}' SLPlugin mac-crash-logger") - set(VIEWER_EXE_GLOBS "'${product}' mac-crash-logger") - set(VIEWER_LIB_GLOB "*.dylib") - endif (DARWIN) - if (LINUX) - list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/packaged") - set(VIEWER_EXE_GLOBS "do-not-directly-run-secondlife-bin SLPlugin") - set(VIEWER_EXE_GLOBS "do-not-directly-run-secondlife-bin") - set(VIEWER_LIB_GLOB "*${CMAKE_SHARED_MODULE_SUFFIX}*") - set(VIEWER_COPY_MANIFEST copy_l_viewer_manifest) - endif (LINUX) +if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIEWER_SYMBOL_FILE) + if (NOT BUGSPLAT_DB) + # Breakpad symbol-file generation + set(SYMBOL_SEARCH_DIRS "") + if (WINDOWS) + list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}") + # slplugin.exe failing symbols dump - need to debug, might have to do with updated version of google breakpad + # set(VIEWER_EXE_GLOBS "${VIEWER_BINARY_NAME}${CMAKE_EXECUTABLE_SUFFIX} slplugin.exe") + set(VIEWER_EXE_GLOBS "${VIEWER_BINARY_NAME}${CMAKE_EXECUTABLE_SUFFIX}") + set(VIEWER_LIB_GLOB "*${CMAKE_SHARED_MODULE_SUFFIX}") + set(VIEWER_COPY_MANIFEST copy_w_viewer_manifest) + endif (WINDOWS) + if (DARWIN) + list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}") + # *TODO: Generate these search dirs in the cmake files related to each binary. + list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/llplugin/slplugin/${CMAKE_CFG_INTDIR}") + list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/mac_crash_logger/${CMAKE_CFG_INTDIR}") + list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/media_plugins/gstreamer010/${CMAKE_CFG_INTDIR}") + set(VIEWER_EXE_GLOBS "'${product}' SLPlugin mac-crash-logger") + set(VIEWER_EXE_GLOBS "'${product}' mac-crash-logger") + set(VIEWER_LIB_GLOB "*.dylib") + endif (DARWIN) + if (LINUX) + list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_CURRENT_BINARY_DIR}/packaged") + set(VIEWER_EXE_GLOBS "do-not-directly-run-secondlife-bin SLPlugin") + set(VIEWER_EXE_GLOBS "do-not-directly-run-secondlife-bin") + set(VIEWER_LIB_GLOB "*${CMAKE_SHARED_MODULE_SUFFIX}*") + set(VIEWER_COPY_MANIFEST copy_l_viewer_manifest) + endif (LINUX) - if(CMAKE_CFG_INTDIR STREQUAL ".") - set(LLBUILD_CONFIG ${CMAKE_BUILD_TYPE}) - else(CMAKE_CFG_INTDIR STREQUAL ".") - # set LLBUILD_CONFIG to be a shell variable evaluated at build time - # reflecting the configuration we are currently building. - set(LLBUILD_CONFIG ${CMAKE_CFG_INTDIR}) - endif(CMAKE_CFG_INTDIR STREQUAL ".") - add_custom_command(OUTPUT "${VIEWER_SYMBOL_FILE}" - COMMAND "${PYTHON_EXECUTABLE}" - ARGS - "${CMAKE_CURRENT_SOURCE_DIR}/generate_breakpad_symbols.py" - "${LLBUILD_CONFIG}" - "${SYMBOL_SEARCH_DIRS}" - "${VIEWER_EXE_GLOBS}" - "${VIEWER_LIB_GLOB}" - "${AUTOBUILD_INSTALL_DIR}/bin/dump_syms" - "${VIEWER_SYMBOL_FILE}" - DEPENDS generate_breakpad_symbols.py - VERBATIM) + if(CMAKE_CFG_INTDIR STREQUAL ".") + set(LLBUILD_CONFIG ${CMAKE_BUILD_TYPE}) + else(CMAKE_CFG_INTDIR STREQUAL ".") + # set LLBUILD_CONFIG to be a shell variable evaluated at build time + # reflecting the configuration we are currently building. + set(LLBUILD_CONFIG ${CMAKE_CFG_INTDIR}) + endif(CMAKE_CFG_INTDIR STREQUAL ".") + add_custom_command(OUTPUT "${VIEWER_SYMBOL_FILE}" + COMMAND "${PYTHON_EXECUTABLE}" + ARGS + "${CMAKE_CURRENT_SOURCE_DIR}/generate_breakpad_symbols.py" + "${LLBUILD_CONFIG}" + "${SYMBOL_SEARCH_DIRS}" + "${VIEWER_EXE_GLOBS}" + "${VIEWER_LIB_GLOB}" + "${AUTOBUILD_INSTALL_DIR}/bin/dump_syms" + "${VIEWER_SYMBOL_FILE}" + DEPENDS generate_breakpad_symbols.py + VERBATIM) - add_custom_target(generate_breakpad_symbols DEPENDS "${VIEWER_SYMBOL_FILE}" "${VIEWER_BINARY_NAME}" "${VIEWER_COPY_MANIFEST}") - add_dependencies(generate_breakpad_symbols "${VIEWER_BINARY_NAME}") - if (WINDOWS OR LINUX) - add_dependencies(generate_breakpad_symbols "${VIEWER_COPY_MANIFEST}") - endif (WINDOWS OR LINUX) - add_dependencies(llpackage generate_breakpad_symbols) + add_custom_target(generate_symbols DEPENDS "${VIEWER_SYMBOL_FILE}" "${VIEWER_BINARY_NAME}" "${VIEWER_COPY_MANIFEST}") + add_dependencies(generate_symbols "${VIEWER_BINARY_NAME}") + if (WINDOWS OR LINUX) + add_dependencies(generate_symbols "${VIEWER_COPY_MANIFEST}") + endif (WINDOWS OR LINUX) + + else (NOT BUGSPLAT_DB) + # BugSplat symbol-file generation + if (WINDOWS) + # Just pack up a tarball containing only the .pdb file for the executable. + add_custom_command(OUTPUT "${VIEWER_SYMBOL_FILE}" + # Use of 'tar ...j' here assumes VIEWER_SYMBOL_FILE endswith .tar.bz2; + # testing a string suffix is painful enough in CMake language that + # we'll continue assuming it until forced to generalize. + COMMAND "tar" + ARGS + "cjf" + "${VIEWER_SYMBOL_FILE}" + "-C" + "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}" + "secondlife-bin.pdb" + DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/secondlife-bin.pdb" + COMMENT "Packing viewer PDB into ${VIEWER_SYMBOL_FILE}" + ) + add_custom_target(generate_symbols DEPENDS "${VIEWER_SYMBOL_FILE}") + endif (WINDOWS) + if (DARWIN) + # Have to run dsymutil first, then pack up the resulting .dSYM directory + add_custom_command(OUTPUT "${VIEWER_APP_DSYM}" + COMMAND "dsymutil" + ARGS + "${VIEWER_APP_EXECUTABLE}" + DEPENDS "${VIEWER_APP_EXECUTABLE}" + COMMENT "Generating ${VIEWER_APP_DSYM}" + ) + add_custom_command(OUTPUT "${VIEWER_SYMBOL_FILE}" + # See above comments about "tar ...j" + COMMAND "tar" + ARGS + "cjf" + "${VIEWER_SYMBOL_FILE}" + "-C" + "${VIEWER_APP_DSYM}/.." + "${product}.dSYM" + DEPENDS "${VIEWER_APP_DSYM}" + COMMENT "Packing dSYM into ${VIEWER_SYMBOL_FILE}" + ) + add_custom_command(OUTPUT "${VIEWER_APP_XCARCHIVE}" + COMMAND "zip" + ARGS + "-r" + "${VIEWER_APP_XCARCHIVE}" + "." + WORKING_DIRECTORY "${VIEWER_APP_DSYM}/.." + DEPENDS "${VIEWER_APP_DSYM}" + COMMENT "Generating xcarchive.zip for upload to BugSplat" + ) + # Have to create a stamp file, and depend on it, to force CMake to run + # the cleanup step. + add_custom_command(OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/dsym.stamp" + COMMAND rm -rf "${VIEWER_APP_DSYM}" + COMMAND touch "${CMAKE_CURRENT_BINARY_DIR}/dsym.stamp" + DEPENDS "${VIEWER_SYMBOL_FILE}" "${VIEWER_APP_XCARCHIVE}" + COMMENT "Cleaning up dSYM" + ) + add_custom_target(generate_symbols DEPENDS + "${VIEWER_APP_DSYM}" + "${VIEWER_SYMBOL_FILE}" + "${VIEWER_APP_XCARCHIVE}" + "${CMAKE_CURRENT_BINARY_DIR}/dsym.stamp" + ) + endif (DARWIN) + if (LINUX) + # TBD + endif (LINUX) + endif (NOT BUGSPLAT_DB) + + # for both BUGSPLAT_DB and Breakpad + add_dependencies(llpackage generate_symbols) endif () if (LL_TESTS) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index e5f0575e86..f73b444a6e 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1037,39 +1037,6 @@ open "%s" --args "$@" if ("package" in self.args['actions'] or "unpacked" in self.args['actions']): - # only if we're engaging BugSplat - if self.args.get('bugsplat'): - # Create a symbol archive BEFORE stripping the - # binary. - self.run_command(['dsymutil', exepath]) - # This should produce a Second Life.dSYM bundle directory. - try: - # Now pretend we're Xcode making a .xcarchive file. - # Put it as a sibling of the top-level .app. - # From "Dave" at BugSplat support: - # "More from our Mac lead: I think zipping - # a folder containing the binary and - # symbols would be sufficient. Assuming - # symbol files are created with CMake. I'm - # not sure if CMake strips symbols into - # separate files at build time, and if so - # they're in a supported format." - xcarchive = os.path.join(parentdir, - exename + '.xcarchive.zip') - with zipfile.ZipFile(xcarchive, 'w', - compression=zipfile.ZIP_DEFLATED) as zf: - print "Creating {}".format(xcarchive) - for base, dirs, files in os.walk(here): - for fn in files: - fullfn = os.path.join(base, fn) - relfn = os.path.relpath(fullfn, here) - print " {}".format(relfn) - zf.write(fullfn, relfn) - finally: - # Whether or not we were able to create the - # .xcarchive file, clean up the .dSYM bundle - shutil.rmtree(self.dst_path_of(exename + '.dSYM')) - # NOTE: the -S argument to strip causes it to keep # enough info for annotated backtraces (i.e. function # names in the crash log). 'strip' with no arguments @@ -1089,7 +1056,7 @@ open "%s" --args "$@" if bugsplat_db: # https://www.bugsplat.com/docs/platforms/os-x#configuration Info["BugsplatServerURL"] = \ - "https://{}.bugsplatsoftware.com/".format(bugsplat_db) + "https://{}.bugsplat.com/".format(bugsplat_db) self.put_in_file( plistlib.writePlistToString(Info), os.path.basename(Info_plist), From 34e6d5321df54d99fd91943f49c764849d77bff0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 Aug 2018 14:29:00 -0400 Subject: [PATCH 12/14] SL-957: Use cygpath to set up output pathname for cygwin tar. --- indra/newview/CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 717c835031..7711488c0f 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2261,7 +2261,12 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE else (NOT BUGSPLAT_DB) # BugSplat symbol-file generation if (WINDOWS) - # Just pack up a tarball containing only the .pdb file for the executable. + # Just pack up a tarball containing only the .pdb file for the + # executable. Because we intend to use cygwin tar, we must render + # VIEWER_SYMBOL_FILE in cygwin path syntax. + execute_process(COMMAND "cygpath" "-u" "${VIEWER_SYMBOL_FILE}" + OUTPUT_VARIABLE VIEWER_SYMBOL_FILE_CYGWIN + OUTPUT_STRIP_TRAILING_WHITESPACE) add_custom_command(OUTPUT "${VIEWER_SYMBOL_FILE}" # Use of 'tar ...j' here assumes VIEWER_SYMBOL_FILE endswith .tar.bz2; # testing a string suffix is painful enough in CMake language that @@ -2269,12 +2274,11 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE COMMAND "tar" ARGS "cjf" - "${VIEWER_SYMBOL_FILE}" - "-C" - "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}" + "${VIEWER_SYMBOL_FILE_CYGWIN}" "secondlife-bin.pdb" + WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}" DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/secondlife-bin.pdb" - COMMENT "Packing viewer PDB into ${VIEWER_SYMBOL_FILE}" + COMMENT "Packing viewer PDB into ${VIEWER_SYMBOL_FILE_CYGWIN}" ) add_custom_target(generate_symbols DEPENDS "${VIEWER_SYMBOL_FILE}") endif (WINDOWS) From 29a29db7dd154b694c1ac414ceec8e37cb745335 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 Aug 2018 14:52:54 -0400 Subject: [PATCH 13/14] SL-957: Try to add enough CMake dependencies to generate Mac symbols. --- indra/newview/CMakeLists.txt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 7711488c0f..36dafa52ed 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2129,7 +2129,11 @@ if (DARWIN) ) set(VIEWER_APP_BUNDLE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${product}.app") - set(VIEWER_APP_EXECUTABLE "${VIEWER_APP_BUNDLE}/Contents/MacOS/${product}") + # VIEWER_APP_EXECUTABLE was originally a prediction of the executable + # pathname that would result from running viewer_manifest.py, but CMake + # complained that there was no rule to make that executable. Try using the + # existing target. + set(VIEWER_APP_EXECUTABLE "${VIEWER_BINARY_NAME}") set(VIEWER_APP_DSYM "${VIEWER_APP_EXECUTABLE}.dSYM") set(VIEWER_APP_XCARCHIVE "${VIEWER_APP_BUNDLE}/../${product}.xcarchive.zip") @@ -2291,6 +2295,8 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE DEPENDS "${VIEWER_APP_EXECUTABLE}" COMMENT "Generating ${VIEWER_APP_DSYM}" ) + add_custom_target(dsym_generate DEPENDS "${VIEWER_APP_DSYM}") + add_dependencies(dsym_generate "${VIEWER_APP_EXECUTABLE}") add_custom_command(OUTPUT "${VIEWER_SYMBOL_FILE}" # See above comments about "tar ...j" COMMAND "tar" @@ -2303,6 +2309,8 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE DEPENDS "${VIEWER_APP_DSYM}" COMMENT "Packing dSYM into ${VIEWER_SYMBOL_FILE}" ) + add_custom_target(dsym_tarball DEPENDS "${VIEWER_SYMBOL_FILE}") + add_dependencies(dsym_tarball dsym_generate) add_custom_command(OUTPUT "${VIEWER_APP_XCARCHIVE}" COMMAND "zip" ARGS @@ -2313,6 +2321,8 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE DEPENDS "${VIEWER_APP_DSYM}" COMMENT "Generating xcarchive.zip for upload to BugSplat" ) + add_custom_target(dsym_xcarchive DEPENDS "${VIEWER_APP_XCARCHIVE}") + add_dependencies(dsym_xcarchive dsym_generate) # Have to create a stamp file, and depend on it, to force CMake to run # the cleanup step. add_custom_command(OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/dsym.stamp" @@ -2327,6 +2337,7 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE "${VIEWER_APP_XCARCHIVE}" "${CMAKE_CURRENT_BINARY_DIR}/dsym.stamp" ) + add_dependencies(generate_symbols dsym_tarball dsym_xcarchive) endif (DARWIN) if (LINUX) # TBD From aa6d178b6917af0d933dd2fdcd0c0906285b768b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 Aug 2018 15:19:23 -0400 Subject: [PATCH 14/14] SL-957: Delay trying to pack up Windows PDB file until linker done. --- indra/newview/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 36dafa52ed..10210cd918 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2284,7 +2284,8 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/secondlife-bin.pdb" COMMENT "Packing viewer PDB into ${VIEWER_SYMBOL_FILE_CYGWIN}" ) - add_custom_target(generate_symbols DEPENDS "${VIEWER_SYMBOL_FILE}") + add_custom_target(generate_symbols DEPENDS "${VIEWER_SYMBOL_FILE}" "${VIEWER_BINARY_NAME}") + add_dependencies(generate_symbols "${VIEWER_BINARY_NAME}") endif (WINDOWS) if (DARWIN) # Have to run dsymutil first, then pack up the resulting .dSYM directory